-
Notifications
You must be signed in to change notification settings - Fork 901
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
Drop Python 3.9 support #16637
Drop Python 3.9 support #16637
Changes from all commits
8fb0823
6fd04eb
8ef260e
ec74d27
987ce3f
fad288f
cf4ab5b
d7e9b7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,7 +17,7 @@ authors = [ | |||||||||||
{ name = "NVIDIA Corporation" }, | ||||||||||||
] | ||||||||||||
license = { text = "Apache 2.0" } | ||||||||||||
requires-python = ">=3.9" | ||||||||||||
requires-python = ">=3.10" | ||||||||||||
dependencies = [ | ||||||||||||
"polars>=1.0,<1.3", | ||||||||||||
"pylibcudf==24.10.*,>=0.0.0a0", | ||||||||||||
|
@@ -28,7 +28,6 @@ classifiers = [ | |||||||||||
"Topic :: Scientific/Engineering", | ||||||||||||
"License :: OSI Approved :: Apache Software License", | ||||||||||||
"Programming Language :: Python", | ||||||||||||
"Programming Language :: Python :: 3.9", | ||||||||||||
"Programming Language :: Python :: 3.10", | ||||||||||||
"Programming Language :: Python :: 3.11", | ||||||||||||
] | ||||||||||||
|
@@ -62,7 +61,7 @@ exclude_also = [ | |||||||||||
[tool.ruff] | ||||||||||||
line-length = 88 | ||||||||||||
indent-width = 4 | ||||||||||||
target-version = "py39" | ||||||||||||
target-version = "py310" | ||||||||||||
fix = true | ||||||||||||
|
||||||||||||
[tool.ruff.lint] | ||||||||||||
|
@@ -115,6 +114,9 @@ ignore = [ | |||||||||||
"TD003", # Missing issue link on the line following this TODO | ||||||||||||
# tryceratops | ||||||||||||
"TRY003", # Avoid specifying long messages outside the exception class | ||||||||||||
# pyupgrade | ||||||||||||
"UP035", # Import from `collections.abc` instead: `Callable` | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything else in git grep -E 'import.*Callable' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confused by this one, is this an auto-upgrade fix that ruff can do for us? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this is one But either way... it recommended this for
cudf/python/cudf_polars/pyproject.toml Line 89 in 8d6b261
All of the other Python code in the repo only has specific Lines 87 to 90 in 8d6b261
Happy to do whatever you want me to with this particular check, I don't have a strong preference for either pattern so I chose the one that would at least be consistent with the rest of the Python code in this repo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wence- could you please let me know what you want to do with this one, and if there's anything else I could help clarify? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would propose deferring this change to a separate PR unless we can knock it out today. Completing the drop of Python 3.9 is important so we can start adding Python 3.12. We want all these platform changes done before we get too close to burndown. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok sure, happy to do that in a follow-up PR after this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Put up #16670 making this change across |
||||||||||||
"UP038", # Use `X | Y` in `isinstance` call instead of `(X, Y)` | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @grlee77 pointed out to me that there's actually a performance penalty associated with using |
||||||||||||
# Lints below are turned off because of conflicts with the ruff | ||||||||||||
# formatter | ||||||||||||
# See https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules | ||||||||||||
|
@@ -137,6 +139,10 @@ fixable = ["ALL"] | |||||||||||
|
||||||||||||
[tool.ruff.lint.per-file-ignores] | ||||||||||||
"**/tests/**/*.py" = ["D"] | ||||||||||||
"**/cudf_polars/typing/__init__.py" = [ | ||||||||||||
# pyupgrade | ||||||||||||
"UP007", # Use `X | Y` for type annotations | ||||||||||||
] | ||||||||||||
jameslamb marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
||||||||||||
[tool.ruff.lint.flake8-pytest-style] | ||||||||||||
# https://docs.astral.sh/ruff/settings/#lintflake8-pytest-style | ||||||||||||
|
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.
TypeAlias
moved totyping
in Python 3.10: https://docs.python.org/3/library/typing.html#typing.TypeAliasThere are still other imports from
typing_extensions
incudf-polars
though, so we can't drop the dependency on it.