From 2f1bf1f8f191caf5e44e3652aad1c40bd405a3bb Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:25:34 +0000 Subject: [PATCH 1/7] PP-10d: move the masks step out of the CLI into run.mask_step (move only) The pairing with the recorded inputs, the scan and the write of mask_relationships.json under the output folder's lock move from cli_masks.masks_command into run.mask_step (orchestration): prepare_mask_step returns a MaskStep (preflight, output folder, input changes, blocking issues, or the experiment_not_recorded refusal) and describe_masks scans and writes, returning the output_locked refusal instead of raising. The CLI only prints their results; its output is unchanged (same text and exit codes over 13 scenarios, HPMEC included). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- pyproject.toml | 1 + src/morphobricks_preprocessing/cli_masks.py | 89 +++----- .../run/__init__.py | 5 + .../run/mask_step.py | 198 ++++++++++++++++++ tests/test_mask_step.py | 100 +++++++++ 5 files changed, 328 insertions(+), 65 deletions(-) create mode 100644 src/morphobricks_preprocessing/run/mask_step.py create mode 100644 tests/test_mask_step.py diff --git a/pyproject.toml b/pyproject.toml index 60781780..0bade8a0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -194,6 +194,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.run.pipeline" = "orchestration" "morphobricks_preprocessing.run.entries" = "io" "morphobricks_preprocessing.run.check_step" = "orchestration" +"morphobricks_preprocessing.run.mask_step" = "orchestration" "morphobricks_preprocessing.run.planned" = "orchestration" "morphobricks_preprocessing.run.design_file" = "io" "morphobricks_preprocessing.run.lock" = "io" diff --git a/src/morphobricks_preprocessing/cli_masks.py b/src/morphobricks_preprocessing/cli_masks.py index 29fd1597..de1740f7 100644 --- a/src/morphobricks_preprocessing/cli_masks.py +++ b/src/morphobricks_preprocessing/cli_masks.py @@ -5,30 +5,17 @@ with each mask type and the relationships between them (contract §15). Like ``run``, it takes every input it is not given from the run in the output folder (design v0.4 §1): the experiment folder (``experiment_not_recorded`` without one), the masks folder, the mask -axes and the exclusions; a value given that differs is used and named. +axes and the exclusions; a value given that differs is used and named. The pairing, the +scan and the write under the lock are :mod:`.run.mask_step`'s, shared with the notebook. """ from __future__ import annotations import argparse -from dataclasses import replace -from pathlib import Path -from .cli_run import command_preflight, use_recorded_experiment -from .discovery import default_output_folder -from .mask_scan import ( - DEFAULT_THRESHOLDS, - MASK_REPORT_FILENAME, - mask_thresholds, - scan_masks, - write_mask_report, -) -from .masks import MaskInventory +from .mask_scan import DEFAULT_THRESHOLDS, mask_thresholds from .report import format_issues, mask_report_summary, mask_summary -from .run import OutputLockedError -from .run.lock import lock_issue, output_lock -from .run.recorded import GivenInputs, InputChange, resolve_inputs -from .run.reuse import read_previous +from .run.mask_step import describe_masks, prepare_mask_step from .validation import has_errors THRESHOLD_HELP = { @@ -42,11 +29,6 @@ } -_RECORDED = ("experiment_root", "exclude", "masks_folder", "mask_axes") -"""The recorded inputs ``masks`` uses.""" -_EFFECT = "used for this scan only; the run keeps the recorded value until it is given this one" - - def add_masks_arguments(masks: argparse.ArgumentParser) -> None: """Add the options of ``masks`` after the shared ones. @@ -82,21 +64,25 @@ def masks_command(args: argparse.Namespace) -> int: except ValueError as exc: print(exc) return 1 - if not use_recorded_experiment(args): + step = prepare_mask_step( + args.experiment, + args.output, + exclude=() if args.no_exclude else (tuple(args.exclude) or None), + masks_folder=args.masks, + mask_axes=args.mask_axes or None, + ) + if step.refusal is not None: + print(format_issues([step.refusal])) return 1 - changes = _recorded_inputs(args) - pre = command_preflight(args) - pre = replace(pre, masks=pre.masks or MaskInventory(folder=None)) # none found - print(mask_summary(pre.masks)) - for change in changes: + print(mask_summary(step.masks)) + for change in step.changes: print(f"Changed {change.name}: {change.recorded} -> {change.given} ({change.effect})") - blocking = [*pre.discovery.issues, *pre.exclusion_issues, *pre.masks.issues] - if blocking: + if step.issues: print() - print(format_issues(blocking)) - if has_errors(blocking): + print(format_issues(step.issues)) + if has_errors(step.issues): return 1 - if not pre.masks.types: + if not step.masks.types: print("Give the masks folder with --masks, or use OME-Zarr images with label images.") return 1 @@ -106,43 +92,16 @@ def progress(done: int, total: int, path: str) -> None: print() try: - report = scan_masks(pre.masks, pre.discovery.root, progress=progress, thresholds=thresholds) + result = describe_masks(step, progress=progress, thresholds=thresholds) except ValueError as exc: # masks whose pixels cannot be used (planes, labels) print(f"The masks cannot be used: {exc}") return 1 - output = Path(args.output) if args.output else default_output_folder(pre.discovery.root) - output.mkdir(parents=True, exist_ok=True) - try: - with output_lock(output): # a run may be replacing or removing the same file - path = write_mask_report(report, output / MASK_REPORT_FILENAME) - except OutputLockedError as exc: + if result.refusal is not None: print() - print(format_issues([lock_issue(exc)])) + print(format_issues([result.refusal])) return 1 print() - print(mask_report_summary(report)) + print(mask_report_summary(result.report)) print() - print(f"Report written to {path}") + print(f"Report written to {result.path}") return 0 - - -def _recorded_inputs(args: argparse.Namespace) -> tuple[InputChange, ...]: - """Fill the masks folder, mask axes and exclusions not given from the recorded run. - - Returns the inputs given that differ from the recorded ones, with their effect. - """ - root = Path(args.experiment).resolve() - output = Path(args.output) if args.output else default_output_folder(root) - exclude = () if args.no_exclude else (tuple(args.exclude) or None) - given = GivenInputs( - experiment_root=root, - exclude=exclude, - masks_folder=Path(args.masks).resolve() if args.masks else None, - mask_axes=args.mask_axes or None, - ) - resolved = resolve_inputs(given, read_previous(output.resolve())[0]) - args.masks, args.mask_axes = resolved.masks_folder, resolved.mask_axes - args.exclude = list(resolved.exclude) - return tuple( - replace(change, effect=_EFFECT) for change in resolved.changes if change.name in _RECORDED - ) diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py index ab091235..daeed1ae 100644 --- a/src/morphobricks_preprocessing/run/__init__.py +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -26,6 +26,7 @@ from .check_step import CheckOutputs, write_check_outputs, write_check_report from .design_file import edit_design, write_design_template from .lock import LOCK_FILENAME, LockHolder, OutputLockedError, read_lock, remove_stale_lock +from .mask_step import MaskStep, MaskStepResult, describe_masks, prepare_mask_step from .modes import FULL, MODES, RESTART from .pipeline import run_experiment from .preflight import Preflight, SamplePlane, preflight, sample_planes @@ -38,12 +39,16 @@ "RESTART", "CheckOutputs", "LockHolder", + "MaskStep", + "MaskStepResult", "OutputLockedError", "Preflight", "RunResult", "SamplePlane", + "describe_masks", "edit_design", "preflight", + "prepare_mask_step", "read_lock", "remove_stale_lock", "run_experiment", diff --git a/src/morphobricks_preprocessing/run/mask_step.py b/src/morphobricks_preprocessing/run/mask_step.py new file mode 100644 index 00000000..d0b69f11 --- /dev/null +++ b/src/morphobricks_preprocessing/run/mask_step.py @@ -0,0 +1,198 @@ +"""The optional mask step: describe the masks and write ``mask_relationships.json``. + +The command line's ``masks`` and the notebook's **Describe the masks** (plan §10, PP-10d) +run the same two functions (contract §15): + +- :func:`prepare_mask_step` pairs every image with its masks, with the inputs the run in the + output folder recorded unless they are given (design v0.4 §1): the experiment folder + (``experiment_not_recorded`` without one), the masks folder, the mask axes and the + exclusions; a value given that differs is used and named. It reads no pixels. +- :func:`describe_masks` reads the masks (:func:`.mask_scan.scan_masks`) and writes the + report, holding the output folder's lock while it writes: a run may be replacing or + removing the same file. +""" + +from __future__ import annotations + +import os +from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass, replace +from pathlib import Path + +from ..discovery import default_output_folder +from ..mask_scan import MASK_REPORT_FILENAME, MaskReport, scan_masks, write_mask_report +from ..masks import MaskInventory +from ..validation import ValidationIssue, has_errors +from .lock import OutputLockedError, lock_issue, output_lock +from .preflight import Preflight, preflight +from .recorded import GivenInputs, InputChange, resolve_inputs +from .reuse import read_previous, recorded_experiment_root + +_RECORDED = ("experiment_root", "exclude", "masks_folder", "mask_axes") +"""The recorded inputs the mask step uses.""" +_EFFECT = "used for this scan only; the run keeps the recorded value until it is given this one" + + +@dataclass(frozen=True) +class MaskStep: + """The masks the mask step would read, found with the inputs it uses. + + Attributes + ---------- + preflight + The folder check with the masks paired; None when refused. + output_folder + Where the report is written; None when refused. + changes + The inputs given that differ from the recorded ones, with their effect. + issues + The issues that stop the step when one is an error: the folder's, the + exclusions' and the masks'. + refusal + ``experiment_not_recorded`` when no experiment folder was given or recorded. + """ + + preflight: Preflight | None + output_folder: Path | None + changes: tuple[InputChange, ...] = () + issues: tuple[ValidationIssue, ...] = () + refusal: ValidationIssue | None = None + + @property + def masks(self) -> MaskInventory: + """The masks found (none when refused).""" + if self.preflight is None or self.preflight.masks is None: + return MaskInventory(folder=None) + return self.preflight.masks + + @property + def ready(self) -> bool: + """True when the masks can be read: no refusal, no error, at least one mask type.""" + return self.refusal is None and not has_errors(self.issues) and bool(self.masks.types) + + +@dataclass(frozen=True) +class MaskStepResult: + """What :func:`describe_masks` did. + + Attributes + ---------- + report + The description of the masks. + path + The report written, or None when the output folder's lock refused it. + refusal + ``output_locked`` when another run holds the output folder's lock, else None. + """ + + report: MaskReport + path: Path | None + refusal: ValidationIssue | None = None + + +def prepare_mask_step( + experiment_root: str | os.PathLike | None, + output_folder: str | os.PathLike | None, + *, + exclude: Sequence[str] | None = None, + masks_folder: str | os.PathLike | None = None, + mask_axes: Mapping[str, str] | None = None, +) -> MaskStep: + """Pair the masks the mask step reads, with the inputs the earlier run recorded. + + Parameters + ---------- + experiment_root + The experiment folder; None takes the one the run in ``output_folder`` recorded. + output_folder + The output folder; None is the default next to the experiment. + exclude + Images to leave out; None takes the recorded exclusions, ``()`` clears them. + masks_folder + The masks folder; None takes the recorded one. + mask_axes + Mask axes stated per mask type; they add to (and replace per type) the recorded ones. + + Returns + ------- + MaskStep + With ``refusal`` set when no experiment folder was given or recorded. + """ + if experiment_root is None: + recorded, missing = recorded_experiment_root( + output_folder and Path(output_folder).resolve() + ) + if missing is not None: + return MaskStep(preflight=None, output_folder=None, refusal=missing) + experiment_root = str(recorded) + root = Path(experiment_root).resolve() + output = Path(output_folder) if output_folder else default_output_folder(root) + given = GivenInputs( + experiment_root=root, + exclude=None if exclude is None else tuple(exclude), + masks_folder=Path(masks_folder).resolve() if masks_folder else None, + mask_axes=mask_axes or None, + ) + resolved = resolve_inputs(given, read_previous(output.resolve())[0]) + changes = tuple( + replace(change, effect=_EFFECT) for change in resolved.changes if change.name in _RECORDED + ) + pre = preflight( + experiment_root, + output_folder, + exclude=list(resolved.exclude), + masks_folder=resolved.masks_folder, + mask_axes=resolved.mask_axes, + ) + pre = replace(pre, masks=pre.masks or MaskInventory(folder=None)) # none found + return MaskStep( + preflight=pre, + output_folder=output, + changes=changes, + issues=(*pre.discovery.issues, *pre.exclusion_issues, *pre.masks.issues), + ) + + +def describe_masks( + step: MaskStep, + *, + thresholds: Mapping[str, float] | None = None, + progress: Callable[[int, int, str], None] | None = None, +) -> MaskStepResult: + """Read the masks of a ready :class:`MaskStep` and write ``mask_relationships.json``. + + Parameters + ---------- + step + The masks to read; :attr:`MaskStep.ready`. + thresholds + Thresholds to change from their defaults (:func:`.mask_scan.mask_thresholds`). + progress + Called as ``progress(done, total, image)`` before each image is read, and once + with ``image`` empty at the end. + + Returns + ------- + MaskStepResult + The report is written to the output folder holding its lock; ``refusal`` when + another run holds it (nothing is written). + + Raises + ------ + ValueError + For a step that is not ready, and for masks whose pixels cannot be used (planes, + labels; :func:`.mask_scan.scan_masks`). + """ + if not step.ready or step.preflight is None or step.output_folder is None: + raise ValueError("the masks cannot be described: the step has errors or no masks") + report = scan_masks( + step.masks, step.preflight.discovery.root, progress=progress, thresholds=thresholds + ) + output = step.output_folder + output.mkdir(parents=True, exist_ok=True) + try: + with output_lock(output): # a run may be replacing or removing the same file + path = write_mask_report(report, output / MASK_REPORT_FILENAME) + except OutputLockedError as exc: + return MaskStepResult(report=report, path=None, refusal=lock_issue(exc)) + return MaskStepResult(report=report, path=path) diff --git a/tests/test_mask_step.py b/tests/test_mask_step.py new file mode 100644 index 00000000..be3df7e5 --- /dev/null +++ b/tests/test_mask_step.py @@ -0,0 +1,100 @@ +"""PP-10d: the mask step shared by the command line's ``masks`` and the notebook. + +:func:`prepare_mask_step` pairs the masks with the inputs the run in the output folder +recorded unless given, and :func:`describe_masks` scans them and writes +``mask_relationships.json`` holding the folder's lock; ``masks`` only prints their results. +""" + +import json + +import pytest + +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.mask_scan import MASK_REPORT_FILENAME +from morphobricks_preprocessing.run import ( + MaskStep, + describe_masks, + prepare_mask_step, +) +from morphobricks_preprocessing.run.lock import output_lock +from morphobricks_preprocessing.testing import make_clean_run + + +@pytest.fixture +def clean(tmp_path): + return make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells", "Nuclei")) + + +def without_versions(path): + data = json.loads(path.read_text(encoding="utf-8")) + data.pop("versions") + return data + + +def test_the_step_takes_every_input_the_run_recorded(clean): + step = prepare_mask_step(None, clean.output) + assert step.refusal is None and step.ready + assert step.changes == () + assert step.preflight.discovery.root == clean.experiment.resolve() + assert step.masks.folder == clean.masks.resolve() + assert step.masks.types == ("Cells", "Nuclei") + assert step.output_folder == clean.output + + +def test_describe_masks_writes_the_report_the_command_writes(clean, capsys): + seen = [] + result = describe_masks( + prepare_mask_step(None, clean.output), progress=lambda *a: seen.append(a) + ) + assert result.refusal is None + assert result.path == clean.output / MASK_REPORT_FILENAME + assert seen[0] == (0, 4, "ctrl/rep1/a.ome.tif") and seen[-1] == (4, 4, "") + assert set(result.report.types) == {"Cells", "Nuclei"} + written = without_versions(result.path) + result.path.unlink() + assert main(["masks", "--output", str(clean.output)]) == 0 + assert without_versions(clean.output / MASK_REPORT_FILENAME) == written + assert f"Report written to {result.path}" in capsys.readouterr().out + + +def test_without_an_experiment_or_a_recorded_run_the_step_is_refused(tmp_path): + step = prepare_mask_step(None, tmp_path / "out") + assert step.refusal.code == "experiment_not_recorded" + assert step.preflight is None and not step.ready + assert step.masks.types == () + with pytest.raises(ValueError, match="cannot be described"): + describe_masks(step) + + +def test_a_masks_folder_given_is_used_and_named_with_its_effect(clean, tmp_path): + other = tmp_path / "other" + clean.masks.rename(other) + step = prepare_mask_step(None, clean.output, masks_folder=other) + assert step.ready + (change,) = step.changes + assert change.name == "masks_folder" + assert "used for this scan only" in change.effect + + +def test_mask_errors_stop_the_step_before_any_mask_is_read(clean): + step = prepare_mask_step(clean.experiment, clean.output, masks_folder=clean.experiment / "ctrl") + assert "masks_inside_experiment" in {issue.code for issue in step.issues} + assert not step.ready + with pytest.raises(ValueError): + describe_masks(step) + + +def test_a_locked_output_folder_gets_the_report_but_no_file(clean): + step = prepare_mask_step(None, clean.output) + with output_lock(clean.output): + result = describe_masks(step) + assert result.refusal.code == "output_locked" + assert result.path is None and result.report.images == 4 + assert not (clean.output / MASK_REPORT_FILENAME).exists() + + +def test_no_masks_found_is_not_ready(tmp_path): + bare = make_clean_run(tmp_path, axes="YX", shape=(8, 8)) + step = prepare_mask_step(None, bare.output) + assert isinstance(step, MaskStep) + assert step.refusal is None and step.masks.types == () and not step.ready From 96d8ede47271725e6d1702096f5e180086e0cfe4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:36:05 +0000 Subject: [PATCH 2/7] PP-10d: the notebook's Describe the masks button (step 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a run that recorded masks, step 3's "Describe the masks (optional)" card runs the masks step with the run's recorded inputs through run.mask_step (the functions the CLI calls), with a progress bar, and shows the mask types, relationships with their counts, masks of the same objects and the hierarchy as suggestions to check, with the path of mask_relationships.json. Refusals use the existing cards (lock card with the stale-lock button; red cards for mask errors and unusable masks). Default thresholds only, said on the card. Hidden with folder_step(show_output=False); embedding API unchanged (private handler). run.mask_step.recorded_mask_types gates the button. The masks' texts move from report.py to mask_text.py (pure) with the new table rows, keeping report.py within 600 lines. Docs: notebook, user guide, contract §9.2/§15.1, plan §10, AGENTS.md. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 15 +- docs/contract.md | 8 +- docs/plan.md | 24 +- docs/user_guide.md | 25 +- notebooks/MorphoBricks_Preprocessing.ipynb | 30 ++- pyproject.toml | 2 + src/morphobricks_preprocessing/cli.py | 2 +- src/morphobricks_preprocessing/cli_masks.py | 3 +- src/morphobricks_preprocessing/mask_text.py | 175 +++++++++++++ .../notebook_mask_step.py | 243 ++++++++++++++++++ .../notebook_run.py | 2 + src/morphobricks_preprocessing/notebook_ui.py | 8 +- src/morphobricks_preprocessing/report.py | 107 +------- .../run/__init__.py | 9 +- .../run/mask_step.py | 23 +- tests/test_notebook_file.py | 1 + tests/test_notebook_mask_step.py | 183 +++++++++++++ 17 files changed, 734 insertions(+), 126 deletions(-) create mode 100644 src/morphobricks_preprocessing/mask_text.py create mode 100644 src/morphobricks_preprocessing/notebook_mask_step.py create mode 100644 tests/test_notebook_mask_step.py diff --git a/AGENTS.md b/AGENTS.md index 57e82e23..ba240b2a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,11 +120,16 @@ src/morphobricks_preprocessing/ pipeline.py (run_experiment, incremental), steps.py (header checks, rows, outcome), modes.py (restart/full), recorded.py (inputs reused from the earlier run, InputChange), lock.py (the output folder's O_EXCL lock, - remove_stale_lock), check_step.py (write_check_report), result.py (RunResult), + remove_stale_lock), check_step.py (write_check_report), mask_step.py + (the masks step shared by CLI `masks` and the notebook: prepare_mask_step, + describe_masks, recorded_mask_types; PP-10d), result.py (RunResult), entries.py (manifest entries, new/changed/unchanged by RowKey), reuse.py (which earlier rows are reused), rows.py (a file's rows and their pixel checks), publish.py (staged outputs, published together) report.py plain-language summaries shared by CLI and notebook, group statistics tables + mask_text.py the masks' texts (mask_summary, mask_report_summary) and the notebook's + tables of the mask step (mask_type_rows, mask_relationship_rows, + mask_hierarchy_lines); moved from report.py in PP-10d discovery_report.py the run report (discovery_report.json and .txt, design v0.4 §4): status, manifest SHA-256, what was found, reused, recomputed and replaced cli.py, __main__ `morphobricks-preprocessing check|run|masks|calibrate` (also `python -m morphobricks_preprocessing`); @@ -151,6 +156,8 @@ src/morphobricks_preprocessing/ notebook_style.py notebook look and feel: cards, status cards, tiles, tables, ConfirmToggle, Browse dialogs notebook_axes.py the notebook's axis-check card and the marks under the role dropdowns notebook_masks.py step 1's masks folder, Masks card and mask-axis statements (PP-10c) + notebook_mask_step.py step 3's Describe the masks card: the masks step after a run that + recorded masks (PP-10d) notebook_ui.py PreprocessingNotebook: the four notebook steps (needs the `notebook` extra); folder_step/axis_step take title, purpose, intro and show_next (and show_output and show_design for step 1) so other bricks can embed them @@ -882,8 +889,10 @@ jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb `repeat_group`, repeat groups differing only in case, or a `confirmed_on` that is not `YYYY-MM-DD` are refused, naming the line and column. - **Notebook masks field (PP-10c):** step 1 takes the masks folder and the mask axes per - mask type (design v0.4 §2), see plan §10. Next, PP-10d: a notebook button for the - `masks` step (`mask_relationships.json`). + mask type (design v0.4 §2), see plan §10. +- **Notebook masks step (PP-10d):** step 3's **Describe the masks** runs the `masks` step + (`mask_relationships.json`) with the run's recorded inputs, through `run.mask_step`, which + the CLI calls too; see plan §10. - **Next (not started):** the notebook step for the masks (name each mask, confirm or correct the relationships) and a file saving those answers for the other bricks (question 32); 3D and time-lapse masks later. diff --git a/docs/contract.md b/docs/contract.md index 63391426..0d8fcd10 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -251,13 +251,14 @@ the bricks' tests. | Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json` and its own temporary folders; never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | | Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | -| `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). | +| `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report, the file written, or the `output_locked` refusal with nothing written; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run and for plain stacks that are new or changed. | | Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` while it writes `mask_relationships.json`, and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | | Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | | `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). The notebook's **Check folder** does the same with the same functions (`run.check_step.suggested_review`, `write_check_outputs`; v0.4.0, PP-10): the report, and the template with the design card; nothing when an embedding brick hides the output folder (`folder_step(show_output=False)`). | | Notebook (v0.4.0, PP-10) | Step 3 offers the mode (`restart`, `full`) with its effect, and lists before the run the inputs that differ from the recorded ones with their effect (`run.planned.planned_changes(given, output_folder, design=, calibration=)`, from `resolve_inputs`, `design_for_run` and `calibration_change`: the changes the run then reports). After a run it names the mode, the files replaced and the input changes, and shows the report; a stopped run says what it wrote (question 55). A refusal `output_locked` (check, run, a design answer or a stated size) shows the holder (computer, process ID, start time) and a **Remove the lock of a stopped run** button: `remove_stale_lock` only on that click, never the lock of a process still running on the same computer; on Google Drive one session per output folder (question 58). | -| Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) stays on the command line (`masks`) until PP-10d (plan §10). | +| Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) is step 3's **Describe the masks** (next row). | +| Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled only after a run in this session that wrote its outputs and recorded masks (`run.mask_step.recorded_mask_types(output_folder)`, from the manifest's `masks` key). A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which writes `mask_relationships.json` under the lock. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, mask errors found since the run (shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | ### 9.3 Calibration (`calibration.csv`, v0.4.0, design v0.4 §3) @@ -445,7 +446,8 @@ changed: `inside_pixel_share` (0.5), `inside_object_share` (0.9), `parent_larger and `grouped_median_pieces` (2). Shares and the intersection over union must be above 0 and at most 1, the median number of pieces at least 1; an unknown name or another value raises `ValueError` before any mask is read (`mask_thresholds`). The command line takes -one option per threshold (`--inside-object-share 0.8`, ...). +one option per threshold (`--inside-object-share 0.8`, ...); the notebook's **Describe the +masks** uses the defaults (§9.2). **Hierarchy.** A relation `inside` or `grouped_by` is **direct** unless another type lies between: nucleoli inside nuclei inside cells have the direct parent nuclei, and the diff --git a/docs/plan.md b/docs/plan.md index 18b2c37a..c2f5b07e 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -466,11 +466,25 @@ API, CLI). The notebook and CLI only call the API. folder (MorphoRelate has its own masks card); the embedding API is unchanged. - Not in this PR: a notebook button for the `masks` step (`mask_relationships.json`); it is PP-10d. -- **PP-10d (the notebook's masks-step button)** *(not started; owner decision of 2 November - 2026: right after PP-10c)*: a notebook button for the `masks` step (describe the masks: - `mask_relationships.json`). It first moves the CLI's scan-and-write-under-lock out of - `cli_masks.masks_command` into a shared orchestration function that the CLI and the - notebook both call. +- **PP-10d (the notebook's masks-step button)** *(implemented, PR #123; owner decision of 2 + November 2026: right after PP-10c, before PP-11)*: a notebook button for the `masks` step + (describe the masks: `mask_relationships.json`; contract §9.2 "Notebook mask step", §15): + - Move only, first: the CLI's pairing with the recorded inputs, scan and write under the + lock leave `cli_masks.masks_command` for `run.mask_step` (orchestration): + `prepare_mask_step` (a `MaskStep`) and `describe_masks` (a `MaskStepResult`, the lock + refusal returned rather than raised). The command only prints; its output is unchanged + (same text and exit status over 13 scenarios, the HPMEC data included). + - Step 3's **Describe the masks (optional)** card (`notebook_mask_step`, ui): enabled only + after a run that recorded masks (`recorded_mask_types`); runs the step with the run's + recorded inputs and the default thresholds (the notebook offers no threshold option; + the command line does), with a progress bar; shows the mask types, the relationships + with their counts, masks of the same objects and the hierarchy as suggestions to check, + and the path of `mask_relationships.json`. Refusals use the existing cards (the lock + card with **Remove the lock of a stopped run**, red cards for mask errors and masks that + cannot be used). Hidden with `folder_step(show_output=False)`; the pinned embedding API + is unchanged. + - The masks' texts and the new table rows moved from `report` to `mask_text` (pure), so + `report.py` stays within §9's 600 lines. - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; diff --git a/docs/user_guide.md b/docs/user_guide.md index cbdcadac..8f1d16e7 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -191,7 +191,8 @@ masks/ measured. Masks do not change `plane_histograms.csv`. They are checked and recorded for the bricks -that measure objects; the optional `masks` command (§5) also tells how they relate. +that measure objects; the optional `masks` command (§5), or **Describe the masks** in the +notebook's step 3 (§4), also tells how they relate. ## 3. Supported images @@ -451,6 +452,23 @@ results or comparing another grouping. repeats as amber boxes; and one panel per condition with the intensity distribution of each biological repeat (comparable repeats lie close together; a shifted one is dashed). The plots are only shown here, never saved. + - **Describe the masks (optional)**, after a run that recorded masks: the notebook's + counterpart of the `masks` command (§5), with the same functions and the inputs the + run recorded (experiment folder, masks folder, mask axes, excluded images). The button + is greyed out until such a run, saying why. Click it to read every mask, one image at a + time (a progress bar names the image); the card then shows each mask type (kind, + images, objects, median pieces per label), the relationships found (which type is + inside or grouped by which, how many objects are inside, how many objects of the + other type hold 0, 1, 2 or 3 or more, and how many are outside or split), masks of + the same objects, and the hierarchy of direct parents. These are suggestions to + check, not decisions. The description is written to **mask_relationships.json** in + the output folder (its path is shown), holding the output folder's lock: when another + run holds it, **The output folder is locked** is shown with **Remove the lock of a + stopped run**, and nothing is written. Mask errors found since the run (a mask + changed or removed) and masks that cannot be used (a label above 2,147,483,647) are + shown as red cards, and nothing is written. The notebook uses the default thresholds + (contract §15.1); the `masks` command can change them. A new run clears the card; + an embedding brick that hides the output folder does not show it. 4. **Calculate the statistics (optional).** The table from step 3 is all that later bricks need; this step only reads it to compare groups and writes no files. - **Histogram table** is filled in with the table from step 3. To look at another @@ -714,7 +732,8 @@ The default output folder is next to the experiment folder: - **`ome_zarr/`**: written only by v0.3 runs that converted TIFF images to OME-Zarr. That option was removed in v0.4.0; such a folder is left as it is, and may be deleted. -- **`mask_relationships.json`** (only from the `masks` command): each kind of mask, the +- **`mask_relationships.json`** (only from the `masks` command or the notebook's + **Describe the masks**): each kind of mask, the relationship between every two kinds with its counts, and the hierarchy. The rules and thresholds are in [contract.md](contract.md) §15. @@ -741,7 +760,7 @@ while they write their reports, `design` and `calibrate` while they change their and are refused the same way. A stopped run leaves the last good results in place; rerunning checks the table against the manifest and rebuilds a mismatched pair. `mask_relationships.json` is removed by a `full` run and when the masks changed since it -was written; run the `masks` command again. +was written; run the `masks` command (or **Describe the masks**) again. ## 7. Running again, and excluding images diff --git a/notebooks/MorphoBricks_Preprocessing.ipynb b/notebooks/MorphoBricks_Preprocessing.ipynb index ca4b9ed0..111cfb0c 100644 --- a/notebooks/MorphoBricks_Preprocessing.ipynb +++ b/notebooks/MorphoBricks_Preprocessing.ipynb @@ -22,6 +22,9 @@ " conditions, whether they are comparable, and which conditions are controls.\n", "- **Pairs your masks with the images** (optional): segmentations such as cells or nuclei, kept\n", " in a masks folder, are checked and recorded for the later bricks; their pixels are not read.\n", + "- **Describes your masks** (optional, after the run): each mask type, how the types relate\n", + " (nuclei inside cells, mitochondria grouped by cell) and their hierarchy, as suggestions for\n", + " you to check.\n", "- **Shows the pixel sizes** of every image with where they come from (the image metadata, or a\n", " size you state), and lets you state them where the metadata are missing or wrong.\n", "- **Asks you to confirm how the images are organised:** which dimension is depth, channel or\n", @@ -44,7 +47,8 @@ " **Yes, the preview looks right** and then **Confirm axis roles**. Check the suggested image\n", " type and press **Yes, this is the right image type**.\n", "4. **Step 3:** press **Compute histograms**. This writes the table the next bricks need. Read\n", - " the **Intensity checks** that follow.\n", + " the **Intensity checks** that follow. With masks, you may then press **Describe the masks**\n", + " (optional).\n", "5. **Step 4 (optional):** press **Calculate statistics** to compare your groups and check the\n", " intensities of any table.\n", "\n", @@ -516,7 +520,24 @@ "\n", "> ℹ️ Images are compared only within their condition, so a treatment effect is never reported.\n", "> The limits (3 robust deviations, 1 % saturated pixels, at least 5 images) are under\n", - "> **Check settings**; changing them updates the warnings at once, and the next run records them." + "> **Check settings**; changing them updates the warnings at once, and the next run records them.\n", + "\n", + "**Describe the masks (optional).** After a run that recorded masks, press **Describe the masks**.\n", + "It reads every mask the run recorded (same masks folder, mask axes and excluded images) and\n", + "shows, with the counts behind each:\n", + "\n", + "- **Mask types:** one object per label (cells, nuclei), each label in several pieces (for example\n", + " mitochondria carrying the label of their cell), or one value per image (a binary mask);\n", + "- **Relationships found:** which mask type is inside, or grouped by, another, how many objects\n", + " are inside, and how many objects of the other type have 0, 1, 2 or 3 or more of them; two masks\n", + " of the same objects are listed apart;\n", + "- **Hierarchy:** each mask type under its direct parent.\n", + "\n", + "These are **suggestions to check**, from the masks' pixels: nothing is decided for you. The\n", + "description is written to **mask_relationships.json** in the output folder, and its path is\n", + "shown. The notebook uses the default thresholds; the command line's `masks` command can change\n", + "them. The button stays disabled until a run has recorded masks. A later run in mode **Full**, or\n", + "one that finds the masks changed, removes the file: describe them again." ] }, { @@ -592,7 +613,7 @@ "\n", "- [ ] The output folder contains **plane_histograms.csv**, **experiment_design.csv**,\n", " **run_manifest.json** and the report, **discovery_report.txt** (with **calibration.csv** when\n", - " you stated pixel sizes).\n", + " you stated pixel sizes, and **mask_relationships.json** when you described the masks).\n", "- [ ] Keep these files together; the next MorphoBricks brick reads them.\n", "- [ ] Rerun this notebook whenever you add, change or remove images.\n", "\n", @@ -660,6 +681,9 @@ "one plane …\"**: if one 2D mask is meant for every plane, choose **YX: one 2-D mask shared by every\n", "plane of its image** under **Mask axes**; otherwise save one mask plane per slice and time point.\n", "**\"The masks folder is inside the experiment folder …\"**: move it next to the experiment folder.\n", + "**\"The masks cannot be used\"** (after **Describe the masks**): the message names the mask, for\n", + "example one holding labels above 2,147,483,647; relabel its objects from 1, run step 3 again and\n", + "press **Describe the masks** again.\n", "\n", "**The preview looks like stripes**: X or Y is on the wrong dimension; change the roles in step 2.\n", "\n", diff --git a/pyproject.toml b/pyproject.toml index 0bade8a0..fd355943 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,6 +140,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_axis_step" = "ui" "morphobricks_preprocessing.notebook_design" = "ui" "morphobricks_preprocessing.notebook_masks" = "ui" +"morphobricks_preprocessing.notebook_mask_step" = "ui" "morphobricks_preprocessing.notebook_folder" = "ui" "morphobricks_preprocessing.notebook_options" = "ui" "morphobricks_preprocessing.notebook_plots" = "ui" @@ -189,6 +190,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.run.calibrate" = "io" "morphobricks_preprocessing.planes" = "io" "morphobricks_preprocessing.report" = "pure" +"morphobricks_preprocessing.mask_text" = "pure" "morphobricks_preprocessing.sources" = "io" "morphobricks_preprocessing.run" = "orchestration" "morphobricks_preprocessing.run.pipeline" = "orchestration" diff --git a/src/morphobricks_preprocessing/cli.py b/src/morphobricks_preprocessing/cli.py index 27bc4d3e..5f5e5f9c 100644 --- a/src/morphobricks_preprocessing/cli.py +++ b/src/morphobricks_preprocessing/cli.py @@ -41,6 +41,7 @@ ) from .discovery import default_output_folder from .image_type import review_image_type +from .mask_text import mask_summary from .masks import mask_axes_option from .report import ( axis_check_summary, @@ -48,7 +49,6 @@ design_summary, format_issues, label_tree, - mask_summary, preflight_summary, ) from .run import ( diff --git a/src/morphobricks_preprocessing/cli_masks.py b/src/morphobricks_preprocessing/cli_masks.py index de1740f7..99aa46a7 100644 --- a/src/morphobricks_preprocessing/cli_masks.py +++ b/src/morphobricks_preprocessing/cli_masks.py @@ -14,7 +14,8 @@ import argparse from .mask_scan import DEFAULT_THRESHOLDS, mask_thresholds -from .report import format_issues, mask_report_summary, mask_summary +from .mask_text import mask_report_summary, mask_summary +from .report import format_issues from .run.mask_step import describe_masks, prepare_mask_step from .validation import has_errors diff --git a/src/morphobricks_preprocessing/mask_text.py b/src/morphobricks_preprocessing/mask_text.py new file mode 100644 index 00000000..0bc6f74b --- /dev/null +++ b/src/morphobricks_preprocessing/mask_text.py @@ -0,0 +1,175 @@ +"""Plain-language descriptions of the masks found and of the mask step's report (§15). + +The command line's ``check`` and ``masks`` print these texts and the notebook shows the +tables (moved from :mod:`.report`), so both tell the user the same thing: what each mask +type looks like, how the types relate, and the hierarchy, as suggestions to check. Nothing +here reads masks or writes files. +""" + +from __future__ import annotations + +from .mask_scan import DEFAULT_THRESHOLDS, MaskReport, PairRelation +from .masks import MaskInventory +from .validation import plural + +_MASK_KINDS = { + "instances": "one object per label", + "grouped": "each label in several pieces (for example organelles grouped by cell)", + "one_value": "one value per image (a binary mask or a single object)", + "empty": "no objects", +} +_RELATIONS = { + "inside": "inside", + "grouped_by": "grouped by the labels of", + "same_objects": "the same objects as", +} + + +def mask_summary(masks: MaskInventory) -> str: + """Summarise the masks found: each mask type and how many images have it.""" + if not masks.types: + return "Masks: none found." + lines = [ + f"Masks: {plural(len(masks.types), 'mask type')} for {plural(len(masks.images), 'image')}:" + ] + for name in masks.types: + found = [f for f in masks.files if f.mask_type == name] + sources = " and ".join(sorted({f.source for f in found})) + lines.append(f" {name}: {plural(len(found), 'image')} ({sources})") + return "\n".join(lines) + + +def mask_report_summary(report: MaskReport) -> str: + """Describe the mask types, their relationships and the hierarchy in plain words. + + Every statement is a suggestion from the masks' pixels; the text ends by asking the + user to name the masks and check the relationships. + """ + lines = [f"Mask types ({plural(report.images, 'image')} scanned):"] + for name, summary in sorted(report.types.items()): + lines.append( + f" {name}: {_MASK_KINDS[summary.kind]}; {plural(summary.objects, 'object')}, " + f"median {summary.median_pieces_per_label:g} piece(s) per label" + ) + relationships = [pair for pair in report.relationships if pair.relation != "same_objects"] + if relationships: + lines += ["", "Relationships found:"] + lines += [_relationship_line(pair) for pair in relationships] + if report.same_objects: + lines += ["", "Masks that look like two segmentations of the same objects:"] + lines += [f" {first} and {second}" for first, second in report.same_objects] + lines += ["", "Hierarchy (direct parents only):"] + lines += _hierarchy_lines(report.hierarchy) + lines += ["", f"Thresholds: {mask_thresholds_used(report)}"] + lines += [ + "", + "These are suggestions from the masks' pixels: name each mask, then check the " + "relationships before using them.", + ] + return "\n".join(lines) + + +def mask_thresholds_used(report: MaskReport) -> str: + """Name the thresholds the report was made with: "the defaults", or those changed.""" + changed = [ + f"{name} {value:g}" + for name, value in report.thresholds.items() + if value != DEFAULT_THRESHOLDS[name] + ] + return ", ".join(changed) + "; the others the defaults" if changed else "the defaults" + + +def mask_type_rows(report: MaskReport) -> list[list[str]]: + """Return the mask types as table rows, header first: kind, images, objects, pieces.""" + rows = [["Mask type", "Kind (suggested)", "Images", "Objects", "Median pieces per label"]] + for name, summary in sorted(report.types.items()): + kind = _MASK_KINDS[summary.kind] + numbers = (summary.images, summary.objects) + rows.append( + [name, kind, *(f"{n:,}" for n in numbers), f"{summary.median_pieces_per_label:g}"] + ) + return rows + + +def mask_relationship_rows(report: MaskReport) -> list[list[str]]: + """Return the relationships found as table rows, header first, with their evidence. + + Two masks of the same objects are left out, as in :func:`mask_report_summary`: they are + :attr:`.mask_scan.MaskReport.same_objects`. + """ + rows = [ + [ + "Mask", + "Relationship (suggested)", + "Other mask", + "Objects inside", + "Other mask's objects with 0/1/2/3+", + "Outside the other mask", + "Split between two of its objects", + ] + ] + for pair in report.relationships: + if pair.relation == "same_objects": + continue + e = pair.evidence + relation = _RELATIONS[pair.relation] + ("" if pair.direct else " (through another mask)") + counts = ( + e["parents_with_0_children"], + e["parents_with_1_child"], + e["parents_with_2_children"], + e["parents_with_3_or_more_children"], + ) + rows.append( + [ + pair.child, + relation, + pair.parent, + f"{e['inside']:,} of {e['children']:,}", + "/".join(f"{n:,}" for n in counts), + f"{e['outside_parents']:,}", + f"{e['split_between_parents']:,}", + ] + ) + return rows + + +def mask_hierarchy_lines(report: MaskReport) -> list[str]: + """Return the hierarchy of the mask types (direct parents only) as an indented tree.""" + return _hierarchy_lines(report.hierarchy) + + +def _relationship_line(pair: PairRelation) -> str: + """One relationship with its evidence, e.g. how many parents have 0, 1, 2, 3+ children.""" + e = pair.evidence + text = ( + f" {pair.child} {_RELATIONS[pair.relation]} {pair.parent}" + f"{'' if pair.direct else ' (through another mask)'}: " + f"{e['inside']} of {plural(e['children'], 'object')} inside; " + f"{pair.parent} with 0/1/2/3+ {pair.child}: {e['parents_with_0_children']}/" + f"{e['parents_with_1_child']}/{e['parents_with_2_children']}/" + f"{e['parents_with_3_or_more_children']}" + ) + extra = [] + if e["outside_parents"]: + extra.append(f"{e['outside_parents']} outside every {pair.parent}") + if e["split_between_parents"]: + extra.append(f"{e['split_between_parents']} split between {pair.parent}") + return text + (f"; {', '.join(extra)}" if extra else "") + + +def _hierarchy_lines(hierarchy) -> list[str]: # noqa: ANN001 + """Return the mask types as an indented tree; a type with two parents shows twice.""" + children: dict[str, list[str]] = {name: [] for name in hierarchy} + for name, parents in hierarchy.items(): + for parent in parents: + children[parent].append(name) + lines: list[str] = [] + + def show(name: str, depth: int) -> None: + lines.append(f"{' ' * (depth + 1)}{name}") + for child in sorted(children[name]): + show(child, depth + 1) + + for name in sorted(n for n, parents in hierarchy.items() if not parents): + show(name, 0) + return lines diff --git a/src/morphobricks_preprocessing/notebook_mask_step.py b/src/morphobricks_preprocessing/notebook_mask_step.py new file mode 100644 index 00000000..2656d31d --- /dev/null +++ b/src/morphobricks_preprocessing/notebook_mask_step.py @@ -0,0 +1,243 @@ +"""Step 3 of the notebook: **Describe the masks**, the optional mask step (plan §10, PP-10d). + +After a run that recorded masks, the button runs the command line's ``masks`` step with the +same functions (:func:`.run.mask_step.prepare_mask_step`, :func:`.run.mask_step.describe_masks`) +and the inputs the run recorded: the experiment folder, the masks folder, the mask axes and +the exclusions. It reads the masks, writes ``mask_relationships.json`` to the output folder +holding its lock, and shows each mask type, the relationships between the types and their +hierarchy, as suggestions to check (contract §15). The thresholds are the defaults; the +command line's ``masks`` can change them. A refusal is shown on the card (the locked output +folder with :class:`.notebook_output._LockCard`), and an embedding brick that hides the +output folder (``show_output=False``) does not get the card. +""" + +from __future__ import annotations + +import html +import traceback + +import ipywidgets as widgets + +from . import notebook_style as ui +from .mask_scan import MASK_REPORT_FILENAME +from .mask_text import ( + mask_hierarchy_lines, + mask_relationship_rows, + mask_thresholds_used, + mask_type_rows, +) +from .notebook_output import _LockCard +from .notebook_render import issues_html +from .run import MaskStep, MaskStepResult, mask_step +from .validation import has_errors, plural + +_STALE = object() +"""Marks the step as describing no run (a run is going on).""" +_LABEL = " Describe the masks" +_ICON = "sitemap" +_SUGGESTIONS = ( + "These are suggestions from the masks' pixels: name each mask, then check the " + "relationships before using them." +) + + +class _MaskStepCard: + """Step 3's optional mask step: the button, its progress and the description.""" + + def _build_mask_step_card(self) -> None: + """Build the card, its button disabled until a run records masks.""" + self._mask_step_run: object = _STALE + self.mask_step_button = ui.button( + _LABEL, + style="info", + icon=_ICON, + tooltip="Read the masks the run recorded and describe how they relate", + main=True, + ) + self.mask_step_hint = widgets.HTML() + self.mask_step_progress = widgets.IntProgress( + value=0, min=0, max=1, bar_style="info", layout=widgets.Layout(width="99%") + ) + ui.set_visible(self.mask_step_progress, False) + self.mask_step_status = widgets.HTML() + self.mask_step_lock = _LockCard("Then click Describe the masks again.") + self.mask_step_result = widgets.HTML() + self.mask_step_card = ui.card( + "Describe the masks (optional)", + [ + ui.help_text( + "Reads every mask the run recorded and describes each mask type (one " + "object per label, or labels in several pieces), how the types relate " + "(inside, grouped by, the same objects) and their hierarchy, with the " + f"counts behind each. Writes {MASK_REPORT_FILENAME} to the output " + "folder, as the command line's masks step does. The relationship " + "thresholds are the defaults; the command line's masks step can change " + "them. Optional: the histogram table does not need it." + ), + self.mask_step_hint, + self.mask_step_button, + self.mask_step_progress, + self.mask_step_status, + self.mask_step_lock.card, + self.mask_step_result, + ], + ) + self.mask_step_button.on_click(lambda _: self._on_describe_masks()) + + def _refresh_mask_step(self) -> None: + """Offer the step only after a run that recorded masks; clear it for another run.""" + ui.set_visible(self.mask_step_card, self._show_output) + if self.result is self._mask_step_run: + return + self._mask_step_run = self.result + self._clear_mask_step() + types = self._recorded_mask_types() + self.mask_step_button.disabled = not types + if types: + hint = ( + f"The run recorded {plural(len(types), 'mask type')}: " + f"{html.escape(', '.join(types))}." + ) + elif self.result is not None and self.result.written: + hint = "The run recorded no masks: give a masks folder in step 1 and run again." + else: + hint = "Available after a run that recorded masks (step 1's masks folder)." + self.mask_step_hint.value = f"
{hint}
" + + def _recorded_mask_types(self) -> tuple[str, ...]: + """Return the mask types the last run recorded (none before a written run).""" + if self.result is None or not self.result.written: + return () + with self.log.attached(): + return mask_step.recorded_mask_types(self.result.output_folder) + + def _mask_step_stale(self) -> None: + """Clear the step and disable it while a run replaces the results it describes.""" + self._mask_step_run = _STALE + self._clear_mask_step() + self.mask_step_button.disabled = True + + def _clear_mask_step(self) -> None: + """Clear the progress, status and description of an earlier click.""" + ui.set_visible(self.mask_step_progress, False) + self.mask_step_status.value = self.mask_step_result.value = "" + self.mask_step_lock.hide() + + def _on_describe_masks(self) -> MaskStepResult | None: + """Describe the masks the last run recorded and write ``mask_relationships.json``. + + Returns + ------- + MaskStepResult or None + None when the button is disabled or the step was refused or failed (the card + says why). + """ + self._refresh_mask_step() + if self.mask_step_button.disabled: + return None + self._busy(self.mask_step_button, "Reading the masks…") + self._clear_mask_step() + self.mask_step_status.value = ui.working("Pairing the masks the run recorded…") + try: + with self.log.attached(): + return self._describe_recorded_masks() + except Exception as exc: # unexpected failures still get a readable message + self.log.lines.append(traceback.format_exc()) + self._mask_step_failed( + "The masks could not be described because of an unexpected problem.", + [f"{type(exc).__name__}: {exc}", "Open Technical details for the full report."], + ) + return None + finally: + self._idle(self.mask_step_button, _LABEL, _ICON) + self._show_log() + + def _describe_recorded_masks(self) -> MaskStepResult | None: + """Pair, read and describe the masks with the run's recorded inputs; show the result.""" + step = mask_step.prepare_mask_step(None, self.result.output_folder) + if not step.ready: + self._show_mask_step_refused(step) + return None + ui.set_visible(self.mask_step_progress, True) + self.mask_step_progress.value, self.mask_step_progress.bar_style = 0, "info" + try: + result = mask_step.describe_masks(step, progress=self._mask_step_progressed) + except ValueError as exc: # masks whose pixels cannot be used (planes, labels) + self._mask_step_failed( + "The masks cannot be used.", + [str(exc), "Correct or replace the mask named, run step 3 again, then retry."], + ) + return None + if result.refusal is not None: + self.mask_step_progress.bar_style = "danger" + self.mask_step_status.value = ui.status( + "error", + "The description was not written: the output folder is locked.", + ["The masks were read, but nothing was written."], + ) + self.mask_step_lock.show(step.output_folder) + return None + self.mask_step_progress.bar_style = "success" + self._show_mask_description(step, result) + return result + + def _mask_step_progressed(self, done: int, total: int, image: str) -> None: + self.mask_step_progress.max = max(total, 1) + self.mask_step_progress.value = done + if image: + self.mask_step_status.value = ui.working( + f"Reading the masks of image {done + 1} of {total}: {image}" + ) + + def _mask_step_failed(self, title: str, lines: list[str]) -> None: + """Show a failure as a red card; nothing was written.""" + self.mask_step_progress.bar_style = "danger" + self.mask_step_status.value = ui.status("error", title, [*lines, "Nothing was written."]) + + def _show_mask_step_refused(self, step: MaskStep) -> None: + """Say why the masks cannot be read: no recorded experiment, mask errors or no masks.""" + if step.refusal is not None: + title, issues = "The run's experiment folder is not recorded.", [step.refusal] + elif has_errors(step.issues): + title, issues = "The masks have errors: none was read.", list(step.issues) + else: + title, issues = "No masks were found with the inputs the run recorded.", [] + self.mask_step_status.value = ui.status( + "error", title, ["Nothing was written. Read the messages below; each says what to do."] + ) + self.mask_step_result.value = issues_html(issues) if issues else "" + + def _show_mask_description(self, step: MaskStep, result: MaskStepResult) -> None: + """Show the mask types, relationships and hierarchy, and where the report is.""" + report = result.report + self.mask_step_status.value = ui.status( + "ok", + f"The masks are described: {plural(len(report.types), 'mask type')} in " + f"{plural(report.images, 'image')}.", + [ + f"Written: {html.escape(str(result.path))}", + f"Thresholds: {html.escape(mask_thresholds_used(report))}.", + f"{html.escape(_SUGGESTIONS)}", + ], + raw=True, + ) + relationships = mask_relationship_rows(report) + same = "".join( + f"
  • {html.escape(first)} and {html.escape(second)}
  • " + for first, second in report.same_objects + ) + hierarchy = html.escape("\n".join(mask_hierarchy_lines(report))) + parts = [ + "Mask types (suggestions to check)", + ui.table(mask_type_rows(report), numeric_from=2), + "Relationships found (suggestions to check)", + ui.table(relationships, numeric_from=3) + if len(relationships) > 1 + else "
    None: no mask type is inside or grouped by another.
    ", + ] + if same: + parts += ["Two segmentations of the same objects (to check)", f"
      {same}
    "] + parts += ["Hierarchy (direct parents only)", f"
    {hierarchy}
    "] + if step.issues: + parts.append(issues_html(step.issues)) + self.mask_step_result.value = "".join(parts) diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index db29ee5d..53e39b40 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -251,6 +251,7 @@ def _refresh_run_step(self) -> None: self.readiness.value = title + ui.checklist(items) self.run_button.disabled = bool(missing) self.plan.value = self._plan_html(review if folder_ok else None) + self._refresh_mask_step() def run(self) -> RunResult | None: """Compute or update the histogram table (pass 2) and show what happened.""" @@ -285,6 +286,7 @@ def _start_run(self) -> None: self.run_checks.clear() self.run_lock.hide() self.run_report.clear() + self._mask_step_stale() ui.set_visible(self.progress, True) self.progress.value, self.progress.max, self.progress.bar_style = 0, 1, "info" self.run_status.value = ui.working("Preparing: checking everything again…") diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index db3cd7aa..4bc2946b 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -15,7 +15,8 @@ 3. **Run** - check that everything is ready, see what will happen (the mode and the inputs that changed since the earlier run), and compute or update the histogram table; then the intensity checks of the table, with - their plots. + their plots; after a run that recorded masks, the optional mask step + (**Describe the masks**, as the command line's ``masks``). 4. **Results** - pooled statistics and intensity distributions per group, and the intensity checks of any histogram table. @@ -53,6 +54,7 @@ from .notebook_calibration import _CalibrationCard from .notebook_design import _DesignCard from .notebook_folder import _FolderStep +from .notebook_mask_step import _MaskStepCard from .notebook_masks import _MasksCard from .notebook_options import ( _PACKAGE_LOGGER, @@ -123,6 +125,7 @@ class PreprocessingNotebook( _AxisStep, _AxisPreview, _RunStep, + _MaskStepCard, _RunPlan, _ResultsStep, ): @@ -178,6 +181,7 @@ def __init__(self, experiment_folder: str = "", output_folder: str = ""): # noq self._build_masks_cards() self._build_folder_step(str(experiment_folder), str(output_folder)) self._build_axis_step() + self._build_mask_step_card() self._build_run_step() self._build_results_step() self._refresh_axis_step() @@ -260,6 +264,7 @@ def folder_step( ui.set_visible(self.folder_next, show_next) self._show_output = show_output ui.set_visible(self.output_card, show_output) + ui.set_visible(self.mask_step_card, show_output) self.labels_confirmed.explain(_labels_explanation(show_design)) if show_design != self._show_design: self._show_design = show_design @@ -368,6 +373,7 @@ def run_step(self) -> widgets.Widget: self.run_tiles, self.run_issues, self.run_checks.card, + self.mask_step_card, self.details_box, self.run_next, ], diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index 98dbb976..bdfe2930 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -1,7 +1,8 @@ -"""Plain-language summaries of a preflight, a run, the group statistics and the masks. +"""Plain-language summaries of a preflight, a run and the group statistics. The command-line interface prints these texts and the notebook shows them, so -both tell the user the same thing. Nothing here reads images or writes files. +both tell the user the same thing. Nothing here reads images or writes files. The +masks have theirs in :mod:`.mask_text`. """ from __future__ import annotations @@ -19,8 +20,6 @@ from .design import DESIGN_FILENAME, ExperimentDesign from .discovery import DiscoveryResult from .discovery_report import REPORT_FILENAME, REPORT_TEXT_FILENAME -from .mask_scan import DEFAULT_THRESHOLDS, MaskReport, PairRelation -from .masks import MaskInventory from .run import Preflight, RunResult from .validation import Severity, ValidationIssue, plural @@ -435,103 +434,3 @@ def _number(value: float | None) -> str: if value is None: return "–" return f"{value:.6g}" - - -_MASK_KINDS = { - "instances": "one object per label", - "grouped": "each label in several pieces (for example organelles grouped by cell)", - "one_value": "one value per image (a binary mask or a single object)", - "empty": "no objects", -} -_RELATIONS = { - "inside": "inside", - "grouped_by": "grouped by the labels of", - "same_objects": "the same objects as", -} - - -def mask_summary(masks: MaskInventory) -> str: - """Summarise the masks found: each mask type and how many images have it.""" - if not masks.types: - return "Masks: none found." - lines = [ - f"Masks: {plural(len(masks.types), 'mask type')} for {plural(len(masks.images), 'image')}:" - ] - for name in masks.types: - found = [f for f in masks.files if f.mask_type == name] - sources = " and ".join(sorted({f.source for f in found})) - lines.append(f" {name}: {plural(len(found), 'image')} ({sources})") - return "\n".join(lines) - - -def mask_report_summary(report: MaskReport) -> str: - """Describe the mask types, their relationships and the hierarchy in plain words. - - Every statement is a suggestion from the masks' pixels; the text ends by asking the - user to name the masks and check the relationships. - """ - lines = [f"Mask types ({plural(report.images, 'image')} scanned):"] - for name, summary in sorted(report.types.items()): - lines.append( - f" {name}: {_MASK_KINDS[summary.kind]}; {plural(summary.objects, 'object')}, " - f"median {summary.median_pieces_per_label:g} piece(s) per label" - ) - relationships = [pair for pair in report.relationships if pair.relation != "same_objects"] - if relationships: - lines += ["", "Relationships found:"] - lines += [_relationship_line(pair) for pair in relationships] - if report.same_objects: - lines += ["", "Masks that look like two segmentations of the same objects:"] - lines += [f" {first} and {second}" for first, second in report.same_objects] - lines += ["", "Hierarchy (direct parents only):"] - lines += _hierarchy_lines(report.hierarchy) - changed = [ - f"{name} {value:g}" - for name, value in report.thresholds.items() - if value != DEFAULT_THRESHOLDS[name] - ] - used = ", ".join(changed) + "; the others the defaults" if changed else "the defaults" - lines += ["", f"Thresholds: {used}"] - lines += [ - "", - "These are suggestions from the masks' pixels: name each mask, then check the " - "relationships before using them.", - ] - return "\n".join(lines) - - -def _relationship_line(pair: PairRelation) -> str: - """One relationship with its evidence, e.g. how many parents have 0, 1, 2, 3+ children.""" - e = pair.evidence - text = ( - f" {pair.child} {_RELATIONS[pair.relation]} {pair.parent}" - f"{'' if pair.direct else ' (through another mask)'}: " - f"{e['inside']} of {plural(e['children'], 'object')} inside; " - f"{pair.parent} with 0/1/2/3+ {pair.child}: {e['parents_with_0_children']}/" - f"{e['parents_with_1_child']}/{e['parents_with_2_children']}/" - f"{e['parents_with_3_or_more_children']}" - ) - extra = [] - if e["outside_parents"]: - extra.append(f"{e['outside_parents']} outside every {pair.parent}") - if e["split_between_parents"]: - extra.append(f"{e['split_between_parents']} split between {pair.parent}") - return text + (f"; {', '.join(extra)}" if extra else "") - - -def _hierarchy_lines(hierarchy) -> list[str]: # noqa: ANN001 - """Return the mask types as an indented tree; a type with two parents shows twice.""" - children: dict[str, list[str]] = {name: [] for name in hierarchy} - for name, parents in hierarchy.items(): - for parent in parents: - children[parent].append(name) - lines: list[str] = [] - - def show(name: str, depth: int) -> None: - lines.append(f"{' ' * (depth + 1)}{name}") - for child in sorted(children[name]): - show(child, depth + 1) - - for name in sorted(n for n, parents in hierarchy.items() if not parents): - show(name, 0) - return lines diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py index daeed1ae..a1194a50 100644 --- a/src/morphobricks_preprocessing/run/__init__.py +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -26,7 +26,13 @@ from .check_step import CheckOutputs, write_check_outputs, write_check_report from .design_file import edit_design, write_design_template from .lock import LOCK_FILENAME, LockHolder, OutputLockedError, read_lock, remove_stale_lock -from .mask_step import MaskStep, MaskStepResult, describe_masks, prepare_mask_step +from .mask_step import ( + MaskStep, + MaskStepResult, + describe_masks, + prepare_mask_step, + recorded_mask_types, +) from .modes import FULL, MODES, RESTART from .pipeline import run_experiment from .preflight import Preflight, SamplePlane, preflight, sample_planes @@ -50,6 +56,7 @@ "preflight", "prepare_mask_step", "read_lock", + "recorded_mask_types", "remove_stale_lock", "run_experiment", "sample_planes", diff --git a/src/morphobricks_preprocessing/run/mask_step.py b/src/morphobricks_preprocessing/run/mask_step.py index d0b69f11..f82c9a98 100644 --- a/src/morphobricks_preprocessing/run/mask_step.py +++ b/src/morphobricks_preprocessing/run/mask_step.py @@ -7,6 +7,8 @@ output folder recorded unless they are given (design v0.4 §1): the experiment folder (``experiment_not_recorded`` without one), the masks folder, the mask axes and the exclusions; a value given that differs is used and named. It reads no pixels. +- :func:`recorded_mask_types` says whether the run in the output folder recorded masks: + the notebook offers the step only then. - :func:`describe_masks` reads the masks (:func:`.mask_scan.scan_masks`) and writes the report, holding the output folder's lock while it writes: a run may be replacing or removing the same file. @@ -25,7 +27,7 @@ from ..validation import ValidationIssue, has_errors from .lock import OutputLockedError, lock_issue, output_lock from .preflight import Preflight, preflight -from .recorded import GivenInputs, InputChange, resolve_inputs +from .recorded import GivenInputs, InputChange, resolve_inputs, usable from .reuse import read_previous, recorded_experiment_root _RECORDED = ("experiment_root", "exclude", "masks_folder", "mask_axes") @@ -196,3 +198,22 @@ def describe_masks( except OutputLockedError as exc: return MaskStepResult(report=report, path=None, refusal=lock_issue(exc)) return MaskStepResult(report=report, path=path) + + +def recorded_mask_types(output_folder: str | os.PathLike) -> tuple[str, ...]: + """Return the mask types the run in the output folder recorded (v0.4.0 manifest). + + Parameters + ---------- + output_folder + The output folder. + + Returns + ------- + tuple of str + The recorded mask types; empty without a usable manifest or without masks. + """ + manifest = usable(read_previous(Path(output_folder).resolve())[0]) + if manifest is None: + return () + return tuple((manifest.masks or {}).get("types") or ()) diff --git a/tests/test_notebook_file.py b/tests/test_notebook_file.py index 92da3d6c..f43b9456 100644 --- a/tests/test_notebook_file.py +++ b/tests/test_notebook_file.py @@ -153,6 +153,7 @@ def test_notebook_names_buttons_that_exist(): nb.confirm_button, nb.image_type_checked.button, nb.run_button, + nb.mask_step_button, nb.statistics_button, ): assert f"**{button.description.strip()}**" in text diff --git a/tests/test_notebook_mask_step.py b/tests/test_notebook_mask_step.py new file mode 100644 index 00000000..559313d4 --- /dev/null +++ b/tests/test_notebook_mask_step.py @@ -0,0 +1,183 @@ +"""PP-10d: the notebook's **Describe the masks** button (plan §10, contract §15). + +After a run that recorded masks, step 3's button runs the command line's ``masks`` step with +the run's recorded inputs (:mod:`morphobricks_preprocessing.run.mask_step`): it writes +``mask_relationships.json`` holding the lock and shows the mask types, the relationships and +the hierarchy as suggestions to check. It is disabled without such a run, shows refusals with +the existing cards, and is hidden when an embedding brick hides the output folder. +""" + +import importlib +import json + +import numpy as np +import pytest + +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.mask_scan import MASK_REPORT_FILENAME +from morphobricks_preprocessing.run.lock import output_lock +from tiff_tree import write_data + +pytest.importorskip("ipywidgets") +pytest.importorskip("matplotlib") +ui = importlib.import_module("morphobricks_preprocessing.notebook_style") +PreprocessingNotebook = importlib.import_module( + "morphobricks_preprocessing.notebook_ui" +).PreprocessingNotebook + +IMAGES = ["ctrl/rep1/a.tif", "ctrl/rep2/a.tif", "drug/rep1/a.tif", "drug/rep2/a.tif"] + + +def experiment(tmp_path): + root = tmp_path / "exp" + rng = np.random.default_rng(0) + for rel in IMAGES: + write_data(root / rel, rng.integers(0, 200, (16, 16), dtype=np.uint8), axes="YX") + return root + + +def nested_masks(tmp_path): + """Write cells (two squares) and nuclei (one smaller square inside each cell).""" + masks = tmp_path / "masks" + cells = np.zeros((16, 16), np.uint16) + cells[1:7, 1:7], cells[9:15, 9:15] = 1, 2 + nuclei = np.zeros((16, 16), np.uint16) + nuclei[3:5, 3:5], nuclei[11:13, 11:13] = 1, 2 + for rel in IMAGES: + write_data(masks / "cells" / rel, cells) + write_data(masks / "nuclei" / rel, nuclei) + return masks + + +def output_of(root): + return root.parent / f"{root.name}_MorphoBricks_Preprocessing" + + +def ran(root, masks=None, *, show_output=True): + nb = PreprocessingNotebook(str(root)) + nb.welcome() + nb.folder_step(show_output=show_output) + nb.axis_step() + page = nb.run_step() + if masks is not None: + nb.masks_folder.value = str(masks) + nb.check_button.click() + nb.labels_confirmed.button.click() + for toggle in nb.group_confirmations.values(): + if not toggle.value: + toggle.button.click() + nb.preview_button.click() + nb.preview_checked.button.click() + nb.confirm_button.click() + nb.image_type_checked.button.click() + return nb, page + + +def test_the_button_is_on_step_3_and_waits_for_a_run_that_recorded_masks(tmp_path): + root = experiment(tmp_path) + nb, page = ran(root, nested_masks(tmp_path)) + assert nb.mask_step_card in page.children[1].children + assert ui.is_visible(nb.mask_step_card) + assert nb.mask_step_button.disabled + assert "Available after a run that recorded masks" in nb.mask_step_hint.value + assert nb._on_describe_masks() is None + assert nb.run().written + assert not nb.mask_step_button.disabled + assert "2 mask types" in nb.mask_step_hint.value and "cells, nuclei" in nb.mask_step_hint.value + + +def test_without_recorded_masks_the_button_stays_disabled(tmp_path): + root = experiment(tmp_path) + nb, _ = ran(root) + assert nb.run().written + assert nb.mask_step_button.disabled + assert "The run recorded no masks" in nb.mask_step_hint.value + nb.mask_step_button.click() + assert nb.mask_step_status.value == "" + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() + + +def test_describe_the_masks_writes_the_report_and_shows_the_cards(tmp_path, capsys): + root = experiment(tmp_path) + nb, _ = ran(root, nested_masks(tmp_path)) + assert nb.run().written + nb.mask_step_button.click() + path = output_of(root).resolve() / MASK_REPORT_FILENAME + assert path.is_file() + status, shown = nb.mask_step_status.value, nb.mask_step_result.value + assert "The masks are described: 2 mask types in 4 images." in status + assert str(path) in status and "Thresholds: the defaults." in status + assert "suggestions from the masks' pixels" in status + assert "Mask types (suggestions to check)" in shown + assert "one object per label" in shown + assert "nucleiinsidecells" in shown + assert "8 of 8" in shown and "0/8/0/0" in shown + assert "
        cells\n        nuclei
    " in shown + assert nb.mask_step_progress.bar_style == "success" + assert not nb.mask_step_button.disabled and nb.mask_step_button.icon == "sitemap" + # The same report as the command line's masks step, with the recorded inputs. + written = json.loads(path.read_text(encoding="utf-8")) + path.unlink() + assert main(["masks", "--output", str(output_of(root))]) == 0 + again = json.loads(path.read_text(encoding="utf-8")) + assert {k: v for k, v in again.items() if k != "versions"} == { + k: v for k, v in written.items() if k != "versions" + } + + +def test_a_locked_output_folder_shows_the_lock_card_and_writes_nothing(tmp_path): + root = experiment(tmp_path) + nb, _ = ran(root, nested_masks(tmp_path)) + assert nb.run().written + with output_lock(output_of(root)): + assert nb._on_describe_masks() is None + assert ui.is_visible(nb.mask_step_lock.card) + assert "the output folder is locked" in nb.mask_step_status.value + assert nb.mask_step_result.value == "" + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() + assert "Describe the masks again" in nb.mask_step_lock.again + assert nb._on_describe_masks() is not None + assert not ui.is_visible(nb.mask_step_lock.card) + + +def test_mask_errors_since_the_run_are_shown_and_nothing_is_read(tmp_path): + root = experiment(tmp_path) + masks = nested_masks(tmp_path) + nb, _ = ran(root, masks) + assert nb.run().written + write_data(masks / "cells" / IMAGES[0], np.zeros((8, 8), np.uint16)) # wrong size now + assert nb._on_describe_masks() is None + assert "The masks have errors: none was read." in nb.mask_step_status.value + assert "mask size mismatch" in nb.mask_step_result.value + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() + + +def test_masks_whose_labels_cannot_be_used_get_a_red_card(tmp_path): + root = experiment(tmp_path) + masks = nested_masks(tmp_path) + nb, _ = ran(root, masks) + assert nb.run().written + write_data(masks / "cells" / IMAGES[0], np.full((16, 16), 2**31, np.uint32)) + assert nb._on_describe_masks() is None + assert "The masks cannot be used." in nb.mask_step_status.value + assert "Nothing was written." in nb.mask_step_status.value + assert nb.mask_step_progress.bar_style == "danger" + + +def test_a_new_run_clears_the_description(tmp_path): + root = experiment(tmp_path) + nb, _ = ran(root, nested_masks(tmp_path)) + assert nb.run().written + nb.mask_step_button.click() + assert nb.mask_step_result.value + assert nb.run().written + assert nb.mask_step_result.value == "" and nb.mask_step_status.value == "" + assert not nb.mask_step_button.disabled + + +def test_an_embedding_brick_that_hides_the_output_folder_gets_no_card(tmp_path): + root = experiment(tmp_path) + nb, _ = ran(root, show_output=False) + assert not ui.is_visible(nb.mask_step_card) + nb.run() + assert not ui.is_visible(nb.mask_step_card) From 0aab0863757b57a78371976676ef9ca5590040ff Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:40:14 +0000 Subject: [PATCH 3/7] PP-10d: space the headings of the masks description Checked on the HPMEC data rendered in a browser: the headings sat on the table above them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- .../notebook_mask_step.py | 16 ++++++++++++---- tests/test_notebook_mask_step.py | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/morphobricks_preprocessing/notebook_mask_step.py b/src/morphobricks_preprocessing/notebook_mask_step.py index 2656d31d..7631333c 100644 --- a/src/morphobricks_preprocessing/notebook_mask_step.py +++ b/src/morphobricks_preprocessing/notebook_mask_step.py @@ -41,6 +41,11 @@ ) +def _heading(text: str) -> str: + """Return a bold heading inside the card, spaced from the table above it.""" + return f"
    {html.escape(text)}
    " + + class _MaskStepCard: """Step 3's optional mask step: the button, its progress and the description.""" @@ -228,16 +233,19 @@ def _show_mask_description(self, step: MaskStep, result: MaskStepResult) -> None ) hierarchy = html.escape("\n".join(mask_hierarchy_lines(report))) parts = [ - "Mask types (suggestions to check)", + _heading("Mask types (suggestions to check)"), ui.table(mask_type_rows(report), numeric_from=2), - "Relationships found (suggestions to check)", + _heading("Relationships found (suggestions to check)"), ui.table(relationships, numeric_from=3) if len(relationships) > 1 else "
    None: no mask type is inside or grouped by another.
    ", ] if same: - parts += ["Two segmentations of the same objects (to check)", f"
      {same}
    "] - parts += ["Hierarchy (direct parents only)", f"
    {hierarchy}
    "] + parts += [ + _heading("Two segmentations of the same objects (to check)"), + f"
      {same}
    ", + ] + parts += [_heading("Hierarchy (direct parents only)"), f"
    {hierarchy}
    "] if step.issues: parts.append(issues_html(step.issues)) self.mask_step_result.value = "".join(parts) diff --git a/tests/test_notebook_mask_step.py b/tests/test_notebook_mask_step.py index 559313d4..e213c2e8 100644 --- a/tests/test_notebook_mask_step.py +++ b/tests/test_notebook_mask_step.py @@ -108,7 +108,7 @@ def test_describe_the_masks_writes_the_report_and_shows_the_cards(tmp_path, caps assert "The masks are described: 2 mask types in 4 images." in status assert str(path) in status and "Thresholds: the defaults." in status assert "suggestions from the masks' pixels" in status - assert "Mask types (suggestions to check)" in shown + assert "Mask types (suggestions to check)" in shown assert "one object per label" in shown assert "nucleiinsidecells" in shown assert "8 of 8" in shown and "0/8/0/0" in shown From 12b2a89c9319657f49ef5e033d9df9f9709eb9a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:51:10 +0000 Subject: [PATCH 4/7] Review PP-10d: hold the output folder's lock for the whole mask scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coordinator decision 1: describe_masks takes the lock before the first mask is read and holds it to the write of mask_relationships.json, so the command line's masks and the notebook's Describe the masks refuse a locked output folder (output_locked) before reading any mask. MaskStepResult's report is None on that refusal. Regression tests: the refusal reads no mask, the lock is held during every progress call, and the command prints no progress line when locked. Docs: contract §9.2 and §15.1, user guide, plan §10, the CLI module's notes; the notebook's lock card says no mask was read. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 10 +++--- docs/plan.md | 4 ++- docs/user_guide.md | 15 ++++++--- src/morphobricks_preprocessing/cli_masks.py | 7 ++-- .../notebook_mask_step.py | 4 +-- .../run/mask_step.py | 26 ++++++++------- tests/test_mask_step.py | 32 ++++++++++++++++--- 7 files changed, 68 insertions(+), 30 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index 0d8fcd10..b04f1dec 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -251,14 +251,14 @@ the bricks' tests. | Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json` and its own temporary folders; never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | | Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | -| `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report, the file written, or the `output_locked` refusal with nothing written; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | +| `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report and the file written, or the `output_locked` refusal with no mask read and nothing written: the lock is taken before the first mask is read and held to the write; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run and for plain stacks that are new or changed. | -| Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` while it writes `mask_relationships.json`, and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | +| Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` from before its first mask read to the write of `mask_relationships.json` (a locked folder is refused before any mask is read), and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | | Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | | `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). The notebook's **Check folder** does the same with the same functions (`run.check_step.suggested_review`, `write_check_outputs`; v0.4.0, PP-10): the report, and the template with the design card; nothing when an embedding brick hides the output folder (`folder_step(show_output=False)`). | | Notebook (v0.4.0, PP-10) | Step 3 offers the mode (`restart`, `full`) with its effect, and lists before the run the inputs that differ from the recorded ones with their effect (`run.planned.planned_changes(given, output_folder, design=, calibration=)`, from `resolve_inputs`, `design_for_run` and `calibration_change`: the changes the run then reports). After a run it names the mode, the files replaced and the input changes, and shows the report; a stopped run says what it wrote (question 55). A refusal `output_locked` (check, run, a design answer or a stated size) shows the holder (computer, process ID, start time) and a **Remove the lock of a stopped run** button: `remove_stale_lock` only on that click, never the lock of a process still running on the same computer; on Google Drive one session per output folder (question 58). | | Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) is step 3's **Describe the masks** (next row). | -| Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled only after a run in this session that wrote its outputs and recorded masks (`run.mask_step.recorded_mask_types(output_folder)`, from the manifest's `masks` key). A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which writes `mask_relationships.json` under the lock. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, mask errors found since the run (shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | +| Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled only after a run in this session that wrote its outputs and recorded masks (`run.mask_step.recorded_mask_types(output_folder)`, from the manifest's `masks` key). A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which reads the masks and writes `mask_relationships.json` holding the lock throughout. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, mask errors found since the run (shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | ### 9.3 Calibration (`calibration.csv`, v0.4.0, design v0.4 §3) @@ -407,7 +407,9 @@ mask type and every ordered pair of types over the whole experiment. Pieces are a negative value (possible in a signed integer type, which the header check accepts) raises an error when its pixels are read, and no report is written, so the background or a negative value is never counted as an object. Every conclusion is a suggestion with its evidence: -the user names the masks and confirms or corrects the relationships. +the user names the masks and confirms or corrects the relationships. The step holds the +output folder's lock (§9.2) from before the first mask is read to the write of the report; +another run holding it refuses the step (`output_locked`) before any mask is read. | Kind of mask | Rule | |---|---| diff --git a/docs/plan.md b/docs/plan.md index c2f5b07e..51d2eabc 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -472,7 +472,9 @@ API, CLI). The notebook and CLI only call the API. - Move only, first: the CLI's pairing with the recorded inputs, scan and write under the lock leave `cli_masks.masks_command` for `run.mask_step` (orchestration): `prepare_mask_step` (a `MaskStep`) and `describe_masks` (a `MaskStepResult`, the lock - refusal returned rather than raised). The command only prints; its output is unchanged + refusal returned rather than raised). Review (coordinator decision): the lock is held + from before the first mask is read to the write, so a locked folder is refused before + any mask is read, in the command and the notebook. The command only prints; its output is unchanged (same text and exit status over 13 scenarios, the HPMEC data included). - Step 3's **Describe the masks (optional)** card (`notebook_mask_step`, ui): enabled only after a run that recorded masks (`recorded_mask_types`); runs the step with the run's diff --git a/docs/user_guide.md b/docs/user_guide.md index 8f1d16e7..d5bba194 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -462,9 +462,9 @@ results or comparing another grouping. other type hold 0, 1, 2 or 3 or more, and how many are outside or split), masks of the same objects, and the hierarchy of direct parents. These are suggestions to check, not decisions. The description is written to **mask_relationships.json** in - the output folder (its path is shown), holding the output folder's lock: when another - run holds it, **The output folder is locked** is shown with **Remove the lock of a - stopped run**, and nothing is written. Mask errors found since the run (a mask + the output folder (its path is shown). The output folder's lock is held from before the + first mask is read until the file is written: when another run holds it, **The output folder is locked** is shown with **Remove the lock of a + stopped run**, no mask is read and nothing is written. Mask errors found since the run (a mask changed or removed) and masks that cannot be used (a label above 2,147,483,647) are shown as red cards, and nothing is written. The notebook uses the default thresholds (contract §15.1); the `masks` command can change them. A new run clears the card; @@ -637,6 +637,9 @@ you give that differs from the recorded one is used for this scan and printed as the run keeps its recorded value until you give it the new one. Label 0 is the background and objects have positive labels: a mask with a negative value stops the command with a message naming the mask, and no report is written. +The output folder's lock is held from before the first mask is read until the report is +written: when another run holds it, the command stops with `output_locked` before reading +any mask. The rules behind each suggestion use thresholds you can change, for example `--inside-object-share 0.8` to call a kind of mask "inside" another when 80% of its objects @@ -755,8 +758,10 @@ with `chmod a+r /*`, or delete them before running again. Only one run at a time can use an output folder: a run holds a lock file (`.morphobricks-preprocessing.lock`) and a second run is refused (`output_locked`). If the process is forcibly stopped, its lock stays; remove it with `--remove-stale-lock` once you -are sure no run is going on (every command takes it). `check` and `masks` hold the lock -while they write their reports, `design` and `calibrate` while they change their files, +are sure no run is going on (every command takes it). `check` holds the lock +while it writes its report, `masks` (and **Describe the masks**) from before the first +mask is read until `mask_relationships.json` is written (a locked folder is refused before +any mask is read), `design` and `calibrate` while they change their files, and are refused the same way. A stopped run leaves the last good results in place; rerunning checks the table against the manifest and rebuilds a mismatched pair. `mask_relationships.json` is removed by a `full` run and when the masks changed since it diff --git a/src/morphobricks_preprocessing/cli_masks.py b/src/morphobricks_preprocessing/cli_masks.py index 99aa46a7..9e5dddfe 100644 --- a/src/morphobricks_preprocessing/cli_masks.py +++ b/src/morphobricks_preprocessing/cli_masks.py @@ -6,7 +6,9 @@ takes every input it is not given from the run in the output folder (design v0.4 §1): the experiment folder (``experiment_not_recorded`` without one), the masks folder, the mask axes and the exclusions; a value given that differs is used and named. The pairing, the -scan and the write under the lock are :mod:`.run.mask_step`'s, shared with the notebook. +scan and the write are :mod:`.run.mask_step`'s, shared with the notebook: the output +folder's lock is held from before the first mask is read to the write, so a locked folder +is refused before any mask is read. """ from __future__ import annotations @@ -97,8 +99,7 @@ def progress(done: int, total: int, path: str) -> None: except ValueError as exc: # masks whose pixels cannot be used (planes, labels) print(f"The masks cannot be used: {exc}") return 1 - if result.refusal is not None: - print() + if result.refusal is not None: # refused before any mask is read print(format_issues([result.refusal])) return 1 print() diff --git a/src/morphobricks_preprocessing/notebook_mask_step.py b/src/morphobricks_preprocessing/notebook_mask_step.py index 7631333c..0bc5de73 100644 --- a/src/morphobricks_preprocessing/notebook_mask_step.py +++ b/src/morphobricks_preprocessing/notebook_mask_step.py @@ -177,8 +177,8 @@ def _describe_recorded_masks(self) -> MaskStepResult | None: self.mask_step_progress.bar_style = "danger" self.mask_step_status.value = ui.status( "error", - "The description was not written: the output folder is locked.", - ["The masks were read, but nothing was written."], + "The masks were not read: the output folder is locked.", + ["No mask was read and nothing was written."], ) self.mask_step_lock.show(step.output_folder) return None diff --git a/src/morphobricks_preprocessing/run/mask_step.py b/src/morphobricks_preprocessing/run/mask_step.py index f82c9a98..00c115d9 100644 --- a/src/morphobricks_preprocessing/run/mask_step.py +++ b/src/morphobricks_preprocessing/run/mask_step.py @@ -10,8 +10,9 @@ - :func:`recorded_mask_types` says whether the run in the output folder recorded masks: the notebook offers the step only then. - :func:`describe_masks` reads the masks (:func:`.mask_scan.scan_masks`) and writes the - report, holding the output folder's lock while it writes: a run may be replacing or - removing the same file. + report, holding the output folder's lock from the first mask read to the write: a run + may be replacing or removing the same file. A locked folder is refused before any mask + is read. """ from __future__ import annotations @@ -80,14 +81,15 @@ class MaskStepResult: Attributes ---------- report - The description of the masks. + The description of the masks, or None when the output folder's lock refused the + step (no mask was read). path The report written, or None when the output folder's lock refused it. refusal ``output_locked`` when another run holds the output folder's lock, else None. """ - report: MaskReport + report: MaskReport | None path: Path | None refusal: ValidationIssue | None = None @@ -176,8 +178,8 @@ def describe_masks( Returns ------- MaskStepResult - The report is written to the output folder holding its lock; ``refusal`` when - another run holds it (nothing is written). + The masks are read and the report written holding the output folder's lock; + ``refusal`` when another run holds it (no mask is read, nothing is written). Raises ------ @@ -187,16 +189,18 @@ def describe_masks( """ if not step.ready or step.preflight is None or step.output_folder is None: raise ValueError("the masks cannot be described: the step has errors or no masks") - report = scan_masks( - step.masks, step.preflight.discovery.root, progress=progress, thresholds=thresholds - ) output = step.output_folder output.mkdir(parents=True, exist_ok=True) try: - with output_lock(output): # a run may be replacing or removing the same file + # Held from the first mask read to the write: a run may be replacing the masks' + # recorded inputs or removing the report; a locked folder is refused before any read. + with output_lock(output): + report = scan_masks( + step.masks, step.preflight.discovery.root, progress=progress, thresholds=thresholds + ) path = write_mask_report(report, output / MASK_REPORT_FILENAME) except OutputLockedError as exc: - return MaskStepResult(report=report, path=None, refusal=lock_issue(exc)) + return MaskStepResult(report=None, path=None, refusal=lock_issue(exc)) return MaskStepResult(report=report, path=path) diff --git a/tests/test_mask_step.py b/tests/test_mask_step.py index be3df7e5..d463151e 100644 --- a/tests/test_mask_step.py +++ b/tests/test_mask_step.py @@ -16,7 +16,8 @@ describe_masks, prepare_mask_step, ) -from morphobricks_preprocessing.run.lock import output_lock +from morphobricks_preprocessing.run import mask_step +from morphobricks_preprocessing.run.lock import output_lock, read_lock from morphobricks_preprocessing.testing import make_clean_run @@ -84,12 +85,35 @@ def test_mask_errors_stop_the_step_before_any_mask_is_read(clean): describe_masks(step) -def test_a_locked_output_folder_gets_the_report_but_no_file(clean): +def test_a_locked_output_folder_is_refused_before_any_mask_is_read(clean, monkeypatch): step = prepare_mask_step(None, clean.output) + seen = [] + monkeypatch.setattr(mask_step, "scan_masks", lambda *a, **k: seen.append(a)) with output_lock(clean.output): - result = describe_masks(step) + result = describe_masks(step, progress=lambda *a: seen.append(a)) assert result.refusal.code == "output_locked" - assert result.path is None and result.report.images == 4 + assert result.path is None and result.report is None + assert seen == [] + assert not (clean.output / MASK_REPORT_FILENAME).exists() + + +def test_the_lock_is_held_from_the_first_mask_read_to_the_write(clean): + held = [] + result = describe_masks( + prepare_mask_step(None, clean.output), + progress=lambda *a: held.append(read_lock(clean.output) is not None), + ) + assert result.refusal is None + assert held and all(held) + assert read_lock(clean.output) is None + + +def test_the_command_refuses_a_locked_folder_before_reading_a_mask(clean, capsys): + with output_lock(clean.output): + assert main(["masks", "--output", str(clean.output)]) == 1 + out = capsys.readouterr().out + assert "output_locked" in out or "Another run is using this output folder" in out + assert "[1/4]" not in out assert not (clean.output / MASK_REPORT_FILENAME).exists() From 959075b75a71a4701f90cb8e7e8254326ec06532 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:51:21 +0000 Subject: [PATCH 5/7] Review PP-10d: sort the mask step tests' imports Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_mask_step.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_mask_step.py b/tests/test_mask_step.py index d463151e..0e3b3eeb 100644 --- a/tests/test_mask_step.py +++ b/tests/test_mask_step.py @@ -14,9 +14,9 @@ from morphobricks_preprocessing.run import ( MaskStep, describe_masks, + mask_step, prepare_mask_step, ) -from morphobricks_preprocessing.run import mask_step from morphobricks_preprocessing.run.lock import output_lock, read_lock from morphobricks_preprocessing.testing import make_clean_run From a4b8d534cb05548036f0f44c2348ff58a9eb5d24 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:56:28 +0000 Subject: [PATCH 6/7] Review PP-10d: Describe the masks works for an earlier run in a reopened notebook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coordinator decision 2: the button is enabled when step 1's output folder holds a v2 manifest that recorded masks, not only after a run in this session. run.mask_step.recorded_masks (replacing recorded_mask_types) returns a RecordedMasks (output folder, experiment folder, masks folder, mask types, when written); the help line names the run it describes, and a click runs prepare_mask_step(None, output_folder), the path the CLI's masks --output takes. The card follows step 1's output folder (re-read when the manifest's fingerprint changes), is never offered with show_output=False (folder_step refreshes it), and a moved experiment or masks folder is refused with the existing messages under a title that no longer says the masks have errors. Regression tests: reopened notebook (report equal to the CLI's), output folder edits, moved experiment and masks folders, no run, embedding brick. Docs: contract §9.2, user guide, plan §10, AGENTS.md, notebook. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 2 +- docs/contract.md | 2 +- docs/plan.md | 6 +- docs/user_guide.md | 13 ++- notebooks/MorphoBricks_Preprocessing.ipynb | 2 + .../notebook_mask_step.py | 93 +++++++++------ src/morphobricks_preprocessing/notebook_ui.py | 2 +- .../run/__init__.py | 6 +- .../run/mask_step.py | 54 +++++++-- tests/test_mask_step.py | 12 ++ tests/test_notebook_mask_step.py | 107 +++++++++++++++++- 11 files changed, 243 insertions(+), 56 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ba240b2a..8c003819 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -122,7 +122,7 @@ src/morphobricks_preprocessing/ the earlier run, InputChange), lock.py (the output folder's O_EXCL lock, remove_stale_lock), check_step.py (write_check_report), mask_step.py (the masks step shared by CLI `masks` and the notebook: prepare_mask_step, - describe_masks, recorded_mask_types; PP-10d), result.py (RunResult), + describe_masks, recorded_masks; PP-10d), result.py (RunResult), entries.py (manifest entries, new/changed/unchanged by RowKey), reuse.py (which earlier rows are reused), rows.py (a file's rows and their pixel checks), publish.py (staged outputs, published together) diff --git a/docs/contract.md b/docs/contract.md index b04f1dec..235add92 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -258,7 +258,7 @@ the bricks' tests. | `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). The notebook's **Check folder** does the same with the same functions (`run.check_step.suggested_review`, `write_check_outputs`; v0.4.0, PP-10): the report, and the template with the design card; nothing when an embedding brick hides the output folder (`folder_step(show_output=False)`). | | Notebook (v0.4.0, PP-10) | Step 3 offers the mode (`restart`, `full`) with its effect, and lists before the run the inputs that differ from the recorded ones with their effect (`run.planned.planned_changes(given, output_folder, design=, calibration=)`, from `resolve_inputs`, `design_for_run` and `calibration_change`: the changes the run then reports). After a run it names the mode, the files replaced and the input changes, and shows the report; a stopped run says what it wrote (question 55). A refusal `output_locked` (check, run, a design answer or a stated size) shows the holder (computer, process ID, start time) and a **Remove the lock of a stopped run** button: `remove_stale_lock` only on that click, never the lock of a process still running on the same computer; on Google Drive one session per output folder (question 58). | | Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) is step 3's **Describe the masks** (next row). | -| Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled only after a run in this session that wrote its outputs and recorded masks (`run.mask_step.recorded_mask_types(output_folder)`, from the manifest's `masks` key). A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which reads the masks and writes `mask_relationships.json` holding the lock throughout. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, mask errors found since the run (shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | +| Notebook mask step (v0.4.0, PP-10d) | Step 3's **Describe the masks (optional)** card. Its button is enabled when step 1's output folder holds a v2 manifest that recorded masks (`run.mask_step.recorded_masks(output_folder)`, a `RecordedMasks` from the manifest's `experiment_root`, `written_at` and `masks` keys): after a run in this session, or for an earlier run in a reopened notebook. The help line names the run it describes (output folder, when written, experiment folder, masks folder and mask types). The output folder is the one step 1 shows (default: next to the experiment folder); the experiment folder typed in step 1 only locates it, and the recorded one is read. A click runs the `masks` command's step with the run's recorded inputs (`prepare_mask_step(None, output_folder)`, the path the CLI's `masks --output` takes: experiment folder, masks folder, mask-axis statements, exclusions) and the default thresholds (the notebook offers no threshold; the command line's options change them), with a progress bar per image, then `describe_masks`, which reads the masks and writes `mask_relationships.json` holding the lock throughout. The card shows the mask types, the relationships with their counts (`mask_text.mask_type_rows`, `mask_relationship_rows`), masks of the same objects and the hierarchy (`mask_hierarchy_lines`), labelled as suggestions to check, and the path written. A refused step changes nothing: `experiment_not_recorded`, errors found since the run (an experiment or masks folder moved or removed, a mask changed; shown with their messages), no masks, masks that cannot be used (a red card), and `output_locked` (the lock card with **Remove the lock of a stopped run**). A new run clears the card. `folder_step(show_output=False)` hides the card; the pinned embedding API is unchanged (the click handler is private). | ### 9.3 Calibration (`calibration.csv`, v0.4.0, design v0.4 §3) diff --git a/docs/plan.md b/docs/plan.md index 51d2eabc..90cf2dcc 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -476,8 +476,10 @@ API, CLI). The notebook and CLI only call the API. from before the first mask is read to the write, so a locked folder is refused before any mask is read, in the command and the notebook. The command only prints; its output is unchanged (same text and exit status over 13 scenarios, the HPMEC data included). - - Step 3's **Describe the masks (optional)** card (`notebook_mask_step`, ui): enabled only - after a run that recorded masks (`recorded_mask_types`); runs the step with the run's + - Step 3's **Describe the masks (optional)** card (`notebook_mask_step`, ui): enabled when + step 1's output folder holds a v2 manifest that recorded masks (`recorded_masks`), after + a run in this session or for an earlier run in a reopened notebook (review, coordinator + decision); the help line names the run it describes; runs the step with the run's recorded inputs and the default thresholds (the notebook offers no threshold option; the command line does), with a progress bar; shows the mask types, the relationships with their counts, masks of the same objects and the hierarchy as suggestions to check, diff --git a/docs/user_guide.md b/docs/user_guide.md index d5bba194..d9c203f7 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -455,7 +455,11 @@ results or comparing another grouping. - **Describe the masks (optional)**, after a run that recorded masks: the notebook's counterpart of the `masks` command (§5), with the same functions and the inputs the run recorded (experiment folder, masks folder, mask axes, excluded images). The button - is greyed out until such a run, saying why. Click it to read every mask, one image at a + works for the run in step 1's output folder: after a run in this session, and also for + an earlier run when you reopen the notebook (choose the same experiment or output + folder in step 1). The line above it names the run it describes (output folder, when + it was written, experiment and masks folders, mask types); it is greyed out, saying + why, when that folder holds no run that recorded masks. Click it to read every mask, one image at a time (a progress bar names the image); the card then shows each mask type (kind, images, objects, median pieces per label), the relationships found (which type is inside or grouped by which, how many objects are inside, how many objects of the @@ -464,9 +468,10 @@ results or comparing another grouping. check, not decisions. The description is written to **mask_relationships.json** in the output folder (its path is shown). The output folder's lock is held from before the first mask is read until the file is written: when another run holds it, **The output folder is locked** is shown with **Remove the lock of a - stopped run**, no mask is read and nothing is written. Mask errors found since the run (a mask - changed or removed) and masks that cannot be used (a label above 2,147,483,647) are - shown as red cards, and nothing is written. The notebook uses the default thresholds + stopped run**, no mask is read and nothing is written. Errors found since the run + (the experiment or masks folder moved, a mask changed or removed) and masks that + cannot be used (a label above 2,147,483,647) are shown as red cards, and nothing is + written. The notebook uses the default thresholds (contract §15.1); the `masks` command can change them. A new run clears the card; an embedding brick that hides the output folder does not show it. 4. **Calculate the statistics (optional).** The table from step 3 is all that later bricks diff --git a/notebooks/MorphoBricks_Preprocessing.ipynb b/notebooks/MorphoBricks_Preprocessing.ipynb index 111cfb0c..c687990d 100644 --- a/notebooks/MorphoBricks_Preprocessing.ipynb +++ b/notebooks/MorphoBricks_Preprocessing.ipynb @@ -523,6 +523,8 @@ "> **Check settings**; changing them updates the warnings at once, and the next run records them.\n", "\n", "**Describe the masks (optional).** After a run that recorded masks, press **Describe the masks**.\n", + "It also works for an earlier run when you reopen this notebook: choose the same folders in step 1;\n", + "the line above the button names the run it describes.\n", "It reads every mask the run recorded (same masks folder, mask axes and excluded images) and\n", "shows, with the counts behind each:\n", "\n", diff --git a/src/morphobricks_preprocessing/notebook_mask_step.py b/src/morphobricks_preprocessing/notebook_mask_step.py index 0bc5de73..4b96082d 100644 --- a/src/morphobricks_preprocessing/notebook_mask_step.py +++ b/src/morphobricks_preprocessing/notebook_mask_step.py @@ -1,9 +1,11 @@ """Step 3 of the notebook: **Describe the masks**, the optional mask step (plan §10, PP-10d). -After a run that recorded masks, the button runs the command line's ``masks`` step with the -same functions (:func:`.run.mask_step.prepare_mask_step`, :func:`.run.mask_step.describe_masks`) -and the inputs the run recorded: the experiment folder, the masks folder, the mask axes and -the exclusions. It reads the masks, writes ``mask_relationships.json`` to the output folder +When the output folder of step 1 holds a run that recorded masks (a run of this session, or +an earlier one in a reopened notebook), the button runs the command line's ``masks`` step +with the same functions (:func:`.run.mask_step.prepare_mask_step`, +:func:`.run.mask_step.describe_masks`) and the inputs the run recorded: the experiment +folder, the masks folder, the mask axes and the exclusions; the help line names the run it +describes. It reads the masks and writes ``mask_relationships.json`` to the output folder holding its lock, and shows each mask type, the relationships between the types and their hierarchy, as suggestions to check (contract §15). The thresholds are the defaults; the command line's ``masks`` can change them. A refusal is shown on the card (the locked output @@ -15,10 +17,14 @@ import html import traceback +from pathlib import Path import ipywidgets as widgets from . import notebook_style as ui +from .discovery import default_output_folder +from .fingerprint import current_fingerprint +from .manifest import MANIFEST_FILENAME from .mask_scan import MASK_REPORT_FILENAME from .mask_text import ( mask_hierarchy_lines, @@ -28,11 +34,11 @@ ) from .notebook_output import _LockCard from .notebook_render import issues_html -from .run import MaskStep, MaskStepResult, mask_step +from .run import MaskStep, MaskStepResult, RecordedMasks, mask_step from .validation import has_errors, plural _STALE = object() -"""Marks the step as describing no run (a run is going on).""" +"""Marks the step as describing no run (a run is going on): the next refresh reads it again.""" _LABEL = " Describe the masks" _ICON = "sitemap" _SUGGESTIONS = ( @@ -50,8 +56,9 @@ class _MaskStepCard: """Step 3's optional mask step: the button, its progress and the description.""" def _build_mask_step_card(self) -> None: - """Build the card, its button disabled until a run records masks.""" - self._mask_step_run: object = _STALE + """Build the card, its button disabled until the output folder holds recorded masks.""" + self._mask_step_seen: object = _STALE + self._mask_step_recorded: RecordedMasks | None = None self.mask_step_button = ui.button( _LABEL, style="info", @@ -90,35 +97,33 @@ def _build_mask_step_card(self) -> None: self.mask_step_button.on_click(lambda _: self._on_describe_masks()) def _refresh_mask_step(self) -> None: - """Offer the step only after a run that recorded masks; clear it for another run.""" + """Offer the step when the output folder's run recorded masks; clear it for another.""" ui.set_visible(self.mask_step_card, self._show_output) - if self.result is self._mask_step_run: + output = self._mask_step_output() + manifest = None if output is None else current_fingerprint(output / MANIFEST_FILENAME) + if self._mask_step_seen == (output, manifest): return - self._mask_step_run = self.result + self._mask_step_seen = (output, manifest) self._clear_mask_step() - types = self._recorded_mask_types() - self.mask_step_button.disabled = not types - if types: - hint = ( - f"The run recorded {plural(len(types), 'mask type')}: " - f"{html.escape(', '.join(types))}." - ) - elif self.result is not None and self.result.written: - hint = "The run recorded no masks: give a masks folder in step 1 and run again." - else: - hint = "Available after a run that recorded masks (step 1's masks folder)." - self.mask_step_hint.value = f"
    {hint}
    " - - def _recorded_mask_types(self) -> tuple[str, ...]: - """Return the mask types the last run recorded (none before a written run).""" - if self.result is None or not self.result.written: - return () with self.log.attached(): - return mask_step.recorded_mask_types(self.result.output_folder) + recorded = None if output is None else mask_step.recorded_masks(output) + self._mask_step_recorded = recorded if recorded and recorded.mask_types else None + self.mask_step_button.disabled = self._mask_step_recorded is None + self.mask_step_hint.value = f"
    {_recorded_hint(recorded)}
    " + + def _mask_step_output(self) -> Path | None: + """Return step 1's output folder, resolved; None without a folder or when hidden.""" + if not self._show_output: + return None + output = self._output_folder() + folder = ui.clean_path(self.folder.value) + if output: + return Path(output).resolve() + return default_output_folder(Path(folder).resolve()).resolve() if folder else None def _mask_step_stale(self) -> None: """Clear the step and disable it while a run replaces the results it describes.""" - self._mask_step_run = _STALE + self._mask_step_seen = _STALE self._clear_mask_step() self.mask_step_button.disabled = True @@ -159,7 +164,7 @@ def _on_describe_masks(self) -> MaskStepResult | None: def _describe_recorded_masks(self) -> MaskStepResult | None: """Pair, read and describe the masks with the run's recorded inputs; show the result.""" - step = mask_step.prepare_mask_step(None, self.result.output_folder) + step = mask_step.prepare_mask_step(None, self._mask_step_recorded.output_folder) if not step.ready: self._show_mask_step_refused(step) return None @@ -204,7 +209,7 @@ def _show_mask_step_refused(self, step: MaskStep) -> None: if step.refusal is not None: title, issues = "The run's experiment folder is not recorded.", [step.refusal] elif has_errors(step.issues): - title, issues = "The masks have errors: none was read.", list(step.issues) + title, issues = "An error stops the step: no mask was read.", list(step.issues) else: title, issues = "No masks were found with the inputs the run recorded.", [] self.mask_step_status.value = ui.status( @@ -249,3 +254,27 @@ def _show_mask_description(self, step: MaskStep, result: MaskStepResult) -> None if step.issues: parts.append(issues_html(step.issues)) self.mask_step_result.value = "".join(parts) + + +def _recorded_hint(recorded: RecordedMasks | None) -> str: + """Name the run the step describes, or say why the button is disabled.""" + if recorded is None: + return ( + "Available when the output folder holds a run that recorded masks " + "(step 1's masks folder)." + ) + when = f", written {html.escape(recorded.written_at)}" if recorded.written_at else "" + run = ( + f"the run in {html.escape(str(recorded.output_folder))}{when}, of " + f"{html.escape(str(recorded.experiment_root))}" + ) + if not recorded.mask_types: + return ( + f"Describes {run}: it recorded no masks. Give a masks folder in step 1 and run again." + ) + masks = recorded.masks_folder + source = f" from {html.escape(str(masks))}" if masks else "" + return ( + f"Describes {run}: {plural(len(recorded.mask_types), 'mask type')}{source}: " + f"{html.escape(', '.join(recorded.mask_types))}." + ) diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index 4bc2946b..f80b54b7 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -264,7 +264,7 @@ def folder_step( ui.set_visible(self.folder_next, show_next) self._show_output = show_output ui.set_visible(self.output_card, show_output) - ui.set_visible(self.mask_step_card, show_output) + self._refresh_mask_step() # hidden, and never offered, without the output folder self.labels_confirmed.explain(_labels_explanation(show_design)) if show_design != self._show_design: self._show_design = show_design diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py index a1194a50..5afab4c9 100644 --- a/src/morphobricks_preprocessing/run/__init__.py +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -29,9 +29,10 @@ from .mask_step import ( MaskStep, MaskStepResult, + RecordedMasks, describe_masks, prepare_mask_step, - recorded_mask_types, + recorded_masks, ) from .modes import FULL, MODES, RESTART from .pipeline import run_experiment @@ -49,6 +50,7 @@ "MaskStepResult", "OutputLockedError", "Preflight", + "RecordedMasks", "RunResult", "SamplePlane", "describe_masks", @@ -56,7 +58,7 @@ "preflight", "prepare_mask_step", "read_lock", - "recorded_mask_types", + "recorded_masks", "remove_stale_lock", "run_experiment", "sample_planes", diff --git a/src/morphobricks_preprocessing/run/mask_step.py b/src/morphobricks_preprocessing/run/mask_step.py index 00c115d9..3f8d460b 100644 --- a/src/morphobricks_preprocessing/run/mask_step.py +++ b/src/morphobricks_preprocessing/run/mask_step.py @@ -7,8 +7,9 @@ output folder recorded unless they are given (design v0.4 §1): the experiment folder (``experiment_not_recorded`` without one), the masks folder, the mask axes and the exclusions; a value given that differs is used and named. It reads no pixels. -- :func:`recorded_mask_types` says whether the run in the output folder recorded masks: - the notebook offers the step only then. +- :func:`recorded_masks` says what the run in the output folder recorded (a v2 manifest): + the notebook offers the step only when it recorded masks, also for a run of an earlier + session. - :func:`describe_masks` reads the masks (:func:`.mask_scan.scan_masks`) and writes the report, holding the output folder's lock from the first mask read to the write: a run may be replacing or removing the same file. A locked folder is refused before any mask @@ -94,6 +95,31 @@ class MaskStepResult: refusal: ValidationIssue | None = None +@dataclass(frozen=True) +class RecordedMasks: + """The run in an output folder, as the mask step would describe it. + + Attributes + ---------- + output_folder + The output folder, resolved. + experiment_root + The experiment folder the run recorded. + masks_folder + The masks folder it recorded, or None (masks of OME-Zarr images only, or none). + mask_types + The mask types it recorded; empty when it recorded no masks. + written_at + When its manifest was written (UTC, ISO 8601), or empty when unknown. + """ + + output_folder: Path + experiment_root: Path + masks_folder: Path | None + mask_types: tuple[str, ...] + written_at: str = "" + + def prepare_mask_step( experiment_root: str | os.PathLike | None, output_folder: str | os.PathLike | None, @@ -204,8 +230,8 @@ def describe_masks( return MaskStepResult(report=report, path=path) -def recorded_mask_types(output_folder: str | os.PathLike) -> tuple[str, ...]: - """Return the mask types the run in the output folder recorded (v0.4.0 manifest). +def recorded_masks(output_folder: str | os.PathLike) -> RecordedMasks | None: + """Return what the run in the output folder recorded for the mask step (v2 manifest). Parameters ---------- @@ -214,10 +240,20 @@ def recorded_mask_types(output_folder: str | os.PathLike) -> tuple[str, ...]: Returns ------- - tuple of str - The recorded mask types; empty without a usable manifest or without masks. + RecordedMasks or None + None without a usable (v0.4.0 or later) manifest; ``mask_types`` is empty when the + run recorded no masks. """ - manifest = usable(read_previous(Path(output_folder).resolve())[0]) + output = Path(output_folder).resolve() + manifest = usable(read_previous(output)[0]) if manifest is None: - return () - return tuple((manifest.masks or {}).get("types") or ()) + return None + masks = manifest.masks or {} + folder = masks.get("folder") + return RecordedMasks( + output_folder=output, + experiment_root=Path(manifest.experiment_root), + masks_folder=Path(folder) if folder else None, + mask_types=tuple(masks.get("types") or ()), + written_at=manifest.written_at, + ) diff --git a/tests/test_mask_step.py b/tests/test_mask_step.py index 0e3b3eeb..c10b1484 100644 --- a/tests/test_mask_step.py +++ b/tests/test_mask_step.py @@ -16,6 +16,7 @@ describe_masks, mask_step, prepare_mask_step, + recorded_masks, ) from morphobricks_preprocessing.run.lock import output_lock, read_lock from morphobricks_preprocessing.testing import make_clean_run @@ -122,3 +123,14 @@ def test_no_masks_found_is_not_ready(tmp_path): step = prepare_mask_step(None, bare.output) assert isinstance(step, MaskStep) assert step.refusal is None and step.masks.types == () and not step.ready + + +def test_recorded_masks_names_the_run_the_step_describes(clean, tmp_path): + recorded = recorded_masks(clean.output) + assert recorded.output_folder == clean.output.resolve() + assert recorded.experiment_root == clean.experiment.resolve() + assert recorded.masks_folder == clean.masks.resolve() + assert recorded.mask_types == ("Cells", "Nuclei") and recorded.written_at + assert recorded_masks(tmp_path / "nothing") is None + bare = make_clean_run(tmp_path / "bare", axes="YX", shape=(8, 8)) + assert recorded_masks(bare.output).mask_types == () diff --git a/tests/test_notebook_mask_step.py b/tests/test_notebook_mask_step.py index e213c2e8..218ea4cf 100644 --- a/tests/test_notebook_mask_step.py +++ b/tests/test_notebook_mask_step.py @@ -79,11 +79,16 @@ def test_the_button_is_on_step_3_and_waits_for_a_run_that_recorded_masks(tmp_pat assert nb.mask_step_card in page.children[1].children assert ui.is_visible(nb.mask_step_card) assert nb.mask_step_button.disabled - assert "Available after a run that recorded masks" in nb.mask_step_hint.value + assert "Available when the output folder holds a run that recorded masks" in ( + nb.mask_step_hint.value + ) assert nb._on_describe_masks() is None assert nb.run().written assert not nb.mask_step_button.disabled - assert "2 mask types" in nb.mask_step_hint.value and "cells, nuclei" in nb.mask_step_hint.value + hint = nb.mask_step_hint.value + assert "2 mask types" in hint and "cells, nuclei" in hint + assert f"Describes the run in {output_of(root).resolve()}" in hint + assert f"of {root.resolve()}" in hint def test_without_recorded_masks_the_button_stays_disabled(tmp_path): @@ -91,7 +96,7 @@ def test_without_recorded_masks_the_button_stays_disabled(tmp_path): nb, _ = ran(root) assert nb.run().written assert nb.mask_step_button.disabled - assert "The run recorded no masks" in nb.mask_step_hint.value + assert "it recorded no masks" in nb.mask_step_hint.value nb.mask_step_button.click() assert nb.mask_step_status.value == "" assert not (output_of(root) / MASK_REPORT_FILENAME).exists() @@ -147,7 +152,7 @@ def test_mask_errors_since_the_run_are_shown_and_nothing_is_read(tmp_path): assert nb.run().written write_data(masks / "cells" / IMAGES[0], np.zeros((8, 8), np.uint16)) # wrong size now assert nb._on_describe_masks() is None - assert "The masks have errors: none was read." in nb.mask_step_status.value + assert "An error stops the step: no mask was read." in nb.mask_step_status.value assert "mask size mismatch" in nb.mask_step_result.value assert not (output_of(root) / MASK_REPORT_FILENAME).exists() @@ -181,3 +186,97 @@ def test_an_embedding_brick_that_hides_the_output_folder_gets_no_card(tmp_path): assert not ui.is_visible(nb.mask_step_card) nb.run() assert not ui.is_visible(nb.mask_step_card) + + +def reopened(root, *, show_output=True): + """Open a new notebook on an experiment, as after a restart: step 1's folders, no run.""" + nb = PreprocessingNotebook(str(root)) + nb.welcome() + nb.folder_step(show_output=show_output) + nb.axis_step() + nb.run_step() + return nb + + +def test_a_reopened_notebook_describes_the_earlier_run(tmp_path): + root = experiment(tmp_path) + first, _ = ran(root, nested_masks(tmp_path)) + assert first.run().written + nb = reopened(root) + assert nb.result is None + assert not nb.mask_step_button.disabled + hint = nb.mask_step_hint.value + assert f"Describes the run in {output_of(root).resolve()}, written " in hint + assert "cells, nuclei" in hint and str((tmp_path / "masks").resolve()) in hint + result = nb._on_describe_masks() + assert result is not None and result.path == output_of(root).resolve() / MASK_REPORT_FILENAME + written = json.loads(result.path.read_text(encoding="utf-8")) + result.path.unlink() + assert main(["masks", "--output", str(output_of(root))]) == 0 + again = json.loads(result.path.read_text(encoding="utf-8")) + assert {k: v for k, v in again.items() if k != "versions"} == { + k: v for k, v in written.items() if k != "versions" + } + + +def test_a_reopened_notebook_follows_the_output_folder_of_step_1(tmp_path): + root = experiment(tmp_path) + first, _ = ran(root, nested_masks(tmp_path)) + assert first.run().written + nb = reopened(root) + assert not nb.mask_step_button.disabled + nb.output.value = str(tmp_path / "elsewhere") + assert nb.mask_step_button.disabled + assert "Available when the output folder holds" in nb.mask_step_hint.value + nb.output.value = str(output_of(root)) + assert not nb.mask_step_button.disabled + + +def test_a_reopened_notebook_refuses_when_the_experiment_folder_moved(tmp_path): + root = experiment(tmp_path) + first, _ = ran(root, nested_masks(tmp_path)) + assert first.run().written + output = output_of(root) + root.rename(tmp_path / "moved") + nb = PreprocessingNotebook(str(tmp_path / "moved"), str(output)) + nb.welcome() + nb.folder_step() + nb.axis_step() + nb.run_step() + assert not nb.mask_step_button.disabled + assert nb._on_describe_masks() is None + assert "An error stops the step: no mask was read." in nb.mask_step_status.value + assert "does not exist or is not a folder" in nb.mask_step_result.value + assert not (output / MASK_REPORT_FILENAME).exists() + + +def test_a_reopened_notebook_refuses_when_the_masks_folder_moved(tmp_path): + root = experiment(tmp_path) + masks = nested_masks(tmp_path) + first, _ = ran(root, masks) + assert first.run().written + masks.rename(tmp_path / "moved_masks") + nb = reopened(root) + assert nb._on_describe_masks() is None + assert "The masks folder" in nb.mask_step_result.value + assert "does not exist or is not a folder" in nb.mask_step_result.value + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() + + +def test_a_reopened_notebook_without_a_run_keeps_the_button_disabled(tmp_path): + root = experiment(tmp_path) + nb = reopened(root) + assert nb.mask_step_button.disabled + assert nb._on_describe_masks() is None + assert nb.mask_step_status.value == "" + + +def test_an_embedding_brick_never_gets_the_earlier_run_described(tmp_path): + root = experiment(tmp_path) + first, _ = ran(root, nested_masks(tmp_path)) + assert first.run().written + nb = reopened(root, show_output=False) + assert not ui.is_visible(nb.mask_step_card) + assert nb.mask_step_button.disabled + assert nb._on_describe_masks() is None + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() From 3f3b2064205281756307348aaee055913dd5554e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:59:07 +0000 Subject: [PATCH 7/7] Review PP-10d: annotate the moved hierarchy helper, drop its noqa MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mask_text._hierarchy_lines kept its # noqa: ANN001 through the move; §9 asks moved functions to be annotated. The ANN noqa ratchet shrinks 55 -> 54. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/mask_text.py | 4 +++- tools/code_rules_baseline.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/morphobricks_preprocessing/mask_text.py b/src/morphobricks_preprocessing/mask_text.py index 0bc6f74b..91d0476c 100644 --- a/src/morphobricks_preprocessing/mask_text.py +++ b/src/morphobricks_preprocessing/mask_text.py @@ -8,6 +8,8 @@ from __future__ import annotations +from collections.abc import Mapping, Sequence + from .mask_scan import DEFAULT_THRESHOLDS, MaskReport, PairRelation from .masks import MaskInventory from .validation import plural @@ -157,7 +159,7 @@ def _relationship_line(pair: PairRelation) -> str: return text + (f"; {', '.join(extra)}" if extra else "") -def _hierarchy_lines(hierarchy) -> list[str]: # noqa: ANN001 +def _hierarchy_lines(hierarchy: Mapping[str, Sequence[str]]) -> list[str]: """Return the mask types as an indented tree; a type with two parents shows twice.""" children: dict[str, list[str]] = {name: [] for name in hierarchy} for name, parents in hierarchy.items(): diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 9dd42214..d96a4100 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -1,6 +1,6 @@ { "noqa": { - "ANN": 55, + "ANN": 54, "C901": 0, "PLR0913": 3, "PLR0915": 0