From a6949b88904aff4c5a9a05c8eff10f1caa10f936 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 17:37:43 +0000 Subject: [PATCH 1/5] Mask step: same_objects counts a maximum one-to-one matching (#67) The one-to-one counts took every object with any partner at the IoU threshold, so two children halving one parent counted as two matches and could give same_objects. The matches are now a maximum bipartite matching of the pairs at or above the threshold (Hopcroft-Karp on a sparse graph, per plane); its size is unique, so the counts are deterministic. The evidence adds the unmatched objects and the objects with several partners at the threshold. Report version 2. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 10 +- src/morphobricks_preprocessing/mask_scan.py | 67 ++++++++++- tests/test_mask_matching.py | 122 ++++++++++++++++++++ tests/test_mask_scan.py | 2 +- tests/test_masks.py | 2 +- 5 files changed, 191 insertions(+), 12 deletions(-) create mode 100644 tests/test_mask_matching.py diff --git a/docs/contract.md b/docs/contract.md index c50de113..725737ac 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -406,7 +406,7 @@ masks/ masks folder, outside the experiment folder | Warnings | `mask_missing` (images without a mask of a type that other images have: their objects of that type cannot be measured), `mask_without_image` (files matching no image of the experiment), `mask_of_excluded_image` (masks of excluded images, not used), `mask_outside_type_folder` (files directly in the masks folder that are not TIFF masks). | | Run | `run_experiment(..., masks_folder=..., mask_axes=...)` stops on a mask error and records the masks in the manifest's `masks` key (§9). Without masks, the run and its outputs are unchanged. | -### 15.1 The mask step (`mask_relationships.json`, report version 1) +### 15.1 The mask step (`mask_relationships.json`, report version 2) The optional mask step reads every image's masks, one image at a time, and describes each mask type and every ordered pair of types over the whole experiment. Pieces are counted with @@ -435,7 +435,7 @@ Each child object's parent is the parent object sharing most of its pixels. | Relation of child to parent | Rule (default thresholds; the values used are written in the report) | |---|---| -| `same_objects` | At least 90% of the children and 90% of the parents are matched one to one at an intersection over union of at least 0.5: two segmentations of the same objects. | +| `same_objects` | At least 90% of the children and 90% of the parents are matched one to one at an intersection over union of at least 0.5: two segmentations of the same objects. The matches are a maximum matching of the pairs at or above the threshold, in each plane: each object is matched at most once, and as many objects as possible are matched. Two children that each cover half of one parent are one match, not two (issue #67; before report version 2 every object with any partner at the threshold counted as matched). | | `grouped_by` | The child is `grouped`, at least 90% of its objects are inside as below, and at least 99% of its pixels carry the label of the parent object under them. | | `inside` | At least 90% of the child objects have at least half of their pixels in one parent object, and that parent is larger than the child for at least 90% of them. | | `none` | Otherwise. | @@ -447,7 +447,9 @@ Each child object's parent is the parent object sharing most of its pixels. | `split_between_parents`, `outside_parents` | Children touching parents but with less than half of their pixels in any one; children touching no parent. | | `parents_with_0_children`, `parents_with_1_child`, `parents_with_2_children`, `parents_with_3_or_more_children` | How many children each parent holds (two nuclei in one cell counts as 2). | | `child_pixels`, `child_pixels_with_parent_label` | Child pixels; those whose label equals the parent label under them. | -| `children_matched_one_to_one`, `parents_matched_one_to_one` | Objects with a partner at an intersection over union of at least 0.5. | +| `children_matched_one_to_one`, `parents_matched_one_to_one` | Objects matched one to one (the maximum matching above, at an intersection over union of at least 0.5); the two counts are equal. The number of matches does not depend on which of several equally large matchings is found, so it is deterministic. | +| `children_not_matched`, `parents_not_matched` | Objects of each type left without a match. | +| `children_over_iou_with_several_parents`, `parents_over_iou_with_several_children` | Objects with two or more partners at or above the threshold: a split or merged segmentation. Above 0.5 an object can overlap only one partner that much, so these are 0 at a threshold above 0.5. | **Thresholds.** The numbers above are defaults (`DEFAULT_THRESHOLDS`), and each may be changed: `inside_pixel_share` (0.5), `inside_object_share` (0.9), `parent_larger_share` @@ -468,7 +470,7 @@ The report is UTF-8 JSON (two-space indent, LF), written atomically to the outpu | Key | Meaning | |---|---| -| `report_version` | `1`. | +| `report_version` | `2` (since issue #67: the one-to-one matches are a matching, and the four counts of unmatched and several-partner objects were added). | | `thresholds` | The thresholds used, all seven. | | `images` | Images scanned (every image with at least one mask). | | `masks` | Per mask type: `kind`, `images`, `objects` (labels over all images) and `median_pieces_per_label`. | diff --git a/src/morphobricks_preprocessing/mask_scan.py b/src/morphobricks_preprocessing/mask_scan.py index ba10aca4..69ebc091 100644 --- a/src/morphobricks_preprocessing/mask_scan.py +++ b/src/morphobricks_preprocessing/mask_scan.py @@ -12,7 +12,9 @@ - **Relationship of a child type to a parent type:** - ``same_objects``: one-to-one matches at intersection over union >= 0.5 for at least 90% of - the objects of both types (two segmentations of the same objects); + the objects of both types (two segmentations of the same objects). The matches are a + maximum matching (:func:`one_to_one_matches`): each object is matched at most once, so + two children covering halves of one parent count as one match, not two; - ``grouped_by``: the child is ``grouped``, at least 99% of its pixels carry the label of the parent object under them, and it is inside as below; - ``inside``: at least 90% of the child objects have at least half of their pixels in one @@ -51,6 +53,8 @@ import numpy as np from scipy import ndimage +from scipy.sparse import csr_matrix +from scipy.sparse.csgraph import maximum_bipartite_matching from ._io import write_text_atomic from .manifest import package_versions @@ -61,8 +65,8 @@ MASK_REPORT_FILENAME = "mask_relationships.json" """Name of the report written to the output folder.""" -MASK_REPORT_VERSION = "1" -"""Version of the report's format.""" +MASK_REPORT_VERSION = "2" +"""Version of the report's format: 2 since the one-to-one matches are a matching (issue #67).""" DEFAULT_THRESHOLDS = MappingProxyType( { @@ -95,6 +99,10 @@ "child_pixels_with_parent_label", "children_matched_one_to_one", "parents_matched_one_to_one", + "children_not_matched", + "parents_not_matched", + "children_over_iou_with_several_parents", + "parents_over_iou_with_several_children", ) @@ -296,6 +304,8 @@ def pair_counts( child_labels, child_of = np.unique(child[on], return_inverse=True) parent_labels, parent_sizes = np.unique(parent[parent != 0], return_counts=True) counts["children"], counts["parents"] = child_labels.size, parent_labels.size + counts["children_not_matched"] = child_labels.size # until matched below + counts["parents_not_matched"] = parent_labels.size if child_labels.size == 0: counts["parents_with_0_children"] = parent_labels.size return counts @@ -315,9 +325,8 @@ def pair_counts( counts["parents_with_0_children"] = slots return counts union = child_sizes[kid] + parent_sizes[host] - shared - matched = shared / union >= limits["same_objects_iou"] - counts["children_matched_one_to_one"] = int(np.unique(kid[matched]).size) - counts["parents_matched_one_to_one"] = int(np.unique(host[matched]).size) + eligible = shared / union >= limits["same_objects_iou"] + counts.update(_match_counts(kid[eligible], host[eligible], child_labels.size, slots)) best = np.lexsort((-shared, kid)) lead = np.r_[True, kid[best][1:] != kid[best][:-1]] best_kid, best_host, best_shared = kid[best][lead], host[best][lead], shared[best][lead] @@ -336,6 +345,52 @@ def pair_counts( return counts +def one_to_one_matches(kid: np.ndarray, host: np.ndarray, children: int, parents: int) -> int: + """Return the number of one-to-one matches: a maximum matching of the eligible pairs. + + Each child and each parent is matched at most once, and the number of matches is the + largest possible (Hopcroft-Karp, :func:`scipy.sparse.csgraph.maximum_bipartite_matching`). + That number does not depend on which of several equally good matchings is found, so + the counts are deterministic. Above an intersection over union of 0.5 an object can + overlap only one partner that much, and every eligible pair is matched; at 0.5 or below, + two children covering one parent are one match, not two (issue #67). + + Parameters + ---------- + kid, host + The child and parent of each eligible pair (intersection over union at least the + threshold), as indices below ``children`` and ``parents``; each pair at most once. + children, parents + Numbers of child and parent objects. + + Returns + ------- + int + """ + if kid.size == 0: + return 0 + graph = csr_matrix((np.ones(kid.size, dtype=np.int8), (kid, host)), shape=(children, parents)) + partner = maximum_bipartite_matching(graph, perm_type="column") + return int(np.count_nonzero(partner >= 0)) + + +def _match_counts(kid: np.ndarray, host: np.ndarray, children: int, parents: int) -> dict: + """Return the matched, unmatched and several-partner counts of the eligible pairs.""" + matched = one_to_one_matches(kid, host, children, parents) + return { + "children_matched_one_to_one": matched, + "parents_matched_one_to_one": matched, + "children_not_matched": children - matched, + "parents_not_matched": parents - matched, + "children_over_iou_with_several_parents": int( + np.count_nonzero(np.bincount(kid, minlength=children) > 1) + ), + "parents_over_iou_with_several_children": int( + np.count_nonzero(np.bincount(host, minlength=parents) > 1) + ), + } + + def scan_masks( inventory: MaskInventory, experiment_root: str | os.PathLike, diff --git a/tests/test_mask_matching.py b/tests/test_mask_matching.py new file mode 100644 index 00000000..f67f49ad --- /dev/null +++ b/tests/test_mask_matching.py @@ -0,0 +1,122 @@ +"""One-to-one matches of the mask step are a matching: each object matched once (issue #67).""" + +from itertools import permutations + +import numpy as np +import pytest + +from morphobricks_preprocessing import pair_counts, write_mask_report +from morphobricks_preprocessing.mask_scan import one_to_one_matches +from test_mask_scan import boxes, by_pair, scan + + +def matches(counts): + return ( + counts["children_matched_one_to_one"], + counts["parents_matched_one_to_one"], + counts["children_not_matched"], + counts["parents_not_matched"], + counts["children_over_iou_with_several_parents"], + counts["parents_over_iou_with_several_children"], + ) + + +def test_identical_objects_are_matched_one_to_one(): + cells = boxes({1: (0, 10, 0, 10), 2: (0, 10, 20, 30), 3: (20, 30, 0, 10)}) + assert matches(pair_counts(cells, cells)) == (3, 3, 0, 0, 0, 0) + + +def test_two_children_halving_one_parent_are_one_match_not_two(): + # The issue's counterexample: one 4-pixel parent, two 2-pixel children; each IoU 0.5. + parent = boxes({1: (0, 2, 0, 2)}) + children = boxes({10: (0, 1, 0, 2), 11: (1, 2, 0, 2)}) + counts = pair_counts(children, parent) + assert matches(counts) == (1, 1, 1, 0, 0, 1) + # The other way round: one child split over two parents. + assert matches(pair_counts(parent, children)) == (1, 1, 0, 1, 1, 0) + + +def test_many_to_one_at_a_lower_threshold(): + # Three children in thirds of one parent: IoU 1/3 each, eligible at 0.3. + parent = boxes({1: (0, 3, 0, 3)}) + children = boxes({10: (0, 1, 0, 3), 11: (1, 2, 0, 3), 12: (2, 3, 0, 3)}) + counts = pair_counts(children, parent, thresholds={"same_objects_iou": 0.3}) + assert matches(counts) == (1, 1, 2, 0, 0, 1) + assert matches(pair_counts(children, parent)) == (0, 0, 3, 1, 0, 0) # not at 0.5 + + +def test_partial_overlaps_at_the_threshold(): + parents = boxes({1: (0, 1, 0, 2), 2: (5, 6, 0, 2)}) + # Child 10 holds parent 1 in 4 pixels (IoU 0.5: eligible), child 11 parent 2 in 5 (0.4). + children = boxes({10: (0, 2, 0, 2), 11: (5, 6, 0, 5)}) + assert matches(pair_counts(children, parents)) == (1, 1, 1, 1, 0, 0) + loose = pair_counts(children, parents, thresholds={"same_objects_iou": 0.4}) + assert matches(loose) == (2, 2, 0, 0, 0, 0) + + +def test_the_matching_is_maximum_not_greedy(): + # Child 10 overlaps parents 1 and 2 at IoU 1/3, child 11 only parent 1 (IoU 0.5): + # matching child by child in label order (10 with 1) would leave 11 alone; the maximum + # matching pairs both. + parents = boxes({1: (0, 1, 0, 4), 2: (0, 1, 4, 8)}) + children = boxes({10: (0, 1, 2, 6), 11: (0, 1, 0, 2)}) + counts = pair_counts(children, parents, thresholds={"same_objects_iou": 0.3}) + assert matches(counts) == (2, 2, 0, 0, 1, 1) + + +def test_counts_do_not_depend_on_label_numbers(): + parent = boxes({1: (0, 2, 0, 2), 2: (0, 2, 2, 4)}) + halves = {(0, 1, 0, 2), (1, 2, 0, 2), (0, 2, 2, 4)} + found = set() + for labels in permutations((3, 5, 9)): + children = boxes(dict(zip(labels, sorted(halves), strict=True))) + found.add(matches(pair_counts(children, parent))) + assert found == {(2, 2, 1, 0, 0, 1)} + + +def reference_matching(kid, host): + """Largest matching by trying every assignment (independent of the brick's code).""" + edges = set(zip(kid.tolist(), host.tolist(), strict=True)) + children = sorted({k for k, _ in edges}) + best = 0 + + def walk(i, used, size): + nonlocal best + best = max(best, size) + if i == len(children): + return + walk(i + 1, used, size) + for k, h in edges: + if k == children[i] and h not in used: + walk(i + 1, used | {h}, size + 1) + + walk(0, frozenset(), 0) + return best + + +@pytest.mark.parametrize("seed", range(20)) +def test_matching_size_against_a_brute_force_reference(seed): + rng = np.random.default_rng(seed) + graph = rng.random((5, 4)) < 0.4 + kid, host = np.nonzero(graph) + assert one_to_one_matches(kid, host, 5, 4) == reference_matching(kid, host) + + +def test_a_split_segmentation_is_not_the_same_objects(tmp_path): + cells = {1: (2, 12, 2, 12), 2: (2, 12, 20, 30), 3: (20, 30, 2, 12), 4: (20, 30, 20, 30)} + # Each cell segmented again as two halves: IoU 0.5 each. + halves = {} + for label, (r0, r1, c0, c1) in cells.items(): + middle = (r0 + r1) // 2 + halves[10 * label] = (r0, middle, c0, c1) + halves[10 * label + 1] = (middle, r1, c0, c1) + report = scan(tmp_path, {"Cells": boxes(cells), "Halves": boxes(halves)}) + pair = by_pair(report)[("Halves", "Cells")] + assert pair.relation != "same_objects" and report.same_objects == () + e = pair.evidence # two images, four cells and eight halves each + assert (e["children_matched_one_to_one"], e["parents_matched_one_to_one"]) == (8, 8) + assert (e["children_not_matched"], e["parents_not_matched"]) == (8, 0) + assert e["parents_over_iou_with_several_children"] == 8 + data = write_mask_report(report, tmp_path / "out.json").read_text(encoding="utf-8") + assert '"report_version": "2"' in data + assert '"children_over_iou_with_several_parents": 0' in data diff --git a/tests/test_mask_scan.py b/tests/test_mask_scan.py index 9a0e7bd7..4c1fc8d4 100644 --- a/tests/test_mask_scan.py +++ b/tests/test_mask_scan.py @@ -226,7 +226,7 @@ def test_report_file(tmp_path): raw = path.read_bytes() assert b"\r\n" not in raw data = json.loads(raw) - assert data["report_version"] == "1" + assert data["report_version"] == "2" assert data["thresholds"]["inside_object_share"] == 0.9 assert data["hierarchy"]["Nucleoli"] == ["Nuclei", "NucleiAlt"] assert data["same_objects"] == [["Nuclei", "NucleiAlt"]] diff --git a/tests/test_masks.py b/tests/test_masks.py index f543face..ebe545a9 100644 --- a/tests/test_masks.py +++ b/tests/test_masks.py @@ -320,7 +320,7 @@ def test_masks_command_writes_the_report(experiment, masks, tmp_path, capsys): assert "[3/3] drug/rep1/a.tif" in printed assert "Hierarchy (direct parents only):" in printed report = json.loads((out / "mask_relationships.json").read_text(encoding="utf-8")) - assert report["report_version"] == "1" + assert report["report_version"] == "2" assert sorted(report["masks"]) == ["Cells", "Nuclei"] assert report["same_objects"] == [["Cells", "Nuclei"]] From 3e6f7574a8d5af666c73984167b212540454e39f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 17:43:56 +0000 Subject: [PATCH 2/5] Size and plane-count warnings follow the confirmed roles (#74) different_sizes and different_plane_counts compared the header letters, also of plain stacks whose letters are not their roles: a plain QYX stack of (Y, X, C) arrays confirmed as YXC was compared by its X and channels (a real height difference missed) and gave "planes along Q". Preflight now leaves plain stacks out; AxisReview.combined_issues drops the header versions and recomputes both warnings with each image's confirmed roles (header letters for images with an axis error or without a mapping). The checks move to the pure size_checks module (metadata.py shrinks). Docs: contract 11 and 15.1, user guide, plan questions 60 and 61. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 4 +- docs/plan.md | 25 ++++ docs/user_guide.md | 8 +- pyproject.toml | 1 + src/morphobricks_preprocessing/axes.py | 19 ++- src/morphobricks_preprocessing/metadata.py | 109 ++++++--------- src/morphobricks_preprocessing/size_checks.py | 125 ++++++++++++++++++ tests/test_axes.py | 2 +- tests/test_role_sizes.py | 107 +++++++++++++++ tools/code_rules_baseline.json | 6 +- 10 files changed, 327 insertions(+), 79 deletions(-) create mode 100644 src/morphobricks_preprocessing/size_checks.py create mode 100644 tests/test_role_sizes.py diff --git a/docs/contract.md b/docs/contract.md index 725737ac..d64ded70 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -304,8 +304,8 @@ row (follow-up P3b, normalization plan N43). | `missing_pixel_size` | Warning | The pixel size (or the z-step of images with more than one depth slice) is known for some images and not for others. Nothing is reported when no image has it. | | `unsupported_pixel_size_unit` | Warning | An image's metadata give a pixel size or z-step (OME `PhysicalSize{X,Y,Z}`, ImageJ resolution or `spacing`, OME-Zarr level-0 scale) in a length unit that is not one of decision D-a2 (§16), for example OME `in` or ImageJ `inch`. The size is not converted and the axis is treated as uncalibrated (as before); the warning names each image, its axes and the unit as written, so the missing calibration is never silent. A unit without a size, and a plain TIFF's resolution tags (decision D-c), are not reported. The run is not refused. | | `different_channel_counts` | Error | The number of channels differs between images: the `C` dimension, or the RGB samples `S` when there is no `C`; once the axis roles are confirmed, the length of the C axis (1 without one), which replaces the header's count. The images not having the most common number are listed, with each number's image count. Every image of an experiment must have the same channels: split the others into another experiment or exclude them. | -| `different_plane_counts` | Warning | The number of depth slices (`Z`), time points (`T`) or planes along any other dimension except X, Y, C and S differs between images. A missing dimension counts as 1. Channels are compared by `different_channel_counts`. | -| `different_sizes` | Warning | The image width or height differs between images (TIFF and OME-Zarr). The other dimensions are compared by `different_plane_counts`. | +| `different_plane_counts` | Warning | The number of depth slices (`Z`), time points (`T`) or planes along any other dimension except X, Y, C and S differs between images. A missing dimension counts as 1. Channels are compared by `different_channel_counts`. Before the axis roles are confirmed (`check`, preflight) the axes each header names are compared, and a plain stack (its header names no axes, §2) is left out; once they are confirmed (`run`, the notebook's run, `AxisReview.combined_issues`) each image's depth slices and time points under its confirmed roles are compared instead, and the header version is dropped. An image whose confirmed roles contradict its header (an axis error, never read) is compared by its header. Before issue #74 a plain stack was compared by its header letters, so a plain `QYX` stack confirmed as `YXC` gave "planes along Q". | +| `different_sizes` | Warning | The image width or height differs between images (TIFF and OME-Zarr). The other dimensions are compared by `different_plane_counts`. As for `different_plane_counts`, the header's Y and X before the roles are confirmed (a plain stack left out) and the confirmed Y and X after (issue #74: before, a plain `QYX` stack of (Y, X, C) arrays confirmed as `YXC` was compared by its X and channels, and a real difference in height was missed). | | Rule | Detail | |---|---| diff --git a/docs/plan.md b/docs/plan.md index 8cd2d733..6c8145b8 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1496,3 +1496,28 @@ Raised during review; to be decided before the milestones that need them: above 0 (`1.5GiB` is accepted, `1.5` and `0.5B` are refused before anything is read). +60. How are the one-to-one matches of `same_objects` counted (issue #67)? + *Decided (2 November 2026, implemented for issue #67; the rule is the issue's + acceptance): as a maximum matching of the child and parent objects of each plane whose + intersection over union is at least `same_objects_iou` + (`scipy.sparse.csgraph.maximum_bipartite_matching`, a sparse graph, no K x K matrix). + Each object is matched at most once, so two children halving one parent are one match. + The number of matches is the same for every maximum matching, so the counts do not + depend on label numbers or tie-breaking. Above an intersection over union of 0.5 the + matching is every eligible pair (an object can overlap only one partner that much), so + the default threshold changes nothing for real one-to-one segmentations (the HPMEC + masks give the same report). The evidence adds `children_not_matched`, + `parents_not_matched`, `children_over_iou_with_several_parents` and + `parents_over_iou_with_several_children`; `mask_relationships.json` is report version 2. + Contract §15.1.* +61. Which axes do the size and plane-count warnings use (issue #74)? + *Decided (2 November 2026, implemented for issue #74): the confirmed roles, once they + are confirmed. Before (`check`, preflight), `different_sizes` and + `different_plane_counts` compare the axes each header names, and a plain stack is left + out, since its header's letters are not its roles (design v0.4 §3). After the + confirmation, `AxisReview.combined_issues` (used by `run`, the notebook's run and the + conversion plan) drops the header versions and compares each image under its own + confirmed roles; an image with an axis error is compared by its header, never by the + contradicted roles. Images without confirmed roles that the header names are compared + by their header, as before. The notebook's axis card still shows only the axis review's + own issues; the corrected warnings appear in the run's result. Contract §11.* diff --git a/docs/user_guide.md b/docs/user_guide.md index b84de9d5..dcd9f82e 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -624,7 +624,9 @@ Hierarchy (direct parents only): ``` Nucleoli inside nuclei inside cells have the nuclei as their direct parent; two kinds of -mask that segment the same objects are reported as such. These are suggestions from the +mask that segment the same objects are reported as such: at least 90% of the objects of +both are matched one to one, each object matched at most once (two halves of one cell are +one match, not two). These are suggestions from the pixels, not decisions: you name each kind of mask and check the relationships in the bricks that use them. The report is written to `mask_relationships.json` in the output folder (§6). The command takes `--output`, `--exclude` and `--log` like the others, and exits with @@ -912,8 +914,8 @@ continues. | `image_named_as_mask` | Error | An image is named like a mask of another image (`f1_mask.tif` beside `f1.tif`; the endings `_mask`, `_masks`, `_cp_masks`, `_labels`, `_label`, `_seg`, `_segmentation`), and a mask is never analysed as an image. If it is a mask, move it into a masks folder outside the experiment, or exclude it; if it is an image, rename it. | | `mask_folder_in_experiment` | Warning | A folder inside the experiment is named like a masks folder (`masks`, `ctrl_masks`, `CellLabels`) and holds files named like masks of the images; they would be analysed as images. Move it out of the experiment and give it as the masks folder, or rename it if these are images. | | `planes_read_in_blocks` | Warning | Planes too large for the memory limit are read in blocks. The results are the same; nothing to do. Raise the limit to read them whole, which is faster. | -| `different_sizes` | Warning | Images have different widths or heights. Usually expected; otherwise check the camera settings. | -| `different_plane_counts` | Warning | Images have different numbers of depth slices, time points or other planes. Images with more planes weigh more in pooled statistics. Check the acquisition if this is not expected. | +| `different_sizes` | Warning | Images have different widths or heights. Usually expected; otherwise check the camera settings. For a plain stack (no axes in its file), the sizes are compared once you confirm its axis roles, with those roles. | +| `different_plane_counts` | Warning | Images have different numbers of depth slices, time points or other planes. Images with more planes weigh more in pooled statistics. Check the acquisition if this is not expected. For a plain stack, compared with its confirmed axis roles, as `different_sizes`. | | `different_pixel_sizes` | Warning | The physical pixel size or z-step (from the metadata or `calibration.csv`) differs between images by more than 0.1 % (for example different objectives). Images outside the largest group of matching sizes are listed; when sizes only change by small steps from image to image, none is listed. Check the microscope settings if this is not intended. | | `missing_pixel_size` | Warning | Some images have a pixel size (from the metadata or `calibration.csv`) and others do not, so it cannot be checked; images stated as measured in pixels count as without one, and the message says how many. Export them again with metadata, or state their sizes with `calibrate`, if needed. | | `unsupported_pixel_size_unit` | Warning | An image gives its pixel size or z-step in a unit that is not converted (for example inches); only pm, Å, nm, µm, mm, cm and m are. The message names each image, axis and unit. Those sizes are not used: the image is treated as uncalibrated on those axes. Export it again with the pixel size in a supported unit, or state it with `calibrate`, if the physical size matters. Not reported for an axis whose size is stated. | diff --git a/pyproject.toml b/pyproject.toml index 7b3f9faa..039df889 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -196,6 +196,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_ui" = "ui" "morphobricks_preprocessing.physical_sizes" = "io" "morphobricks_preprocessing.pixel_size_checks" = "pure" +"morphobricks_preprocessing.size_checks" = "pure" "morphobricks_preprocessing.calibration" = "pure" "morphobricks_preprocessing.calibration_io" = "io" "morphobricks_preprocessing.run.calibrate" = "io" diff --git a/src/morphobricks_preprocessing/axes.py b/src/morphobricks_preprocessing/axes.py index 3d4da9f1..b53c8231 100644 --- a/src/morphobricks_preprocessing/axes.py +++ b/src/morphobricks_preprocessing/axes.py @@ -30,6 +30,7 @@ from .metadata import ( MetadataReport, TiffMetadata, + dimension_issues, ) from .validation import ValidationIssue, has_errors @@ -63,7 +64,13 @@ # Metadata issues that the axis review checks again against the confirmed mapping. _SUPERSEDED_METADATA_CODES = frozenset( - {"missing_axes", "inconsistent_dimensions", "different_channel_counts"} + { + "missing_axes", + "inconsistent_dimensions", + "different_channel_counts", + "different_sizes", + "different_plane_counts", + } ) @@ -254,10 +261,16 @@ def combined_issues(self, metadata: MetadataReport) -> tuple[ValidationIssue, .. The review checks the arrangement of dimensions and the numbers of channels again against the confirmed mapping, so the metadata's ``missing_axes``, ``inconsistent_dimensions`` and - ``different_channel_counts`` issues are replaced by the review's own. + ``different_channel_counts`` issues are replaced by the review's own. The + ``different_sizes`` and ``different_plane_counts`` warnings are computed again with + each image's confirmed roles (issue #74; :func:`.metadata.dimension_issues`), except + for an image whose roles contradict its header (:attr:`axis_error_files`, never read), + which is measured by the axes its header names. """ kept = [i for i in metadata.issues if i.code not in _SUPERSEDED_METADATA_CODES] - merged = [*kept, *self.issues] + refused = set(self.axis_error_files) + roles = {p: m.roles for p, m in self.file_mappings.items() if p not in refused} + merged = [*kept, *dimension_issues(metadata.files, roles), *self.issues] return tuple(sorted(merged, key=lambda issue: not issue.is_error)) diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index f9277171..b2dd945e 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -18,7 +18,7 @@ import json import logging import os -from collections.abc import Callable, Hashable, Iterable, Mapping +from collections.abc import Callable, Hashable, Iterable, Mapping, Sequence from dataclasses import dataclass from pathlib import Path from xml.etree import ElementTree @@ -31,7 +31,14 @@ from .discovery import DiscoveryResult from .physical_sizes import PhysicalSizes, _tiff_sizes from .pixel_size_checks import pixel_size_issues -from .validation import ValidationIssue, error, has_errors, plural, warning +from .size_checks import ( + image_size_issues, + lettered_plane_counts, + plane_count_issues, + role_plane_counts, + role_size, +) +from .validation import ValidationIssue, error, has_errors, plural from .zarr_input import ( NgffError, ZarrUnavailableError, @@ -382,6 +389,8 @@ def check_metadata(files: Iterable[TiffMetadata]) -> list[ValidationIssue]: known for only some images (``missing_pixel_size``), and pixel sizes given in a length unit that is not converted (``unsupported_pixel_size_unit``, decision D-a2): those images are uncalibrated, and the unit is named. + Sizes and plane counts are compared by the axes the headers name; a plain stack is + left out until its roles are confirmed (:func:`dimension_issues`, issue #74). Parameters ---------- @@ -489,10 +498,9 @@ def check_metadata(files: Iterable[TiffMetadata]) -> list[ValidationIssue]: ) issues.extend(_check_dimensions([f for f in readable if not f.is_ome_zarr])) - issues.extend(_check_image_sizes(readable)) issues.extend(_check_channel_names(readable)) issues.extend(_check_channel_counts(readable)) - issues.extend(_check_plane_counts(readable)) + issues.extend(dimension_issues(readable)) issues.extend(pixel_size_issues(readable)) return issues @@ -640,32 +648,6 @@ def _check_dimensions(files: list[TiffMetadata]) -> list[ValidationIssue]: return [] -def _check_image_sizes(files: list[TiffMetadata]) -> list[ValidationIssue]: - # Width and height only: the other dimensions are compared by _check_plane_counts. - sized = [f for f in files if "X" in f.axes and "Y" in f.axes] - if len({(f.length("Y"), f.length("X")) for f in sized}) < 2: - return [] - summary, minority = _breakdown(sized, lambda f: (f.length("Y"), f.length("X")), _format_shape) - return [ - warning( - "different_sizes", - "The images have different widths or heights (height x width in pixels): " - f"{summary}. They can still be processed together.", - minority, - "No action needed if the sizes are expected, for example images cropped " - "differently. Otherwise check that these images were acquired with the same " - "camera settings.", - ) - ] - - -# What the planes along each dimension are called in messages. -_PLANE_NAMES = {"Z": "depth slices (Z)", "T": "time points"} -# Dimensions that do not hold planes: the image itself, and channels (or RGB samples), -# which are compared by _check_channel_counts. -_NOT_PLANES = frozenset("XYSC") - - def header_channel_count(f: TiffMetadata) -> int: """Return the channel count the header suggests: the C axis, else the RGB samples.""" return f.length("C") if "C" in f.axes else f.length("S") @@ -715,40 +697,39 @@ def _check_channel_counts(files: list[TiffMetadata]) -> list[ValidationIssue]: return channel_count_issues({f.relative_path: header_channel_count(f) for f in files}) -def _plane_counts(f: TiffMetadata) -> dict[str, int]: - counts = {"Z": 1, "T": 1} - counts.update({axis: f.length(axis) for axis in f.axes if axis not in _NOT_PLANES}) - return counts +def dimension_issues( + files: Iterable[TiffMetadata], roles: Mapping[str, Sequence[str | None]] | None = None +) -> list[ValidationIssue]: + """Report images of different sizes or plane counts (contract §11). + Each readable image is measured by its confirmed roles when ``roles`` has them, else by + the letters its header names. A plain stack without confirmed roles is left out: the Y + and X of its header are only where its rows and columns are stored (:func:`named_axes`), + so its sizes are compared only once its roles are confirmed (issue #74). -def _check_plane_counts(files: list[TiffMetadata]) -> list[ValidationIssue]: - counts = {f.relative_path: _plane_counts(f) for f in files} - axes = sorted({axis for c in counts.values() for axis in c}, key=lambda a: ("ZT".find(a), a)) - descriptions: list[str] = [] - differing: list[str] = [] - for axis in axes: - if len({c.get(axis, 1) for c in counts.values()}) < 2: - continue - summary, minority = _breakdown( - files, lambda f, a=axis: counts[f.relative_path].get(a, 1), str - ) - descriptions.append(f"{_PLANE_NAMES.get(axis, f'planes along {axis}')}: {summary}") - differing.extend(minority) - if not differing: - return [] - return [ - warning( - "different_plane_counts", - "The images have different numbers of planes (" - + "; ".join(descriptions) - + "). Every plane is still analysed, but images with more planes weigh more " - "in pooled statistics.", - sorted(set(differing)), - "No action needed if this is expected, for example time series of different " - "lengths. Otherwise check that these images were acquired with the same " - "depth range and time points.", - ) - ] + Parameters + ---------- + files + The image headers. + roles + The confirmed role of each axis, by relative path (:attr:`.axes.AxisReview.file_mappings`). + + Returns + ------- + list of ValidationIssue + The warnings ``different_sizes`` and ``different_plane_counts``, or nothing. + """ + roles = roles or {} + sizes: dict[str, tuple[int, int]] = {} + planes: dict[str, dict[str, int]] = {} + for f in (f for f in files if f.readable): + path, given = f.relative_path, roles.get(f.relative_path) + if given is not None and {"Y", "X"} <= set(given): + sizes[path], planes[path] = role_size(f.shape, given), role_plane_counts(f.shape, given) + elif given is None and {"Y", "X"} <= set(named_axes(f)): + sizes[path] = f.length("Y"), f.length("X") + planes[path] = lettered_plane_counts(f.axes, f.shape) + return [*image_size_issues(sizes), *plane_count_issues(planes)] def _breakdown( @@ -770,10 +751,6 @@ def _breakdown( return summary, minority -def _format_shape(shape: Hashable) -> str: - return " x ".join(str(n) for n in shape) - - def _is_subsequence(short: str, long: str) -> bool: remaining = iter(long) return all(char in remaining for char in short) diff --git a/src/morphobricks_preprocessing/size_checks.py b/src/morphobricks_preprocessing/size_checks.py new file mode 100644 index 00000000..fbb0aa94 --- /dev/null +++ b/src/morphobricks_preprocessing/size_checks.py @@ -0,0 +1,125 @@ +"""Image sizes and plane counts across the experiment (contract §11). + +The warnings ``different_sizes`` and ``different_plane_counts`` compare, for every image, +its height and width and its numbers of depth slices, time points and planes along any +other dimension. :mod:`.metadata` measures each image by the letters its header names; +after the axis review, :meth:`.axes.AxisReview.combined_issues` measures each image by its +confirmed roles instead (issue #74): the header letters of a plain stack are only where its +pages, rows and columns are stored, so they are never used for these warnings. +""" + +from __future__ import annotations + +from collections.abc import Callable, Hashable, Mapping, Sequence + +from .validation import ValidationIssue, plural, warning + +_PLANE_NAMES = {"Z": "depth slices (Z)", "T": "time points"} +"""What the planes along each dimension are called in messages.""" + +NOT_PLANE_AXES = frozenset("XYSC") +"""Dimensions that do not hold planes: the image itself, and channels (or RGB samples), +which are compared by ``different_channel_counts``.""" + + +def lettered_plane_counts(axes: str, shape: Sequence[int]) -> dict[str, int]: + """Return the planes along each header letter except X, Y, S and C; Z and T default to 1.""" + counts = {"Z": 1, "T": 1} + counts.update( + {axis: n for axis, n in zip(axes, shape, strict=True) if axis not in NOT_PLANE_AXES} + ) + return counts + + +def role_size(shape: Sequence[int], roles: Sequence[str | None]) -> tuple[int, int]: + """Return the height and width of an image under its confirmed roles (one per axis).""" + return shape[list(roles).index("Y")], shape[list(roles).index("X")] + + +def role_plane_counts(shape: Sequence[int], roles: Sequence[str | None]) -> dict[str, int]: + """Return the depth slices and time points under the confirmed roles (1 without the role).""" + lengths = dict(zip(roles, shape, strict=True)) + return {role: lengths.get(role, 1) for role in ("Z", "T")} + + +def image_size_issues(sizes: Mapping[str, tuple[int, int]]) -> list[ValidationIssue]: + """Report images of different heights or widths. + + Parameters + ---------- + sizes + Height and width of each image, by relative path, in the order to report them. + + Returns + ------- + list of ValidationIssue + The warning ``different_sizes`` naming the images not of the most common size, or + nothing. + """ + if len(set(sizes.values())) < 2: + return [] + summary, minority = _breakdown(sizes, lambda shape: " x ".join(str(n) for n in shape)) + return [ + warning( + "different_sizes", + "The images have different widths or heights (height x width in pixels): " + f"{summary}. They can still be processed together.", + minority, + "No action needed if the sizes are expected, for example images cropped " + "differently. Otherwise check that these images were acquired with the same " + "camera settings.", + ) + ] + + +def plane_count_issues(counts: Mapping[str, Mapping[str, int]]) -> list[ValidationIssue]: + """Report images with different numbers of planes along a dimension. + + Parameters + ---------- + counts + Planes along each dimension of each image, by relative path; a dimension an image + lacks counts as 1. + + Returns + ------- + list of ValidationIssue + The warning ``different_plane_counts``, or nothing. + """ + axes = sorted({axis for c in counts.values() for axis in c}, key=lambda a: ("ZT".find(a), a)) + descriptions: list[str] = [] + differing: list[str] = [] + for axis in axes: + along = {path: c.get(axis, 1) for path, c in counts.items()} + if len(set(along.values())) < 2: + continue + summary, minority = _breakdown(along, str) + descriptions.append(f"{_PLANE_NAMES.get(axis, f'planes along {axis}')}: {summary}") + differing.extend(minority) + if not differing: + return [] + return [ + warning( + "different_plane_counts", + "The images have different numbers of planes (" + + "; ".join(descriptions) + + "). Every plane is still analysed, but images with more planes weigh more " + "in pooled statistics.", + sorted(set(differing)), + "No action needed if this is expected, for example time series of different " + "lengths. Otherwise check that these images were acquired with the same " + "depth range and time points.", + ) + ] + + +def _breakdown( + values: Mapping[str, Hashable], describe: Callable[[Hashable], str] +) -> tuple[str, list[str]]: + """Summarise the images by value, most common first; return it and the other images.""" + groups: dict[Hashable, list[str]] = {} + for path, value in values.items(): + groups.setdefault(value, []).append(path) + ordered = sorted(groups.items(), key=lambda item: (-len(item[1]), describe(item[0]))) + summary = ", ".join(f"{describe(k)} ({plural(len(paths), 'image')})" for k, paths in ordered) + return summary, [path for _, paths in ordered[1:] for path in paths] diff --git a/tests/test_axes.py b/tests/test_axes.py index 1372a302..39760330 100644 --- a/tests/test_axes.py +++ b/tests/test_axes.py @@ -259,9 +259,9 @@ def test_channel_count_follows_the_confirmed_channel_axis(): # A plain stack never names its axes: read with the confirmed roles, with a warning. assert as_time.ok and codes(as_time.issues) == ["axes_not_named"] as_channel = review_axes(report, prefill_mapping("IYX").with_role(0, "C")) + # I confirmed as C holds channels, not planes: no different_plane_counts (issue #74). assert codes(as_channel.combined_issues(report)) == [ "different_channel_counts", - "different_plane_counts", "axes_not_named", ] diff --git a/tests/test_role_sizes.py b/tests/test_role_sizes.py new file mode 100644 index 00000000..43cc8c24 --- /dev/null +++ b/tests/test_role_sizes.py @@ -0,0 +1,107 @@ +"""Size and plane-count warnings follow the confirmed roles of plain stacks (issue #74).""" + +import numpy as np +import pytest + +from morphobricks_preprocessing import inspect_images, prefill_mapping, run_experiment +from morphobricks_preprocessing.axes import AxisMapping +from morphobricks_preprocessing.axis_review import review_axes +from tiff_tree import write_data, write_plain, write_tiff + + +def codes(issues): + return [issue.code for issue in issues] + + +def issue(issues, code): + (found,) = [i for i in issues if i.code == code] + return found + + +def plain_yxc(tmp_path): + """Two (Y, X, C) arrays without axis metadata: tifffile reads them as QYX.""" + write_data(tmp_path / "a.tif", np.zeros((16, 16, 3), np.uint8)) + write_data(tmp_path / "b.tif", np.zeros((32, 16, 3), np.uint8)) + return inspect_images(tmp_path, ["a.tif", "b.tif"]) + + +def test_plain_stacks_are_not_compared_by_their_header_letters(tmp_path): + report = plain_yxc(tmp_path) + assert report.files[0].axes == "QYX" + # Before the roles are confirmed, the header's Q, Y and X say nothing: no warning. + assert not {"different_sizes", "different_plane_counts"} & set(codes(report.issues)) + + +def test_confirmed_yxc_compares_the_real_heights(tmp_path): + report = plain_yxc(tmp_path) + review = review_axes(report, AxisMapping("QYX", "YXC")) + issues = review.combined_issues(report) + assert codes(issues) == ["different_sizes", "axes_not_named"] + sizes = issue(issues, "different_sizes") + assert "16 x 16 (1 image), 32 x 16 (1 image)" in sizes.message + assert sizes.paths == ("b.tif",) + + +@pytest.mark.parametrize(("role", "name"), [("Z", "depth slices (Z)"), ("T", "time points")]) +def test_a_plain_stack_confirmed_as_depth_or_time(tmp_path, role, name): + write_data(tmp_path / "a.tif", np.zeros((3, 8, 8), np.uint8)) + write_data(tmp_path / "b.tif", np.zeros((5, 8, 8), np.uint8)) + report = inspect_images(tmp_path, ["a.tif", "b.tif"]) + assert "different_plane_counts" not in codes(report.issues) + review = review_axes(report, AxisMapping("QYX", f"{role}YX")) + issues = review.combined_issues(report) + planes = issue(issues, "different_plane_counts") + assert f"{name}: 3 (1 image), 5 (1 image)" in planes.message + assert "planes along Q" not in planes.message and planes.paths == ("b.tif",) + assert "different_sizes" not in codes(issues) + + +def test_a_sequence_of_pages_confirmed_as_time(tmp_path): + write_plain(tmp_path / "a.tif", np.zeros((2, 8, 8), np.uint8)) + write_plain(tmp_path / "b.tif", np.zeros((4, 8, 8), np.uint8)) + report = inspect_images(tmp_path, ["a.tif", "b.tif"]) + assert report.files[0].axes == "IYX" + issues = review_axes(report, prefill_mapping("IYX").with_role(0, "T")).combined_issues(report) + assert codes(issues) == ["different_plane_counts", "axes_not_named"] + planes = issue(issues, "different_plane_counts") + assert "time points: 2 (1 image), 4 (1 image)" in planes.message + + +def test_named_and_plain_images_together(tmp_path): + write_tiff(tmp_path / "named.tif", (8, 9), axes="YX") + write_data(tmp_path / "plain.tif", np.zeros((5, 8, 8), np.uint8)) + report = inspect_images(tmp_path, ["named.tif", "plain.tif"]) + # Only the named image is measured before the confirmation: nothing to compare. + assert not {"different_sizes", "different_plane_counts"} & set(codes(report.issues)) + review = review_axes(report, AxisMapping("QYX", "TYX"), missing_as_singleton=True) + issues = review.combined_issues(report) + assert codes(issues).count("different_sizes") == 1 + assert codes(issues).count("different_plane_counts") == 1 + assert "8 x 8 (1 image), 8 x 9 (1 image)" in issue(issues, "different_sizes").message + planes = issue(issues, "different_plane_counts") + assert "time points: 1 (1 image), 5 (1 image)" in planes.message + + +def test_a_plain_tiff_with_an_ome_zarr_image(tmp_path): + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + write_ngff(tmp_path / "z.ome.zarr", np.zeros((3, 8, 10), np.uint8), "tyx") + write_data(tmp_path / "plain.tif", np.zeros((5, 8, 8), np.uint8)) + report = inspect_images(tmp_path, ["plain.tif", "z.ome.zarr"]) + review = review_axes(report, AxisMapping("QYX", "TYX")) + issues = review.combined_issues(report) + assert "8 x 10 (1 image), 8 x 8 (1 image)" in issue(issues, "different_sizes").message + planes = issue(issues, "different_plane_counts") + assert "time points: 3 (1 image), 5 (1 image)" in planes.message + assert "planes along Q" not in planes.message + + +def test_the_run_reports_the_warnings_of_the_confirmed_roles(tmp_path): + root = tmp_path / "exp" + write_data(root / "a.tif", np.ones((16, 16, 3), np.uint16)) + write_data(root / "b.tif", np.ones((32, 16, 3), np.uint16)) + result = run_experiment(root, AxisMapping("QYX", "YXC")) + assert "different_plane_counts" not in codes(result.issues) + sizes = issue(result.issues, "different_sizes") + assert "16 x 16 (1 image), 32 x 16 (1 image)" in sizes.message diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index d96a4100..9f67be4c 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -26,10 +26,8 @@ "src/morphobricks_preprocessing/masks/files.py _name_issues: private function of # lines without a docstring": 15, "src/morphobricks_preprocessing/metadata.py _check_channel_names: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 30, - "src/morphobricks_preprocessing/metadata.py _check_image_sizes: private function of # lines without a docstring": 15, - "src/morphobricks_preprocessing/metadata.py _check_plane_counts: private function of # lines without a docstring": 27, - "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 97, - "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 779, + "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 96, + "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 756, "src/morphobricks_preprocessing/physical_sizes.py _imagej_sizes: private function of # lines without a docstring": 27, "src/morphobricks_preprocessing/physical_sizes.py _ome_sizes: private function of # lines without a docstring": 20, "src/morphobricks_preprocessing/physical_sizes.py: # lines (max #)": 706, From 477c4a04e1a815d997c6abb027d96205d635fb7c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 20:28:12 +0000 Subject: [PATCH 3/5] Review #131: one breakdown helper for the metadata and size checks Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/metadata.py | 15 +++---------- src/morphobricks_preprocessing/size_checks.py | 22 +++++++++++++++---- tools/code_rules_baseline.json | 2 +- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index b2dd945e..f554e28c 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -32,6 +32,7 @@ from .physical_sizes import PhysicalSizes, _tiff_sizes from .pixel_size_checks import pixel_size_issues from .size_checks import ( + breakdown, image_size_issues, lettered_plane_counts, plane_count_issues, @@ -737,18 +738,8 @@ def _breakdown( key: Callable[[TiffMetadata], Hashable], describe: Callable[[Hashable], str], ) -> tuple[str, list[str]]: - """Summarise files by ``key``, most common first. - - Returns the summary text, e.g. ``"ZCYX (10 images), ZYX (2 images)"``, and the - paths of the files outside the most common group. - """ - groups: dict[Hashable, list[str]] = {} - for f in files: - groups.setdefault(key(f), []).append(f.relative_path) - ordered = sorted(groups.items(), key=lambda item: (-len(item[1]), describe(item[0]))) - summary = ", ".join(f"{describe(k)} ({plural(len(paths), 'image')})" for k, paths in ordered) - minority = [path for _, paths in ordered[1:] for path in paths] - return summary, minority + """Summarise files by ``key``, most common first (:func:`.size_checks.breakdown`).""" + return breakdown({f.relative_path: key(f) for f in files}, describe) def _is_subsequence(short: str, long: str) -> bool: diff --git a/src/morphobricks_preprocessing/size_checks.py b/src/morphobricks_preprocessing/size_checks.py index fbb0aa94..9ae3fa48 100644 --- a/src/morphobricks_preprocessing/size_checks.py +++ b/src/morphobricks_preprocessing/size_checks.py @@ -58,7 +58,7 @@ def image_size_issues(sizes: Mapping[str, tuple[int, int]]) -> list[ValidationIs """ if len(set(sizes.values())) < 2: return [] - summary, minority = _breakdown(sizes, lambda shape: " x ".join(str(n) for n in shape)) + summary, minority = breakdown(sizes, lambda shape: " x ".join(str(n) for n in shape)) return [ warning( "different_sizes", @@ -93,7 +93,7 @@ def plane_count_issues(counts: Mapping[str, Mapping[str, int]]) -> list[Validati along = {path: c.get(axis, 1) for path, c in counts.items()} if len(set(along.values())) < 2: continue - summary, minority = _breakdown(along, str) + summary, minority = breakdown(along, str) descriptions.append(f"{_PLANE_NAMES.get(axis, f'planes along {axis}')}: {summary}") differing.extend(minority) if not differing: @@ -113,10 +113,24 @@ def plane_count_issues(counts: Mapping[str, Mapping[str, int]]) -> list[Validati ] -def _breakdown( +def breakdown( values: Mapping[str, Hashable], describe: Callable[[Hashable], str] ) -> tuple[str, list[str]]: - """Summarise the images by value, most common first; return it and the other images.""" + """Summarise images by value, most common first. + + Parameters + ---------- + values + The value of each image, by relative path. + describe + How a value is written in the summary; groups of equal size are ordered by it. + + Returns + ------- + tuple of (str, list of str) + The summary, e.g. ``"ZCYX (10 images), ZYX (2 images)"``, and the paths of the + images outside the most common group. + """ groups: dict[Hashable, list[str]] = {} for path, value in values.items(): groups.setdefault(value, []).append(path) diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index bbdc93a3..a0f6d66b 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -26,7 +26,7 @@ "src/morphobricks_preprocessing/metadata.py _check_channel_names: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 30, "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 96, - "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 756, + "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 747, "src/morphobricks_preprocessing/physical_sizes.py _imagej_sizes: private function of # lines without a docstring": 27, "src/morphobricks_preprocessing/physical_sizes.py _ome_sizes: private function of # lines without a docstring": 20, "src/morphobricks_preprocessing/physical_sizes.py: # lines (max #)": 684, From bc0116c50bc40bfafa43eb2d81e0d2b6513ce87a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 20:28:12 +0000 Subject: [PATCH 4/5] Review #131: contradicted roles are never used for size warnings (tests) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_role_sizes.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_role_sizes.py b/tests/test_role_sizes.py index 43cc8c24..4bab0364 100644 --- a/tests/test_role_sizes.py +++ b/tests/test_role_sizes.py @@ -105,3 +105,32 @@ def test_the_run_reports_the_warnings_of_the_confirmed_roles(tmp_path): assert "different_plane_counts" not in codes(result.issues) sizes = issue(result.issues, "different_sizes") assert "16 x 16 (1 image), 32 x 16 (1 image)" in sizes.message + + +def test_contradicted_roles_are_never_used_for_the_warnings(tmp_path): + # OME-Zarr ZYX confirmed as TYX contradicts its OME-NGFF axes (issue #72): it is + # measured by the Z its metadata names, never as time points. + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + write_ngff(tmp_path / "a.ome.zarr", np.zeros((3, 8, 8), np.uint8), "zyx") + write_ngff(tmp_path / "b.ome.zarr", np.zeros((5, 8, 8), np.uint8), "zyx") + report = inspect_images(tmp_path, ["a.ome.zarr", "b.ome.zarr"]) + review = review_axes(report, AxisMapping("ZYX", "TYX")) + issues = review.combined_issues(report) + assert "ome_zarr_axes_mismatch" in codes(issues) + planes = issue(issues, "different_plane_counts") + assert "depth slices (Z): 3 (1 image), 5 (1 image)" in planes.message + assert "time points" not in planes.message + + +def test_an_image_with_an_axis_error_is_measured_by_its_header(tmp_path): + write_tiff(tmp_path / "a.tif", (3, 8, 9), axes="ZYX") + write_tiff(tmp_path / "b.tif", (3, 8, 12), axes="ZYX") + report = inspect_images(tmp_path, ["a.tif", "b.tif"]) + review = review_axes(report, AxisMapping("ZYX", "ZXY")) + issues = review.combined_issues(report) + assert "axes_yx_swapped" in codes(issues) + # Height x width as the header names them, not as the swapped roles would give. + assert "8 x 12 (1 image), 8 x 9 (1 image)" in issue(issues, "different_sizes").message + assert codes(issues).count("different_sizes") == 1 From 7bed643351860cbb3fd0475867625b77e97c5183 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 20:29:46 +0000 Subject: [PATCH 5/5] Review #131: plan questions 64 and 65 follow main's 63; exact-0.5 wording Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/plan.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/plan.md b/docs/plan.md index e4fcf466..7638f769 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1589,7 +1589,7 @@ Raised during review; to be decided before the milestones that need them: answer (the folder may still be being typed), and the design card says which. The run is gated as before, so it runs only on the new folder's confirmed design. No copy action: the user answers again, or copies the file. Contract §12; user guide §4. -66. How are the one-to-one matches of `same_objects` counted (issue #67)? +64. How are the one-to-one matches of `same_objects` counted (issue #67)? *Decided (2 November 2026, implemented for issue #67; the rule is the issue's acceptance): as a maximum matching of the child and parent objects of each plane whose intersection over union is at least `same_objects_iou` @@ -1597,13 +1597,14 @@ Raised during review; to be decided before the milestones that need them: Each object is matched at most once, so two children halving one parent are one match. The number of matches is the same for every maximum matching, so the counts do not depend on label numbers or tie-breaking. Above an intersection over union of 0.5 the - matching is every eligible pair (an object can overlap only one partner that much), so - the default threshold changes nothing for real one-to-one segmentations (the HPMEC + matching is every eligible pair (an object can overlap only one partner that much); at + the default of exactly 0.5 only an object split into exact halves has two eligible + partners, so the default changes nothing for real one-to-one segmentations (the HPMEC masks give the same report). The evidence adds `children_not_matched`, `parents_not_matched`, `children_over_iou_with_several_parents` and `parents_over_iou_with_several_children`; `mask_relationships.json` is report version 2. Contract §15.1.* -67. Which axes do the size and plane-count warnings use (issue #74)? +65. Which axes do the size and plane-count warnings use (issue #74)? *Decided (2 November 2026, implemented for issue #74): the confirmed roles, once they are confirmed. Before (`check`, preflight), `different_sizes` and `different_plane_counts` compare the axes each header names, and a plain stack is left