Switch to ruff check and ruff format - #237
Open
djhoese wants to merge 9 commits into
Open
Conversation
The pre-commit config pinned pre-commit/pre-commit-hooks at v1.2.3 (2018) and pulled a flake8 hook from it, but that hook moved to PyCQA/flake8 years ago and no longer exists at that rev. Nothing has actually linted this repo in a long time, so flake8-docstrings, flake8-debugger and flake8-bugbear were all inert. Consolidate on ruff. Lint config now lives only in pyproject.toml, which lets setup.cfg (which held nothing but [flake8]) and .stickler.yml (a service that shut down in 2023) be deleted. The rule set is satpy's plus the polar2grid rules that are cheap here. Families that would need large mechanical rewrites (ANN, N, PTH, UP031, RUF005) are deferred with comments so each can be its own reviewable pass. T10 preserves the flake8-debugger property the old config intended to enforce. ruff-check runs before ruff-format because --fix rewrites are not formatter-stable. Vendored and generated files are excluded via ruff's own extend-exclude, which the hooks honor through --force-exclude. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
Mechanical, produced by `ruff check --fix` followed by `--unsafe-fixes`. B905 was excluded from the unsafe pass: its fix inserts a blanket strict=False, which preserves the silent-truncation behaviour rather than stating intent, so those seven call sites are handled by hand in a following commit. Covers import sorting (I001), redundant placeholders (PIE790), dict/list literal calls (C408, C419, C420), f-strings (UP032), parametrize value types (PT007) and assorted UP/SIM/RUF/FURB fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
zip(strict=): every one of the seven call sites pairs sequences that are equal in length by construction, so strict=True is correct and a mismatch there would be a bug worth surfacing. ruff's autofix inserts strict=False, which would have preserved silent truncation instead. raise ... from: the caught KeyError in the three mode-conversion lookups is an internal dict-lookup detail, so those chain from None. The AttributeError in rio_save does say which attribute was missing, so that one keeps its cause. Docstrings: several Args entries were missing the colon Google style requires, which meant they were not being parsed as parameter descriptions at all. Also documents rio_save's deprecated include_scale_offset_tags and **format_kwargs, the *args/**kwargs forwarding on from_string/from_np, and corrects from_csv, whose docstring named the parameter `string` when it is `path`. Tests: pytest.raises/warns calls now assert on the message rather than only the type. utilities.cmap_from_text no longer leaks the file handle it opened. A002 is ignored for utilities.py rather than fixed: `hex` shadows a builtin but is a public keyword argument of cmap_from_text, so renaming it breaks callers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
Mechanical only: no behaviour change, no manual edits. This is the commit to add to .git-blame-ignore-revs. Most of the diff is quote normalisation to double quotes, which is why the lint rule set deliberately does not select Q -- quote style is owned by the formatter, not the linter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
Nothing has ever linted this file: neither flake8 nor ruff reads .pyx, so its 531 lines had no coverage at all. cython-lint reported nine findings, all cosmetic bar one. The exception is `cimport cython`, which was genuinely unused -- the compiler directives are given in the `# cython:` header comment, not as decorators. That header is itself split across two lines to fit in 120 columns, which Cython supports. Verified by cythonizing before and after: the generated C is identical apart from the three now-absent empty "Module declarations from cython" blocks and shifted source line references. The extension was rebuilt and the suite re-run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
The spec assumed py.typed was not being packaged. It is: setuptools 69.0.0 added py.typed to its implicit data files, so recent setuptools picks it up with no configuration. Verified against a built wheel and sdist, both of which already contained it. That behaviour is flagged EXPERIMENTAL upstream, though, and build-system.requires only asks for setuptools >= 42, which gives no such guarantee. Declaring it makes the packaging independent of that. package_data rather than include_package_data=True on purpose: the latter would also honour the existing "recursive-include trollimage *.pyx" and ship the Cython source in every wheel. Confirmed the rebuilt wheel contains py.typed and still contains no .pyx. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
==========================================
+ Coverage 92.06% 92.10% +0.03%
==========================================
Files 12 12
Lines 4161 4128 -33
==========================================
- Hits 3831 3802 -29
+ Misses 330 326 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The PR template asked contributors to run `flake8 --diff` against origin/master: --diff was removed in flake8 6.0 and the branch has been main for years. It now points at pre-commit, which is what pre-commit.ci runs on every pull request. pre-commit is deliberately not added to continuous_integration/environment.yaml -- that file drives ten conda solves in CI, and pre-commit pins its own ruff. AGENTS.md documents ruff and cython-lint, the blame.ignoreRevsFile opt-in, the Google-style colon rule that D417 enforces, and a warning not to widen the rule set inside unrelated changes. It also drops .stickler.yml from the list of relics kept for history, since that file is now deleted. The end-of-file-fixer hook added missing final newlines to four files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqd9odjKZKZn3CibwvxqBP
djhoese
force-pushed
the
modernize-linting
branch
from
September 6, 2026 13:39
cc564d2 to
eb643d9
Compare
Member
Author
|
Ok I've reviewed this and I think I agree with all of the changes except maybe all the comments in .pyproject.toml about what ruff rules still need to be worked on. But those are all left for future PRs so I think it is fine to leave them for now. 90% of this PR is single quotes to double quotes, reordering imports, and reindenting/newlining lines of code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I told Claude to look at Satpy's config as a base and also my Polar2Grid project where I recently added more strict rules. Claude suggested a cython-lint pre-commit hook which looks pretty interesting. So this is what it created. This is built on top of the AGENTS.md PR (#235).
git diff origin/master **/*py | flake8 --diff(remove if you did not edit any Python files)