Repository navigation
Adopt ruff for linting and formatting - #37
Merged
Merged
Conversation
The repo had no lint or format configuration, so style was whatever each
contributor's editor did: mixed quote styles, import blocks in arbitrary
order, and a handful of imports left behind by refactors.
Configuration is a `[tool.ruff]` block in pyproject.toml. `target-version`
is pinned to py39 so the linter cannot suggest syntax the package's
`requires-python` floor rejects, even when ruff runs under 3.13. The rule
set is E, W, F, I, UP, B, C4 -- the pycodestyle/pyflakes baseline, sorted
imports, version-bounded modernisation, and the two check families this
code can actually trip: loop-variable and mutable-default mistakes, and
needless intermediate collections in array code. E501 is off because line
length is the formatter's job; leaving it on would flag exactly the long
comment and docstring lines `ruff format` is unable to split.
Most of the diff is the mechanical `ruff format` pass over 27 files. The
substantive changes are small:
- Dropped genuinely unused imports -- `itertools` in projection.py and
`h5py`, `nrrd`, `pandas`, `logging`, `tqdm` in metrics.py, all noted as
dead in CLAUDE.md. Every module still imports, which confirms it.
- Rewrote three `dict(...)` calls as literals and two `for k, v in
.items()` loops that ignored the key as `.values()`.
- tests/test_real_data.py carries the only suppression in the tree: its
`tests.mini_ccf` import must follow `pytest.importorskip("h5py")`, so
it takes a `# noqa: E402` with the reason.
CI gains a `lint` job alongside `test`, which the workflow's own comment
already anticipated. It runs once on x86-64 -- ruff's findings are a
property of the source, not the machine, so the test job's architecture
matrix buys nothing here -- and uses `--only-group dev` to get the
lockfile-pinned ruff without installing numpy, scipy or scikit-image.
Lint and format are separate steps so a formatting-only failure is
distinguishable at a glance from a real finding.
Suite is unchanged: 366 passed, 36 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #36 landed while this branch was in flight and touched two files it also changes. docs/source/conf.py conflicted twice. Both were main's content against this branch's reformatting, and both resolve to main's content with the formatter re-run over it. The second one mattered: main deliberately removed `html_static_path`, because pointing it at a `_static` directory the tree does not have was the docs build's only warning, and .readthedocs.yaml builds with `fail_on_warning: true`. Taking this branch's side would have reinstated the line and broken the RTD build. CLAUDE.md conflicted twice as well: - The Commands block. This branch was cut before #36 corrected the Sphinx version, so merging its side verbatim would have reverted `sphinx 9.1.0` back to `5.2.3`. Kept main's line, added the two ruff commands. - The Gotchas list. Kept main's new Read the Docs entry and this branch's note about the one `# noqa: E402`. Dropped the dead-imports entry, which this branch made obsolete by removing those imports, and the processing.rst heading entry, which #36 fixed and removed the claim for. Merged tree verified: ruff check clean, 38 files already formatted, 366 passed / 36 skipped, and conf.py still scrapes version 1.2.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The repo had no lint or format configuration, so style was whatever each contributor's editor did: mixed quote styles, arbitrary import order, and a handful of imports left behind by refactors.
Configuration
A
[tool.ruff]block inpyproject.toml:target-version = "py39"— pinned to therequires-pythonfloor so the linter cannot suggest syntax the package rejects, even when ruff runs under 3.13.select = ["E", "W", "F", "I", "UP", "B", "C4"]— the pycodestyle/pyflakes baseline, sorted imports, version-bounded modernisation, and the two check families this code can actually trip: loop-variable and mutable-default mistakes (B), and needless intermediate collections in array code (C4).ignore = ["E501"]— line length is the formatter's job. Leaving E501 on would flag exactly the long comment and docstring linesruff formatis unable to split.What changed
Most of the diff is the mechanical
ruff formatpass over 27 files. The substantive changes are small:itertoolsinprojection.py, andh5py/nrrd/pandas/logging/tqdminmetrics.py— all already noted as dead inCLAUDE.md. Every module still imports, which confirms it.dict(...)calls as literals, and twofor k, v in ....items()loops that ignored the key as.values().tests/test_real_data.pycarries the only suppression in the tree: itstests.mini_ccfimport must followpytest.importorskip("h5py"), so it takes a# noqa: E402with the reason stated.CI
A
lintjob alongsidetest, which the workflow's own comment already anticipated. It runs once on x86-64 — ruff's findings are a property of the source, not the machine, so the test job's architecture matrix buys nothing here — and usesuv run --only-group devto get the lockfile-pinned ruff without installing numpy, scipy or scikit-image. Lint and format are separate steps so a formatting-only failure is distinguishable at a glance from a real finding.Verification
ruff check .→ clean;ruff format --check .→ 38 files already formatted.uv run pytest→ 366 passed, 36 skipped (the opt-in real-data tier), unchanged frommain.docs/source/conf.pystill scrapes version1.2.0out ofpyproject.toml.Note for reviewers
The reformat and the real changes are in one commit — the working-tree revert needed to split them was not available in this session. Reviewing with
git diff --ignore-all-spaceor hiding whitespace in the GitHub diff view isolates the substantive changes listed above.🤖 Generated with Claude Code