Use ruff to lint Python code #518
Conversation
cclauss
force-pushed
the
ruff
branch
2 times, most recently
from
1a20cf3 to
00f15fe
Compare
There was a problem hiding this comment.
ruff is a great tool. This allows dev workflows with constant linting. It makes no significant difference to ci times as flake 8 runs in parallel with the tests and finishes first.
Django uses flake8 but I don't see a good reason why this library needs to wait for django to move
I've looked through the changes and can't see anything which should break.
| ] | ||
| ignore = ["F401"] | ||
|
|
||
| [tool.ruff.lint.mccabe] |
There was a problem hiding this comment.
question: why complexity 23 ?
There was a problem hiding this comment.
Set it to 22 and see what happens...
% pipx run ruff check --config mccabe.max-complexity=22
| line-length = 120 | ||
| target-version = "py38" | ||
|
|
||
| [tool.ruff.lint] |
There was a problem hiding this comment.
question: why this set of lint rules?
There was a problem hiding this comment.
These are the ones that currently pass. Future PRs can uncomment rules that might be of interest. We do not want this PR to contain so many changes that it slows the review process.
| @@ -41,16 +41,15 @@ def render(self, context): | |||
| def handle_token(cls, parser, token, kind, condition): | |||
| bits = token.split_contents() | |||
| if len(bits) < 2: | |||
| raise template.TemplateSyntaxError("%r tag requires an argument" % | |||
| bits[0]) | |||
| raise template.TemplateSyntaxError(f"{bits[0]!r} tag requires an argument") | |||
There was a problem hiding this comment.
suggestion: use repr(bits[0]) for readability in the spirit of https://peps.python.org/pep-0498/#s-r-and-a-are-redundant
There was a problem hiding this comment.
This PR is about automatic changes, not manual changes. Turning on different rules may make other automatic changes.
|
I think the pre-commit.yaml file needs to be updated as well |
|
|
||
| [tool.ruff.lint.per-file-ignores] | ||
| "docs/conf.py" = ["INP001"] | ||
| "test_app/models.py" = ["DJ008"] # FIXME |
There was a problem hiding this comment.
praise: nice to highlight this in the comment
|
|
||
| [tool.ruff.lint.pylint] | ||
| allow-magic-value-types = ["float", "int", "str"] | ||
| max-args = 6 # default is 5 |
There was a problem hiding this comment.
praise: nice to have the defaults mentioned here as a possible future target for refactoring
| "ICN", # flake8-import-conventions | ||
| "INP", # flake8-no-pep420 | ||
| "INT", # flake8-gettext | ||
| "NPY", # NumPy-specific rules |
There was a problem hiding this comment.
nit: this library doesn't use numpy or pandas and seems unlikely to start doing so so these could be omitted for a minor gain in readability
There was a problem hiding this comment.
this library doesn't use numpy or pandas and seems unlikely to start doing so
Famous last words. You never know when code will be forked or when contributors will have new ideas. Given that ruff can lint the entire CPython codebase in less than a third of a second with no cache, I take the stance that if a family of rules pass then run them.
This reverts commit 60c3dc1.
This reverts commit dfae8be.
This reverts commit 70a5751.
Ruff supports over 800 lint rules and can be used to replace Flake8 (plus dozens of plugins), isort, pydocstyle, yesqa, eradicate, pyupgrade, and autoflake, all while executing (in Rust) tens or hundreds of times faster than any individual tool.
ruff check --output-format=githubprovides intuitive GitHub Annotations to contributors...Related to
@ulgens