Skip to content

PP-0: code standards with a shrink-only baseline; one atomic writer, one grammar helper - #65

Merged
guijacquemet merged 4 commits into
mainfrom
claude/pp-0-tooling
Nov 1, 2026
Merged

guijacquemet merged 4 commits into
mainfrom
claude/pp-0-tooling

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

PP-0 of the v0.4.0 plan (preprocessing-design-v0.4.md revision 4, §8 and §9): tooling and dedup, with no behaviour change. Its own diff adds tooling, dedup and a baseline; it changes no behaviour.

Merge order: #64 (PP-1, merged) is already in this branch (1fa7ccb), so the baseline covers the combined code.

Summary

  • tools/code_rules.py: the shared file copied from MorphoFeature origin/main (9951a58), extended with §9:
    • the private-docstring rule: private functions over 10 executable lines need a docstring;
    • the layer check: each package module is declared pure, io, ui or orchestration in pyproject.toml. Pure modules may not import I/O or UI modules, or call open, print, read_text and the like; UI modules may not import scipy;
    • RESERVED_NAMES: shared logic listed by its owner repository (normalization, preprocessing, CellPatcher). A definition anywhere else fails;
    • a baseline that may only shrink. baseline_key drops line numbers, so code that moves keeps its entry. New or grown violations fail, and so do entries the code no longer needs;
    • a # noqa count ratchet for C901, PLR0913, PLR0915 and ANN, counting line and file-level (ruff: noqa:) comments in src, tools and tests.
  • tools/check.py: runs ruff, pytest and the code rules.
    • --rules runs only the code rules; the CI lint job runs it.
    • --update-baseline only lowers the baseline.
  • Ruff now selects C90 (max 15), PLR0913 (max 8), PLR0915 (max 50), ANN, PGH004 (a noqa must name its rules) and I002 (from __future__ import annotations in every module of src and tools).
    • Existing sites carry # noqa, counted by the ratchet: 63 ANN, 2 C901, 3 PLR0913 (one in tests/zarr_tree.py) and 2 PLR0915.
    • Tests are exempt from ANN and I002, as they already are from D. The experiment scripts are exempt as run; notebooks from I002.
    • Two annotations were added instead of a noqa: planes._tiff_blocking's keyframe, and the notebook's Step 0 function.
  • tools/code_rules_baseline.json: the 113 violations present in PP-1: the axis check (swaps and contradictions are errors; plain stacks confirmed) #64 + main (oversized files and functions, private functions without docstrings, duplicated test helpers).
  • Dedup (audit D1, D7):
    • _io.write_text_atomic replaces the three temp-file-then-os.replace copies (design.py, manifest.py, mask_scan.py);
    • validation.agree replaces _are/_contain (discovery) and _do/_their/_are/_match/_have (masks).
  • Docs: a "Code standards" section in development.md; AGENTS.md "Done means" now refers to tools/check.py.

Review fixes (0608025)

  • A bare # noqa or # ruff: noqa: … hid rules from the ratchet (both passed --rules). Now PGH004 refuses the bare form, and file-level codes and tests' comments are counted.
  • baseline_key used the first number of an "identical bodies" message, a line number, as its size: moving a duplicated helper down failed as "grew". Line numbers and digits inside names are no longer sizes; those four entries are lowered to 0.
  • §9's from __future__ import annotations was not enforced; I002 now does (added to __init__.py and __main__.py).

Notes for review

  • The SHA-256 test pins this extended copy, with line endings normalised to LF. MorphoFeature's copy has to be updated to the same file for the "identical in every repo" rule; this PR does not edit other repositories.
  • Owner question: tests are exempt from ANN here (2,367 test sites); MorphoFeature annotates its tests. Should this repo annotate its tests too, as a follow-up?
  • The layer check is deliberately simple: it looks at imports and call names. The existing modules are classified so that none violates its layer.
  • The baseline file can still be edited by hand; shrink-only is enforced against the code, so a hand edit that adds an entry shows only in the diff. A follow-up could compare it with origin/main's baseline in CI.

Tests

  • New tests:
    • tests/test_code_rules.py (18): the shared-file SHA; the repository against its baseline; every module has a layer; ruff guards for ANN001, C901, PLR0913, PGH004 and I002; size and suppress guards; private docstrings; layers; reserved names; the baseline's shrink and grow rules; line numbers are never sizes; line and file-level noqa counts.
    • tests/test_shared_helpers.py (4): write_text_atomic and agree.
  • All of these fail on old main, where tools/, _io and agree do not exist.
  • Full gate as CI (ruff check, ruff format --check, tools/check.py --rules, pytest with .[dev,notebook,zarr]):
    • Python 3.13 with zarr 3: 1579 passed, 1 skipped (running as root ignores file modes).
    • Python 3.11 with zarr 2.18: 1561 passed, 19 skipped (that root skip plus 18 OME-NGFF 0.5 tests that need zarr 3).

🤖 Generated with Claude Code

https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a

…nd one grammar helper

Tooling (spec revision 4, section 9):
- tools/code_rules.py is the shared file from MorphoFeature, extended with
  the private-docstring rule, the layer check (pure/io/ui/orchestration,
  declared per module in pyproject.toml), RESERVED_NAMES (shared logic may
  only be defined by its owner repository), a baseline that may only
  shrink, and a ratchet on the # noqa count of C901/PLR0913/PLR0915/ANN.
  The SHA-256 test keeps the copy identical.
- tools/check.py runs ruff, pytest and the code rules. Its --rules flag
  runs the rules alone (CI lint job); --update-baseline only lowers the
  baseline, never raises it.
- Ruff now selects C90 (15), PLR0913 (8), PLR0915 (50) and ANN. The 63
  existing ANN sites in src/tools, two of each other rule in src and one
  PLR0913 in tests carry # noqa, counted by the ratchet. Tests are exempt
  from ANN, as from D. The experiments scripts are exempt as run.
- tools/code_rules_baseline.json records the 110 violations that existed.
- Guard fixtures prove each rule fires.

Dedup with no behaviour change (plan-structure audit D1, D7):
- _io.write_text_atomic replaces the three copies in design.py,
  manifest.py and mask_scan.py.
- validation.agree replaces _are/_contain (discovery.py) and
  _do/_their/_are/_match/_have (masks.py).

Docs: development.md "Code standards", AGENTS.md "Done means".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
PP-0 now merges after #64 (PP-1). Its baseline covers the combined code, so
main stays green whichever PR is reviewed first. It adds layers for #64's
modules: axis_check and mask_checks are pure, notebook_axes is ui.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
The baseline is regenerated on the new main: the review commit 5f9b015 adds
a line to axes.py. PP-0 creates the baseline, so it records main as it is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
…es, the future import

- A bare `# noqa` or `# ruff: noqa` hid every Ruff rule from the noqa ratchet. Ruff now
  selects PGH004, and the ratchet also counts file-level `ruff: noqa:` codes and the
  `# noqa` comments in tests (one existing PLR0913 in tests/zarr_tree.py is now counted).
- `baseline_key` took the first number of a message as its size, which for identical
  bodies was a line number: moving a duplicated helper down failed as "grew". A line
  number or a digit inside a name is no longer a size; those entries lower to 0.
- Spec §9 asks for `from __future__ import annotations`; I002 now enforces it in src and
  tools (added to `__init__` and `__main__`; tests, experiments and notebooks exempt).

The shared file's SHA-256 in the identity test is updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a

Copy link
Copy Markdown
Contributor Author

Independent review: approved after one fix commit (0608025).

Checked

  • tools/code_rules.py is MorphoFeature origin/main (9951a58)'s file with only additions: §9 private docstrings, layers, RESERVED_NAMES, baseline and noqa ratchet. The SHA-256 identity test pins the extended copy; MorphoFeature still has to adopt it (follow-up there).
  • Adversarial runs of tools/check.py --rules:
    • a new 70-line function fails ("new violation");
    • growing baselined _bin_count by 2 lines fails ("grew from 12 to 14"), and so does growing run.py by 3 lines;
    • --update-baseline never adds the new violation.
  • It is wired into tools/check.py and the CI lint job (stdlib only, so the ruff-only lint job runs it).
  • No behaviour change:
    • write_text_atomic matches the three copies it replaces (no newline translation; only the temp file's name differs for mask_scan);
    • agree gives the same words as the eight helpers;
    • the rest of the src diff is annotations and noqa comments.

Fixed in 0608025

  1. A bare # noqa (ruff then hides C901/PLR/ANN) and a file-level # ruff: noqa: ANN001, PLR0913 both passed --rules. The fix selects PGH004 and counts file-level codes. The ratchet also counts tests (one existing PLR0913, so the limit is 3).
  2. baseline_key took an "identical bodies" message's line number as its size, so moving a duplicated helper down failed as "grew". Line numbers and digits in names are no longer sizes; the four entries are lowered to 0.
  3. §9's from __future__ import annotations was not enforced. I002 now enforces it for src and tools.

Follow-ups (not blocking)

  • Baseline keys collapse violations with the same function name in one file (max size). A third same-named oversized nested function would pass.
  • The baseline JSON can be grown by hand. A CI check against origin/main's baseline would close that.

Gate on 0608025: ruff check and format clean, --rules clean.

  • 3.13 with zarr 3.4.2: 1579 passed, 1 skipped (root ignores file modes).
  • 3.11 with zarr 2.18.7: 1561 passed, 19 skipped (that one plus 18 OME-NGFF 0.5 tests needing zarr 3).
  • CI is green on all 7 jobs.

Generated by Claude Code

@guijacquemet
guijacquemet marked this pull request as ready for review November 1, 2026 17:07
@guijacquemet
guijacquemet merged commit 1336648 into main Nov 1, 2026
7 checks passed
guijacquemet pushed a commit that referenced this pull request Nov 3, 2026
Normalization main 534c63d (#65, issue #47) refuses a PlaneHistogram with
valid pixels and no pixel_min or pixel_max. The test now passes them from
the version-2 columns; it passes with v0.3.3 (still the pin) and 534c63d.
It is the only PlaneHistogram construction in src/ and tests/.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet deleted the claude/pp-0-tooling branch November 14, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants