Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated Flatten and Tile Ops #948

Merged
merged 2 commits into from
May 12, 2022

Conversation

hectormz
Copy link
Contributor

@hectormz hectormz commented May 7, 2022

This PR closes #399 and removes long deprecated Flatten and Tile Ops.

I'm not sure if unittest.rst is maintained, but I made a change to the code there.

My dev environment is not quite right, so I couldn't confirm if the debug output in debug_faq.rst (line 158) needed to be updated.

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

@codecov
Copy link

codecov bot commented May 7, 2022

Codecov Report

Merging #948 (7ff73e6) into main (2fee841) will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #948      +/-   ##
==========================================
+ Coverage   79.10%   79.20%   +0.10%     
==========================================
  Files         152      152              
  Lines       48061    47937     -124     
  Branches    10934    10916      -18     
==========================================
- Hits        38017    37967      -50     
+ Misses       7534     7464      -70     
+ Partials     2510     2506       -4     
Impacted Files Coverage Δ
aesara/tensor/basic.py 90.65% <ø> (+2.85%) ⬆️
aesara/tensor/basic_opt.py 85.93% <ø> (+0.83%) ⬆️

ricardoV94
ricardoV94 previously approved these changes May 11, 2022
Copy link
Contributor

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Need to fix a merge conflict

@hectormz
Copy link
Contributor Author

@ricardoV94 Thanks for the review! I merged in changes that @brandonwillard made after my PR

@ricardoV94
Copy link
Contributor

I think you will have to rebase from main instead of merging. Can you confirm @brandonwillard?

@hectormz
Copy link
Contributor Author

I can do that as well and re-push

@hectormz hectormz force-pushed the remove-deprecated branch from a719c9b to 8c88266 Compare May 11, 2022 13:35
@hectormz
Copy link
Contributor Author

Are you able to restart the checks? Seemed to cancel with force push, but not restart

@ricardoV94 ricardoV94 force-pushed the remove-deprecated branch from 8c88266 to 7ff73e6 Compare May 12, 2022 14:08
@ricardoV94
Copy link
Contributor

Are you able to restart the checks? Seemed to cancel with force push, but not restart

I rebased on main again, tests should rerun now

Copy link
Member

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; much appreciated!

@ricardoV94 ricardoV94 merged commit e2e2366 into aesara-devs:main May 12, 2022
@hectormz hectormz deleted the remove-deprecated branch May 12, 2022 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated Flatten and Tile Ops
3 participants