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

Fix type hints Python 3.9 compatibility #6944

Closed
wants to merge 1 commit into from

Conversation

michaelosthege
Copy link
Member

@michaelosthege michaelosthege commented Oct 6, 2023

What is this PR about?
Trying to fix one of the reasons why our CIs are going ❌.

Closes #6941

Checklist

  • Explain important implementation details πŸ‘‰ Literal | None doesn't work in Python 3.9
  • Make sure that the pre-commit linting/style checks pass.
  • Link relevant issues (preferably in nice commit messages)
  • Are the changes covered by tests and docstrings?
  • Fill out the short summary sections πŸ‘‡

Bugfixes

  • Fixed a type hint incompatibility of the jax submodule under Python 3.9

πŸ“š Documentation preview πŸ“š: https://pymc--6944.org.readthedocs.build/en/6944/

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #6944 (daa8a86) into main (602234b) will decrease coverage by 5.53%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6944      +/-   ##
==========================================
- Coverage   90.69%   85.17%   -5.53%     
==========================================
  Files         100      100              
  Lines       16833    16833              
==========================================
- Hits        15267    14337     -930     
- Misses       1566     2496     +930     
Files Coverage Ξ”
pymc/sampling/jax.py 0.00% <ΓΈ> (ΓΈ)

... and 21 files with indirect coverage changes

@ricardoV94
Copy link
Member

ricardoV94 commented Oct 6, 2023

Should we run pre-commit on the oldest supported python version? Would that have caught the problem?

@ricardoV94
Copy link
Member

@michaelosthege
Copy link
Member Author

Sounds like we just need a with: https://github.com/pymc-devs/pytensor/blob/36df37981381dad62a24d46f1f21945b3b649c57/.github/workflows/test.yml#L59-L60

Looks like we're running pre-commit multiple times then, and here it's already under Python 3.9:

key: ${{ runner.os }}-py39-conda-${{ env.CACHE_NUMBER }}-${{

@michaelosthege
Copy link
Member Author

jaxlib version 0.4.17 is newer than and incompatible with jax version 0.4.16. Please update your jax and/or jaxlib packages.

also this error again. It shouldn't be our job to pin jax's dependencies πŸ˜–

@ricardoV94
Copy link
Member

Sounds like we just need a with: https://github.com/pymc-devs/pytensor/blob/36df37981381dad62a24d46f1f21945b3b649c57/.github/workflows/test.yml#L59-L60

Looks like we're running pre-commit multiple times then, and here it's already under Python 3.9:

key: ${{ runner.os }}-py39-conda-${{ env.CACHE_NUMBER }}-${{

That's just the mypy job which apparently didn't pick the incompatible type hints. I think the pre-commit itself would have

@ricardoV94
Copy link
Member

jaxlib version 0.4.17 is newer than and incompatible with jax version 0.4.16. Please update your jax and/or jaxlib packages.

also this error again. It shouldn't be our job to pin jax's dependencies πŸ˜–

Is that in our CI?

@michaelosthege
Copy link
Member Author

closing in favor of #6945 to which I cherry-picked the commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jax sampling type-hints incompatible with Python 3.9
2 participants