chore: normalise repo-wide ruff formatting and gate it in CI - #489
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
`ruff check .` was already clean; the drift was entirely in the formatter, which CI never ran. Reformatting split four long lines whose trailing `# noqa` then no longer sat on the code it suppressed, so those directives move with the code they cover: - significance.py: the E501 directive is dropped, the line is no longer long. - _adaptive.py update(): ARG002 moves to `y_row` (the ignored argument); `label` is used, so it never needed one. - _adaptive.py _channels_frame(): ANN401 now repeated per parameter, matching what the single pre-split directive covered. - spec.py: PERF401 moves to the `append` call it is reported against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
No lint fallout here: no `# noqa` directives were displaced by the reflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
Only code-cell source lines change; no cell outputs or execution counts are touched, so the strict notebook-executing Sphinx build sees the same code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
The lint job ran only `ruff check .`, so formatter drift was never caught: 118 of 266 files were unformatted on main. Add `ruff format --check .` so it cannot accumulate again. Also move the ruff-pre-commit rev from v0.15.6 to v0.15.22, matching the version resolved from the `ruff>=0.11.0,<0.16` pin in pyproject.toml. With the new gate the two must agree, otherwise pre-commit could format a file one way and CI reject it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
CodeQL flags `combined` as potentially uninitialised at the point it is read.
The reads were guarded by the same `if results` that guards the assignment, so
this was never reachable, but the correlation is not something the analyser can
prove. Compute the two counts inside the branch that owns `combined` instead.
Behaviour is unchanged. The alert predates this branch; the reflow of the
`clean({...})` call moved the reads onto a line this PR touches, which is what
re-reported a pre-existing alert as a new one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
10 tasks
kgdunn
pushed a commit
that referenced
this pull request
Aug 9, 2026
Each of these sends a new contributor somewhere the repository no longer is: - Setup said `pip install -e ".[dev]"`, which omits the optional runtime dependencies. pyDOE3 lives in the `expt`/`all` extras, so that environment fails a large part of the suite on ImportError rather than skipping. Now `".[dev,all]"`, matching CI's `uv sync --dev --all-extras`. - The lint block told contributors to run `black .`, which is configured nowhere in the repository and contradicts the ruff-only policy, and `mypy process_improve`, which is the wrong path under the src layout. It now also documents `ruff format --check .` as the second, independent gate added in #489, and the pre-commit rev/pin alignment that gate depends on. - "Adding new methods" pointed at `multivariate/methods.py`, which has been a re-export shim since ENG-01; a method added there has no effect. Now points at `_pca.py` / `_pls.py` and names the sibling modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD
kgdunn
added a commit
that referenced
this pull request
Aug 9, 2026
#490) * docs: record the two-command lint gate in CLAUDE.md Follow-up to #489. That PR made `ruff format --check .` a CI gate alongside `ruff check .`, but nothing wrote the convention down, so the next agent would have gone on assuming a clean `ruff check .` meant the lint job would pass. - Code Quality: state that the gate is two independent commands, that the ruff-pre-commit rev must track the ruff pin in pyproject.toml, and how to handle a `# noqa` stranded by a reflow (move it, do not delete it). - CI/CD: the run-tests.yml description still listed `lint` as `ruff check .` alone, which is now wrong. No version bump: CLAUDE.md is agent instructions rather than shipped code or configuration, and this file already classes edits to itself as internal-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD * docs: correct three stale instructions in CONTRIBUTING.md Each of these sends a new contributor somewhere the repository no longer is: - Setup said `pip install -e ".[dev]"`, which omits the optional runtime dependencies. pyDOE3 lives in the `expt`/`all` extras, so that environment fails a large part of the suite on ImportError rather than skipping. Now `".[dev,all]"`, matching CI's `uv sync --dev --all-extras`. - The lint block told contributors to run `black .`, which is configured nowhere in the repository and contradicts the ruff-only policy, and `mypy process_improve`, which is the wrong path under the src layout. It now also documents `ruff format --check .` as the second, independent gate added in #489, and the pre-commit rev/pin alignment that gate depends on. - "Adding new methods" pointed at `multivariate/methods.py`, which has been a re-export shim since ENG-01; a method added there has no effect. Now points at `_pca.py` / `_pls.py` and names the sibling modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD * docs: restructure CLAUDE.md around single sources of truth The file was accurate but was acting as a parallel contributor guide: it referenced none of the repository's other authoritative documents, restated policy that CONTRIBUTING.md owns, and carried reference data that duplicates the source and had already gone stale in one place. Removed, because the code or CONTRIBUTING.md is the better source: - The PCA and PLS fitted-attribute lists. They duplicated each class's Attributes docstring and had already drifted; PCA sets `algorithm_`, which the list omitted. Replaced with a pointer to read the class. - The `_model_method` code snippet. The rule and its rationale are kept; an agent editing that class reads the real code anyway. - Long-form Docstrings, Scaling and "Adding New Methods to PCA/PLS" sections that CONTRIBUTING.md already covers. One-line rules stay so common cases do not need a second file open. Added, because nothing pointed at them: - An Authoritative documents table: CONTRIBUTING.md plus the five policy documents under docs/development/ (error handling, reproducibility, deprecation, logging, tool authoring) and SKLEARN_COMPATIBILITY.md. - Test tier markers. pytest.ini registers unit/integration/slow/dataset, and an untagged slow or network-bound test silently breaks `-m 'not slow'`. - `Expt` and the `pi_` metadata prefix, a public-API convention the file did not mention at all. - MAJOR in the versioning list. The section defined only PATCH and MINOR, then asked the reader to decide whether a change was "major or minor" using a word it never defined, and never mentioned the deprecation schedule. - The `_LazyFrame` descriptor (ENG-18). Seven fitted attributes are descriptors over a private ndarray, so the "set only in fit()" rule alone would mislead anyone adding one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MT8LQS1YtrAGzir5SUeqWD --------- Co-authored-by: Claude <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
3 participants
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.
Summary
The recurring "ruff keeps changing files I never touched" problem is not the linter.
ruff check .was already clean onmain. It is the formatter, which CI never ran: 118 of 266 files were unformatted.The asymmetry is what made it feel random.
.pre-commit-config.yamlrunsruff-format, but pre-commit only reformats the files already staged. So editing one file reformatted that file in place, producing hunks unrelated to the actual edit, while the other 117 files stayed drifted and waited for the next contributor to trip over them.ruff formatacross the whole repository:src/(75 files),tests/+tools/(33), and the documentation notebooks (10).ruff format --check .to the CIlintjob so the drift cannot silently accumulate again. This is the part that actually stops the recurrence.ruff-pre-commitrev fromv0.15.6tov0.15.22, matching the version resolved from theruff>=0.11.0,<0.16pin inpyproject.toml. With the new gate the two must agree, or pre-commit could format a file one way and CI reject it.Two things the reformat exposed
Four
# noqadirectives stopped covering their code. Each sat at the end of a long line that the reflow split, so the directive stayed behind while the offending code moved. Ruff then reported both the unsuppressed rule and the now-unused directive:experiments/visualization/plots/significance.pyE501multivariate/_adaptive.py(update)ARG002y_row, the ignored argument;labelis used and never needed itmultivariate/_adaptive.py(_channels_frame)ANN401visualization/spec.pyPERF401appendcall it is reported againstOne CodeQL alert.
combinedinbatch/tools.pyis flagged as potentially uninitialised. The reads were guarded by the sameif resultsthat guards the assignment, so it was never reachable, but the analyser cannot prove that correlation. The condition predates this branch; reflowing theclean({...})call moved the reads onto a line this PR touches, which is what re-reported a pre-existing alert as new. Fixed by computing the two counts inside the branch that ownscombined.Test plan
uv run ruff format --check .reports266 files already formatteduv run ruff check .passesuv run mypy src/process_improvepasses (145 source files).pyfiles were parsed before and after and compared withast.dump: every AST is identical tomain. For the 10 notebooks, each code cell's AST was compared the same way, and cell outputs, execution counts and markdown were verified untouched, so the notebook-executing Sphinx build sees the same code.Codecov reports reduced patch coverage on this PR. That is an artifact of the reformat: moved lines count as new lines in the patch, so already-uncovered lines are re-reported as "missing coverage in your changes". No new code was added, and total coverage is unchanged at 94.09%.
Checklist
pyproject.toml(1.66.0 -> 1.66.1, PATCH),CITATION.cffin syncruff check .passesCHANGELOG.mdupdated