Drop Python 3.9 support #16637

jameslamb · 2024-08-21T22:43:47Z

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:

git grep -E '3\.9'
git grep '39'
git grep 'py39'

And similar for variations on Python 3.8 (to catch things that were missed the last time this was done).

jameslamb · 2024-08-21T22:51:49Z

There are a bunch of new ruff errors here:

python/cudf_polars/cudf_polars/containers/dataframe.py:108:38: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/containers/dataframe.py:138:50: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/containers/dataframe.py:169:29: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:21:1: UP035 [*] Import from `collections.abc` instead: `Callable`
python/cudf_polars/cudf_polars/dsl/ir.py:309:32: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:429:51: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:547:12: UP038 Use `X | Y` in `isinstance` call instead of `(X, Y)`
python/cudf_polars/cudf_polars/dsl/ir.py:551:14: UP038 Use `X | Y` in `isinstance` call instead of `(X, Y)`
python/cudf_polars/cudf_polars/dsl/ir.py:603:24: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:605:50: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:755:33: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:764:33: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:803:52: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:912:31: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:977:52: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/dsl/ir.py:1092:43: B905 [*] `zip()` without an explicit `strict=` parameter
python/cudf_polars/cudf_polars/typing/__init__.py:16:5: UP035 [*] Import from `collections.abc` instead: `Callable`
python/cudf_polars/cudf_polars/typing/__init__.py:18:5: UP035 [*] Import from `typing` instead: `TypeAlias`
python/cudf_polars/cudf_polars/typing/__init__.py:22:17: UP007 [*] Use `X | Y` for type annotations
python/cudf_polars/cudf_polars/typing/__init__.py:42:19: UP007 [*] Use `X | Y` for type annotations
python/cudf_polars/cudf_polars/utils/sorting.py:48:27: B905 [*] `zip()` without an explicit `strict=` parameter

(build link)

The zip() ones look related to these comments:

cudf/python/cudf_polars/cudf_polars/containers/dataframe.py

Lines 137 to 138 in bf2ee32

    
           # TODO: strict=True when we drop py39 
        
           [NamedColumn(c, name) for c, name in zip(table.columns(), names)]

cudf/python/cudf_polars/cudf_polars/containers/dataframe.py

Lines 168 to 169 in bf2ee32

    
           # TODO: strict=True when we drop py39 
        
           for c, other in zip(self.columns, like.columns)

Could use some help from someone more familiar with cudf-polars (@wence- ?).

vyasr · 2024-08-22T06:17:34Z

I didn't realize ruff had this rule implemented. That's nice. The comments are related, but are there because I didn't expect a linter to enforce this for us.

The strict=True parameter was added in Python 3.10, so I'm guessing that you are seeing this issue now because you bumped the requires-python in pyproject.toml and ruff uses that to determine what rules to enforce because this particular rule would be unenforceable if you supported Python<3.9. I would go ahead and add strict=True and see what, if anything, breaks. Hopefully nothing does.

…hon-3.9

jameslamb · 2024-08-22T16:42:02Z

python/cudf_polars/pyproject.toml

@@ -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`


Everything else in cudf uses typing.Callable.

git grep -E 'import.*Callable'

Confused by this one, is this an auto-upgrade fix that ruff can do for us?

I'm not sure if this is one ruff will do automatically if asked.

But either way... it recommended this for cudf_polars but not for the rest of cudf, because cudf_polars has its own ruff config separate from the rest of cudf.

cudf_polars uses all the pyupgrade checks

cudf/python/cudf_polars/pyproject.toml

Line 89 in 8d6b261

"UP", # pyupgrade

All of the other Python code in the repo only has specific pyupgrade checks run over it

cudf/pyproject.toml

Lines 87 to 90 in 8d6b261

# non-pep585-annotation

"UP006",

# non-pep604-annotation

"UP007"

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.

@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?

typing.Callable is the deprecated alias, so in cudf-polars can we migrate to using collections.abc.Callable?
I realise in cudf-classic this is more work, so we can defer doing it there. Thanks!

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.

Ok sure, happy to do that in a follow-up PR after this.

in a follow-up PR after this

Put up #16670 making this change across cudf.

jameslamb · 2024-08-22T16:43:22Z

python/cudf_polars/pyproject.toml

@@ -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`
+  "UP038",  # Use `X | Y` in `isinstance` call instead of `(X, Y)`


@grlee77 pointed out to me that there's actually a performance penalty associated with using | instead of a tuple in isinstance().

rapidsai/cucim#766 (comment)

python/cudf_polars/pyproject.toml

jameslamb · 2024-08-22T16:50:04Z

python/cudf_polars/cudf_polars/typing/__init__.py

-    from typing import Callable
-
-    from typing_extensions import TypeAlias
+    from typing import Callable, TypeAlias


TypeAlias moved to typing in Python 3.10: https://docs.python.org/3/library/typing.html#typing.TypeAlias

There are still other imports from typing_extensions in cudf-polars though, so we can't drop the dependency on it.

jameslamb · 2024-08-22T16:52:58Z

I would go ahead and add strict=True and see what, if anything, breaks. Hopefully nothing does.

Alright sounds good, I've done that in recent commits. Just wasn't sure if there were places where we were knowingly relying on the non-strict behavior (i.e. zip()-ing over objects of different lengths).

I've addressed everything ruff complained about in cudf_polars. Left inline comments for some things to provide more context.

bdice

Two missed py39s noted in comments -- otherwise LGTM!

python/cudf_polars/cudf_polars/containers/dataframe.py

Co-authored-by: Bradley Dice <[email protected]>

vyasr · 2024-08-22T18:36:16Z

Alright sounds good, I've done that in recent commits. Just wasn't sure if there were places where we were knowingly relying on the non-strict behavior (i.e. zip()-ing over objects of different lengths).

I seriously doubt it, and if we are I would want to find out about it!

wence- · 2024-08-23T16:19:30Z

question: should we upgrade ruff in pre-commit as part of these changes? I haven't tried recently if there are any additional issues. If it produces lots of warnings, let's not and we can do it separately.

wence-

cudf-polars changes all look good, with one tiny question about the typing -> collections.abc import ignore.

wence- · 2024-08-23T16:21:23Z

python/cudf_polars/pyproject.toml

@@ -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`


Confused by this one, is this an auto-upgrade fix that ruff can do for us?

python/cudf_polars/pyproject.toml

jameslamb · 2024-08-23T16:49:31Z

question: should we upgrade ruff in pre-commit as part of these changes? I haven't tried recently if there are any additional issues. If it produces lots of warnings, let's not and we can do it separately.

I just tried updating it to the latest version (0.6.2). 41 new non-auto-fixable errors were raised, mainly the those about using == to compare dtypes, similar to rapidsai/cucim#767.

I think we should not try to upgrade ruff as part of this PR.

jameslamb · 2024-08-27T16:02:18Z

/merge

Follow-up to #16637. Once this project's minimum support Python version was bumped up to Python 3.10, `ruff` started raising this error from `pyupgrade`: ```text Import from `collections.abc` instead: `Callable` ``` * ruff docs: https://docs.astral.sh/ruff/rules/deprecated-import/ * `typing` docs saying that `typing.Callable` is deprecated starting in Python 3.9 https://docs.python.org/3/library/typing.html#typing.Callable * context: #16637 (comment) This proposes accepting that suggestion, so that `cudf` won't be broken whenever `Callable` is removed from the `typing` module. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Lawrence Mitchell (https://github.com/wence-) - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #16670

Drop Python 3.9 support

8fb0823

jameslamb added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Aug 21, 2024

github-actions bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. CMake CMake build issue cudf.polars Issues specific to cudf.polars labels Aug 21, 2024

jameslamb changed the title ~~Drop Python 3.9 support~~ WIP: Drop Python 3.9 support Aug 21, 2024

jameslamb mentioned this pull request Aug 21, 2024

Drop support for Python 3.9 rapidsai/build-planning#88

Closed

11 tasks

jameslamb added 2 commits August 22, 2024 11:16

Merge branch 'branch-24.10' of github.com:rapidsai/cudf into drop-pyt…

6fd04eb

…hon-3.9

address new ruff errors

8ef260e

jameslamb commented Aug 22, 2024

View reviewed changes

python/cudf_polars/pyproject.toml Show resolved Hide resolved

jameslamb commented Aug 22, 2024

View reviewed changes

jameslamb changed the title ~~WIP: Drop Python 3.9 support~~ Drop Python 3.9 support Aug 22, 2024

jameslamb marked this pull request as ready for review August 22, 2024 16:58

jameslamb requested review from a team as code owners August 22, 2024 16:58

jameslamb requested review from raydouglass, isVoid and lithomas1 August 22, 2024 16:58

bdice approved these changes Aug 22, 2024

View reviewed changes

python/cudf_polars/cudf_polars/containers/dataframe.py Outdated Show resolved Hide resolved

python/cudf_polars/cudf_polars/containers/dataframe.py Outdated Show resolved Hide resolved

Apply suggestions from code review

ec74d27

Co-authored-by: Bradley Dice <[email protected]>

Merge branch 'branch-24.10' into drop-python-3.9

987ce3f

wence- approved these changes Aug 23, 2024

View reviewed changes

merge branch-24.10

fad288f

jameslamb added 2 commits August 26, 2024 17:03

Merge branch 'branch-24.10' into drop-python-3.9

cf4ab5b

Merge branch 'branch-24.10' into drop-python-3.9

d7e9b7d

rapids-bot bot merged commit efa9770 into rapidsai:branch-24.10 Aug 27, 2024
87 checks passed

jameslamb deleted the drop-python-3.9 branch August 27, 2024 16:02

jameslamb mentioned this pull request Aug 27, 2024

switch from typing.Callable to collections.abc.Callable #16670

Merged

3 tasks

vyasr mentioned this pull request Aug 29, 2024

[ENH] Use strict=True argument to zip once py39 support is dropped #15835

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop Python 3.9 support #16637

Drop Python 3.9 support #16637

jameslamb commented Aug 21, 2024 •

edited

Loading

jameslamb commented Aug 21, 2024

vyasr commented Aug 22, 2024

jameslamb Aug 22, 2024

wence- Aug 23, 2024

jameslamb Aug 23, 2024

jameslamb Aug 26, 2024

wence- Aug 27, 2024 •

edited

Loading

bdice Aug 27, 2024 •

edited

Loading

jameslamb Aug 27, 2024

jameslamb Aug 27, 2024

jameslamb Aug 22, 2024

jameslamb Aug 22, 2024

jameslamb commented Aug 22, 2024

bdice left a comment

vyasr commented Aug 22, 2024

wence- commented Aug 23, 2024

wence- left a comment

wence- Aug 23, 2024

jameslamb commented Aug 23, 2024

jameslamb commented Aug 27, 2024

	# non-pep585-annotation
	"UP006",
	# non-pep604-annotation
	"UP007"

Drop Python 3.9 support #16637

Drop Python 3.9 support #16637

Conversation

jameslamb commented Aug 21, 2024 • edited Loading

Description

Notes for Reviewers

How I tested this

jameslamb commented Aug 21, 2024

vyasr commented Aug 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wence- Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

bdice Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameslamb commented Aug 22, 2024

bdice left a comment

Choose a reason for hiding this comment

vyasr commented Aug 22, 2024

wence- commented Aug 23, 2024

wence- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameslamb commented Aug 23, 2024

jameslamb commented Aug 27, 2024

jameslamb commented Aug 21, 2024 •

edited

Loading

wence- Aug 27, 2024 •

edited

Loading

bdice Aug 27, 2024 •

edited

Loading