-
Notifications
You must be signed in to change notification settings - Fork 29
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] Allow any python version for pre-commit #184
Conversation
@jakevdp Github doesn't allow me to assign reviewers to this PR - is this intended? |
Thanks - I wonder if this change will cause flakiness across systems? My understanding is that this not only controls the python version used to run the hook, but also the precise set of rules enforced by pyink. Do you know if that's right? |
From the PyInk docs: # It is recommended to specify the latest version of Python
# supported by your project here, or alternatively use
# pre-commit's default_language_version, see
# https://pre-commit.com/#top_level-default_language_version
language_version: python3.9 Perhaps we should use |
I checked pyink sources, and the python version defines the set of features supported by the language version: This set is backwards compatible, i.e. features are only added, not removed, so I believe bumping the version up to 3.12 should be safe from pyink perspective. |
29744cf
to
a79ae1f
Compare
Updated the PR. |
It looks like we'll also need to update the Python version for the ml_dtypes/.github/workflows/test.yml Lines 16 to 24 in b65a1f6
|
a79ae1f
to
30f5ef7
Compare
Good catch, I haven't noticed this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem with pulldown. Could you rebase on main to trigger a new job? |
30f5ef7
to
1a2a88b
Compare
Rebased on main. |
Currently "pre-commit" doesn't work if python3.9 is not installed (e.g. on a typical Ubuntu which has python3.12)