-
Notifications
You must be signed in to change notification settings - Fork 41
MNT add python version requirement #289
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
Conversation
My development tools (uv) were complaining about the missing `requires-python` field in `pyproject.toml`. This adds a dependency on at least 3.11, which is what scipy and numpy currently demands.
pyproject.toml
Outdated
| ] | ||
| dynamic = ["version"] | ||
|
|
||
| requires-python = ">=3.8" |
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.
scikit-learn requires at least 3.9 (e.g classifier)
I don't know to which extent skglm would work with lower version
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.
3.8 has reached its EOL 6 months ago so go for 3.9 !
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.
Seems reasonable. I originally set this to 3.11 actually, but then saw this:
ERROR: Package 'skglm' requires a different Python: 3.8.1 not in '>=3.9'
in the circle CI test logs, so I figured you needed 3.8. But the error still remains so I guess it's something different.
Scipy and numpy both require at least 3.11 now, numba 3.10.
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.
I just bumped the version of Python used in our circleci workflow to 3.10 instead
|
@Badr-MOUFAD pytest fails because of something that we should fix, merge if OK for you |
Badr-MOUFAD
left a comment
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.
It doesn't hurt us to update this
skglm/.github/workflows/flake8.yml
Line 23 in 6309a6a
| python-version: 3.8 |
Only needed for code format checks.
LGTM, thnx @jolars @mathurinm 💪
Co-authored-by: mathurinm <[email protected]>
Co-authored-by: mathurinm <[email protected]>
Context of the PR
The
requires-pythonfield inpyproject.tomlis missing.Contributions of the PR
So this PR pins the python version and adds a couple of classifiers related to it for pypi.
Checks before merging PR
[ ] added documentation for any new feature[ ] added unit tests[ ] edited the what's new (if applicable)