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

Update Python version for the pre-commit hooks #844

Conversation

fabcor-maxiv
Copy link
Contributor

Since the Python range is >=3.8,<3.11 in conda-environment.yml (and in the Poetry config in pyproject.toml as well), the version installed by default will be Python 3.10.

It makes sense to use the same version in the pre-commit hooks.

GitHub: fix #836

Since the Python range is `>=3.8,<3.11` in `conda-environment.yml`
(and in the Poetry config in `pyproject.toml` as well),
the version installed by default will be Python 3.10.

It makes sense to use the same version in the pre-commit hooks.

GitHub: fix mxcube#836
Copy link
Collaborator

@rhfogh rhfogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this. The point of the pre-commit hooks is to run tests, right? And the agreed minimum version of Python is 3.8, right? Then, surely, you should run the tests under Python 3.8, so that you catch it if someone is using code that does not work in Python 3.8? Personally I would love it if we could upgrade the minimum version to something higher, but until we can do that, can we upgrade it in the tests?

Or have I misunderstood something?

@marcus-oscarsson
Copy link
Member

marcus-oscarsson commented Jan 25, 2024

I agree with @rhfogh and as I said previously if I'm not mistaken the pre-commit scripts runs in a separate environment specified by the pre-commit config file so it should not be effected by what's in the conda environment. That is also why I previously asked what the exact problem was so that we can deal with it correctly.

Edit: I might of-course be mistaken and there is a Python version issue to deal with but it then becomes a bit more complicated for the reason @rhfogh highlights above, but we need to know what the issue was.

@fabcor-maxiv fabcor-maxiv deleted the update-pre-commit-python-version branch January 26, 2024 07:55
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.

Old Python version (3.8) in pre-commit hooks
3 participants