Use ruff instead of flake8 to check PEP8 #29762
Conversation
83ec31d to
accaf12
Compare
|
I can see the case for leaving the |
Fair. We should then add a comment to the file that we don't use it anymore. |
There was a problem hiding this comment.
Should decide on whether to keep .flake8 or not. Also, IIRC, @ksunden added the ruff config and did not make it the primary thing because it didn't support everything yet. I suppose he can confirm whether this is true now or not.
|
Ruff implements
Of the
I'm not sure what E901 means, but I understand it's a Python syntax error that I'm 90% sure would be caught just by importing a file. For the rest, I'm not sure how much we care about keeping strictly to these visual indentation rules, since we do not enforce a code formatter (e.g., black) at the moment anyway). My two cents is that it's worth sacrificing these rules for switching to ruff and providing the benefits of being able to autofix issues. Also worth noting that On keeping |
|
It seems we should just remove flake8 if we don't use it? Developers who still need it for something could still copy one into their directory. Given that there is no way to deprecate its use, it seems the easiest way to let people know is to remove it. |
|
The only two rules I saw as "missing" when I last checked was E122 and E302, which is why they are listed as the "external rules" (which prevents warnings for e.g. ignore comments on those rules) Looking into it, looks like E302 is now implemented, so it really is just E122, which has to do with indentation on continuation lines, which is relatively niche to begin with. Personally, I'd be good with swapping to ruff only at this point. I had been hoping for 100% parity, but we are so close it's probably good enough. |
0eaee63 to
a93d55e
Compare
|
Consensus seems to be this is ready to go then. The last open question seems to be keeping the |
a93d55e to
d53121d
Compare
|
Ok, let's go. |
PR summary
This switches the way we check PEP8 from using
flake8toruff. There are two main motivations for this:ruffandflake8in the repository.ruff, PEP8 violations can in many instances be automatically fixed (fixes [ENH]: Add way to automatically fix flake8 errors #27588).For me 2) is a big developer win - instead of having to manually fix many PEP8 violations, I can just run either
ruff check --fix, orpre-commit --all-filesto automatically make the required fixes.The initial commit here raised a PEP8 error, to check if
reviewdogis working correctly - it is 🎉 , and the second commit fixes the error.PR checklist