Fixing GitHub Actions #202
Conversation
Mambaforge is sunset, see https://conda-forge.org/news/2024/07/29/sunsetting-mambaforge/
This was done to prevent `TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'` being thrown by Jax. It occurs because Python 3.9 and lower doesn't support the `str | None` syntax, but Jax>0.4.28 uses it. See https://stackoverflow.com/questions/76712720/typeerror-unsupported-operand-types-for-type-and-nonetype
Why wouldn't we want the tests to honor dependencies the same way an average user would during installation? This way the change made in 2069f44 is actually respected and tests can work.
The location doesn't functionally matter as far as I can tell, but I like the idea of keeping as many dependencies as possible confined to one file.
| @@ -56,7 +56,7 @@ jobs: | |||
| - name: Install package | |||
| shell: bash -l {0} | |||
| run: | | |||
| python -m pip install --no-deps . | |||
There was a problem hiding this comment.
I removed --no-deps when I included the "python3.9 requires jax<=0.4.28" logic in this file. I moved the logic to devtools/conda-envs/test_env.yaml in 9ad2d31, but I opted to keep the removal of --no-deps. I don't think it really makes any difference either way, as the only dependency specified here is numpy and that specification seems to be redundant.
|
@slochower @jeff231li if either of you have time in the near future, could you please review this PR? I want to get tests up and running before I make a new release. |
| os: [macOS-latest, ubuntu-latest] | ||
| python-version: ["3.8", "3.9", "3.10"] | ||
| os: [ubuntu-latest] | ||
| python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] |
There was a problem hiding this comment.
Nice, I like that pAPRika is passing with versions 3.11 and 3.12. You can probably drop support for 3.8, but it's probably okay to leave it here for now.
| @@ -56,7 +56,7 @@ jobs: | |||
| - name: Install package | |||
| shell: bash -l {0} | |||
| run: | | |||
| python -m pip install --no-deps . | |||
| @@ -39,4 +39,5 @@ dependencies: | |||
| # Pip-only installs | |||
| - pip: | |||
| - codecov | |||
|
|
|||
| - jax<=0.4.28; python_version == '3.9' # Since python<=3.9 doesn't support `str | None` syntax, pin jax to a version that uses `None` or `Optional[str]` | |||
| - jaxlib<=0.4.28; python_version == '3.9' | |||
There was a problem hiding this comment.
Neat, haven't seen this trick before (I guess it's unbounded when python != 3.9).
|
Thanks both! |
This PR aims to get the lint, ci, and codeql workflows working properly. To do so, it was necessary to introduce logic into
devtools/conda-envs/test_env.yamlthat says if python 3.9 is being used, enforce jax and jaxlib <= 0.4.28. This is because jax above that version containsstr | Nonetype operations, which aren't supported below python 3.10.Python 3.8 doesn't encounter this issue because it defaults to a jax version way older (~0.4.14) which doesn't contain those type operations (Instead, depending on the version, it will use
Optional[str]or simplyNoneas the type hint).One small thing I noticed while making these changes:
pAPRika/setup.py
Line 37 in 688cc2c
Is including numpy here necessary? Commenting out this line doesn't cause errors for me.