pre-commit: Replace flake8, isort, and pyupgrade with ruff #584
Conversation
|
Who can take the decision to drop python 3.6 ? It's been EOL for a long time at this point. |
|
Yes we can |
|
#567 drops py36 |
|
Ruff looks nice if it can replace the all the tools with a single config. Are there any downsides with ruff or anything that would need attention? |
|
I have not seen downsides (except maybe it is not yet v1.0) and I have been using it broadly: |
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Apart from the fact that without fix it's not equivalent to having pyupgrade LGTM.
|
I've played a bit with ruff now, its isort support is incomplete (no support for default-section or src-paths at least). The old isort hook needs to be kept. |
|
This cannot be achieved by using forced separate, known parties, known local folder and src? If not then I can drop the
|
.ruff.toml
Outdated
| "dist", | ||
| ] | ||
| line-length = 140 | ||
| select = [ |
There was a problem hiding this comment.
Shouldn't this be extend-select instead?
There was a problem hiding this comment.
If you're wondering how to configure Ruff, here are some recommended guidelines:
- Prefer select and ignore over extend-select and extend-ignore, to make your rule set explicit.
There was a problem hiding this comment.
Well all I want is to not hardcode a ruleset (which select = [stuff] exactly is). I don't think any of those recommendations are worth considering blindly given the tool's age and popularity.
How about select = ["ALL"] and ignore whatever doesn't make sense for pytest-cov?
There was a problem hiding this comment.
If you're wondering how to configure Ruff, here are some recommended guidelines:
- Prefer select and ignore over extend-select and extend-ignore, to make your rule set explicit.
- Use
ALLwith discretion. EnablingALLwill implicitly enable new rules whenever you upgrade.- Start with a small set of rules (
select = ["E", "F"]) and add a category at-a-time. For example, you might consider expanding toselect = ["E", "F", "B"]to enable the popular flake8-bugbear extension.- By default, Ruff's autofix is aggressive. If you find that it's too aggressive for your liking, consider turning off autofix for specific rules or categories (see FAQ).
There was a problem hiding this comment.
There are way too many categories to enable manually. I'd expect that there are more good checks than broken or bad ones.
I noticed it's aggressive ... or rather destructive. That's why I think --show-fixes is a must. I think avoiding --fix actually adds more friction (too much back and forth for avoidable manual stuff).
There was a problem hiding this comment.
Ok so I've played with it a bit more.... now I am not really sure what the best way... eg: these are either invalid checks or would be a pain to sort out, or not even worth fixing:
ignore = [
"D", # pydocstyle
"ANN", # flake8-annotations
"ARG", # flake8-unused-arguments
"BLE", # flake8-blind-except
"C90", # mccabe
"EM", # flake8-errmsg
"FBT", # flake8-boolean-trap
"INP", # flake8-no-pep420
"PLR", # Pylint Refactor
"PLW", # Pylint Warning
"Q", # flake8-quotes
"RET", # flake8-return
"SIM102", # flake8-simplify collapsible-if
"SIM105", # flake8-simplify use-contextlib-suppress
"SLF", # flake8-self
"T20", # flake8-print
"S101", # flake8-bandit assert
"S102", # flake8-bandit exec
"S110", # flake8-bandit try-except-pass
"TRY", # tryceratops
]
Now considering that rsync features are being dropped in xdist ... a lot of bulk using that in pytest-cov becomes unnecessary, if it isn't already as some people are suggesting pytest-cov is not necessary for getting xdist support...
I guess select with a few is fine for now.
There was a problem hiding this comment.
So... How should we proceed? Given your explorations and your knowledge of the codebase, do you want to open a new PR that cherry-picks from this one? I have no trouble closing this with unmerged commits. Or should I press on?
There was a problem hiding this comment.
I'd also recommend the C4 checks as they are popular, increase performance, and have autofixes enabled for all rules.
There was a problem hiding this comment.
Is there somewhere proof that C4 mods increase performance?
|
As the Ruff docs say, |
6 participants
Ruff supports over
400500 lint rules including flake8, isort, pylint, and pyupgrade and is written in Rust for speed.Also added rules for pylint conventions and pylint errors.