Update to ruff (instead of pylint (and isort)) #192
Conversation
Remove pylint as a dependency. Remove all "# pylint: disable=..." statements. Remove pylint and isort as pre-commit hooks. Add ruff as a pre-commit hook. Avoid running pylint in the CI. Add a 'ruff' CI job. Add (back) disable E501 statements. E501: Line too long. Add it for either lines with string-formatted type (which has to be on one line) or an f-string with a single variable (i.e., a single-line f-string part with an implemented logic for a variable that results in the line being too long).
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
+ Coverage 81.06% 81.29% +0.22%
==========================================
Files 12 12
Lines 898 909 +11
==========================================
+ Hits 728 739 +11
Misses 170 170 ☔ View full report in Codecov by Sentry. |
Ruff is already run as part of pre-commit.
This is a good point - something I've thought glancingly about as well. |
Extend arguments to ruff CLI, ensuring no unsafe fixes are made and any fixes are shown explicitly in the output.
Extend rule set for job on code base to include flake8-blind-except and pylint. Ignore self assignment of variable rule. Ignore all too-many-* rules from pylint, because we will extend the recommended number of branches, statements, function args, etc. in this code base. Otherwise, run with a rule set that includes pycodestyle, pyflakes, pyupgrade, flake8-bugbear, flake8-simplify, isort, and ruff.
So, in relation to this comment, I have now tried to explicitly define some rules. It indeed again ended up in having to have two separate pre-commit jobs (if we want linting on non-code base files as well, i.e., tests). This has ended up showing that the previous pylint rules were indeed not checked (but are now). I have disabled some of them globally (see the pre-commit config file), but otherwise, this rule set should be configured specifically for the package. The full list of ruff rules to be used can be found here. |
Also: - Remove pyupgrade (UP) rule for ruff. An actual pyupgrade hook should be used instead. - Remove bandit (S) rule for ruff hooks for core code base. The bandit hook takes care of these. - Update hook versions. - Switch hook activation order; run black before ruff. - Add python_version-specific pre-commit version in the 'dev' extra. - Add a typing.NamedTuple for an ignore entry pair. This is used to minimize mypy ignore statements.
Enforce importing __future__.annotations in all Python files. Update code accordingly.
|
For reference, this may be a nice guide on which rules to use (and general Python code styling) https://learn.scientific-python.org/development/guides/style/. |
Or rather, base it on scientific-python and then update it to fit this package. Update the code base accordingly.
There was a problem hiding this comment.
I didn't notice any comments that weren't already covered by @francescalb (and addressed)
Closes #191
Remove pylint as a dependency.
Remove all "# pylint: disable=..." statements.
Remove pylint and isort as pre-commit hooks.
Add ruff as a pre-commit hook.
Avoid running pylint in the CI.
Add a 'ruff' CI job.
Add (back) disable E501 statements.
E501: Line too long.
Add it for either lines with string-formatted type (which has to be on one line) or an f-string with a single variable (i.e., a single-line f-string part with an implemented logic for a variable that results in the line being too long).
Also, add pyupgrade as a pre-commit hook and enfore importing
__future__.annotationsin all Python files.