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

Removed the requirement for a particular version of Python in pre-commit #925

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

patrick-kidger
Copy link
Owner

@patrick-kidger patrick-kidger commented Jan 5, 2025

I've realised that this only works if you have that particular version of Python installed locally -- pre-commit doesn't download this for you.

This unacceptably raises the bar for making contributions -- folks shouldn't have to modify their global system just to offer a PR against Equinox.

Unfortunately this has meant that I've had to remove a couple of dependencies from the additional_dependencies list, as they don't support Python 3.13, which is what is sometimes selected.

(Alternatives considered:

  • Install the specified version of Python as part of the pre-commit hook. This would be ideal. Unfortunately pre-commit passes the specified language_version to python -m virtualenv under the hood, and that doesn't seem to offer a way to do this.

  • Based on the above: something with uv? Unfortunately pre-commit have also elected not to support uv (Add support for uv-based venvs/install via PRE_COMMIT_USE_UV pre-commit/pre-commit#3131, Python language: add option to use uv instead of pip for venv creation & dependency installs pre-commit/pre-commit#3222), which would have done this automatically. Maybe we just need to wait until the folks at Astral write their own version of pre-commit as well!

  • Specify a range of versions for Python, so that we use whatever the system Python is, as long as it is below 3.13. Unfortunately pre-commit doesn't seem to support this.

  • Write our own local hook that does whatever we damn well please: uv to install the right version of Python, downloads pyright, and run it. If this becomes problematic amongst the rest of the ecosystem then it may be worth doing exactly this.

)

I've realised that this only works if you have that particular version of Python installed locally -- pre-commit doesn't download this for you.

This unacceptably raises the bar for making contributions -- folks shouldn't have to modify their global system just to offer a PR against Equinox.

Unfortunately this has meant that I've had to remove a couple of dependencies from the `additional_dependencies` list, as they don't support Python 3.13, which is what is sometimes selected.

(Alternatives considered:

- Install the specified version of Python as part of the pre-commit hook. This would be ideal. Unfortunately `pre-commit` passes the specified `language_version` to `python -m virtualenv` under the hood, and that doesn't seem to offer a way to do this.

- Based on the above: something with `uv`? Unfortunately `pre-commit` have also elected *not* to support `uv` (pre-commit/pre-commit#3131, pre-commit/pre-commit#3222), which would have done this automatically. Maybe we just need to wait until the folks at Astral write their own version of pre-commit as well!

- Specify a range of versions for Python, so that we use whatever the system Python is, as long as it is below 3.13. Unfortunately pre-commit doesn't seem to support this.

- Write our own local hook that does whatever we damn well please: `uv` to installs the right version of Python, downloads pyright, and run it. If this becomes problematic amongst the other repos then I may well do this.
)
@patrick-kidger patrick-kidger merged commit 90626ee into main Jan 5, 2025
1 check passed
@patrick-kidger patrick-kidger deleted the no-more-python-version-in-ci branch January 5, 2025 14:06
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.

1 participant