[nc-time-axis] QA refresh linter hooks #293
Conversation
- trailing-whitespace hook - CI chore - global file list
- Required manually disabling on @savefig lines in docs/examples.rst - Update files list to include doc/*.rst in filespre-commit-config.yaml
Includes Ruff autofixes: - 11 × D212 (multi-line-summary-first-line) - 1 × RET506 (superfluous-else-raise) - 1 × RET505 (superfluous-else-return) - 1 × I001 (unsorted-imports) Added `PLR5501` to exclude list to preserve readability of TODO block in convert method
- Applied some fixes to [tool.pytest.ini_options]
There was a problem hiding this comment.
Looking good, just a bit more annotations and I think we can merge this in. It's worth noting that some of the errors we were getting were due to a planned outage of mambaforge https://conda-forge.org/news/2024/07/29/sunsetting-mambaforge/. This problem fixed itself since today is not a planned outage day but this issue will still need to be resolved (probably in its own PR though).
| ] | ||
| enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] | ||
| strict = true | ||
| warn_unreachable = true |
There was a problem hiding this comment.
I'm guessing that the idea here is to signal that this is the intention while functionally turning this off with "unreachable" in the disabled error codes. I guess the alternative would be to have this be false but with a comment saying that's temporary. On reflection I think the approach here is probably best since it keeps all the TODOs in one place.
There was a problem hiding this comment.
I could comment it out and add a TODO, like I did with the redundant-expr rule (in the cf-units repo):
enable_error_code = [
"ignore-without-code",
# "redundant-expr", # TODO: Add back in when above ignores fixed
"truthy-bool"
]
There was a problem hiding this comment.
If you think it would be neater that way, to be honest I could see an argument for doing it either way.
There was a problem hiding this comment.
I commented it out for the moment and added a comment to say it needs putting in when the error code is fixed; see bf0774f
This also required me to add an exception to repo-review (MY103)
It's probably better this way as previously I was sort of tricking repo-review that I had it set (then was sneakily putting in an ignore for it in mypy!)
🚀 Pull Request
Description
Adds/updates linter hooks to pre-commit.
Excluded/ignored tests documented in following issues for follow up:
Noteworthy changes due to addition of QA hooks:
docs/example.rst- had to add.. blacken-doc: offdirective to stop blacken-docs parsing two code blocks as there is a@savefigstatement (used for generating plots for the docs) that causes blacken-docs to fail.xfail_strict. Left filterwarnings = ["error"] commented out until warnings in code are addressed. Tested with pytest.One of the commits (de7d178) alphabetises the order of sections in pyproject.toml. In hindsight this might make comparison of the changes in the file a bit tricky, so it might be worth checking the changes by commit (I tried to add each QA tool and its affected files in one commit)