Improving ruff setup #224
Improving ruff setup #224
Conversation
Let's remove all rules so far, and re-introduce those by following the definitions of pylint. The first rule is target-version infered from the package definition that requires-python >= 3.8.
Patterns defined at .python-lint lines 10 & 14.
Current setup defined between lines 57-105.
Defining `max-nested-blocks` as .pyhton-lint line 141.
While pylint defines `expected-line-ending-format`, ruff uses `line-ending` instead. Defined in .python-lint line 254.
Equivalent to single-line-if-stmt at line 278 of .python-lint
We don't need that with ruff, but here is a placeholder if we want to add more than default `logger` in the future.
Addressing "init-import=no" in line 402 from .python-lint
Interestingly, max-args is defined as 5 in line 434, but PLR0913 rule is ignored.
To reflect DESIGN section in .python-lint, pages 431-463.
Ruff doesn't implement the exactly the same, but E722 is good enough if not even better.
sup3r do not follow: - Q000 bad-quotes-inline-string - Q004 unnecessary-escaped-quote
sup3r do not follow I001: unsorted-imports.
sup3r doesn't follow NPY002, but we might want to re-consider that in the future.
sup3r doesn't follow: - UP009: utf8-encoding-declaration - UP015: redundant-open-modes - UP032: f-string
sup3r doesn't conform with, but should reconsider that in the future: - A001: builtin-variable-shadowing - A002: builtin-argument-shadowing
sup3r doesn't conform with: - ARG002: unused-method-argument - ARG003: unused-class-method-argument - ARG004: unused-static-method-argument - ARG005: unused-lambda-argument
sup3r doesn't conform with: - COM812: missing-trailing-comma
sup3r doesn't conform with: - N802: invalid-function-name - N803: invalid-argument-name - N806: non-lowercase-variable-in-function
sup3r doesn't conform with: - D105: undocumented-magic-method - D200: fits-on-one-line - D202: no-blank-line-after-function - D204: one-blank-line-after-class - D205: blank-line-after-summary - D207: under-indentation - D209: new-line-after-last-paragraph - D400: ends-in-period - D401: non-imperative-mood - D404: docstring-starts-with-this
sup3r doesn't conform with: - PLR2004: magic-value-comparison - PLW2901: redefined-loop-name
| "UP009", # utf8-encoding-declaration | ||
| "UP015", # redundant-open-modes | ||
| "UP032", # f-string |
There was a problem hiding this comment.
this one is for @grantbuster ;)
As suggested by @bnb32 sup3r doesn't conform with: - C408: unnecessary-collection-call - C414: unnecessary-double-cast-or-process
|
hey @bnb32 and @castelao, nice work and thanks for prioritizing this. There's too many codes and new configs for me to learn and review right now but seems like you two have been having a productive dialogue which is what matters. Let's get this merged in when you two are happy and see what it does with the open PRs and we can tweak as we go. |
As suggested by @bnb32 Using max-complexity as previously defined for flake8 (lintesrs/.flake8) as equal to 12.
|
@castelao One more thing. Lets add |
@bnb32 any other rule that you would like to include?
Done! |
Without going in details on the rules, I tried to reproduce the best I could the previous setup, but there is not always a 1:1 match. After that, I added some standard rules that don't break the current sup3r, which means that those rules were already followed. In summary, there is no change in the pattern of sup3r here, but instead we should have rules that match sup3r. Some specific rules I had to ignore, and a few of them I added a suggestion to review the decision. |
Sounds great 👍 |
There were some misterious situations where super-linter was not properly checking the code. Paul mentioned a setup that limits checks to the very last commit only. I expect that VALIDATE_ALL_CODEBASE should address that.
castelao
force-pushed
the
ruff_for_sup3r
branch
5 times, most recently
from
65ef8a0
to
e1a08ec
Compare
As suggested by @bnb32. sup3r doesn't conform with: - SIM108: if-else-block-instead-of-if-exp - SIM117: multiple-with-statements - SIM118: in-dict-keys - SIM211: if-expr-with-false-true
As suggested by @bnb32 sup3r doesn't conform with: - PERF102: incorrect-dict-iterator - PERF203: try-except-in-loop - PERF401: manual-list-comprehension
|
@bnb32 @grantbuster , new rules setup is working fine. This is just a start. We can later review each one of the specific rules been ignored, but it runs as it is now. There is just one strange error that the former super-linter is complaining in a yaml file that I didn't edit in this PR. It's not clear to me what is the issue there. I suggest we ignore that and move on to merge this PR. I think this is a great start and I really wanted to get back to PresRat. |
With the pull_request trigger we don't need VALIDATE_ALL_CODEBASE set to true anymore. Otherwise it's running twice. Thanks @ppinchuk!
While conforming with the legacy setup, I added a few explicit specific rules but in the final version didn't make sense anymore since those are now implicit. For instance, no need of "E701" is added "E".
|
@grantbuster @bnb32 , passing all tests. Just waiting on your review. |
Sounds good to me. What yaml file is it complaining about? yaml files should be excluded from linting anyway imo |
|
@grantbuster , I guess you're also OK with this PR? Since @bnb32 already approved, I'll merge this one so I can use it in PresRat. |
* Redefining ruff, starting with target-version = "py38" Let's remove all rules so far, and re-introduce those by following the definitions of pylint. The first rule is target-version infered from the package definition that requires-python >= 3.8. * Excluding CSV and ref.*?py Patterns defined at .python-lint lines 10 & 14. * Disabling rules according to .python-lint Current setup defined between lines 57-105. * max-nested-blocks Defining `max-nested-blocks` as .pyhton-lint line 141. * Confirming with expected-line-ending-format While pylint defines `expected-line-ending-format`, ruff uses `line-ending` instead. Defined in .python-lint line 254. * Defining line-lenght as .python-lint line 267 * indent-width * multiple-statements-on-one-line-colon Equivalent to single-line-if-stmt at line 278 of .python-lint * task-tags In reference to notes, line 291 of .python-lint * In respect t ologging-modules, line 285 of .python-lint We don't need that with ruff, but here is a placeholder if we want to add more than default `logger` in the future. * unused imports Addressing "init-import=no" in line 402 from .python-lint * Defining max-args Interestingly, max-args is defined as 5 in line 434, but PLR0913 rule is ignored. * Defining DESIGN explicit setup To reflect DESIGN section in .python-lint, pages 431-463. * Closest option of satisfy EXCEPTIONS Ruff doesn't implement the exactly the same, but E722 is good enough if not even better. * Re-introducing all (W) Warning rules * Re-introducing (Q) flake8-quotes rules sup3r do not follow: - Q000 bad-quotes-inline-string - Q004 unnecessary-escaped-quote * Re-introducing (I) isort rules sup3r do not follow I001: unsorted-imports. * Re-introducing (NPY) NymPy-specific rules sup3r doesn't follow NPY002, but we might want to re-consider that in the future. * style: Conforming with UP039 * Re-introducing (UP) pyupgrade rules sup3r doesn't follow: - UP009: utf8-encoding-declaration - UP015: redundant-open-modes - UP032: f-string * Re-introducing (A) flake8-builtins sup3r doesn't conform with, but should reconsider that in the future: - A001: builtin-variable-shadowing - A002: builtin-argument-shadowing * Re-introducing (ARG) flake8-unused-arguments rules sup3r doesn't conform with: - ARG002: unused-method-argument - ARG003: unused-class-method-argument - ARG004: unused-static-method-argument - ARG005: unused-lambda-argument * Re-introducing (E) pycodestyle rules * Re-introducing (F) Pyflakes rules * Re-introducing (COM) flake8-commas rules sup3r doesn't conform with: - COM812: missing-trailing-comma * Introducing (N) pep8-naming rules sup3r doesn't conform with: - N802: invalid-function-name - N803: invalid-argument-name - N806: non-lowercase-variable-in-function * Introducing (D) pydocstyle rules sup3r doesn't conform with: - D105: undocumented-magic-method - D200: fits-on-one-line - D202: no-blank-line-after-function - D204: one-blank-line-after-class - D205: blank-line-after-summary - D207: under-indentation - D209: new-line-after-last-paragraph - D400: ends-in-period - D401: non-imperative-mood - D404: docstring-starts-with-this * Introducing (PL) Pylint rules sup3r doesn't conform with: - PLR2004: magic-value-comparison - PLW2901: redefined-loop-name * Running ruff with GA * Nothing is fixable for now * Using ruff with pre-commmit * style: Conforming with NPY003 & NPY201 * style: Conforming with COM819 * Forgot to include: quote-style & indent-style * Updating requirements on ruff * GA outputs ruff with github style * Adding (C4) flake8-comprehensions As suggested by @bnb32 sup3r doesn't conform with: - C408: unnecessary-collection-call - C414: unnecessary-double-cast-or-process * Adding rule (C90) mccabe As suggested by @bnb32 Using max-complexity as previously defined for flake8 (lintesrs/.flake8) as equal to 12. * Adding convention (C) and flake8-logging (LOG) @bnb32 any other rule that you would like to include? * validate all codebase with super-linter There were some misterious situations where super-linter was not properly checking the code. Paul mentioned a setup that limits checks to the very last commit only. I expect that VALIDATE_ALL_CODEBASE should address that. * Adding (SIM) flake8-simplify rules As suggested by @bnb32. sup3r doesn't conform with: - SIM108: if-else-block-instead-of-if-exp - SIM117: multiple-with-statements - SIM118: in-dict-keys - SIM211: if-expr-with-false-true * Adding (PERF) Perflint rules As suggested by @bnb32 sup3r doesn't conform with: - PERF102: incorrect-dict-iterator - PERF203: try-except-in-loop - PERF401: manual-list-comprehension * We don't need to run on the full code base anymore With the pull_request trigger we don't need VALIDATE_ALL_CODEBASE set to true anymore. Otherwise it's running twice. Thanks @ppinchuk! * clean: Removing implicit rules While conforming with the legacy setup, I added a few explicit specific rules but in the final version didn't make sense anymore since those are now implicit. For instance, no need of "E701" is added "E".
Isolating ruff setup into its own PR to make it easier to review. PR #215 missed a few criteria without warning and hopefully we'll be able to identify those after this PR.
These are not my choices. I reviewed all rules to best match the former Pylint setup. We don't have a perfect match on the rules from Pylint vs Ruff, so it is the closest as possible. After that, a few standard rules were added, and can be easily removed. Several specific rules were included to conform with the current code, and some of those might be worth re-consider and conform.
By using
pyproject.tomlto setup, it should unify the checks, i.e. the checks on GA should be identical to the local run, which was not true with the previous pylint setup.This new check was added with the GA workflow together with the former procedure. We might keep that one for some time for comparison. We most probably will find different checking results which will guide the choices for this repository. Eventually, pylint and flake8 could be removed.