-
Notifications
You must be signed in to change notification settings - Fork 61
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 #766
Conversation
Any file changes here that aren't just removing a "3.9" or changing it to a "3.10" were made automatically by There are also these additional new
@grlee77 how would you like to handle these? I can do the work of either applying or ignoring those suggestions, but will defer to how you want the code to look. |
Thanks @jameslamb, I am in favor of accepting the suggestions related to UP007 unions for type hints. I think we should add
to the ruff config in |
While updating pyproject.toml, it seems we should update the ruff section names to match current recommendations. Here is a patch including the suggested UP038 ignore: diff --git a/python/cucim/pyproject.toml b/python/cucim/pyproject.toml
index 2df37c5a..c664659b 100644
--- a/python/cucim/pyproject.toml
+++ b/python/cucim/pyproject.toml
@@ -147,9 +147,6 @@ filterwarnings = [
]
[tool.ruff]
-# see: https://docs.astral.sh/ruff/rules/
-select = ["E", "F", "W", "I", "UP"]
-fixable = ["ALL"]
exclude = [
# TODO: Remove this in a follow-up where we fix __all__.
".tox",
@@ -166,10 +163,16 @@ exclude = [
line-length = 80
fix = true
-[tool.ruff.per-file-ignores]
+[tool.ruff.lint]
+# see: https://docs.astral.sh/ruff/rules/
+select = ["E", "F", "W", "I", "UP"]
+ignore = ["UP038"]
+fixable = ["ALL"]
+
+[tool.ruff.lint.per-file-ignores]
# "src/cucim/skimage/util/tests/test_shape.py" = ["E201", "E202"]
-[tool.ruff.isort]
+[tool.ruff.lint.isort]
combine-as-imports = true
force-single-line = false
known-first-party = ["cucim"] |
Thanks @grlee77 ! I've applied changes similar to that patch you provided. Using the new format of There's one class of new error that I'd like your opinion on. They look like this:
Are you comfortable with full list of those cases (click me)
|
Thanks James! 🙏 Generally those suggestions sound reasonable. Would be interested in Greg's thoughts as well With the In [1]: import numpy as np
In [2]: np.dtype(bool) == bool
Out[2]: True
In [3]: np.dtype(bool) is bool
Out[3]: False |
Thanks, I think some of those suggestions may be unwanted. Can we just ignore these |
Yep absolutely! I'll ignore add that code to the list |
Put up #767 for you. |
Skimmed through the changes. Generally these all look like solid improvements If Greg agrees and CI passes, we should ship it Thanks for all your hard work here James! 🙏 |
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 James, the changes look great!
Thanks all! 🙏 Let's ship this |
/merge |
Description
Contributes to rapidsai/build-planning#88
Finishes the work of dropping Python 3.9 support.
This project stopped building / testing against Python 3.9 as of rapidsai/shared-workflows#235.
This PR updates configuration and docs to reflect that.
Notes for Reviewers
How I tested this
Checked that there were no remaining uses like this:
And similar for variations on Python 3.8 (to catch things that were missed the last time this was done).