Reduce ruff warnings #778
Reduce ruff warnings #778
Conversation
Based on the below warnings emitted in CI: warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`: - 'extend-per-file-ignores' -> 'lint.extend-per-file-ignores' warning: `incorrect-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `incorrect-blank-line-before-class`. warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
via `pre-commit run --all-files`
docs/CHANGELOG.md
Outdated
| ../CHANGELOG.md |
There was a problem hiding this comment.
I'll revert that second commit.
My pre-commit (on Windows) added newlines to the end of the symlink "files", e.g. compare this with this (if you select all, you see the extra character)
But more importantly, as on macOS, pre-commit didn't do the same, and it's just a symlink, it's not worth it, hence the revert.
This reverts commit 143c3ac.
| "D203", # incorrect-blank-line-before-class | ||
| "D213", # multi-line-summary-second-line |
There was a problem hiding this comment.
Were there previously errors that these were catching? Main is clean in terms of lints?
There was a problem hiding this comment.
So these two were already ignored, because they are conficting two other rules (and hence would catch "errors" if they were active), as the below warning within the CI runs states (e.g. here):
- 'extend-per-file-ignores' -> 'lint.extend-per-file-ignores'
warning: `incorrect-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `incorrect-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
Therefore it comes down to your preference:
- Do you want an extra newline between class definition and docstring? (D203 vs D211)
- Within the docstring, do you want the summary line to start on the first or second line? (D212 vs D213)
In this commit I made the choices so that the existing codebase still passes. If you were to invert either or both of them, then that would require an (automatic) edit all the docstrings elsewhere too.
There was a problem hiding this comment.
Oh... I see.
- Prefer no extra newline between class def and docstring
- Prefer summary line on the first line (d212)
There was a problem hiding this comment.
Then I think it's all OK:
- the repo is already following these preferences
- D203 and D213 would have gone against these preferences -- but they were already ignored as they were conflicting with their opposite rules -- but now with this PR, there won't even be a warning emitted about this clash.
|
Thank you! |
nit: minor changes to clean up ruff / pre-commit output