Skip to content

Make differing channel names at one channel number an error - #42

Merged
guijacquemet merged 4 commits into
mainfrom
claude/channel-names-differ
Oct 30, 2026
Merged

guijacquemet merged 4 commits into
mainfrom
claude/channel-names-differ

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Summary

Fixes #37 (the preprocessing side; the issue tracks the MorphoFeature side separately).

  • metadata.py: channel_names_differ is now an error. Before, it was a warning. The code name, the comparison and the listed paths are unchanged. The message says why this blocks processing: channels are matched by number, so different stains would be pooled as one channel. The action says to fix the channel order, or export again, or exclude the images.
  • Docs:
    • contract §2 (rule), §8 (moved from warnings to errors), §9 (consumers should check their channel names against the manifest's channel_names when it is not empty) and §11 (run stops on it);
    • user guide §8;
    • plan §6, plus a Revised note under question 1;
    • the AGENTS.md decisions log;
    • the step-1 text of the notebook.

Why

At v0.3.1, image A could name OME channel 0 "DAPI" while image B named it "Actin". Their histograms were pooled as one channel with only a warning, and a later brick could fit "DAPI" scaling on Actin pixels.

Decisions / notes for review

  • This is a behaviour change. It reverses the October 2026 decision on plan question 1, which made this a warning. An experiment that ran before with this warning is now blocked. The issue asks for this change; please confirm.
  • Names are compared exactly, as before: "DAPI" and "dapi" also block. There is no "I confirm these are the same" override. The user re-exports the images or excludes them. Adding an override, or a case-insensitive comparison, would be a product decision, so I did not guess one.
  • Channels without a name in some images are still not compared. The manifest records them as null, unchanged.
  • Manifest and CSV formats are unchanged.

Tests

  • tests/test_metadata.py::test_differing_channel_names_are_an_error (renamed from ..._are_a_warning) now checks for an error.
  • New regression test: tests/test_run.py::test_differing_channel_names_stop_the_run, with DAPI and Actin at channel 0. Nothing is written.
  • Both fail on main: the severity is WARNING and the run is written.
  • test_manifest_records_channel_names_and_significant_bits now uses matching names, because differing names no longer give a manifest.
  • Full suite (Python 3.13, zarr 3.4.2, notebook extra installed): 1052 passed, 1 skipped. The skip is test_discovery.py:472: running as root ignores file modes.
  • ruff check . and ruff format --check . are clean.
  • Not run: Windows, macOS, Python 3.11.

Generated by Claude Code

Channels are matched by number: the table's per-channel columns, pooling and
later bricks' scaling. With channel 0 named "DAPI" in one image and "Actin"
in another, both were pooled as one channel and only a warning said so.
channel_names_differ is now an error that blocks check and run, with an
action to fix the channel order or exclude the images. Only named channels
are compared, as before. Contract §2/§8/§9/§11, user guide, plan Q1 (revised
decision), AGENTS.md and the notebook text are updated.

Fixes #37

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Scma6W3LC11s3qjXHZuGDm
Review of #42 (issue #37):

- OME-Zarr copies wrote the placeholder label "Channel <i>" for channels their
  source leaves unnamed, so re-reading the copies (for example beside the copy
  of a named image) failed the now exact channel_names_differ check. Copies now
  write a label only for names from the source's metadata, and a copy carrying
  placeholder labels counts as changed and is converted again
  (copy_channels_current). Its rows do not change.
- Names stay compared exactly (no normalization, no override). When they differ
  only in whitespace, the message quotes them and says so, since the difference
  is otherwise invisible.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VbytvBRB9CmKQ5AAkhEFcv
Owner decision on #42 (issue #37): channel names stay compared exactly with no
override, but leading and trailing whitespace is removed when names are read,
uniformly for every image (OME-TIFF, OME-Zarr and this brick's own copies), so
"DAPI " and "DAPI" are one name. Whitespace inside a name still counts ("DA PI"
and "DAPI" differ). Trimming is recorded per file in the run manifest
(channel_names_trimmed, optional key, false when absent) and logged.

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

Copy link
Copy Markdown
Contributor Author

Review follow-up, pushed as 99b518d and d812269:

  • Copies' placeholder names. OME-Zarr copies used to write Channel <i> for a channel the source leaves unnamed. If you re-read the copies, for example next to the copy of a named image, that placeholder failed the exact name check. Copies now write a label only for a name that comes from the source's metadata. A copy that still carries placeholder labels counts as changed and is converted again (copy_channels_current). Its rows do not change.

  • Owner decision 1: channel names are compared exactly, with no override.

  • Owner decision 2: leading and trailing whitespace is stripped from channel names when they are read. This applies to every image: OME-TIFF, OME-Zarr and the brick's own copies. The manifest records it per file (channel_names_trimmed, an optional key that reads as false when absent), and the run logs it. Whitespace inside a name still has to match: DA PI vs DAPI is refused. When names differ only in inner whitespace, the error message quotes them and says so.

  • New tests:

    • test_leading_and_trailing_whitespace_of_channel_names_is_removed (DAPI vs DAPI passes)
    • test_inner_whitespace_of_channel_names_still_differs
    • test_channel_labels_are_trimmed_but_inner_whitespace_kept[0.4/0.5]
    • test_channel_names_differing_only_in_inner_whitespace_are_named_as_such
    • test_copies_of_unnamed_channels_read_back_unnamed
    • test_copies_with_placeholder_channel_names_are_written_again

    All fail on 3aaa3aa except the DA PI guard, which was already refused there.

  • Checks (Python 3.13, zarr extra): pytest 1059 passed, 1 skipped (test_discovery.py:472, running as root). ruff check and ruff format --check are clean.

  • Docs: contract §2, §9 and §10, plan Q1, user guide, AGENTS.

  • Merge note: this PR conflicts with Give masks a plane identity (Z/T); refuse 2D masks of multi-plane images #43 (docs/contract.md severity table, a docs-only conflict that was already there) and with Data-flow audit minor findings: unknown scale, manifest flags, per-file roles, uint64 labels #44 (docs/contract.md, and manifest.py because the new channel_names_trimmed sits next to Data-flow audit minor findings: unknown scale, manifest flags, per-file roles, uint64 labels #44's roles). Both are mechanical to resolve. After Data-flow audit minor findings: unknown scale, manifest flags, per-file roles, uint64 labels #44 merges, channel_names_trimmed should be read with Data-flow audit minor findings: unknown scale, manifest flags, per-file roles, uint64 labels #44's strict _flag.


Generated by Claude Code

Review of #42 (issue #37): read_channel_names turned a non-text omero label
(a number, list, object or boolean) into an unnamed channel, so the image
silently escaped the exact channel_names_differ check. Such a label is
malformed metadata; open_ngff now raises NgffError, and the image is
unreadable_ome_zarr. Regression test
test_channel_label_that_is_not_text_is_refused.

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

Copy link
Copy Markdown
Contributor Author

Reviewed 99b518d/d812269 and added ea706fd (an OME-Zarr channel label that is not text is refused as unreadable_ome_zarr instead of silently read as unnamed, with regression test); branch was already on main, no conflicts; 1063 passed, 1 skipped (test_discovery.py:472, root), ruff clean, CI 7/7 green — merging.


Generated by Claude Code

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.

Differing channel names across images are only a warning (channels pooled by index)

2 participants