From 72edd2e40a754078a86cac3f6e875a31404bb344 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 08:15:16 +0000 Subject: [PATCH 1/2] Report the layout refusal first in described runs with several layouts (#245) The run lists R_HETEROGENEOUS_LAYOUT_NOT_ENABLED before the axis review's issues, and the arrangement issues no longer advise exporting the images the same way. The notebook's axis step says no roles are suggested because the units declare different layouts. The refusal codes are unchanged. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 6 + .../description_layouts.py | 6 + .../notebook_axis_step.py | 21 +++ .../run/description_run.py | 33 ++++- tests/test_heterogeneous_refusal_first.py | 123 ++++++++++++++++++ 5 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 tests/test_heterogeneous_refusal_first.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5916904b..2c6e1522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ are in [docs/plan.md](docs/plan.md) ยง12. Upgrading from v0.3: see the ## Unreleased +- **Described runs with several layouts report `R_HETEROGENEOUS_LAYOUT_NOT_ENABLED` first (#245).** + The run lists it before the axis review's issues, and the arrangement issues + (`inconsistent_mapping`, `missing_axes`, `ambiguous_missing_axes`) no longer advise exporting the + images the same way: they stay errors and say they follow from the layouts. The notebook's axis + step says that no roles are suggested because the units declare different layouts (fluorescence + and RGB). The refusal codes are unchanged: nothing refused before is accepted. - **Re-pin the vendored Spec to 1bed024 (MorphoBricks-Spec PR 16, owner decisions RF1 to RF6); `read_records` follows section 4.6.** The Spec now fixes the bytes and values of `morphobricks.records.jsonl` and names one refusal, `R_RECORDS_FILE_INVALID`, with the reasons diff --git a/src/morphobricks_preprocessing/description_layouts.py b/src/morphobricks_preprocessing/description_layouts.py index 014ea569..c84d8ba6 100644 --- a/src/morphobricks_preprocessing/description_layouts.py +++ b/src/morphobricks_preprocessing/description_layouts.py @@ -88,3 +88,9 @@ def heterogeneous_units(layouts: tuple[DeclaredLayout, ...]) -> tuple[str, ...]: return tuple( f"{_label(index)}: {unit}" for index, layout in enumerate(layouts) for unit in layout.units ) + + +def layout_kinds(layouts: tuple[DeclaredLayout, ...]) -> str: + """Name the kinds of the layouts in words, e.g. ``"fluorescence and RGB"``.""" + kinds = {"RGB" if layout.image_type == "rgb" else "fluorescence" for layout in layouts} + return " and ".join(sorted(kinds, key=lambda kind: kind == "RGB")) diff --git a/src/morphobricks_preprocessing/notebook_axis_step.py b/src/morphobricks_preprocessing/notebook_axis_step.py index 0fcc4aed..887a71f8 100644 --- a/src/morphobricks_preprocessing/notebook_axis_step.py +++ b/src/morphobricks_preprocessing/notebook_axis_step.py @@ -10,6 +10,7 @@ from .axes import axis_length_ranges, axis_name, file_mapping_for from .axis_check import AXIS_CHECK_CODES from .axis_review import review_axes +from .description_layouts import declared_layouts, is_heterogeneous, layout_kinds from .image_type import FLUORESCENCE from .notebook_axes import axis_check_html, header_marks from .notebook_options import IMAGE_TYPE_OPTIONS, ROLE_OPTIONS @@ -193,6 +194,18 @@ def _axis_waiting(self) -> str: if self.preflight is None: return ui.status("info", "Waiting for step 1.", ["Check the experiment folder first."]) if self.mapping is None: + kinds = self._layout_kinds() + if kinds: + return ui.status( + "error", + "No axis roles are suggested: the units declare different layouts.", + [ + f"The saved description declares {kinds} units, which are arranged " + "differently. This version runs one layout per experiment, so the " + "run stops with R_HETEROGENEOUS_LAYOUT_NOT_ENABLED (see Check and " + "save). Exporting the images again would not change that." + ], + ) return ui.status( "error", "No axis roles can be suggested.", @@ -204,6 +217,14 @@ def _axis_waiting(self) -> str: ) return "" + def _layout_kinds(self) -> str: + """Return the kinds of layout the saved description declares when they differ, else ''.""" + state = self._published() if getattr(self, "_describe", False) else None + if state is None: + return "" + layouts = declared_layouts(state.description) + return layout_kinds(layouts) if is_heterogeneous(layouts) else "" + def _role_grid(self) -> widgets.GridBox: """Build one box per axis, with its length, the suggested role and a dropdown.""" ranges = axis_length_ranges(self.mapping, self.preflight.metadata.files) diff --git a/src/morphobricks_preprocessing/run/description_run.py b/src/morphobricks_preprocessing/run/description_run.py index 5705730b..23708c11 100644 --- a/src/morphobricks_preprocessing/run/description_run.py +++ b/src/morphobricks_preprocessing/run/description_run.py @@ -29,7 +29,7 @@ from ..description_record import description_record, unit_records from ..description_run_text import heterogeneous_issue, physical_issue, reader_issue from ..mask_representation import NOT_CHECKED, file_status, read_checked -from ..physical_check import PhysicalReport, check_physical +from ..physical_check import HETEROGENEOUS_LAYOUT, PhysicalReport, check_physical from ..unit_consistency import declared_type_issues from ..validation import ValidationIssue, error from .description_file import RecordsFileError, read_records @@ -163,9 +163,40 @@ def checked_description(output: Path, pre: Preflight, checked: Checked) -> RunDe roles = None if checked.review is None else checked.review.file_mappings found, refused = check_description(found, pre, roles) checked.issues.extend(refused) + checked.issues[:] = heterogeneous_first(checked.issues) return found +SECONDARY_CODES = frozenset({"inconsistent_mapping", "missing_axes", "ambiguous_missing_axes"}) +"""The axis review's arrangement codes, whose advice is to export the images again.""" + +SECONDARY_ADVICE = ( + "Do not export the images again for this: it follows from the different layouts " + f"({HETEROGENEOUS_LAYOUT}, listed first). This version runs one layout per experiment." +) + + +def heterogeneous_first(issues: Iterable[ValidationIssue]) -> list[ValidationIssue]: + """Put the layout refusal first and make the axis review's arrangement issues secondary. + + With several layouts the arrangement of dimensions differs between units by design, so the + axis review's arrangement issues (:data:`SECONDARY_CODES`) are a consequence, and + exporting the images the same way would not help: their advice is replaced. Only the order + and the advice change: no issue is added, removed or turned into a warning, so the run + refuses exactly as before. + """ + found = list(issues) + layout = [i for i in found if i.code == HETEROGENEOUS_LAYOUT] + if not layout: + return found + rest = [ + replace(i, action=SECONDARY_ADVICE) if i.code in SECONDARY_CODES else i + for i in found + if i.code != HETEROGENEOUS_LAYOUT + ] + return [*layout, *rest] + + def mask_records( description: Description, masks: Mapping | None, output: Path, images: int ) -> dict[str, dict]: diff --git a/tests/test_heterogeneous_refusal_first.py b/tests/test_heterogeneous_refusal_first.py new file mode 100644 index 00000000..33f34d36 --- /dev/null +++ b/tests/test_heterogeneous_refusal_first.py @@ -0,0 +1,123 @@ +"""With several layouts the layout refusal comes first and the axis advice follows (issue #245). + +The Spec refuses an RGB unit next to a fluorescence unit (``R_HETEROGENEOUS_LAYOUT_NOT_ENABLED``, +fixture v29). The axis review's ``inconsistent_mapping`` is a consequence of the layouts, so its +advice to export the images the same way must not lead. Nothing refused before is accepted: the +codes of the refusals are exactly those of before. +""" + +from itertools import combinations + +import pytest + +from description_tree import build_experiment +from morphobricks_preprocessing import prefill_mapping, preflight, run_experiment +from morphobricks_preprocessing.axis_review import review_axes +from morphobricks_preprocessing.description import description_from_bytes +from morphobricks_preprocessing.description_candidate import check_candidate +from morphobricks_preprocessing.image_consistency import arrangement_issues +from morphobricks_preprocessing.notebook_check_view import checks_html +from morphobricks_preprocessing.unit_consistency import unit_checked +from test_described_unit_checks import ( + LAYOUT, + VALID, + _checked_notebook, + mixed_experiment, + publish, +) +from test_metadata import meta + + +def test_the_layout_refusal_comes_first_in_the_run(tmp_path): + root, out = mixed_experiment(tmp_path) + publish(out) + result = run_experiment(root, prefill_mapping("CYX"), output_folder=out) + assert not result.written + assert [i.code for i in result.issues][0] == LAYOUT + assert {i.code for i in result.issues} == {LAYOUT, "inconsistent_mapping"} + + +def test_the_axis_issue_stays_an_error_without_the_advice_to_export_again(tmp_path): + root, out = mixed_experiment(tmp_path) + publish(out) + result = run_experiment(root, prefill_mapping("CYX"), output_folder=out) + (axis,) = [i for i in result.issues if i.code == "inconsistent_mapping"] + assert axis.is_error + assert "Export the images again" not in axis.action + assert LAYOUT in axis.action and "one layout per experiment" in axis.action + + +def test_a_run_of_one_layout_keeps_the_advice_of_the_axis_review(tmp_path): + description = description_from_bytes(VALID["v01"].data) + root, out = tmp_path / "exp", tmp_path / "out" + out.mkdir() + (out / "morphobricks.toml").write_bytes(VALID["v01"].data) + build_experiment(root, description, images=2) + result = run_experiment(root, prefill_mapping("YXC"), output_folder=out) + (axis,) = [i for i in result.issues if i.code == "inconsistent_mapping"] + assert "Export the images again" in axis.action + assert LAYOUT not in {i.code for i in result.issues} + + +@pytest.mark.parametrize("confirmed", [False, True]) +def test_the_check_and_save_list_leads_with_the_layout_refusal(tmp_path, confirmed): + root, out = mixed_experiment(tmp_path) + pre = unit_checked(preflight(root, out)) + roles = review_axes(pre.metadata, prefill_mapping("CYX")).file_mappings if confirmed else None + check = check_candidate(description_from_bytes(VALID["v29"].data), pre, roles, out) + assert check.run_refusals[0].code == LAYOUT + assert check.heterogeneous is check.run_refusals[0] + text = checks_html(check) + assert "2 channel layouts" in text + assert "inconsistent_mapping" not in text + + +def test_the_axis_step_says_the_layouts_are_why_no_roles_are_suggested(tmp_path): + root, out = mixed_experiment(tmp_path) + publish(out) + nb = _checked_notebook(root, out, describe=True) + assert nb.mapping is None + text = nb._axis_waiting() + assert "the units declare different layouts" in text + assert "fluorescence and RGB" in text + assert "one layout per experiment" in text + assert "dimensions are arranged differently" not in text + + +def test_the_axis_step_keeps_its_general_message_without_a_description(tmp_path): + root, out = mixed_experiment(tmp_path) + nb = _checked_notebook(root, out, describe=True) + assert nb.mapping is None + text = nb._axis_waiting() + assert "No axis roles can be suggested." in text + assert "different layouts" not in text + + +BASE = ["TCZYX", "TCYX", "CTYX", "TZYX", "CYX", "TYX", "ZYX", "YX"] + + +def _dropped(short, long): + rest = iter(long) + return all(axis in rest for axis in short) + + +def _found(*axes): + files = [meta(f"{n}.tif", axes=a) for n, a in enumerate(axes)] + return {i.code for i in arrangement_issues(files)} + + +def test_missing_axes_means_one_longest_arrangement_and_the_rest_with_axes_dropped(): + """Pin the ``missing_axes`` rule against an independent statement of it (issue #243 review). + + The clause ``sum(len(a) == len(longest) ...) == 1`` is equivalent to ``>= 1``: two different + arrangements of the longest length are never both subsequences of it, so the ``all(...)`` + before it already refuses them. Mutating it to ``>= 1`` therefore changes no behaviour and no + test can kill it. What this test fixes is the behaviour, for every pair and triple of + arrangements: ``missing_axes`` only when every image is the longest with axes dropped, else + ``inconsistent_dimensions``, and nothing for one arrangement. + """ + for group in [*combinations(BASE, 2), *combinations(BASE, 3)]: + longest = max(group, key=len) + dropped = all(_dropped(axes, longest) for axes in group) + assert _found(*group) == {"missing_axes" if dropped else "inconsistent_dimensions"}, group + assert _found("CYX", "CYX") == set() From bc95ab2ab5495274961a7be47879e5fb4b4850e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 08:42:53 +0000 Subject: [PATCH 2/2] Name the kinds of several layouts of one image type in the axis step note Two fluorescence layouts with different channel lists were described as "fluorescence units, which are arranged differently". Say the channel lists differ. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- .../description_layouts.py | 2 ++ .../notebook_axis_step.py | 4 ++-- tests/test_heterogeneous_refusal_first.py | 13 +++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/morphobricks_preprocessing/description_layouts.py b/src/morphobricks_preprocessing/description_layouts.py index c84d8ba6..4ddbcbdf 100644 --- a/src/morphobricks_preprocessing/description_layouts.py +++ b/src/morphobricks_preprocessing/description_layouts.py @@ -93,4 +93,6 @@ def heterogeneous_units(layouts: tuple[DeclaredLayout, ...]) -> tuple[str, ...]: def layout_kinds(layouts: tuple[DeclaredLayout, ...]) -> str: """Name the kinds of the layouts in words, e.g. ``"fluorescence and RGB"``.""" kinds = {"RGB" if layout.image_type == "rgb" else "fluorescence" for layout in layouts} + if len(kinds) == 1: + return f"{kinds.pop()}, with different channel lists" return " and ".join(sorted(kinds, key=lambda kind: kind == "RGB")) diff --git a/src/morphobricks_preprocessing/notebook_axis_step.py b/src/morphobricks_preprocessing/notebook_axis_step.py index bb997ef1..8f4097bb 100644 --- a/src/morphobricks_preprocessing/notebook_axis_step.py +++ b/src/morphobricks_preprocessing/notebook_axis_step.py @@ -201,8 +201,8 @@ def _axis_waiting(self) -> str: "error", "No axis roles are suggested: the units declare different layouts.", [ - f"The saved description declares {kinds} units, which are arranged " - "differently. This version runs one layout per experiment, so the " + f"The saved description declares different layouts ({kinds}). " + "This version runs one layout per experiment, so the " "run stops with R_HETEROGENEOUS_LAYOUT_NOT_ENABLED (see Check and " "save). Exporting the images again would not change that." ], diff --git a/tests/test_heterogeneous_refusal_first.py b/tests/test_heterogeneous_refusal_first.py index 33f34d36..0a5fe281 100644 --- a/tests/test_heterogeneous_refusal_first.py +++ b/tests/test_heterogeneous_refusal_first.py @@ -121,3 +121,16 @@ def test_missing_axes_means_one_longest_arrangement_and_the_rest_with_axes_dropp dropped = all(_dropped(axes, longest) for axes in group) assert _found(*group) == {"missing_axes" if dropped else "inconsistent_dimensions"}, group assert _found("CYX", "CYX") == set() + + +def test_layout_kinds_names_two_channel_lists_of_one_image_type_in_words(): + """Two fluorescence layouts differ by channels, not by type: say so, not "fluorescence".""" + from morphobricks_preprocessing.description_layouts import DeclaredLayout, layout_kinds + + both = ( + DeclaredLayout(("DNA",), "fluorescence", ()), + DeclaredLayout(("DNA", "ACTIN"), None, ()), + ) + assert layout_kinds(both) == "fluorescence, with different channel lists" + mixed = (both[0], DeclaredLayout(("R", "G", "B"), "rgb", ())) + assert layout_kinds(mixed) == "fluorescence and RGB"