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

Make imports compatible with PyTensor/PyMC v5 #613

Merged
merged 2 commits into from
Dec 18, 2022

Conversation

michaelosthege
Copy link
Contributor

I changed the aesara imports to try PyTensor first, and fall back to aesara otherwise.

Mentions of Aesara were changed to PyTensor, and in pymc.py I had to change where softmax comes from because aesara.tensor.nnet was removed.

Closes #611

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@michaelosthege michaelosthege marked this pull request as ready for review December 18, 2022 00:03
@michaelosthege
Copy link
Contributor Author

cc @tomicapretto @aloctavodia


try:
import pytensor.tensor as pt
except ModuleNotFoundError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should just hard cut over. Theres no reason to stay on Aesara and it just means that we may get user bug reports from two backends rather than one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would recommend to keep compatibility for at least one release and print a warning.

In the four packages I patched in the past few days I maintained compatibility with v4 because it simplifies updating my environments and Docker images.

In any case please take over from here - I'm not even using bambi myself 🙊

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ill merge it. Hopefully no one gets angry at me

@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2022

Codecov Report

Merging #613 (548e1ee) into main (c256549) will decrease coverage by 0.20%.
The diff coverage is 69.44%.

@@            Coverage Diff             @@
##             main     #613      +/-   ##
==========================================
- Coverage   82.21%   82.00%   -0.21%     
==========================================
  Files          40       40              
  Lines        3093     3107      +14     
==========================================
+ Hits         2543     2548       +5     
- Misses        550      559       +9     
Impacted Files Coverage Δ
bambi/families/link.py 65.71% <ø> (ø)
bambi/backend/utils.py 76.92% <50.00%> (-13.08%) ⬇️
bambi/backend/pymc.py 81.42% <55.55%> (-0.84%) ⬇️
bambi/backend/terms.py 93.87% <60.00%> (-1.27%) ⬇️
bambi/backend/links.py 89.65% <80.00%> (-6.50%) ⬇️
bambi/families/multivariate.py 95.38% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@canyon289 canyon289 merged commit 157cbda into bambinos:main Dec 18, 2022
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.

bambi depends on aesara, but pymc forked it to pytensor
3 participants