refactor(ruff): switch from `select` to `ignore` #1412

cmp0xff · 2025-10-07T13:59:53Z

ruff rules usually improve the code quality. By specifying ignore rather than select, we get better hints how to comply to more rules.

In many cases this reduces errors in pyright strict, hence contributing to #1171.

For example, if we comply to ANN201, test functions will be foreced to have -> None, which reduces pyright strict errors. This was discussed in #1405 (comment).

cmp0xff · 2025-10-07T14:51:03Z

Maybe we can freeze versions of all type checkers and upgrade manually to prevent this

Dr-Irv · 2025-10-07T14:32:19Z

tests/extension/decimal/array.py

        # If the operator is not defined for the underlying objects,
        # a TypeError should be raised
-        res = [op(a, b) for (a, b) in zip(lvalues, rvalues)]
+        res = [op(a, b) for (a, b) in zip(lvalues, rvalues, strict=False)]


This is annoying. strict=False is the default, so we shouldn't have to specify it. Can you ignore that rule with ruff?

Dr-Irv · 2025-10-07T14:32:27Z

tests/indexes/test_indexes.py

    check(
-        assert_type(pd.MultiIndex.from_tuples(zip([1, 2], [3, 4])), pd.MultiIndex),
+        assert_type(
+            pd.MultiIndex.from_tuples(zip([1, 2], [3, 4], strict=False)), pd.MultiIndex


same comment about zip

Dr-Irv · 2025-10-07T14:33:24Z

tests/series/test_series.py

    id = [1, 2, 3]
    value = ["a", "b", "c"]
-    df = pd.DataFrame(zip(id, value), columns=["id", "value"])
+    df = pd.DataFrame(zip(id, value, strict=False), columns=["id", "value"])


same comment about zip

Dr-Irv · 2025-10-07T14:37:17Z

tests/test_pandas.py

-        assert_type(
-            pd.concat(map(lambda _: s2, ["some_value", 3]), axis=1), pd.DataFrame
-        ),
+        assert_type(pd.concat((s2 for _ in ["some_value", 3]), axis=1), pd.DataFrame),


Not sure of this change. I think we explicitly wanted to use map and lambda here

Dr-Irv · 2025-10-07T14:37:27Z

tests/test_pandas.py

            "B1970": {0: 2.5, 1: 1.2, 2: 0.7},
            "B1980": {0: 3.2, 1: 1.3, 2: 0.1},
-            "X": dict(zip(range(3), np.random.randn(3))),
+            "X": dict(zip(range(3), np.random.randn(3), strict=False)),


same zip comment

Dr-Irv · 2025-10-07T14:55:15Z

pyproject.toml

+  "PD",      # Pandas is here
+  "PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/
+  "S101",    # https://docs.astral.sh/ruff/rules/assert/
+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/


I think we can limit this to one file tests/test_io.py

Dr-Irv · 2025-10-07T14:56:13Z

pyproject.toml

+  "PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/
+  "S101",    # https://docs.astral.sh/ruff/rules/assert/
+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/
+  "SLF001",  # https://docs.astral.sh/ruff/rules/private-member-access/


I think we should aim to fix this one so put it in the second list below

Moved below

Dr-Irv · 2025-10-07T15:03:12Z

pyproject.toml

+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/
+  "SLF001",  # https://docs.astral.sh/ruff/rules/private-member-access/
+  # The following rules are ignored temporarily. Either fix them or move to the permanent list above.
+  "ANN", "ARG", "ERA", "RUF", "SIM", "TRY", "PT", "NPY", "N", "DTZ", "PLR", "TC", "PGH", "PTH", "S311", "C901", "FIX", "TD", "A001", "PYI042", "ANN201"


FIX and TD relates to TODO, so we can do a per-file ignore

Surprised that ANN201 isn't already taken care of because that's the one that requires return types on functions

FIX and TD now more detailed

ANN201 will be fixed in a separate PR

Dr-Irv · 2025-10-07T15:04:48Z

tests/__init__.py

            and ... not in tuple_args  # fixed-length tuple
-            and (arr_ndim := getattr(actual, "ndim"))
-            != (expected_ndim := len(tuple_args))
+            and (arr_ndim := actual.ndim) != (expected_ndim := len(tuple_args))  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]


This is annoying, because getattr() is a good usage here. If there is a RUFF rule that is causing this fix, let's ignore it on a per-file basis.

getattr is no better than ., it also raises an error when the attribute is not there. Nevertheless, f251900

Dr-Irv · 2025-10-07T15:05:30Z

tests/__init__.py

            and isinstance((expected_dtype := dtype_args[0]), type)
            and issubclass(expected_dtype, np.generic)
-            and (arr_dtype := getattr(actual, "dtype")) != expected_dtype
+            and (arr_dtype := actual.dtype) != expected_dtype  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]


same comment about doing a per-file ignore

getattr is no better than ., it also raises an error when the attribute is not there. Nevertheless, f251900

Dr-Irv · 2025-10-07T15:13:41Z

Maybe we can freeze versions of all type checkers and upgrade manually to prevent this

We can do it on a per-type-checker basis, and if we see that a new version has these kinds of failures, we can pin it. And report it to them so they are aware.

We have pinned mypy because when they released changes, it tended to be disruptive, and we definitely want to fix those issues. But they are better now, we probably don't need to pin mypy any more. They test mypy with pandas-stubs.
We don't pin pyright, because they they release changes, there are typically only a few things we might have to deal with, or they have introduced a bug. They also test pyright with pandas-stubs.
pyrefly and ty are supposed to be tested with pandas-stubs (just the stub file), so if a new version of either of those report a bunch of errors like the above, we should report it to them, and then pin the version until they fix it.

Dr-Irv

I don't think we should do those pyrefly ignores, because they are in disagreement with both mypy and pyright.

I also disagree with them reporting the bad-param-name-override on methods like __getitem__() and __iter__().

We should report this to them, and if we need to, add something in our config to ignore the rules you inserted in the commit #6847e8657878201653b49f401c18b72e680b9eb9 and then put the ignore (for now) in pyproject.toml

…12/files#r2410872686

…12/files/85ab0f97c59b7aa7f5f58d5323f91f6063fb192c#r2410965434

cmp0xff

For pyrefly, I added a TODO in ace7a24

cmp0xff · 2025-10-07T22:07:38Z

tests/extension/decimal/array.py

        # If the operator is not defined for the underlying objects,
        # a TypeError should be raised
-        res = [op(a, b) for (a, b) in zip(lvalues, rvalues)]
+        res = [op(a, b) for (a, b) in zip(lvalues, rvalues, strict=False)]


cmp0xff · 2025-10-07T22:07:43Z

tests/indexes/test_indexes.py

    check(
-        assert_type(pd.MultiIndex.from_tuples(zip([1, 2], [3, 4])), pd.MultiIndex),
+        assert_type(
+            pd.MultiIndex.from_tuples(zip([1, 2], [3, 4], strict=False)), pd.MultiIndex


cmp0xff · 2025-10-07T22:07:48Z

tests/series/test_series.py

    id = [1, 2, 3]
    value = ["a", "b", "c"]
-    df = pd.DataFrame(zip(id, value), columns=["id", "value"])
+    df = pd.DataFrame(zip(id, value, strict=False), columns=["id", "value"])


cmp0xff · 2025-10-07T22:11:24Z

tests/__init__.py

            and ... not in tuple_args  # fixed-length tuple
-            and (arr_ndim := getattr(actual, "ndim"))
-            != (expected_ndim := len(tuple_args))
+            and (arr_ndim := actual.ndim) != (expected_ndim := len(tuple_args))  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]


getattr is no better than ., it also raises an error when the attribute is not there. Nevertheless, f251900

cmp0xff · 2025-10-07T22:11:33Z

tests/__init__.py

            and isinstance((expected_dtype := dtype_args[0]), type)
            and issubclass(expected_dtype, np.generic)
-            and (arr_dtype := getattr(actual, "dtype")) != expected_dtype
+            and (arr_dtype := actual.dtype) != expected_dtype  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]


getattr is no better than ., it also raises an error when the attribute is not there. Nevertheless, f251900

pyproject.toml

cmp0xff · 2025-10-07T22:16:44Z

pyproject.toml

+  "PYI042",  # https://docs.astral.sh/ruff/rules/snake-case-type-alias/
+  "TD002",   # https://docs.astral.sh/ruff/rules/missing-todo-author/
+  # The following rules are ignored temporarily. Either fix them or move to the permanent list above.
+  "ERA", "S", "ANN", "FIX002", "TD003", "TD004", "PLR0402", "PLC0105"


S and most of ERA removed

I don't think we'll comply to FIX002. It is fine for me to leave TODOs.

There are a few TODOs without link. We can fix them by adding issues.

TD004 fixed

cmp0xff · 2025-10-07T22:16:53Z

pyproject.toml

+  "PD",      # Pandas is here
+  "PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/
+  "S101",    # https://docs.astral.sh/ruff/rules/assert/
+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/


cmp0xff · 2025-10-07T22:17:03Z

pyproject.toml

+  "PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/
+  "S101",    # https://docs.astral.sh/ruff/rules/assert/
+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/
+  "SLF001",  # https://docs.astral.sh/ruff/rules/private-member-access/


Moved below

cmp0xff · 2025-10-07T22:20:05Z

pyproject.toml

+  "S301",    # https://docs.astral.sh/ruff/rules/suspicious-pickle-usage/
+  "SLF001",  # https://docs.astral.sh/ruff/rules/private-member-access/
+  # The following rules are ignored temporarily. Either fix them or move to the permanent list above.
+  "ANN", "ARG", "ERA", "RUF", "SIM", "TRY", "PT", "NPY", "N", "DTZ", "PLR", "TC", "PGH", "PTH", "S311", "C901", "FIX", "TD", "A001", "PYI042", "ANN201"


FIX and TD now more detailed

ANN201 will be fixed in a separate PR

cmp0xff · 2025-10-07T22:23:06Z

/pandas_nightly

cmp0xff · 2025-10-07T22:23:18Z

/mypy_nightly

Dr-Irv

Thanks @cmp0xff

cmp0xff added 6 commits October 7, 2025 15:56

chore(ruff): add rules

f99f004

refactor(ruff): safe

c664a0d

refactor(ruff): unsafe

d120bea

fix: typing

1b34b99

fix: conftest

85ab0f9

fix: pyrefly

6847e86

Dr-Irv requested changes Oct 7, 2025

View reviewed changes

Dr-Irv reviewed Oct 7, 2025

View reviewed changes

cmp0xff added 10 commits October 7, 2025 23:33

fix: pyrefly

06f1136

fix(comment): C417 https://github.com/pandas-dev/pandas-stubs/pull/14…

3e7bb89

…12/files#r2410872686

chore(ruff): SLF001

faa516e

feat(ruff): S301

2e81ce6

fix(ruff): TD004

5cb4685

chore(ruff): ERA, S

83518db

fix(ruff): tweak rules

b6394f9

chore: pyrefly and mypy

ace7a24

fix(comment): B009 https://github.com/pandas-dev/pandas-stubs/pull/14…

f251900

…12/files/85ab0f97c59b7aa7f5f58d5323f91f6063fb192c#r2410965434

chore: ANN

0a322ba

cmp0xff commented Oct 7, 2025

View reviewed changes

cmp0xff requested a review from Dr-Irv October 7, 2025 22:23

Dr-Irv approved these changes Oct 8, 2025

View reviewed changes

Dr-Irv merged commit 1cdecd9 into pandas-dev:main Oct 8, 2025
28 checks passed

cmp0xff deleted the feature/cmp0xff/ruff-negative-list branch October 8, 2025 13:50

Uh oh!

refactor(ruff): switch from select to ignore #1412

refactor(ruff): switch from select to ignore #1412

Uh oh!

Conversation

cmp0xff commented Oct 7, 2025

Uh oh!

cmp0xff commented Oct 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dr-Irv commented Oct 7, 2025

Uh oh!

Dr-Irv left a comment

Choose a reason for hiding this comment

Uh oh!

cmp0xff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmp0xff commented Oct 7, 2025

Uh oh!

cmp0xff commented Oct 7, 2025

Uh oh!

refactor(ruff): switch from `select` to `ignore` #1412

refactor(ruff): switch from `select` to `ignore` #1412