-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Linting Package Replacement: Pycodestyle to Ruff #24994
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
Comments
Hi @Luca-Blight, thanks for proposing to work on this. If it's a simple swap of tools, then yes please go for it - |
@rgommers got it I will keep that in mind. Thanks for your response. |
I did a little experiment and ran
It would be a good amount of effort to solve all of these, but I think we should do it. A good plan would be to first replace @Luca-Blight Are you still planning to work on this? |
That's 2004 errors, with 789 "auto-fixable" (not sure how safe the
|
Remember to |
$ 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 --statistics
660 F401 [ ] unused-import
314 F405 [ ] undefined-local-with-import-star-usage
218 F811 [ ] redefined-while-unused
72 F841 [*] unused-variable
66 E402 [ ] module-import-not-at-top-of-file
55 E741 [ ] ambiguous-variable-name
51 E712 [*] true-false-comparison
48 E731 [*] lambda-assignment
33 E721 [ ] type-comparison
26 F403 [ ] undefined-local-with-import-star
15 F821 [ ] undefined-name
11 E714 [*] not-is-test
6 F822 [ ] undefined-export
2 E743 [ ] ambiguous-function-name
$ |
nice work @DimitriPapadopoulos 🙏 |
Pyflakes (
Would it be OK to initially silence Right now, the pycodestyle errors ( $ 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
$ I could easily fix pycodestyle warnings ( $ 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 --extend-select W --ignore F,E --statistics
147 W291 [*] trailing-whitespace
67 W293 [*] blank-line-with-whitespace
3 W292 [*] missing-newline-at-end-of-file
$ |
This sounds like a good idea to me! (disclaimer: not a NumPy maintainer) |
Sounds fine indeed to silence the F rules for now, and fix the rest then enable
Well, we're past that point now:) Seems like we're in a slightly more quiet period now, so there's nothing blocking doing this I think. |
The main issue is the capabilitiy of
I see two options:
Which option would you prefer? |
This is pretty much what we did in SciPy, although applying |
An additional issue is that I am not certain that @lucascolley Do you have more insight on that matter? Have you checked that, for example by running By the way, I guess Numpy maintainers wouldn't want to reformat Numpy with |
run "pycodestyle" on the Numpy codebase$ time pycodestyle --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=W -qq --statistics .
1 E101 indentation contains mixed spaces and tabs
1 E111 indentation is not a multiple of 4
1 E114 indentation is not a multiple of 4 (comment)
1 E116 unexpected indentation (comment)
8 E117 over-indented
27 E121 continuation line under-indented for hanging indent
148 E122 continuation line missing indentation or outdented
225 E123 closing bracket does not match indentation of opening bracket's line
25 E124 closing bracket does not match visual indentation
4 E125 continuation line with same indent as next logical line
288 E126 continuation line over-indented for hanging indent
546 E127 continuation line over-indented for visual indent
1176 E128 continuation line under-indented for visual indent
1 E129 visually indented line with same indent as next logical line
6 E131 continuation line unaligned for hanging indent
320 E201 whitespace after '['
42 E202 whitespace before ')'
123 E203 whitespace before ':'
2 E211 whitespace before '('
136 E221 multiple spaces before operator
3 E222 multiple spaces after operator
22 E225 missing whitespace around operator
3248 E226 missing whitespace around arithmetic operator
2 E227 missing whitespace around bitwise or shift operator
7 E228 missing whitespace around modulo operator
1095 E231 missing whitespace after ','
1769 E241 multiple spaces after ':'
153 E251 unexpected spaces around keyword / parameter equals
39 E261 at least two spaces before inline comment
247 E265 block comment should start with '# '
24 E266 too many leading '#' for block comment
9 E271 multiple spaces after keyword
7 E272 multiple spaces before keyword
51 E275 missing whitespace after keyword
23 E301 expected 1 blank line, found 0
797 E302 expected 2 blank lines, found 1
69 E303 too many blank lines (2)
2 E304 blank lines found after function decorator
87 E305 expected 2 blank lines after class or function definition, found 1
42 E306 expected 1 blank line before a nested definition, found 0
84 E402 module level import not at top of file
1837 E501 line too long (117 > 79 characters)
1 E701 multiple statements on one line (colon)
10 E704 multiple statements on one line (def)
51 E712 comparison to True should be 'if cond is True:' or 'if cond:'
24 E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
48 E731 do not assign a lambda expression, use a def
88 E741 ambiguous variable name 'l'
1 E743 ambiguous function definition 'I'
real 0m18,454s
user 0m18,410s
sys 0m0,030s
$ run "ruff check" on the Numpy codebase$ time 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 --select E --statistics .
785 E501 [ ] line-too-long
66 E402 [ ] module-import-not-at-top-of-file
55 E741 [ ] ambiguous-variable-name
51 E712 [*] true-false-comparison
48 E731 [*] lambda-assignment
33 E721 [ ] type-comparison
11 E714 [*] not-is-test
2 E743 [ ] ambiguous-function-name
1 E101 [ ] mixed-spaces-and-tabs
real 0m0,107s
user 0m0,390s
sys 0m0,080s
$ As you can see, |
Sorry, I can't really help there. We had already transitioned to ruff, but with lots of rules disabled. What I meant by "what we did" was that we used that process to enable ruff rules that had lots of failures.
TLDR: no, not right now. Long answer: astral-sh/ruff#7310 (comment) |
At least in SciPy, I don't think we were concerned about Ruff being less strict than Pycodestyle in places. I think we trusted that Ruff has caught all of the important rules (or at least thought that switching was a net gain overall). |
There has been discussion of adopting an greater linelength for NumPy. Excluding distutils and tools: length <= 79: 1783 A small increase in line length makes a significant difference. Black uses 88 by default, the longest line is 89. |
In SciPy, we adopted and enforce an 88 char limit. It seems to have worked pretty well without causing significant disruption. |
I wanted to suggest bumping to 88 chars as well. I'll prepare a PR... |
A note to NumPy maintainers: for larger style-only PRs, you may wish to utilise a file like https://github.com/scipy/scipy/blob/main/.git-blame-ignore-revs. |
Proposed new feature or change:
There is a newer package that has been implemented by other major packages such as FastAPI and Llama index which has overwhelming performance gains. I would like to make the change to Numpy if that's okay?
More info:
https://docs.astral.sh/ruff/
The text was updated successfully, but these errors were encountered: