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

Test new python versions #324

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Test new python versions #324

wants to merge 4 commits into from

Conversation

gselzer
Copy link
Contributor

@gselzer gselzer commented Mar 31, 2025

Supposedly pyimagej works for Python 3.11, 3.12, 3.13, but we don't test/advertise it

Thoughts here @elevans?

Supposedly pyimagej works for Python 3.11, 3.12, 3.13, but we don't
test/advertise it
@gselzer gselzer added the enhancement New feature or request label Mar 31, 2025
@gselzer gselzer requested a review from elevans March 31, 2025 23:37
@gselzer gselzer self-assigned this Mar 31, 2025
@elevans
Copy link
Member

elevans commented Apr 4, 2025

Thanks @gselzer for taking a look at this. I thought it over and I'm in favor of testing additional Python versions but I do not want to have more test environments than needed. To date it seems like our approach hasn't cause us major issues. Given that, here is the official Python status tracker. From what I can tell 5 versions are always "active" (at the time of writing this post, these versions are 3.9, 3.10, 3.11, 3.12, and 3.13). So it makes sense in my head to expand our tested versions to 5 and match the official Python supported version window.

This also aligns well with our conda-forge releases as conda-forge now enforces a minimum python version that's pinned to the minimum officially supported Python release for packages (see CFEP-25) unless you override it.

elevans added 2 commits April 4, 2025 15:50
This commit bumps the minimum Python version to 3.9, mathcing the
current officially supported minimum version. This commit also increases
the total number of tested Python versions to 5, again matching the
officially supported Python versions.
We recommend installing openjdk=11, so we should also test with it.
@gselzer
Copy link
Contributor Author

gselzer commented Apr 4, 2025

I'm in favor of testing additional Python versions but I do not want to have more test environments than needed.

I'd agree. I believe that at a minimum we should test two python versions:

  • The oldest python version supported by the project, which as of right now is 3.8 (well, 3.9 as of bb068d7). This ensures we do not add code that cannot run with the oldest allowed python version.
  • The newest supported python version, which as of right now seems to be 3.13 (hence I added tests and updated the classifiers in pyproject.toml). This ensures compatibility with the newest version (for example, deprecated/removed code).

If you want to test more, there's not really harm, just inconvenience in the CI checks taking longer. My question, then, is what the benefit might be - it's very rare for code to work in e.g. 3.8 and 3.13 but not in e.g. 3.11. If you have a concern that would be solved by checking more versions, we could add them, just seems like additional headache in all of our builds.

As far as version end of life is concerned, I like to follow @ctrueden's approach of "why remove it"? If there's some Python vulnerability we need to avoid or new feature we want to utilize, makes sense, but bumping the minimum version just to bump the minimum version seems unnecessary.

I also see you bumped the CI Java version to 11, which inherently isn't a bad thing, but is related to the discussion. We could consider adding Java versions to the matrix as well, like we do in napari-imagej

@ctrueden
Copy link
Member

ctrueden commented Apr 4, 2025

In addition to @gselzer's comments (with which I agree), another consideration for how to configure the test matrix is simply being a good citizen of the free OSS cloud. Testing 3×OS × 5×Pythons × 4×Javas (e.g. 8, 11, 17, 21), while comprehensive (and not necessarily slower, with containers running in parallel), would be spinning up 60 containers on every single push, which feels wasteful and irresponsible to me. Even if we install all 5×4 Python+Java versions into the same container, to reduce the matrix to 3, it's still going to be doing 20× compute compared to a single Python+Java version. That's why I like the middle ground of "lowest+highest" for each: 3×OS × 2×Python × 2×Java is only 12 combinations rather than 60.

@gselzer
Copy link
Contributor Author

gselzer commented Apr 4, 2025

Testing 3×OS × 5×Pythons × 4×Javas (e.g. 8, 11, 17, 21), while comprehensive (and not necessarily slower, with containers running in parallel)

Depending on how we do it, it could be slower, as there is a limit on how many jobs we can run concurrently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants