Skip to content

Per-channel group statistics and explicit refusal of cyclic mask hierarchies - #178

Merged
guijacquemet merged 2 commits into
mainfrom
claude/reporting-104-105
Nov 6, 2026
Merged

guijacquemet merged 2 commits into
mainfrom
claude/reporting-104-105

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Fixes #104, fixes #105

Summary (one commit per issue)

#104, report.py, notebook_results.py, notebook_plots.py. group_summaries now gives one GroupSummary per group and channel. It no longer pools every channel of every XYC plane. With C0 near 10 and C1 near 1000, the old code reported a mixed mean near 505; it now reports two rows near 10 and near 1000.

  • New fields on GroupSummary:
    • channel: the channel index, or ALL_CHANNELS (None) for an aggregate.
    • xyc_plane_count: the XYC planes in the group.
    • channel_histogram_count: the channel histograms pooled.
  • plane_count is kept as a deprecated property. It equals channel_histogram_count and emits a DeprecationWarning.
  • group_table adds the columns channel, XYC planes and channel histograms, which replace planes.
  • Step 4 now:
    • plots one panel per channel;
    • counts groups, not summary rows;
    • shows an "XYC planes" tile and a "channels" tile.
  • An all-channel aggregate is produced only on explicit request (pool_channels=True) and is labelled all channels (pooled).
  • A group whose planes have different channel counts is refused with a ValueError.
  • Single-channel tables give the same numbers as before.

#105, mask_scan.py, mask_text.py.

  • New MaskReport.hierarchy_cycles and mask_scan.hierarchy_cycles. They find the groups of types whose direct parents form a cycle (strongly connected groups, including a type that is its own parent).
  • The hierarchy text (CLI masks summary and notebook) now starts with the named warning mask_hierarchy_cycle. It names every type and link in the cycle.
  • The tree shows the condensation: each cycle is one [cycle: A, B, C] entry. Every type is listed and no direction is guessed. The infinite recursion when a cycle sits below a root is gone.
  • Acyclic hierarchies print as before.

Consumer and schema analysis

  • File schemas: no change. plane_histograms.csv (schema 4), run_manifest.json, experiment_design.csv and mask_relationships.json (report version 1) are byte-for-byte unaffected. Step 4 writes no file. mask_relationships.json already kept every direct link; adding a cycle key would need a report-version bump, so it is left as a possible follow-up.
  • Python API change ([Sad Catapella B9][Scientific reporting] Group statistics silently pool distinct fluorescence channels and mislabel channel measurements as planes #104):
  • Consumers searched read-only (MorphoFeature, cellpatcher, morphobricks-normalization, MorphoCoverage, MorphoObjects, MorphoRelate, MorphoEmbed, fiberfeatures, MorphoBricks-Spec):
    • None imports group_summaries, GroupSummary, group_table, channel_histograms, mask_hierarchy_lines or mask_report_summary.
    • No code reads mask_relationships.json; it appears only in design docs.
    • Normalization's own pooling (fitting._channel_histograms) already pools only the selected channels.
  • Inside this repo, the consumers are the notebook's step 4 and mask card, and the CLI masks summary. All are updated and tested.
  • Docs updated: contract §6 and the §15 hierarchy section, user guide (step 4, the mask_hierarchy_cycle code, the API example), AGENTS.md, and a plan §12 Q27 amendment.

Tests

New regressions; each fails on main:

Notes for review

  • The notebook changes were tested headless only, not viewed in a real JupyterLab.

Generated by Claude Code

group_summaries pooled every channel of every XYC plane of a group into one
summary, so two fluorescence markers (C0 near 10, C1 near 1000) gave one mixed
mean near 505, and its plane_count counted channel histograms as planes.

Each group now has one summary per channel (GroupSummary.channel), with
xyc_plane_count and channel_histogram_count reported apart; group_table adds
a channel column and the notebook's step 4 plots one panel per channel and
counts groups, not summaries. The all-channel aggregate is only made with
pool_channels=True and is labelled "all channels (pooled)". A group whose
planes have different channel counts is refused. plane_count stays as a
deprecated alias of channel_histogram_count.

No file schema changes: plane_histograms.csv, the manifest and every output
are unchanged; step 4 writes nothing. Single-channel tables give the same
numbers. Python API change documented in the changelog with migration notes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW
The hierarchy text started only at mask types without parents and followed
children without a visited set. Pairwise evidence summed over different
images can make A inside B, B inside C and C inside A: no type was a root,
so the report printed nothing under "Hierarchy (direct parents only)", and
a cycle below a root would recurse without end.

MaskReport.hierarchy_cycles (mask_scan.hierarchy_cycles) names each group
of types whose direct parents form a cycle, self-parents included. The
hierarchy lines (CLI summary, notebook) start with the named warning
mask_hierarchy_cycle listing every type and link of the cycle, and draw the
condensation: the group as one "[cycle: A, B, C]" entry, so every type is
listed and no direction is guessed. Acyclic hierarchies print as before;
mask_relationships.json is unchanged (report version 1).

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

Copy link
Copy Markdown
Contributor Author

Independent review of 71ce36d: approve, no defects found

I reviewed this in a worktree of origin/main merged with this branch (clean merge), using a fresh venv.

#104 (group statistics per channel): all claims hold in the code

  • group_summaries gives one GroupSummary per (group, channel), with channels in order. xyc_plane_count is len(members) and channel_histogram_count is len(histograms). They are equal for one channel. With an aggregate, the histogram count is planes × channels.
  • Channels are pooled only with the keyword-only pool_channels=True. The aggregate has channel=None, and channel_label shows it as all channels (pooled). By default, nothing is computed across channels, so no statistic mixes channels with different units or calibrations.
  • Each channel is pooled on its own through PooledHistogram, using each plane's own min–max bins. distribution_png draws one panel per channel, and each panel has its own x range.
  • Single-channel tables give the same numbers as before: channel_histograms(members) is the same list as [r.channel_histogram(0) …]. The v2-fixture test checks this, and also checks the legacy aggregate.
  • Planes without finite pixels (NaN handling): a channel with no finite pixels gets pixel_count 0 and None statistics, shown as –. This is documented in the docstring and tested.
  • plane_count is kept as a deprecated property that raises DeprecationWarning. The CHANGELOG documents the change to the positional constructor and to the group_table columns.
  • No file schema changes: step 4 writes nothing.
  • None of the other /home/user/* repositories (MorphoFeature, MorphoEmbed, MorphoObjects, MorphoRelate, cellpatcher, fiberfeatures, MorphoCoverage, normalization, Spec) use GroupSummary, group_summaries, group_table, channel_histograms, distribution_png or the mask-hierarchy names.
  • The ValueError for mixed channel counts in a group is defensive only: the normalization CSV reader already refuses tables with mixed channel counts.

#105 (cycles in the mask hierarchy)
I probed these hierarchy shapes:

  • a self-loop
  • a 2-cycle
  • a 3-cycle
  • two separate cycles
  • a cycle below a root with a subtree under it
  • a type with two parents, one of them in a cycle
  • a figure-8 (two cycles sharing a type)
  • a self-loop with a child
  • a parent that is not a key

Every shape terminated. Each cycle got one mask_hierarchy_cycle warning listing every type and link, and appeared as one [cycle: …] node. Every type was listed, and none was dropped. The condensation into strongly connected groups is a DAG, so the recursion always ends. Acyclic output is byte-identical to main on three hierarchies, including one with a type that has two parents.

Tests are real. Run against main's src, 14 of the new or changed tests fail: every test in test_mask_hierarchy_cycles.py, the per-channel panel spy test, and all six v2-equivalence parametrisations. test_report.py cannot import ALL_CHANNELS on main. The tests assert values, not tautologies.

Gate: python tools/check.py exit 0 (Ruff, code rules and pytest: 2294 passed, 4 skipped). The skips are environmental: three because this file system tells names apart by case, one because the run has root permissions.
CI on 71ce36d: lint and all 6 test jobs (Ubuntu, Windows, macOS × 3.11, 3.13) pass.

Nits, not blocking:

  • The user guide says an XYC plane is "an image at one time point". That is accurate now that Z-stacks are refused.
  • distribution_png would raise IndexError if a caller passed groups with different channel counts directly. The notebook cannot do this, because the reader refuses such tables.

Safe to merge once it is out of draft.


Generated by Claude Code

@guijacquemet
guijacquemet marked this pull request as ready for review November 6, 2026 16:26
@guijacquemet
guijacquemet merged commit ed3a6d8 into main Nov 6, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/reporting-104-105 branch November 6, 2026 16:26
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