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

CI/CD: Update max Python to 3.12 #309

Merged
merged 1 commit into from
Jan 27, 2024

Conversation

kentrutan
Copy link
Collaborator

Try Python 3.12 in the test matrix.

Also since there are so many versions of Python active (3.8 to 3.12), it seems excessive to test every version. This changes the matrix to just three versions: 3.8, 3.10, 3.12. For future updates, I envision picking min, max, and median of the supported Python versions.

@kentrutan
Copy link
Collaborator Author

The casadi wheel build is failing for Python 3.12, so I guess we'll need to wait and try this again at a later date.

@jackvreeken
Copy link
Contributor

jackvreeken commented Nov 9, 2023

@kentrutan CasADi released 3.6.4 with Python 3.12 wheels two days ago, so this MR is now ready.

I made some changes to basically just restart the CI/CD pipelines, I'll leave it to you to squash/properly rebase on top of master.

EDIT:
Welp, nevermind. Many wheels for CasADi 3.6.4 are still missing due to build errors on their side.

@kentrutan
Copy link
Collaborator Author

@jackvreeken Because of these delays, we should probably pin max Python version until all dependencies support the new Python version. What do you think?

At some point, we will want to pin versions of all dependencies. What do you think of using a tool like hatch or poetry to help with this? Not saying we move to that now, but maybe at the same time we are ready to migrate more/all of setup.py into latest recommended packaging practice.

@jackvreeken jackvreeken force-pushed the cicd-test-python-312 branch 5 times, most recently from 0a3b3cf to 9061f99 Compare January 24, 2024 14:42
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e34e2ad) 81.23% compared to head (0906809) 81.21%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #309      +/-   ##
==========================================
- Coverage   81.23%   81.21%   -0.03%     
==========================================
  Files          17       17              
  Lines        4264     4264              
  Branches      950      796     -154     
==========================================
- Hits         3464     3463       -1     
  Misses        651      651              
- Partials      149      150       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jackvreeken jackvreeken force-pushed the cicd-test-python-312 branch 2 times, most recently from 642c75f to 07f0c76 Compare January 24, 2024 14:55
@jackvreeken
Copy link
Contributor

I think I fixed this one, it does depend on #313 and #314 , so those need merging first

@jackvreeken jackvreeken force-pushed the cicd-test-python-312 branch 2 times, most recently from 0c1fcdb to 0449911 Compare January 24, 2024 16:03
@kentrutan
Copy link
Collaborator Author

@jackvreeken What fixed the problem: getting rid of setup.py, the CasADi updates, or both?

Do you have any thoughts on my earlier comment now that we (you) have some more experience with the issue?

@jackvreeken Because of these delays, we should probably pin max Python version until all dependencies support the new Python version. What do you think?

At some point, we will want to pin versions of all dependencies. What do you think of using a tool like hatch or poetry to help with this? Not saying we move to that now, but maybe at the same time we are ready to migrate more/all of setup.py into latest recommended packaging practice.

If you agree, it might be a good time to pin max Python with this PR.

@jackvreeken
Copy link
Contributor

jackvreeken commented Jan 27, 2024

Fixing 3.12 was:

  • Waiting for CasADi to have packages on PyPI (they ran into storage limits on PyPI, their packages are quite big and they have them for all kinds of platform combinations)
  • Fixing distutils deprecation and adding setuptools as a dependency (for the casadi backend). Note that we can then still use import distutils.ccompiler, but we have to have setuptools installed. See Expose compiler classes for customized use pypa/setuptools#2806

Hmm, my thoughts on pinning (exact versions) is that:

  • cli tools/applications probably should
  • libraries should probably not

So for most dependencies we can use semver and pin the major version in major.minor.patch; for CasADi we have to pin major.minor.*. And limiting Python to 3.12 is also fine by me, but then I do think in general we should have pipelines running for the latest version we haven't pinned. Renovate was one of these tools we could use, but there are definitely others. I also spotted something in a .pre-commit-config file the other day about updating those (black/flake8) with some interval of like a month.

We need setuptools as a dependency for distutils.ccompiler to work in
Python 3.12 and higher.

Also since there are so many versions of Python active (3.8 to 3.12),
it seems excessive to test every version. This changes
the matrix to just three versions: 3.8, 3.10, 3.12. For future updates,
I envision picking min, max, and median of the supported Python
versions.
@jackvreeken jackvreeken marked this pull request as ready for review January 27, 2024 13:59
@kentrutan
Copy link
Collaborator Author

kentrutan commented Jan 27, 2024

OK I'll pin some versions per your comment. PR #315 (next to be merged) looks like a better place to do it.

EDIT: I looked into pinning and found that the pyproject.toml section in the Python Packaging User Guide discourages pinning upper versions.

So, plan B, we will follow PyPA's lead and refrain from any additional version pinning for now.

@kentrutan kentrutan merged commit d3653d9 into pymoca:master Jan 27, 2024
13 of 14 checks passed
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.

2 participants