search

nilearn / nilearn

Machine learning for NeuroImaging in Python
http://nilearn.github.io
Other
1.19k stars 605 forks source link

Rerformating with Ruff #4579

Open Remi-Gau opened 1 month ago

Remi-Gau commented 1 month ago

I am listing them here for more transparency and so that things are not just in my head.

After this I think, some more rules could be removed from the ignored list and autofixed by running pre-commit run -a and see how many files are changed and what the changes are.

I would probably start looking into:

List of errors

status number of errors error code autofixable error name
- [ ] 560 N806 [ ] non-lowercase-variable-in-function
- [ ] 184 ERA001 [*] commented-out-code
- [ ] 164 PLR2004 [ ] magic-value-comparison
- [ ] 135 B028 [ ] no-explicit-stacklevel
- [ ] 115 N803 [ ] invalid-argument-name
- [ ] 17 RUF012 [ ] mutable-class-default
- [ ] 34 B904 [ ] raise-without-from-inside-except
- [ ] 8 D105 [ ] undocumented-magic-method
- [ ] 5 E741 [ ] ambiguous-variable-name
- [ ] 3 ARG005 [ ] unused-lambda-argument
- [ ] 3 PERF203 [ ] try-except-in-loop

In progress

status number of errors error code autofixable error name

Probably won't fix

Fixing the following ones would mess up our doc rendering.

status number of errors error code autofixable error name
- [ ] 309 D205 [ ] blank-line-after-summary
- [ ] 61 D208 [*] over-indentation
- [ ] 1 D301 [*] escape-sequence-in-docstring

To fix later (maybe)

issues related to code complexity - silenced for now by adapting the threshold

status number of errors error code autofixable error name
- [ ] 216 PLR0913 [ ] too-many-arguments
- [ ] 48 PLR0912 [ ] too-many-branches
- [ ] 23 PLR0915 [ ] too-many-statements
- [ ] 3 PLR0911 [ ] too-many-return-statements

Fixed

status number of errors error code autofixable error name
- [x] 41 B018 [ ] useless-expression
- [x] 2 SIM117 [ ] multiple-with-statements
- [x] 37 PD011 [ ] pandas-use-of-dot-values
- [x] 57 N802 [ ] invalid-function-name
- [x] 3 PLR1704 [ ] redefined-argument-from-local
- [x] 14 PD901 [ ] pandas-df-variable-name
- [x] 5 ARG003 [ ] unused-class-method-argument
- [x] 41 ARG001 [ ] unused-function-argument
- [x] 34 ARG002 [ ] unused-method-argument
- [x] 81 RUF005 [ ] collection-literal-concatenation
- [x] 9 RUF002 [ ] ambiguous-unicode-character-docstring
- [x] 2 RUF003 [ ] ambiguous-unicode-character-comment
- [x] 2 N813 [ ] camelcase-imported-as-lowercase
- [x] 1 N816 [ ] mixed-case-variable-in-global-scope
- [x] 1 N817 [ ] camelcase-imported-as-acronym
- [x] 1 N818 [ ] error-suffix-on-exception-name
- [x] 7 SIM102 [ ] collapsible-if
- [x] 4 SIM105 [ ] suppressible-exception
- [x] 1 SIM115 [ ] open-file-with-context-handler
- [x] 6 PERF401 [ ] manual-list-comprehension
- [x] 2 PERF102 [*] incorrect-dict-iterator
- [x] 1 PERF403 [ ] manual-dict-comprehension
- [x] 1 PD003 [ ] pandas-use-of-dot-is-null
- [x] 4 PD002 [*] pandas-use-of-inplace-argument
- [x] 2 NPY002 [ ] numpy-legacy-random
- [x] 1 SIM103 [*] needless-bool
- [x] 1 SIM114 [*] if-with-same-arms
- [x] 8 SIM108 [*] if-else-block-instead-of-if-exp
- [x] 37 SIM118 [*] in-dict-keys
- [x] 3 SIM201 [*] negate-equal-op
- [x] 8 SIM300 [*] yoda-conditions
- [x] 1 PLR1714 [*] repeated-equality-comparison
- [x] 2 PLR1730 [*] if-stmt-min-max
- [x] 2 PLR1736 [*] unnecessary-list-index-lookup
- [x] 10 PLR5501 [*] collapsible-else-if
- [x] 2 PLR0402 [*] manual-from-import
- [x] 1 C419 [*] unnecessary-comprehension-in-call
- [x] 8 C414 [*] unnecessary-double-cast-or-process
- [x] 7 C416 [*] unnecessary-comprehension
- [x] 87 C408 [*] unnecessary-collection-call
- [x] 135 D209 [*] new-line-after-last-paragraph
- [x] 15 RUF100 [*] unused-noqa
- [x] 20 RUF010 [*] explicit-f-string-type-conversion
- [x] 7 RUF015 [*] unnecessary-iterable-allocation-for-first-element
- [x] 1 FLY002 [*] static-join-to-f-string
- [x] 10 UP031 [*] printf-string-formatting
PrakharJain1509 commented 1 month ago

I have done removed of the PTH errors. Can I send a pull request now, or do I have to complete the whole list before doing so?

Remi-Gau commented 1 month ago

@PrakharJain1509

PrakharJain1509 commented 1 month ago

@Remi-Gau I have resolved 102, 107, 110, 113, 100, 111, 101, 201, 109, 114, 115, 117. Can you please assign them to me and then I can send you a PR.

Remi-Gau commented 1 month ago

OK I have assigned those to you. To make things easier: do you you could split things into separate PRs? By the number of fixes I would do one PR for each of the following group.

PrakharJain1509 commented 1 month ago

@Remi-Gau Actually I have resolved them(The 12 mentioned above) all and pushed Into a branch on my fork, So could I directly create a PR? All of those are resolved and It is not throwing any error.

Remi-Gau commented 1 month ago

So could I directly create a PR?

A single PR may make it hard to review... Let's see...

Open the PR and then we may rather split the PR by module or subpacklage rather than by error.

Remi-Gau commented 1 month ago
SamiLaayat commented 3 weeks ago

I am an engineering student, and I would like to contribute to this issue. Is it possible to assign myself to it?

Remi-Gau commented 3 weeks ago

hey @SamiLaayat which error from the "List of errors" were you thinking of working on? I discourage people from working on ALL of them at once. Better to pick one error and focus on this one in one pull request before moving to the next.

One that could improve the quality of life of users is fixing the B028 one: https://docs.astral.sh/ruff/rules/no-explicit-stacklevel/

See this video for more context: https://youtu.be/tZSEZ2WG5w8?feature=shared&t=220

Remi-Gau commented 3 weeks ago

The problem with B028 is that there are a LOT of instances to fix and most of them will require to figure the call stack for a given warning and adapt the stacklevel accordingly, so if you want to go for that one start by doing a couple of them to see how long this may take you.