Skip to content

OME-Zarr axes and calibration: nothing guessed, nothing silent (#72, #73, #94) - #129

Merged
guijacquemet merged 5 commits into
mainfrom
claude/fix-ngff-axes-calibration
Nov 2, 2026
Merged

guijacquemet merged 5 commits into
mainfrom
claude/fix-ngff-axes-calibration

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #72, fixes #73, fixes #94

Before-audit batch A (triage 2026-11-02): OME-Zarr axes and calibration. Design v0.4 §2, §3 and §9 apply: nothing guessed, nothing silent.

Summary (by module)

Owner decisions (2026-11-02), recorded in plan question 60

  1. [Audit/Scientific integrity] OME-Zarr with matching axis letters can be processed under contradictory confirmed roles #72: the code is ome_zarr_axes_mismatch, not the axis-check codes, which offer conversion (an OME-Zarr image cannot be converted). A Q axis never gets a role.
  2. [Audit/Scientific integrity] Missing axes metadata in NGFF 0.4/0.5 can silently default 5D images to TCZYX #73: OME-NGFF 0.1/0.2 stores are refused (the PR first kept TCZYX for 5-D arrays; changed in review). Stores without axes are refused, with no confirmation path.
  3. [Sad Catapella B6][Scientific QA] Malformed explicit NGFF coordinate transforms are silently treated as unknown calibration #94: a warning, not an error. The image is uncalibrated with the reason, and the warning goes once the sizes are stated in calibration.csv.

Review (independent reviewer)

Commits: 718e8b5, 8fdf7de (merge of main with #126), eeae9d0, 67bdea5 (merge of main with #127).

  • Owner decision implemented: 0.1/0.2 refusal, with regression tests: a 5-D store, with and without axes, through open_ngff, inspect_image and preflight. ngff_axes loses its unused ndim parameter.
  • Finding (fixed): invalid_physical_calibration disappeared once X and Y (and Z) were stated, even when the malformed scale had also lost the time step of a time series. The time step cannot be stated, so the loss became silent. The new _lost_roles check fixes this; 4 parametrised cases; the T case fails on e0e6c20.
  • Merge with main (PP-11c: conversion integrity (copy SHA-256, metadata epochs, exact CLI sizes) #126): one PP-11c test wrote a zyx store into an experiment confirmed ZYX → ZXY. That is exactly the [Audit/Scientific integrity] OME-Zarr with matching axis letters can be processed under contradictory confirmed roles #72 contradiction, now refused. The store is now tzyx with one time point, matched by role, so the test still checks the reuse of a changed chunk.
  • Verified in code:
    • file_mapping_for and review_axes are the only resolvers, and the preview, sources, preflight and notebook all use file_mapping_for;
    • the version is read before the axes;
    • every malformed case gives invalid with a specific reason, and no silent fallback is left (no broad except on the scale path).
  • Valid stores are unchanged. End to end, a 0.4 and a 0.5 store were run (run_experiment, 2 conditions × 2 images, CZYX, calibrated) on main 8db1f4a and on this branch: run_manifest.json and discovery_report.json differ only in paths, mtimes and timestamps, and plane_histograms.csv is byte-identical.

Tests

  • tests/test_ngff_axes_calibration.py: 35 tests. On main 8db1f4a, 30 fail and 5 pass (the controls).
  • Full gate on 67bdea5 (uv venv, Python 3.11, .[dev,notebook,zarr], zarr 3.1.6, running as root): python tools/check.py passed:
    • ruff check clean, ruff format 181 files;
    • pytest 1831 passed, 1 skipped (test_discovery.py:665, permissions ignored as root: expected), 1 warning;
    • code rules passed.

Code standards (§9), measured

  • Largest file touched: physical_sizes.py 684 lines (baselined, 706 → 684). New ngff_scale.py 113 lines; zarr_input.py 439.
  • Largest function touched: review_axes 67 executable lines (baselined, 106 → 67). New functions are all 40 lines or fewer.
  • Highest complexity among touched functions: _resolve_files 10. Then read_scale and ngff_axes 8; ngff_physical_sizes and _level0 7.
  • Parameters: at most 4.
  • CI green on all 7 jobs (lint; Ubuntu, Windows, macOS × Python 3.11, 3.13).
  • Ratchet: the baseline only shrinks; # noqa 79 → 78 (ANN 54 → 53); no new exemption.

https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc


Generated by Claude Code

…72, #73, #94)

- #72: every OME-Zarr image is matched by the roles its OME-NGFF axes declare,
  also when its axis letters are the mapping's (axes.ome_zarr_mapping, used by
  review_axes and file_mapping_for). NGFF ZYX confirmed TYX or ZXY, and QYX
  confirmed CYX, are ome_zarr_axes_mismatch, naming each image with its
  declared axes and confirmed roles; the image is never read under them.
- #73: missing axes are never guessed. Only OME-NGFF 0.1/0.2 (whose spec fixes
  5-D t,c,z,y,x) keep TCZYX, by the stated version; 0.3-0.5 without axes, and a
  store without axes stating no or another version, are unreadable_ome_zarr.
- #94: a malformed coordinateTransformations calibration (ngff_scale.read_scale)
  gives unknown sizes with the reason in PhysicalSizes.invalid and the warning
  invalid_physical_calibration naming every image; the blanket except with a
  debug log is gone.

Contract, user guide, plan (question 60) and AGENTS.md updated; the code-rules
baseline shrinks (review_axes, physical_sizes.py, test_zarr_input.py, ANN noqa).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet marked this pull request as ready for review November 2, 2026 16:07
…ning for a lost time step

Owner decision (2 November 2026): no spec-default t, c, z, y, x. An OME-NGFF
0.1 or 0.2 store is unreadable_ome_zarr whatever it contains, with the message
to re-save it as OME-NGFF 0.4 or later. ngff_axes loses its unused ndim.

Review finding: invalid_physical_calibration disappeared once X and Y (and Z)
were stated in calibration.csv, also when the malformed scale had lost the
time step of a time series, which cannot be stated there: that loss became
silent. The warning now stays until every size the malformed calibration
leaves unknown is stated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
docs/plan.md: main's items (16) to (18) of question 59 come first, then
question 60 of this PR.

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

test_an_ome_zarr_copy_changed_in_place_is_not_reused (from #126) wrote a
'zyx' store into an experiment confirmed ZYX -> ZXY: the same letters under
contradicted roles, which issue #72 refuses (ome_zarr_axes_mismatch). The
store is now 'tzyx' with one time point, matched by its declared roles, so
the test checks the reuse of a changed chunk as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
No conflicts; plan questions on main end at 59, so 62-64 are renumbered once #128
and #129 land.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit 7f55ba1 into main Nov 2, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/fix-ngff-axes-calibration branch November 2, 2026 17:57

Copy link
Copy Markdown
Contributor Author

Independent review: merged as 7f55ba1 (merge commit). The branch was auto-deleted on merge and has been restored at the PR head, 67bdea5.

Owner decisions implemented (2026-11-02)

  • OME-NGFF 0.1/0.2 stores are refused (unreadable_ome_zarr) whatever they contain, with "re-save it as OME-NGFF 0.4 or later". No spec-default t, c, z, y, x remains (718e8b5).
  • Stores without axes stay refused, with no confirmation path.
  • Malformed calibration stays a warning.
  • ome_zarr_axes_mismatch is kept.

All of these are recorded in plan question 60.

Findings fixed

Verified in code

  • Every OME-Zarr image goes through ome_zarr_mapping, whatever its letters, in both review_axes and file_mapping_for. Preview, sources, preflight and the notebook all use file_mapping_for.
  • ZYX→TYX, ZXY and QYX→CYX are refused and never reach file_mappings, so they are never read.
  • The version is read before the axes. 0.3, 0.4 and 0.5 stores without axes are refused.
  • Each malformed calibration case gives a specific reason in PhysicalSizes.invalid. The broad except is gone.
  • Valid stores are unchanged. End to end with valid 0.4 and 0.5 stores (CZYX, calibrated, 2 conditions × 2 images), main 8db1f4a and this branch give the same run_manifest.json and discovery_report.json apart from paths, mtimes and timestamps, and a byte-identical plane_histograms.csv.
  • New tests: 30 of the 35 in test_ngff_axes_calibration.py fail on main 8db1f4a.

Gate on 67bdea5 (uv venv, Python 3.11, .[dev,notebook,zarr], root)

  • ruff check clean; ruff format: 181 files already formatted.
  • pytest: 1831 passed, 1 skipped (test_discovery.py:665, root, expected).
  • code rules passed.
  • CI green on all 7 jobs.

§9 figures

  • Largest file touched: physical_sizes.py, 684 lines (baselined, down from 706).
  • Largest function: review_axes, 67 executable lines (baselined, down from 106). New functions are 40 lines or fewer.
  • Highest complexity: _resolve_files, 10.
  • At most 4 parameters.
  • # noqa count 79 → 78; the baseline only shrinks.

Issues #72, #73 and #94 are closed.


Generated by Claude Code

guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
docs/plan.md: main's PP-11b and PP-11c items first, then this PR's links
item; this PR's questions 60 and 61 become 61 and 62, after main's question
60 (OME-Zarr axes and calibration, #129). docs/contract.md: both additions to
the error row kept (main's OME-Zarr codes, this PR's link codes).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
…version (#73)

The error row still said a store without axes is refused when its version
does not fix them; since the owner's decision in #129 no version fixes them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
docs/plan.md §12: main's question 60 (#129, OME-Zarr axes and calibration) is kept, and
this PR's questions 62-64 become 61-63 so the numbers are unique and sequential; the
references in contract §9.1, §11, §12 and plan §10 follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
Conflicts resolved hunk by hunk:
- docs/plan.md: main's "Before the audit" entry (#82, #83, #98) kept, then PP-12's;
  main's questions 61 to 63 kept and PP-12's question renumbered 64.
- run_inputs.py RunInputs.design(): main's strict read_confirmed_design, on the file
  PP-12's published_path names (the current generation's design).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 2, 2026
Conflicts: plan questions (main has 60 to 63; this branch's questions
become 66 and 67, 64 and 65 being taken by #128) and the code-rule
layers of pyproject.toml (size_checks and ngff_scale, both pure).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants