MNT Enforce ruff rules (RUF) #30694

DimitriPapadopoulos · 2025-01-21T23:12:07Z

What does this implement/fix? Explain your changes.

Enforce Ruff-specific rules (RUF) rules.

Any other comments?

Some rules have been disabled: either they don't seem that interesting, or they change too much of the codebase.

[doc skip]

github-actions · 2025-01-21T23:13:17Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 0aec43b. Link to the linter CI: here}

jeremiedbb · 2025-01-23T13:15:03Z

Thanks for the PR @DimitriPapadopoulos. These rules mostly summarize to ordering __all__ and properly deal with str and repr in fstrings. I'm okay with that. Some comments though

I checked enabling RUF021 and there was no error. Maybe we can remove it from the exceptions ? This rule is "Parenthesize a and b expressions when chaining and and or together, to make the precedence clear" which I think is fine to enforce.
RUF015 seems good to have imo. I always find these list(some_generator)[0] a bit clunky. I checked enabling it and there are not many occurrences to fix so maybe worth to remove it from the exceptions ?
How come we can remove all these noqa ?

sklearn/neighbors/_base.py

DimitriPapadopoulos · 2025-01-23T13:24:44Z

Of course, I can enable RUF015 and RUF021. Some maintainers don't like them, so I tend not to apply them.

The useless noqa must have been left by previous linters or versions of ruff. Some ruff rules have been deprecated over time. Also, the maximum line length might have changed. Or perhaps reformatting with black reduces line length.

lesteve · 2025-01-28T09:47:37Z

@DimitriPapadopoulos I think you have contributed useful stuff in the past to the project, thanks a lot 🙏!

In this case it looks like you opened multiple PRs on separate aspects on cleaning up/maintenance. Sometimes splitting PRs is good, and sometimes as a busy maintainer it can feel a bit too much.

I would suggest you keep only one PR open and close the others, the one you think is the most important, wait for feed-back, iterate until it is merged, and then open another PR ...

My personal opinion on this kind of PRs, is that they don't have huge benefits and they tend to be controversial. People will have different opinions on whether it is worth it or not, and at the end significant reviewer and contributor time has been spent on something that probably was not so crucial for the project and this time could have been invested somewhere else with a more useful outcome ...

DimitriPapadopoulos · 2025-03-11T13:23:36Z

@lesteve Other maintainers insist on smaller PRs - tastes and colors.

While it's possible to discuss individual rules, linters do result in more maintainable code overall, and sometimes find actual bugs. Perhaps the benefit is not huge, but the cost is small. As for the controversial part of rules, choosing a mainstream linter (or formatter) gets controversy out of the way.

As for closing the PRs, now that they're already opened, I don't know.

warning: Invalid `# noqa` directive: expected `:` followed by a comma-separated list of codes (e.g., `# noqa: F401, F841`).

RUF010 Use explicit conversion flag

RUF015 Prefer `next(...)` over single element slice

RUF017 Avoid quadratic list summation

RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear

RUF022 `__all__` is not sorted

RUF100 Unused blanket `noqa` directive

RUF002 Docstring contains ambiguous ` ` (NO-BREAK SPACE). Did you mean ` ` (SPACE)?

thomasjpfan

For a reviewer, it is a lot of different types of changes to go through. I think if we want to enforce more linting rules, it's best to introduce them one at a time. i.e. One small PR on sorting __all__, one on removing unncessary # noqa, etc and slowly introduce them one at a time.

In any case, given we already have this PR, I'm okay with enforcing the proposed rules.

thomasjpfan · 2025-03-13T02:51:38Z

sklearn/metrics/pairwise.py

@@ -341,7 +341,7 @@ def euclidean_distances(

    Notes
    -----
-    To achieve a better accuracy, `X_norm_squared` and `Y_norm_squared` may be
+    To achieve a better accuracy, `X_norm_squared` and `Y_norm_squared` may be


I can not tell what this change is. A unicode character?

With a single commit per rule, there's the workaround of reviewing the commits individually.

In this case, I manually changed a NO-BREAK SPACE U+00A0 into a regular SPACE U+0020 in 0aec43b, to silence ambiguous-unicode-character-docstring (RUF002).

adrinjalali

LGTM. You need to add a this to .git-blame-ignore-revs after merging this.

DimitriPapadopoulos · 2025-03-18T11:56:25Z

LGTM. You need to add a this to .git-blame-ignore-revs after merging this.

Do you expect me to add the commits to .git-blame-ignore-revs in an upcoming PR? I could do that for all recent ruff-related PRs.

Note however that while these are mostly automatic ruff fixes, I manually fixed some issues.

adrinjalali · 2025-03-18T12:10:48Z

Yes, we prefer to completely ignore styling changes to keep the blame relevant in case we need to know who worked on a feature.

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695)

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695) * black → ruff format (scikit-learn#31015)

DimitriPapadopoulos force-pushed the RUF branch from 2d924a4 to 69b657d Compare January 21, 2025 23:16

DimitriPapadopoulos changed the title ~~MAINT Enforce ruff rules (RUF)~~ MNT Enforce ruff rules (RUF) Jan 21, 2025

jeremiedbb reviewed Jan 23, 2025

View reviewed changes

sklearn/neighbors/_base.py Outdated Show resolved Hide resolved

DimitriPapadopoulos force-pushed the RUF branch 2 times, most recently from ce9a5d8 to 61501d4 Compare January 23, 2025 13:42

DimitriPapadopoulos force-pushed the RUF branch from 61501d4 to 6464e56 Compare March 11, 2025 13:29

DimitriPapadopoulos added 9 commits March 11, 2025 18:21

MNT Address ruff warning

aaa1447

warning: Invalid `# noqa` directive: expected `:` followed by a comma-separated list of codes (e.g., `# noqa: F401, F841`).

MNT Apply ruff rule RUF010

33b9831

RUF010 Use explicit conversion flag

MNT Apply ruff rule RUF015

8c5d15f

RUF015 Prefer `next(...)` over single element slice

MNT Apply ruff rule RUF017

89ff0b7

RUF017 Avoid quadratic list summation

MNT Apply ruff rule RUF021

8d7d3e8

RUF021 Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear

MNT Apply ruff rule RUF022

ccd0da3

RUF022 `__all__` is not sorted

MNT Apply ruff rule RUF100

c09fdca

RUF100 Unused blanket `noqa` directive

MNT Enforce ruff rules (RUF)

c534b49

MNT Apply ruff rule RUF002

0aec43b

RUF002 Docstring contains ambiguous ` ` (NO-BREAK SPACE). Did you mean ` ` (SPACE)?

DimitriPapadopoulos force-pushed the RUF branch from 1511480 to 0aec43b Compare March 11, 2025 17:22

thomasjpfan added the No Changelog Needed label Mar 13, 2025

thomasjpfan approved these changes Mar 13, 2025

View reviewed changes

adrinjalali approved these changes Mar 18, 2025

View reviewed changes

adrinjalali merged commit fe7c417 into scikit-learn:main Mar 18, 2025
36 of 37 checks passed

DimitriPapadopoulos deleted the RUF branch March 18, 2025 11:56

DimitriPapadopoulos added a commit to DimitriPapadopoulos/scikit-learn that referenced this pull request Mar 19, 2025

MNT git ignore recent black/ruff updates

cc98cc5

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695)

DimitriPapadopoulos mentioned this pull request Mar 19, 2025

MNT git ignore recent black/ruff updates #31026

Merged

DimitriPapadopoulos added a commit to DimitriPapadopoulos/scikit-learn that referenced this pull request Mar 19, 2025

MNT git ignore recent black/ruff updates

cf06efd

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695)

DimitriPapadopoulos added a commit to DimitriPapadopoulos/scikit-learn that referenced this pull request Mar 24, 2025

MNT git ignore recent black/ruff updates

03943a8

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695)

DimitriPapadopoulos added a commit to DimitriPapadopoulos/scikit-learn that referenced this pull request Apr 15, 2025

MNT git ignore recent black/ruff updates

dc99887

* Enforce ruff rules (RUF) (scikit-learn#30694) * Apply ruff/flake8-implicit-str-concat rules (ISC) (scikit-learn#30695)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MNT Enforce ruff rules (RUF) #30694

MNT Enforce ruff rules (RUF) #30694

DimitriPapadopoulos commented Jan 21, 2025

github-actions bot commented Jan 21, 2025 •

edited

Loading

jeremiedbb commented Jan 23, 2025

DimitriPapadopoulos commented Jan 23, 2025 •

edited

Loading

lesteve commented Jan 28, 2025

DimitriPapadopoulos commented Mar 11, 2025

thomasjpfan left a comment •

edited

Loading

thomasjpfan Mar 13, 2025

DimitriPapadopoulos Mar 13, 2025 •

edited

Loading

adrinjalali left a comment

DimitriPapadopoulos commented Mar 18, 2025 •

edited

Loading

adrinjalali commented Mar 18, 2025

MNT Enforce ruff rules (RUF) #30694

MNT Enforce ruff rules (RUF) #30694

Conversation

DimitriPapadopoulos commented Jan 21, 2025

What does this implement/fix? Explain your changes.

Any other comments?

github-actions bot commented Jan 21, 2025 • edited Loading

✔️ Linting Passed

jeremiedbb commented Jan 23, 2025

DimitriPapadopoulos commented Jan 23, 2025 • edited Loading

lesteve commented Jan 28, 2025

DimitriPapadopoulos commented Mar 11, 2025

thomasjpfan left a comment • edited Loading

Choose a reason for hiding this comment

thomasjpfan Mar 13, 2025

Choose a reason for hiding this comment

DimitriPapadopoulos Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

adrinjalali left a comment

Choose a reason for hiding this comment

DimitriPapadopoulos commented Mar 18, 2025 • edited Loading

adrinjalali commented Mar 18, 2025

github-actions bot commented Jan 21, 2025 •

edited

Loading

DimitriPapadopoulos commented Jan 23, 2025 •

edited

Loading

thomasjpfan left a comment •

edited

Loading

DimitriPapadopoulos Mar 13, 2025 •

edited

Loading

DimitriPapadopoulos commented Mar 18, 2025 •

edited

Loading