Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
- **Notebook: one step at a time (UX proposal 2026-11-09, PR 17a).** `PreprocessingNotebook.steps()`
shows the shipped notebook's 13 steps one at a time, with a strip of chips (done green with a tick,
current blue, to do grey, changed by a reopen orange; a chip opens its step) and Back/Next (Next is
Expand Down
8 changes: 8 additions & 0 deletions src/morphobricks_preprocessing/description_layouts.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,11 @@ 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}
if len(kinds) == 1:
return f"{kinds.pop()}, with different channel lists"
return " and ".join(sorted(kinds, key=lambda kind: kind == "RGB"))
21 changes: 21 additions & 0 deletions src/morphobricks_preprocessing/notebook_axis_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -194,6 +195,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 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."
],
)
return ui.status(
"error",
"No axis roles can be suggested.",
Expand All @@ -205,6 +218,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)
Expand Down
33 changes: 32 additions & 1 deletion src/morphobricks_preprocessing/run/description_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down
136 changes: 136 additions & 0 deletions tests/test_heterogeneous_refusal_first.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
"""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()


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"
Loading