numpy icon indicating copy to clipboard operation
numpy copied to clipboard

CI: pycodestyle → ruff

Open DimitriPapadopoulos opened this issue 1 year ago • 1 comments

Fixes https://github.com/numpy/numpy/issues/24994.

DimitriPapadopoulos avatar Aug 27 '24 21:08 DimitriPapadopoulos

Remaining pycodestyle errors (E), ignoring Pyflakes (F) issues:

$ ruff check . --exclude vendored-meson --exclude numpy/typing/tests/data --exclude numpy/typing/_char_codes.py --exclude numpy/__config__.py --exclude numpy/f2py --exclude numpy/distutils --ignore F --statistics
66	E402	[ ] module-import-not-at-top-of-file
55	E741	[ ] ambiguous-variable-name
51	E712	[*] true-false-comparison
48	E731	[*] lambda-assignment
$ 

DimitriPapadopoulos avatar Aug 29 '24 08:08 DimitriPapadopoulos

@charris Instead of fixing pycodestyle issues in #27308, I decided to fix ruff/pycodestyle errors (E), as my intent is to address #24994.

This is the current output in the main branch:

ruff check (main branch)
$ ruff check --select E --statistics
1748	E501	[ ] line-too-long
 125	E741	[ ] ambiguous-variable-name
  86	E402	[ ] module-import-not-at-top-of-file
  53	E731	[*] lambda-assignment
  51	E712	[*] true-false-comparison
  44	E701	[ ] multiple-statements-on-one-line-colon
  33	E721	[ ] type-comparison
  11	E713	[*] not-in-test
  11	E714	[*] not-is-test
   6	E702	[ ] multiple-statements-on-one-line-semicolon
   2	E401	[*] multiple-imports-on-one-line
   2	E743	[ ] ambiguous-function-name
   1	E101	[ ] mixed-spaces-and-tabs
   1	E703	[*] useless-semicolon
$ 

I think it's worth applying preview rules for two reasons:

  1. future-proof code base,
  2. moving from pycodestyle to ruff, try to keep most of the existing rules.
ruff check --preview (main branch)
$ ruff check --preview --select E --statistics
3366	E226	[*] missing-whitespace-around-arithmetic-operator
1748	E501	[ ] line-too-long
1442	E241	[*] multiple-spaces-after-comma
1167	E231	[*] missing-whitespace
1049	E302	[*] blank-lines-top-level
 697	E251	[*] unexpected-spaces-around-keyword-parameter-equals
 329	E201	[*] whitespace-after-open-bracket
 297	E203	[*] whitespace-before-punctuation
 281	E265	[*] no-space-after-block-comment
 225	E221	[*] multiple-spaces-before-operator
 145	E225	[*] missing-whitespace-around-operator
 140	E305	[*] blank-lines-after-function-or-class
 133	E303	[*] too-many-blank-lines
 129	E301	[*] blank-line-between-methods
 124	E741	[ ] ambiguous-variable-name
  90	E261	[*] too-few-spaces-before-inline-comment
  86	E402	[ ] module-import-not-at-top-of-file
  77	E275	[*] missing-whitespace-after-keyword
  53	E202	[*] whitespace-before-close-bracket
  53	E502	[*] redundant-backslash
  53	E731	[*] lambda-assignment
  51	E266	[*] multiple-leading-hashes-for-block-comment
  51	E712	[*] true-false-comparison
  49	E306	[*] blank-lines-before-nested-definition
  44	E701	[ ] multiple-statements-on-one-line-colon
  33	E721	[ ] type-comparison
  30	E211	[*] whitespace-before-parameters
  26	E272	[*] multiple-spaces-before-keyword
  14	E252	[*] missing-whitespace-around-parameter-equals
  13	E262	[*] no-space-after-inline-comment
  12	E222	[*] multiple-spaces-after-operator
  12	E271	[*] multiple-spaces-after-keyword
  11	E228	[*] missing-whitespace-around-modulo-operator
  11	E713	[*] not-in-test
  11	E714	[*] not-is-test
   8	E117	[ ] over-indented
   6	E702	[ ] multiple-statements-on-one-line-semicolon
   5	E116	[ ] unexpected-indentation-comment
   4	E227	[*] missing-whitespace-around-bitwise-or-shift-operator
   2	E114	[ ] indentation-with-invalid-multiple-comment
   2	E304	[*] blank-line-after-decorator
   2	E401	[*] multiple-imports-on-one-line
   2	E743	[ ] ambiguous-function-name
   1	E101	[ ] mixed-spaces-and-tabs
   1	E111	[ ] indentation-with-invalid-multiple
   1	E703	[*] useless-semicolon
$ 

This PR fixes all ruff/pycodestyle errors, except E501:

$ ruff check --preview --statistics
1138	E501	line-too-long
$ 

E501 errors are not easy to fix, and there are lots of them. I am not sure how to best handle this. Ideas?

DimitriPapadopoulos avatar Sep 27 '24 15:09 DimitriPapadopoulos

I could add # noqa: E501 at the end of all lines incriminated by ruff. These lines can be modified over time (or not).

DimitriPapadopoulos avatar Sep 27 '24 15:09 DimitriPapadopoulos

Needs rebase.

charris avatar Nov 03 '24 23:11 charris

Rebased.

There are > 1000 occurrences of E501, fixing them sounds unreasonable. And without the assistance of ruff format, it will be painful.

Not sure where the linter errors come from. From the command line:

$ ruff check --statistics --config ruff.toml 
$ 

DimitriPapadopoulos avatar Dec 11 '24 10:12 DimitriPapadopoulos

Needs rebase. Again, leave distutils alone, not only is it not maintained, it is fragile.

charris avatar Dec 14 '24 16:12 charris

Rebased.

I'm not sure why numpy/distutils has been modified, it has been added to exclude. 🤔

DimitriPapadopoulos avatar Dec 14 '24 17:12 DimitriPapadopoulos

I would not worry too much about the lint failures, but it would be good to fix them in another pr.

charris avatar Dec 14 '24 18:12 charris

Generally looks good to me. There are ~two~ three things that might need some discussion.

  • Replacing pycodestyle by ruff. I'm not opposed, and ruff seems actively maintained. Is there a problem with using ruff to only check the PR diffs?
  • Putting spaces around * and / seems to account for a majority of the changes. It isn't really needed, but sometimes it improves readability and I'm getting used to it. It has the advantage that it is an easy rule to teach and apply.
  • This is likely to break some previous PRs. That said, I think we will need to move in this direction at some point and this PR probably covers the noisiest changes.

charris avatar Dec 14 '24 21:12 charris

As far as I can see, ruff cannot run only on staged changes. See https://github.com/astral-sh/ruff/discussions/4049. That's why I have to fix so many warnings before switching to ruff.

Note that running pycodestyle against the whole codebase would probably detect the same issues.

Finally, https://github.com/astral-sh/ruff/discussions/4049 suggests workaround involving the use of darker, but I haven't tested it.

DimitriPapadopoulos avatar Dec 15 '24 13:12 DimitriPapadopoulos

Replacing pycodestyle with ruff is being discussed in #24994.

DimitriPapadopoulos avatar Dec 16 '24 08:12 DimitriPapadopoulos

Well, here goes. Thanks @DimitriPapadopoulos . Would be good to get those linter failures fixed up.

charris avatar Dec 16 '24 15:12 charris

Thank you @charris. I will try to fix the linter failures. Curiously, they don't occur when running ruff from directly from the command line. Perhaps a configuration issue.

DimitriPapadopoulos avatar Dec 16 '24 20:12 DimitriPapadopoulos