From 2e1a956f4d469097b3046865fe458d5f439ba776 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:30:10 +0000 Subject: [PATCH 1/2] PP-8b: refuse X-before-Y masks in the run; RunInputs names unrecorded files - A mask whose own header names X before Y (e.g. XY, TXY) is refused by check and run with mask_layout, naming the mask and its header axes and the numpy.swapaxes fix. mask_issues dropped this error together with the header size comparison it supersedes, so rc1 recorded such a mask with axes "". mask_manifest now refuses a mask without roles, and RunInputs.open refuses such an older record with its own message. - unrecorded_files lists the images discover_experiment would analyse and the masks find_masks would pair with a recorded image (masks folder, OME-Zarr label images) that the run neither recorded nor excluded. RunInputs.open always runs it and raises UnrecordedFilesError (a RunInputsError naming the files). No pixel or TIFF header is read. - Contract 9.1/15, plan questions 50 and 51, user guide, AGENTS map. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 3 + docs/contract.md | 25 ++- docs/plan.md | 26 +++ docs/user_guide.md | 7 +- pyproject.toml | 1 + src/morphobricks_preprocessing/__init__.py | 6 +- src/morphobricks_preprocessing/mask_checks.py | 22 ++- src/morphobricks_preprocessing/mask_record.py | 9 + src/morphobricks_preprocessing/masks/files.py | 7 +- .../masks/stated.py | 3 +- src/morphobricks_preprocessing/run_inputs.py | 94 +++++++++-- src/morphobricks_preprocessing/unrecorded.py | 116 +++++++++++++ tests/test_brick_checks.py | 159 ++++++++++++++++++ 13 files changed, 455 insertions(+), 23 deletions(-) create mode 100644 src/morphobricks_preprocessing/unrecorded.py create mode 100644 tests/test_brick_checks.py diff --git a/AGENTS.md b/AGENTS.md index a0f41cb4..4b5870a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,6 +99,9 @@ src/morphobricks_preprocessing/ pixel_size_checks.py the §11 pixel-size checks on resolved sizes (moved from metadata.py) narrow_ranges.py NarrowRange (moved from manifest.py) sources.py check_sources (moved from manifest.py in PP-4) + unrecorded.py unrecorded_files: images and masks the run neither recorded nor excluded, by + preprocessing's discovery and pairing; RunInputs.open raises + UnrecordedFilesError for them (PP-8b, contract §9.1) mask_record.py the manifest's masks entry with fingerprints and roles (manifest v2) axis_check.py the axis check (contract §2): header_axis_issue, axis_disagreements, likely_axis_cause, check_header_axes, ConfirmedAxes (a confirmation covers diff --git a/docs/contract.md b/docs/contract.md index 6b41b493..a7f93368 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -121,7 +121,7 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | +| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | | Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and @@ -184,8 +184,25 @@ is not empty. Bricks read a run's files only through `RunInputs.open(output_folder, experiment_root=None, masks_root=None)` (design v0.4 §6). It refuses (`RunInputsError`) a version 1 manifest, a `plane_histograms.csv`, `experiment_design.csv` or `calibration.csv` whose SHA-256 is not the -manifest's (also a `calibration.csv` added since the run), and an image or mask changed or -gone since the run; every read checks the file's fingerprint again. +manifest's (also a `calibration.csv` added since the run), an image or mask changed or +gone since the run, and a mask recorded without axes (a v0.4.0rc1 run wrote `axes: ""` for a +mask whose header puts X before Y; the message says to save the mask again with +`numpy.swapaxes` and run again, never "changed or missing"); every read checks the file's +fingerprint again. + +It also refuses an experiment holding files the run neither recorded nor excluded +(`UnrecordedFilesError`, a `RunInputsError`, PP-8b): images that preprocessing's own +discovery (`discover_experiment`) would analyse, and masks it would pair with a recorded +image (masks folder by `find_masks`'s pairing; OME-Zarr label images of recorded images), +less those the run recorded and the images it excluded. Its `files` (`UnrecordedFiles`: +`images`, `masks`) names them, and the message says to run preprocessing again on the +experiment as it is, or exclude the images. Files discovery ignores or refuses (non-image +and system files, images outside a leaf folder or below the technical repeat, masks that +pair with no image or belong to an excluded image) do not count. `open` always runs this +check: it lists folders and reads OME-Zarr label metadata, never a pixel or a TIFF header, +so a brick never leaves out an image added since the run and never discovers files itself. +`unrecorded_files(manifest, experiment_root, masks_root, output_folder=None)` gives the same +list without raising. It gives: - `fovs()`: `Fov` records with the FOV key, `plane_id`s, axes, roles, `axes_from`, channel @@ -350,7 +367,7 @@ masks/ masks folder, outside the experiment folder | Layout | Either one subfolder per mask type, each mirroring the experiment's folders (`//.../.tif`), or the experiment's folders with the type after the image name (`/.../_.tif`, as segmentation tools often write it, e.g. MorphoRelate's JUMP-Target-2 conversion); the two may be mixed. Any number of mask types. The label images of an OME-Zarr image (its OME-NGFF `labels` group, with the `zarr` extra) are masks too, of the type named by the label image; since v0.4.0 they are always used when present, with or without a masks folder. Each label image has its own fingerprint in the manifest, and the `labels` group is left out of its image's fingerprint (`source_fingerprint`), so adding or changing a label image does not recompute the image's rows. | | Pairing | A mask belongs to the image in the same leaf folder with the same `position_id` (§1), ignoring upper/lower case; its file is a TIFF (`.tif`, `.tiff`, optionally `.ome`, any case). Excluded images have no masks: their masks are reported apart (`mask_of_excluded_image`, warning) and not used. | | Names | A mask type is named by its folder, by the suffix after the image name, or by its label image. A file that could belong to two images (an image name that starts another's, `a_b_cells.tif` beside images `a` and `a_b`) or fit both layouts is the error `ambiguous_mask`. Two names differing only in case are the error `mask_names_differ_in_case`; one image with two masks of one type (two extensions, or a file and a label image) is the error `duplicate_mask`. | -| Header checks | From the headers only, no pixels read (`check`, `run`, `preflight`): once its length-1 axes other than Y and X are removed (Y and X are kept whatever their length, so a mask one pixel high or wide, or a single pixel, is a 2D `YX` mask; a TIFF whose series has no Y or X but is one 2D page, such as a 1D array `tifffile` saved as `{"shape": [N]}` and reads as `X`, is that page, `YX` of 1 row; plan §12 question 44), a mask has Y and X and at most a depth axis Z and a time axis T, each named so in its own metadata (TIFF axes as `tifffile` reads them, e.g. an ImageJ hyperstack or OME-TIFF; OME-NGFF axes for a label image); any other or unnamed axis, such as a plain stack that could be depth or time, is `mask_not_2d` and is never guessed, unless the axes are stated for its mask type (`--mask-axes Cells=TYX`, `mask_axes={"Cells": "TYX"}`: one role, Y, X, Z or T, per axis of the header with length-1 axes other than Y and X left out; recorded as `axes_from: "stated"`; a statement that does not fit is `invalid_mask_axes`, one that contradicts an axis the metadata names is `mask_axes_contradict_header`, one for a type not found is `invalid_mask_axes`). Masks are never converted or reordered: a mask whose X is stored before its Y, or whose Y and X are its image's swapped, is `mask_layout`, with a `numpy.swapaxes` hint. A mask has the Y and X sizes of its image (`mask_size_mismatch`; `check` and `preflight` compare with the Y and X the image's header names, and not for a plain stack, whose Y and X are not named; `run` compares with the image's Y and X under the confirmed roles, `mask_size_issues`, and leaves out the masks of an image with an axis error (§2), which is never read — before v0.4.0 `run` compared with the header's letters, so a swapped image gave a misleading size error) and integer or boolean pixels (`mask_pixel_type`; a mask holding a negative label or a label above 2,147,483,647, the int32 limit every brick shares (§16, decision D-b), raises an error when its pixels are read); the masks of one image have the same depth slices and time points (`mask_planes_differ`); a mask that cannot be opened, or holds several series, is `unreadable_mask`, as is an OME-Zarr image's `labels` group that cannot be read or does not list its label images (path `/labels`), so its mask types never disappear unreported. All five are errors. | +| Header checks | From the headers only, no pixels read (`check`, `run`, `preflight`): once its length-1 axes other than Y and X are removed (Y and X are kept whatever their length, so a mask one pixel high or wide, or a single pixel, is a 2D `YX` mask; a TIFF whose series has no Y or X but is one 2D page, such as a 1D array `tifffile` saved as `{"shape": [N]}` and reads as `X`, is that page, `YX` of 1 row; plan §12 question 44), a mask has Y and X and at most a depth axis Z and a time axis T, each named so in its own metadata (TIFF axes as `tifffile` reads them, e.g. an ImageJ hyperstack or OME-TIFF; OME-NGFF axes for a label image); any other or unnamed axis, such as a plain stack that could be depth or time, is `mask_not_2d` and is never guessed, unless the axes are stated for its mask type (`--mask-axes Cells=TYX`, `mask_axes={"Cells": "TYX"}`: one role, Y, X, Z or T, per axis of the header with length-1 axes other than Y and X left out; recorded as `axes_from: "stated"`; a statement that does not fit is `invalid_mask_axes`, one that contradicts an axis the metadata names is `mask_axes_contradict_header`, one for a type not found is `invalid_mask_axes`). Masks are never converted or reordered: a mask whose X is stored before its Y, or whose Y and X are its image's swapped, is `mask_layout`, with a `numpy.swapaxes` hint; the message names each mask with its header axes (`Cells/a.tif: header axes XY: X is stored before Y`). Unlike an image whose own header names X before Y (read in canonical order), a mask is never reordered, so `run` refuses it too (before PP-8b `run` dropped this error with the header size comparison and recorded the mask with `axes: ""`); a manifest never records a mask without axes. A mask has the Y and X sizes of its image (`mask_size_mismatch`; `check` and `preflight` compare with the Y and X the image's header names, and not for a plain stack, whose Y and X are not named; `run` compares with the image's Y and X under the confirmed roles, `mask_size_issues`, and leaves out the masks of an image with an axis error (§2), which is never read — before v0.4.0 `run` compared with the header's letters, so a swapped image gave a misleading size error) and integer or boolean pixels (`mask_pixel_type`; a mask holding a negative label or a label above 2,147,483,647, the int32 limit every brick shares (§16, decision D-b), raises an error when its pixels are read); the masks of one image have the same depth slices and time points (`mask_planes_differ`); a mask that cannot be opened, or holds several series, is `unreadable_mask`, as is an OME-Zarr image's `labels` group that cannot be read or does not list its label images (path `/labels`), so its mask types never disappear unreported. All five are errors. | | Planes | A mask belongs to the image's planes (issue #36): its Z and T lengths (1 for an axis it lacks) must equal the image's under the confirmed axis roles, checked by `run` (`mask_planes_mismatch`, error; `mask_plane_issues`; not for an image with an axis error, §2). So a 2D mask belongs only to an image with one depth slice and one time point, and is refused for a Z-stack or a time-lapse, unless its type is stated `YX` (`--mask-axes Cells=YX`): then it is one plane shared by every plane of its image (`shared: true` in the manifest), read for any `z` and `t`; a mask's plane at index `i` along Z or T belongs to the image's rows with `z = i` or `t = i`. `read_mask(..., z=, t=)` reads one plane and requires `z` and `t` for a mask with those axes. `check` and the `masks` command have no confirmed roles and do not compare with the image. | | Folder | The masks folder must exist (`masks_folder_not_found`) and be outside the experiment folder (`masks_inside_experiment`), where its folders would be read as labels; the output folder may not be inside it (`output_inside_masks`), where the run's files would be read as masks. All errors. | | Warnings | `mask_missing` (images without a mask of a type that other images have: their objects of that type cannot be measured), `mask_without_image` (files matching no image of the experiment), `mask_of_excluded_image` (masks of excluded images, not used), `mask_outside_type_folder` (files directly in the masks folder that are not TIFF masks). | diff --git a/docs/plan.md b/docs/plan.md index 8fcb443b..f5fa95a7 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -287,6 +287,9 @@ API, CLI). The notebook and CLI only call the API. *v0.4.0 (PP-8, question 49): stated calibration in the output folder's `calibration.csv` (experiment, folder or image; a size or `pixels`), recorded per axis with its source; the pixel-size checks run on the resolved sizes; CLI `calibrate` (contract §9.3).* + *v0.4.0 (PP-8b, questions 50 and 51): a mask whose header names X before Y is refused by + the run (`mask_layout`); `RunInputs.open` refuses an experiment holding images or masks + the run did not record (`UnrecordedFilesError`, contract §9.1).* - **Milestone 5 — Integration and docs:** public API, notebook, CLI, user documentation. *Implemented: summaries (`report`), notebook widgets (`notebook_ui`, `notebooks/MorphoBricks_Preprocessing.ipynb`), `check`/`run` command line (`cli`) and the @@ -1122,3 +1125,26 @@ Raised during review; to be decided before the milestones that need them: (owner, 1 November 2026). The manifest records `calibration_sha256`, checked by `RunInputs`; a changed file is an input change that recomputes no row. Y and X are stated per role and never exchanged. Contract §9.3.* +50. What happens to a mask whose own header names X before Y (PP-8b, from CellPatcher PR #69 + question 2)? + *Decided (2 November 2026, owner): the run refuses it. It is the existing `mask_layout` + error (no new code), naming the mask with its header axes and the `numpy.swapaxes` fix, + from `check` and `run` as from `preflight`. Before, `run` dropped it with the header size + comparison it supersedes (`mask_issues`) and recorded the mask with `axes: ""`, which + `RunInputs.read_mask` then called "changed or missing". A manifest never records a mask + without roles (`mask_manifest` raises), and `RunInputs.open` refuses such an older record + with its own message. Images differ: an image whose header names X before Y is read in + canonical order (design change log, 1 November 2026); masks are never reordered. + Contract §9.1, §15.* +51. How do bricks learn of images added after the run (PP-8b, from CellPatcher PR #69 + question 1)? + *Decided (2 November 2026, owner): bricks never discover files and never leave an image + out silently; preprocessing gives the check. `unrecorded_files` lists, with + preprocessing's own rules, the images discovery would analyse and the masks it would pair + with a recorded image (masks folder, OME-Zarr label images) that the run neither recorded + nor excluded; files discovery ignores or refuses do not count. `RunInputs.open` always + runs it and raises `UnrecordedFilesError` (a `RunInputsError` with the files) telling the + user to run preprocessing again or exclude them. Chosen over an explicit call because it + reads no pixel and no TIFF header (a folder walk, plus OME-Zarr label metadata), and a + check bricks must remember to call could be forgotten; there is no opt-out. Contract + §9.1.* diff --git a/docs/user_guide.md b/docs/user_guide.md index fb7f2051..7fe90bcc 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -591,6 +591,10 @@ Run the notebook or `run` again after adding, changing or removing images: - changing only the image type reads nothing again: the histograms are the same for both types, and only the manifest records the new type. +Run the brick again after adding images or masks before using a later brick: later bricks +read the images and masks this brick's run recorded, and refuse an experiment holding images +or masks the run did not record (they name them). + To leave out images, for example a damaged file, list them under *Exclude images* (one per line) or pass `--exclude`. Excluded images are not read, are listed in the messages and the manifest, and their earlier rows are removed. Remove them from the list to analyse them @@ -677,7 +681,7 @@ continues. | `unreadable_mask` | Error | A mask cannot be opened, or holds several images, or the `labels` group of an OME-Zarr image cannot be read (it must list its label images). Save the mask again as a TIFF label image, or repair the OME-Zarr labels. | | `mask_not_2d` | Error | A mask has dimensions other than Y and X that its metadata does not name as depth (Z) or time (T), for example a plain stack. (A mask one pixel high or wide is a 2D mask, not this error.) State the axes of its mask type once (`--mask-axes Cells=TYX`), or save it as a 2D label image, or with its axes named (an ImageJ hyperstack or OME-TIFF with axes TYX or ZYX). | | `invalid_mask_axes`, `mask_axes_contradict_header` | Error | Axes stated for a mask type do not fit its masks (one role, Y, X, Z or T, per axis; Y and X once each), name a type that is not found, or contradict the axes a mask's metadata names. Correct the statement. | -| `mask_layout` | Error | A mask is stored with its Y and X the other way round from its image as read (X before Y, or the image's height and width swapped). Masks are never transposed: save it again with `numpy.swapaxes(mask, -1, -2)`. | +| `mask_layout` | Error | A mask is stored with its Y and X the other way round from its image as read (its header names X before Y, for example `XY`, named in the message with the mask; or the image's height and width swapped). Masks are never transposed: save it again with `numpy.swapaxes(mask, -1, -2)`. | | `mask_planes_differ` | Error | The masks of one image have different numbers of depth slices or time points. Make every mask of an image with the image's planes. | | `mask_planes_mismatch` | Error (`run`) | A mask does not have the depth slices and time points of its image under the confirmed axis roles: a 2D mask fits only an image with one plane (not a Z-stack or time-lapse) unless its type is stated `YX` (`--mask-axes Cells=YX`: one mask shared by every plane), and a mask with Z or T needs as many slices or time points as its image. Save one mask plane per image plane, with the axes named, or check the axis roles. | | `mask_size_mismatch` | Error | A mask does not have the width and height of its image. Check that it was made from that image at full resolution. | @@ -686,6 +690,7 @@ continues. | `ambiguous_mask` | Error | A mask's name could belong to two images (one image name starts another's) or fit both layouts. Rename the masks or the images so each names one image and one kind. | | `mask_without_image`, `mask_outside_type_folder`, `mask_of_excluded_image` | Warning | Files in the masks folder that match no image of the experiment (check the folder and file names), other files directly in the masks folder, or masks of excluded images. They are not used. | | `manifest_before_v040`, `images_not_in_manifest`, `images_missing`, `images_changed`, `position_id_not_recorded`, `identity_differs`, `axis_mapping_mismatch` | Error (other bricks) | A later brick checked its images against this brick's run (`check_sources`): a run written before v0.4.0 (redo it), an image the run did not analyse, no longer there, changed since, recorded with other labels or position (or by a version that did not record positions), or read with axes the run gave other roles. Run this brick again on the experiment as it is, or use the run's labels and axis roles. | +| `UnrecordedFilesError` | Error (other bricks) | The experiment holds images or masks that the preprocessing run neither recorded nor excluded, for example added after the run. Later bricks never look for images themselves, so they refuse rather than leave them out: run this brick again on the experiment as it is, or exclude the images. Files the run ignores (non-image files, system files, masks matching no image) do not count. | | `zarr_not_installed` | Error | OME-Zarr images need the `zarr` extra. Install it, or exclude the OME-Zarr images. | ## 9. Python interface diff --git a/pyproject.toml b/pyproject.toml index da511bd1..7ae9a65d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -146,6 +146,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.narrow_ranges" = "pure" "morphobricks_preprocessing.recorded_sizes" = "pure" "morphobricks_preprocessing.run_inputs" = "io" +"morphobricks_preprocessing.unrecorded" = "io" "morphobricks_preprocessing.testing" = "orchestration" "morphobricks_preprocessing.mask_scan" = "io" "morphobricks_preprocessing.masks" = "io" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index 5e483c58..2f60756b 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -192,8 +192,9 @@ sample_planes, write_calibration, ) -from .run_inputs import Fov, LazyImage, RunInputs, RunInputsError +from .run_inputs import Fov, LazyImage, RunInputs, RunInputsError, UnrecordedFilesError from .sources import check_sources +from .unrecorded import UnrecordedFiles, unrecorded_files from .validation import Severity, ValidationIssue, has_errors from .zarr_input import ( NgffError, @@ -283,6 +284,9 @@ "RunResult", "RunInputs", "RunInputsError", + "UnrecordedFiles", + "UnrecordedFilesError", + "unrecorded_files", "Fov", "LazyImage", "RunSettings", diff --git a/src/morphobricks_preprocessing/mask_checks.py b/src/morphobricks_preprocessing/mask_checks.py index d39d8f6b..09656648 100644 --- a/src/morphobricks_preprocessing/mask_checks.py +++ b/src/morphobricks_preprocessing/mask_checks.py @@ -10,6 +10,7 @@ from __future__ import annotations from collections.abc import Iterable, Mapping +from dataclasses import replace from .axes import AxisMapping, AxisReview from .masks import MaskInventory, mask_plane_issues @@ -87,7 +88,8 @@ def mask_issues( :func:`mask_size_issues`, and the planes are checked by :func:`.masks.mask_plane_issues`. Images with an axis error are never read, so their masks are left out of both: a size or plane message would only repeat the axis error - in another form. + in another form. A mask whose own header puts X before Y has no roles and stays + refused (``mask_layout``): only the header's size comparison is replaced. Parameters ---------- @@ -106,6 +108,24 @@ def mask_issues( skip = review.axis_error_files return [ *(i for i in masks.issues if i.code not in SIZE_CODES), + *_header_refusals(masks), *mask_size_issues(masks, metadata.files, review.file_mappings, skip=skip), *mask_plane_issues(masks, metadata.files, review.file_mappings, skip=skip), ] + + +def _header_refusals(masks: MaskInventory) -> list[ValidationIssue]: + """Return the size-code issues of masks whose header was refused, not size-compared. + + :func:`.masks.find_masks` gives ``mask_layout`` both to a mask whose own header puts X + before Y (it gets no roles) and to a mask whose Y and X are its image's header's + swapped. Only the second is superseded by :func:`mask_size_issues`; the first is kept, + or the run would record the mask without axes. + """ + refused = {mask.path for mask in masks.files if not mask.axes} + kept = [] + for issue in masks.issues: + paths = tuple(path for path in issue.paths if path in refused) + if issue.code in SIZE_CODES and paths: + kept.append(replace(issue, paths=paths)) + return kept diff --git a/src/morphobricks_preprocessing/mask_record.py b/src/morphobricks_preprocessing/mask_record.py index a66b11b5..b10b6370 100644 --- a/src/morphobricks_preprocessing/mask_record.py +++ b/src/morphobricks_preprocessing/mask_record.py @@ -35,7 +35,16 @@ def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) ``roles``, ``axes_from`` and ``shared``. ``axes`` are the mask's axes as stored (its header's letters, ``"IYX"`` for a plain stack), ``roles`` what they are read as (``"TYX"`` when stated), so a brick reads the mask as stored, never reordered. + + Raises + ------ + ValueError + For a mask without roles: its header was refused, so the run has an error and + records no mask without axes. """ + unread = [mask.path for mask in inventory.files if not mask.axes] + if unread: + raise ValueError(f"Masks without roles cannot be recorded: {', '.join(unread)}") data = inventory.to_manifest() by_path = {mask.path: mask for mask in inventory.files} for entry in data["files"]: diff --git a/src/morphobricks_preprocessing/masks/files.py b/src/morphobricks_preprocessing/masks/files.py index 3ed16ac2..a9fa0602 100644 --- a/src/morphobricks_preprocessing/masks/files.py +++ b/src/morphobricks_preprocessing/masks/files.py @@ -7,7 +7,7 @@ import logging import os -from collections.abc import Iterable, Mapping +from collections.abc import Collection, Iterable, Mapping from pathlib import Path from ..discovery import DiscoveryResult, is_system_name, is_tiff_name @@ -127,7 +127,7 @@ def _folder_issues( def _folder_masks( - folder: Path, discovery: DiscoveryResult, images: Mapping[str, TiffMetadata] + folder: Path, discovery: DiscoveryResult, images: Collection[str] ) -> tuple[list[MaskFile], list[ValidationIssue]]: """Pair the TIFF files of the masks folder with the images, in either layout. @@ -137,7 +137,8 @@ def _folder_masks( an analysed image; a file matching two images or both layouts is refused, never guessed. A mask matching no image is the warning ``mask_without_image``, and a mask of an excluded image the warning ``mask_of_excluded_image``: neither is used (design v0.4 §3 keeps the - severity of every existing check). + severity of every existing check). ``images`` are the analysed images' paths (excluded + images left out). """ by_key, by_leaf = {}, {} for leaf in discovery.leaves: diff --git a/src/morphobricks_preprocessing/masks/stated.py b/src/morphobricks_preprocessing/masks/stated.py index a7a5732b..e7e9fd35 100644 --- a/src/morphobricks_preprocessing/masks/stated.py +++ b/src/morphobricks_preprocessing/masks/stated.py @@ -84,7 +84,8 @@ def mask_roles(header: str, named: str, stated: str | None) -> tuple[str, str | detail = f"header axes {header}, stated {stated}: {', '.join(contradicted)}" return "", "mask_axes_contradict_header", detail if roles.index("X") < roles.index("Y"): - return "", "mask_layout", f"axes {roles}: X is stored before Y" + given = f"header axes {header}" + ("" if stated is None else f", stated {stated}") + return "", "mask_layout", f"{given}: X is stored before Y" return roles, None, "" diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index b9260917..be712ed9 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -6,8 +6,11 @@ - :meth:`RunInputs.open` refuses a version 1 manifest (a run before the axis check), a table, design or ``calibration.csv`` whose SHA-256 is not the manifest's (also a - ``calibration.csv`` added since the run), and an image or mask changed or gone since the - run; + ``calibration.csv`` added since the run), an image or mask changed or gone since the + run, a mask recorded without axes (a v0.4.0rc1 run of a mask whose header puts X before + Y), and an image or mask in the experiment the run neither recorded nor excluded + (:class:`UnrecordedFilesError`, from :func:`.unrecorded.unrecorded_files`: folders are + listed, no pixel is read); - every read checks the file's fingerprint again (:class:`.fingerprint.Fingerprint`); - masks are read by the recorded pairing only, with preprocessing's reader and its label check (:func:`.masks.check_labels`); a stated shared 2-D mask serves every plane. @@ -43,12 +46,35 @@ from .masks.header import _mask_header from .planes import DEFAULT_MEMORY_LIMIT, read_plane from .recorded_sizes import RecordedSize +from .unrecorded import UnrecordedFiles, unrecorded_files +from .validation import plural class RunInputsError(ValueError): """The run cannot be used as it is: run preprocessing again (the message says why).""" +class UnrecordedFilesError(RunInputsError): + """The experiment holds images or masks the run neither recorded nor excluded. + + A brick never discovers files itself and never leaves them out without a word: run + preprocessing again on the experiment as it is, or exclude the images. + + Attributes + ---------- + files + The unrecorded images and masks (:class:`.unrecorded.UnrecordedFiles`). + """ + + def __init__(self, files: UnrecordedFiles) -> None: + super().__init__(_unrecorded_text(files)) + self.files = files + + def __reduce__(self) -> tuple: + """Pickle with the files, so a worker process can raise it to its parent.""" + return (type(self), (self.files,)) + + @dataclass(frozen=True) class Fov: """One position (field of view) of a clean run, as the manifest and the table record it. @@ -138,7 +164,10 @@ def open( ------ RunInputsError For a run before v0.4.0, a table, design or calibration changed since the run, - or an image or mask changed or gone. + an image or mask changed or gone, or a mask recorded without axes. + UnrecordedFilesError + For images or masks in the experiment that the run neither recorded nor + excluded (added since the run): it names them. """ output = Path(output_folder).resolve() manifest = _manifest(output) @@ -156,13 +185,7 @@ def open( manifest=manifest, records=tuple(read_plane_histograms(output / OUTPUT_FILENAME)), ) - changed = [e.relative_path for e in manifest.files if not inputs._unchanged(e)] - if changed: - raise RunInputsError(_changed_text(changed)) - recorded = (manifest.masks or {}).get("files", ()) - changed = [m["path"] for m in recorded if not inputs._mask_unchanged(m)] - if changed: - raise RunInputsError(_changed_text(changed, "mask")) + inputs._check_files() return inputs def fovs(self) -> tuple[Fov, ...]: @@ -284,6 +307,24 @@ def image_type(self) -> str | None: """Return the image type the user confirmed (``"fluorescence"`` or ``"rgb"``), or None.""" return self.manifest.image_type + def _check_files(self) -> None: + """Refuse changed or gone files, masks without axes and unrecorded files.""" + changed = [e.relative_path for e in self.manifest.files if not self._unchanged(e)] + if changed: + raise RunInputsError(_changed_text(changed)) + recorded = (self.manifest.masks or {}).get("files", ()) + changed = [m["path"] for m in recorded if not self._mask_unchanged(m)] + if changed: + raise RunInputsError(_changed_text(changed, "mask")) + without_axes = [m["path"] for m in recorded if not m.get("axes") or not m.get("roles")] + if without_axes: + raise RunInputsError(_without_axes_text(without_axes)) + unrecorded = unrecorded_files( + self.manifest, self.experiment_root, self.masks_root, self.output_folder + ) + if unrecorded: + raise UnrecordedFilesError(unrecorded) + def _fov(self, entry: FileEntry) -> Fov: """Describe one position from its entry and its rows of the snapshot.""" rows = [r for r in self.records if r.source_relative_path == entry.relative_path] @@ -403,15 +444,44 @@ def _check_calibration(path: Path, expected: str | None) -> None: def _changed_text(paths: list[str], what: str = "image") -> str: """Say which files changed or went missing since the run.""" - shown = ", ".join(sorted(paths)[:5]) + (" and more" if len(paths) > 5 else "") where = "experiment_root=" if what == "image" else "masks_root=" return ( f"{len(paths)} {what}{'' if len(paths) == 1 else 's'} changed or missing since the " - f"run: {shown}. Run preprocessing again on the experiment as it is (or, if the " + f"run: {_shown(paths)}. Run preprocessing again on the experiment as it is (or, if the " f"{what}s were only moved, give {where} where they are now)." ) +def _shown(paths: tuple[str, ...] | list[str]) -> str: + """Name up to five paths, sorted.""" + return ", ".join(sorted(paths)[:5]) + (" and more" if len(paths) > 5 else "") + + +def _without_axes_text(paths: list[str]) -> str: + """Say which masks the run recorded without axes, and what to do.""" + return ( + f"The run recorded {plural(len(paths), 'mask')} without axes: " + f"{_shown(paths)}. A run before v0.4.0 wrote such a record for a mask whose header " + "puts X before Y; masks are never transposed. Save each such mask again with its " + "last two axes swapped (numpy.swapaxes(mask, -1, -2)), then run preprocessing again." + ) + + +def _unrecorded_text(files: UnrecordedFiles) -> str: + """Name the images and masks the run did not record, and what to do.""" + parts = [ + f"{plural(len(found), what)} ({_shown(found)})" + for what, found in (("image", files.images), ("mask", files.masks)) + if found + ] + return ( + f"The experiment holds {' and '.join(parts)} that the preprocessing run did not " + "record, for example added since the run; they would be left out. Run preprocessing " + "again on the experiment as it is (morphobricks-preprocessing run --output " + "), or exclude the images (--exclude)." + ) + + def _mapping(entry: FileEntry) -> AxisMapping: """Return an image's own mapping as the run recorded it.""" return AxisMapping(entry.axes, tuple(entry.roles or ()), entry.singleton_roles) diff --git a/src/morphobricks_preprocessing/unrecorded.py b/src/morphobricks_preprocessing/unrecorded.py new file mode 100644 index 00000000..8f216bcb --- /dev/null +++ b/src/morphobricks_preprocessing/unrecorded.py @@ -0,0 +1,116 @@ +"""Images and masks in the experiment that a run did not record (design v0.4 §6, PP-8b). + +A brick reads files only through a clean run (:class:`.run_inputs.RunInputs`) and never +discovers them itself, so an image added after the run would be left out without a word. +:func:`unrecorded_files` lists them with preprocessing's own rules, without reading a pixel: + +- images: what :func:`.discovery.discover_experiment` would analyse, less the images the run + recorded and those it excluded. Files discovery ignores or refuses (non-image files, + system files, images outside a leaf folder or too deep) are not listed: a run would not + analyse them either; +- masks: the masks folder's files paired with a recorded image as + :func:`.masks.find_masks` pairs them (masks of excluded images and files that pair with + no image are left out, as the run leaves them out), and the label images of the recorded + OME-Zarr images, less the masks the run recorded. +""" + +from __future__ import annotations + +import logging +from collections.abc import Collection +from dataclasses import dataclass +from pathlib import Path + +from .discovery import DiscoveryResult, discover_experiment +from .manifest import RunManifest +from .masks import FOLDER +from .masks.files import _folder_masks +from .zarr_input import NgffError, ZarrUnavailableError, is_zarr_name, ngff_label_names + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class UnrecordedFiles: + """Files of the experiment that a run neither recorded nor excluded. + + Attributes + ---------- + images + Images, relative to the experiment root, sorted. + masks + Masks: masks-folder files relative to the masks folder, label images as + ``/labels/`` (``/labels`` for a labels group that cannot be + read), sorted. + """ + + images: tuple[str, ...] = () + masks: tuple[str, ...] = () + + def __bool__(self) -> bool: + """Return True when any file is unrecorded.""" + return bool(self.images or self.masks) + + +def unrecorded_files( + manifest: RunManifest, + experiment_root: Path, + masks_root: Path | None, + output_folder: Path | None = None, +) -> UnrecordedFiles: + """List the images and masks a run did not record and did not exclude. + + Folders are listed and OME-Zarr metadata read; no pixel and no TIFF header is read. + + Parameters + ---------- + manifest + The run's manifest. + experiment_root + Where the experiment is now. + masks_root + Where the masks folder is now, or None (only label images are then checked). + output_folder + The run's output folder, never scanned as part of the experiment. + + Returns + ------- + UnrecordedFiles + What the run did not record; empty (false) when it recorded everything. + """ + recorded = {entry.relative_path for entry in manifest.files} + discovery = discover_experiment(experiment_root, output_folder) + images = set(discovery.images) - recorded - set(manifest.settings.exclude) + recorded_masks = {m["path"] for m in (manifest.masks or {}).get("files", ())} + masks = _folder_mask_paths(masks_root, discovery, recorded) | _label_paths( + experiment_root, recorded + ) + return UnrecordedFiles(tuple(sorted(images)), tuple(sorted(masks - recorded_masks))) + + +def _folder_mask_paths( + masks_root: Path | None, discovery: DiscoveryResult, recorded: Collection[str] +) -> set[str]: + """Return the masks-folder files that pair with a recorded image, by the run's rules.""" + if masks_root is None or not masks_root.is_dir(): + return set() + found, _ = _folder_masks(masks_root, discovery, recorded) + return {mask.path for mask in found if mask.source == FOLDER} + + +def _label_paths(experiment_root: Path, recorded: Collection[str]) -> set[str]: + """Return the label images of the recorded OME-Zarr images, as their mask paths.""" + paths = set() + for image in recorded: + if not is_zarr_name(image): + continue + try: + names = ngff_label_names(experiment_root / image) + except ZarrUnavailableError as exc: # the image itself cannot be read either + logger.info("No label images listed for %s: %s", image, exc) + continue + except NgffError: + paths.add(f"{image}/labels") + continue + paths.update(f"{image}/labels/{name}" for name in names) + return paths diff --git a/tests/test_brick_checks.py b/tests/test_brick_checks.py new file mode 100644 index 00000000..f5c097b7 --- /dev/null +++ b/tests/test_brick_checks.py @@ -0,0 +1,159 @@ +"""Checks bricks rely on (PP-8b): X-before-Y masks refused, unrecorded files named.""" + +import json +import pickle + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + MANIFEST_FILENAME, + RunInputs, + RunInputsError, + UnrecordedFilesError, + find_masks, + prefill_mapping, + run_experiment, +) +from morphobricks_preprocessing.discovery import discover_experiment +from morphobricks_preprocessing.mask_record import mask_manifest +from morphobricks_preprocessing.metadata import inspect_experiment +from morphobricks_preprocessing.testing import make_clean_run +from tiff_tree import write_data, write_file +from zarr_tree import write_labels, write_ngff + + +def _labels(shape): + labels = np.zeros(shape, np.uint16) + labels[0, 0] = 1 + return labels + + +def _codes(issues): + return sorted({issue.code for issue in issues}) + + +# 1. A mask whose header puts X before Y ---------------------------------------------- + + +@pytest.mark.parametrize(("axes", "shape"), [("XY", (8, 6)), ("TXY", (3, 8, 6))]) +def test_a_run_refuses_a_mask_whose_header_puts_x_before_y(tmp_path, axes, shape): + # rc1 wrote this run, recording the mask with axes "" (CellPatcher PR 69, question 2). + root = tmp_path / "exp" + image_shape = (6, 8) if len(shape) == 2 else (3, 6, 8) + write_data(root / "ctrl/a.tif", np.zeros(image_shape, np.uint8), axes[:-2] + "YX") + write_data(tmp_path / "masks/Cells/ctrl/a.tif", _labels(shape), axes) + result = run_experiment( + root, prefill_mapping(axes[:-2] + "YX"), masks_folder=tmp_path / "masks" + ) + assert not result.written + assert not (result.output_folder / MANIFEST_FILENAME).exists() + (issue,) = [i for i in result.issues if i.code == "mask_layout"] + assert issue.paths == ("Cells/ctrl/a.tif",) + assert f"Cells/ctrl/a.tif: header axes {axes}: X is stored before Y" in issue.message + assert "numpy.swapaxes(mask, -1, -2)" in issue.action + + +def test_a_mask_without_roles_is_never_recorded(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", np.zeros((6, 8), np.uint8), "YX") + write_data(tmp_path / "masks/Cells/ctrl/a.tif", _labels((8, 6)), "XY") + discovery = discover_experiment(root) + inventory = find_masks(tmp_path / "masks", discovery, inspect_experiment(discovery).files) + assert inventory.files[0].axes == "" + with pytest.raises(ValueError, match="Masks without roles cannot be recorded"): + mask_manifest(inventory, root) + + +def test_run_inputs_refuses_an_older_mask_record_without_axes(tmp_path): + clean = make_clean_run(tmp_path, mask_types=("Cells",)) + path = clean.output / MANIFEST_FILENAME + manifest = json.loads(path.read_text()) + manifest["masks"]["files"][0].update(axes="", shape=[], roles="") + path.write_text(json.dumps(manifest)) + with pytest.raises(RunInputsError) as raised: + RunInputs.open(clean.output) + message = str(raised.value) + assert "without axes: Cells/ctrl/rep1/a.tif" in message + assert "numpy.swapaxes" in message and "changed or missing" not in message + + +# 2. Files the run did not record ----------------------------------------------------- + + +@pytest.fixture +def clean(tmp_path): + return make_clean_run(tmp_path, mask_types=("Cells",)) + + +def test_an_image_added_after_the_run_is_named(clean): + write_data(clean.experiment / "drug/rep1/c.tif", np.zeros((2, 16, 20), np.uint16), "CYX") + with pytest.raises(UnrecordedFilesError) as raised: + RunInputs.open(clean.output) + assert raised.value.files.images == ("drug/rep1/c.tif",) + assert raised.value.files.masks == () + message = str(raised.value) + assert "1 image (drug/rep1/c.tif)" in message + assert "Run preprocessing again" in message and "--exclude" in message + assert isinstance(raised.value, RunInputsError) + again = pickle.loads(pickle.dumps(raised.value)) + assert again.files == raised.value.files and str(again) == message + + +def test_an_image_in_a_moved_experiment_is_named(clean, tmp_path): + moved = tmp_path / "moved" + clean.experiment.rename(moved) + write_data(moved / "ctrl/rep1/new.ome.tif", np.zeros((2, 16, 20), np.uint16), "CYX") + with pytest.raises(UnrecordedFilesError, match="ctrl/rep1/new.ome.tif"): + RunInputs.open(clean.output, experiment_root=moved) + + +def test_files_discovery_ignores_or_refuses_are_not_named(clean): + write_file(clean.experiment / "ctrl/rep1/notes.txt") + write_file(clean.experiment / "ctrl/rep1/a.png") + write_file(clean.experiment / "ctrl/rep1/.DS_Store") + write_file(clean.experiment / "ctrl/rep1/Thumbs.db") + write_file(clean.experiment / "ctrl/rep1/._a.tif") + # A TIFF beside the condition folders is outside every leaf folder: discovery refuses + # it (tiff_outside_leaf), so a run would not analyse it either. + write_data(clean.experiment / "stray.tif", np.zeros((4, 4), np.uint16), "YX") + write_file(clean.masks / "Cells/ctrl/rep1/zz.tif") # pairs with no image + write_file(clean.masks / "readme.txt") + inputs = RunInputs.open(clean.output) + assert len(inputs.fovs()) == 4 + + +def test_excluded_images_are_not_named(tmp_path): + root = tmp_path / "exp" + for name in ("a", "b"): + write_data(root / f"ctrl/{name}.tif", np.arange(48, dtype=np.uint8).reshape(6, 8), "YX") + result = run_experiment(root, prefill_mapping("YX"), exclude=["ctrl/b.tif"]) + assert result.written, result.issues + assert [f.relative_path for f in RunInputs.open(result.output_folder).fovs()] == ["ctrl/a.tif"] + write_data(root / "ctrl/c.tif", np.zeros((6, 8), np.uint8), "YX") + with pytest.raises(UnrecordedFilesError) as raised: + RunInputs.open(result.output_folder) + assert raised.value.files.images == ("ctrl/c.tif",) + + +def test_a_mask_added_after_the_run_is_named(clean): + write_data(clean.masks / "Nuclei/drug/rep1/b.tif", _labels((16, 20)), "YX") + with pytest.raises(UnrecordedFilesError) as raised: + RunInputs.open(clean.output) + assert raised.value.files == type(raised.value.files)((), ("Nuclei/drug/rep1/b.tif",)) + assert "1 mask (Nuclei/drug/rep1/b.tif)" in str(raised.value) + + +def test_a_label_image_added_after_the_run_is_named(tmp_path): + pytest.importorskip("zarr") + image = np.arange(40, dtype=np.uint8).reshape(2, 4, 5) + path = write_ngff(tmp_path / "exp/ctrl/a.ome.zarr", image, "zyx") + write_labels(path, {"cells": _labels((2, 4, 5))}, "zyx") + result = run_experiment(tmp_path / "exp", prefill_mapping("ZYX")) + assert result.written, result.issues + RunInputs.open(result.output_folder) + # Written as the only listed label image: the recorded cells label image is unchanged. + write_labels(path, {"nuclei": _labels((2, 4, 5))}, "zyx") + with pytest.raises(UnrecordedFilesError) as raised: + RunInputs.open(result.output_folder) + assert raised.value.files.masks == ("ctrl/a.ome.zarr/labels/nuclei",) From c37d58f26de7c960779e6dd657d7022b75fc73b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:48:21 +0000 Subject: [PATCH 2/2] PP-8b review: OME-Zarr label images are never skipped; masks message asks for --masks Owner decisions on the review questions (2 November 2026): - Without the zarr extra, RunInputs.open refuses a run holding OME-Zarr images, naming morphobricks-preprocessing[zarr], instead of skipping their label images in the unrecorded-files check. - A recorded OME-Zarr image whose labels group cannot be read is its own refusal naming the image, no longer listed as an unrecorded "/labels" mask. - Unrecorded masks (also a masks_root given to a run that recorded no masks folder): the message says to run preprocessing again with the masks folder (--masks); --exclude is offered only for images. - Contract 9.1, user guide, plan question 51 record the decisions. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 8 ++- docs/plan.md | 10 ++++ docs/user_guide.md | 2 +- src/morphobricks_preprocessing/run_inputs.py | 49 ++++++++++++++--- src/morphobricks_preprocessing/unrecorded.py | 38 ++++++++------ tests/test_brick_checks.py | 55 ++++++++++++++++++++ 6 files changed, 137 insertions(+), 25 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index a7f93368..edb8929a 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -201,8 +201,14 @@ and system files, images outside a leaf folder or below the technical repeat, ma pair with no image or belong to an excluded image) do not count. `open` always runs this check: it lists folders and reads OME-Zarr label metadata, never a pixel or a TIFF header, so a brick never leaves out an image added since the run and never discovers files itself. +Label images are never skipped (owner, 2 November 2026): a run holding OME-Zarr images is +refused (`RunInputsError`) without the `zarr` extra, naming `morphobricks-preprocessing[zarr]`, +and so is a recorded OME-Zarr image whose `labels` group cannot be read (the message names +it). A `masks_root` given for a run that recorded no masks folder makes every mask there +paired with a recorded image unrecorded; the message then says to run preprocessing again +with the masks folder (`--masks`). `unrecorded_files(manifest, experiment_root, masks_root, output_folder=None)` gives the same -list without raising. +list without raising (it raises `ZarrUnavailableError` or `NgffError` for those two cases). It gives: - `fovs()`: `Fov` records with the FOV key, `plane_id`s, axes, roles, `axes_from`, channel diff --git a/docs/plan.md b/docs/plan.md index f5fa95a7..127d7de8 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1148,3 +1148,13 @@ Raised during review; to be decided before the milestones that need them: reads no pixel and no TIFF header (a folder walk, plus OME-Zarr label metadata), and a check bricks must remember to call could be forgotten; there is no opt-out. Contract §9.1.* + *Also decided (2 November 2026, owner, on the PP-8b review questions): (1) a run that + v0.4.0rc1 recorded with a mask of empty axes is refused as a whole by `RunInputs.open`, + not only at `read_mask`; (2) a `masks_root` given for a run that recorded no masks + folder is refused: every mask there paired with a recorded image is unrecorded, and the + message says to run preprocessing again with the masks folder (`--masks`); (3) OME-Zarr + label images are never skipped: without the `zarr` extra, `RunInputs.open` refuses a run + holding OME-Zarr images, naming `morphobricks-preprocessing[zarr]`, and a recorded + OME-Zarr image whose `labels` group cannot be read is its own refusal naming the image, + not a silent pass and not an "unrecorded" file; (4) the version stays 0.3.4 until PP-13, + and v0.4.0rc2 is tagged by the owner after this PR.* diff --git a/docs/user_guide.md b/docs/user_guide.md index 7fe90bcc..ccc22c80 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -690,7 +690,7 @@ continues. | `ambiguous_mask` | Error | A mask's name could belong to two images (one image name starts another's) or fit both layouts. Rename the masks or the images so each names one image and one kind. | | `mask_without_image`, `mask_outside_type_folder`, `mask_of_excluded_image` | Warning | Files in the masks folder that match no image of the experiment (check the folder and file names), other files directly in the masks folder, or masks of excluded images. They are not used. | | `manifest_before_v040`, `images_not_in_manifest`, `images_missing`, `images_changed`, `position_id_not_recorded`, `identity_differs`, `axis_mapping_mismatch` | Error (other bricks) | A later brick checked its images against this brick's run (`check_sources`): a run written before v0.4.0 (redo it), an image the run did not analyse, no longer there, changed since, recorded with other labels or position (or by a version that did not record positions), or read with axes the run gave other roles. Run this brick again on the experiment as it is, or use the run's labels and axis roles. | -| `UnrecordedFilesError` | Error (other bricks) | The experiment holds images or masks that the preprocessing run neither recorded nor excluded, for example added after the run. Later bricks never look for images themselves, so they refuse rather than leave them out: run this brick again on the experiment as it is, or exclude the images. Files the run ignores (non-image files, system files, masks matching no image) do not count. | +| `UnrecordedFilesError` | Error (other bricks) | The experiment holds images or masks that the preprocessing run neither recorded nor excluded, for example added after the run. Later bricks never look for images themselves, so they refuse rather than leave them out: run this brick again on the experiment as it is (with its masks folder, `--masks`, for masks), or exclude the images. Files the run ignores (non-image files, system files, masks matching no image) do not count. A run holding OME-Zarr images also needs the `zarr` extra here (`pip install "morphobricks-preprocessing[zarr]"`), so that their label images can be checked; an OME-Zarr image whose `labels` group cannot be read is refused too. | | `zarr_not_installed` | Error | OME-Zarr images need the `zarr` extra. Install it, or exclude the OME-Zarr images. | ## 9. Python interface diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index be712ed9..85fafc20 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -10,7 +10,9 @@ run, a mask recorded without axes (a v0.4.0rc1 run of a mask whose header puts X before Y), and an image or mask in the experiment the run neither recorded nor excluded (:class:`UnrecordedFilesError`, from :func:`.unrecorded.unrecorded_files`: folders are - listed, no pixel is read); + listed, no pixel is read). OME-Zarr label images are never skipped: a run holding + OME-Zarr images is refused without the ``zarr`` extra, and so is a recorded image whose + ``labels`` group cannot be read; - every read checks the file's fingerprint again (:class:`.fingerprint.Fingerprint`); - masks are read by the recorded pairing only, with preprocessing's reader and its label check (:func:`.masks.check_labels`); a stated shared 2-D mask serves every plane. @@ -48,6 +50,7 @@ from .recorded_sizes import RecordedSize from .unrecorded import UnrecordedFiles, unrecorded_files from .validation import plural +from .zarr_input import NgffError, ZarrUnavailableError class RunInputsError(ValueError): @@ -164,7 +167,9 @@ def open( ------ RunInputsError For a run before v0.4.0, a table, design or calibration changed since the run, - an image or mask changed or gone, or a mask recorded without axes. + an image or mask changed or gone, a mask recorded without axes, OME-Zarr images + without the ``zarr`` extra (``morphobricks-preprocessing[zarr]``), or an OME-Zarr + image whose ``labels`` group cannot be read. UnrecordedFilesError For images or masks in the experiment that the run neither recorded nor excluded (added since the run): it names them. @@ -308,7 +313,7 @@ def image_type(self) -> str | None: return self.manifest.image_type def _check_files(self) -> None: - """Refuse changed or gone files, masks without axes and unrecorded files.""" + """Refuse changed or gone files, masks without axes, unchecked labels, unrecorded files.""" changed = [e.relative_path for e in self.manifest.files if not self._unchanged(e)] if changed: raise RunInputsError(_changed_text(changed)) @@ -319,9 +324,14 @@ def _check_files(self) -> None: without_axes = [m["path"] for m in recorded if not m.get("axes") or not m.get("roles")] if without_axes: raise RunInputsError(_without_axes_text(without_axes)) - unrecorded = unrecorded_files( - self.manifest, self.experiment_root, self.masks_root, self.output_folder - ) + try: + unrecorded = unrecorded_files( + self.manifest, self.experiment_root, self.masks_root, self.output_folder + ) + except ZarrUnavailableError as exc: + raise RunInputsError(_ZARR_MISSING) from exc + except NgffError as exc: + raise RunInputsError(_labels_unreadable_text(exc)) from exc if unrecorded: raise UnrecordedFilesError(unrecorded) @@ -474,11 +484,34 @@ def _unrecorded_text(files: UnrecordedFiles) -> str: for what, found in (("image", files.images), ("mask", files.masks)) if found ] + masks = ( + " with its masks folder (--masks ; the label images of OME-Zarr images " + "are added)" + if files.masks + else "" + ) + exclude = ", or exclude the images (--exclude)" if files.images else "" return ( f"The experiment holds {' and '.join(parts)} that the preprocessing run did not " "record, for example added since the run; they would be left out. Run preprocessing " - "again on the experiment as it is (morphobricks-preprocessing run --output " - "), or exclude the images (--exclude)." + f"again on the experiment as it is{masks} (morphobricks-preprocessing run --output " + f"){exclude}." + ) + + +_ZARR_MISSING = ( + "The run holds OME-Zarr images, and the zarr package is not installed, so their label " + "images cannot be checked against the run (none is ever skipped). Install it with: " + 'pip install "morphobricks-preprocessing[zarr]"' +) + + +def _labels_unreadable_text(exc: NgffError) -> str: + """Say which OME-Zarr image's labels group cannot be read, and what to do.""" + return ( + f"The run cannot be checked: {exc}. Its label images may have changed since the " + "run; check the image's labels group, then run preprocessing again on the " + "experiment as it is." ) diff --git a/src/morphobricks_preprocessing/unrecorded.py b/src/morphobricks_preprocessing/unrecorded.py index 8f216bcb..5ca98119 100644 --- a/src/morphobricks_preprocessing/unrecorded.py +++ b/src/morphobricks_preprocessing/unrecorded.py @@ -11,12 +11,13 @@ - masks: the masks folder's files paired with a recorded image as :func:`.masks.find_masks` pairs them (masks of excluded images and files that pair with no image are left out, as the run leaves them out), and the label images of the recorded - OME-Zarr images, less the masks the run recorded. + OME-Zarr images, less the masks the run recorded. Label images are never skipped: without + the ``zarr`` extra, or with a ``labels`` group that cannot be read, the check raises + instead of passing (owner, 2 November 2026). """ from __future__ import annotations -import logging from collections.abc import Collection from dataclasses import dataclass from pathlib import Path @@ -25,9 +26,7 @@ from .manifest import RunManifest from .masks import FOLDER from .masks.files import _folder_masks -from .zarr_input import NgffError, ZarrUnavailableError, is_zarr_name, ngff_label_names - -logger = logging.getLogger(__name__) +from .zarr_input import NgffError, is_zarr_name, ngff_label_names @dataclass(frozen=True) @@ -40,8 +39,7 @@ class UnrecordedFiles: Images, relative to the experiment root, sorted. masks Masks: masks-folder files relative to the masks folder, label images as - ``/labels/`` (``/labels`` for a labels group that cannot be - read), sorted. + ``/labels/``, sorted. """ images: tuple[str, ...] = () @@ -77,6 +75,15 @@ def unrecorded_files( ------- UnrecordedFiles What the run did not record; empty (false) when it recorded everything. + + Raises + ------ + ZarrUnavailableError + When the run recorded OME-Zarr images and the ``zarr`` extra is not installed: + their label images cannot be listed, and are never skipped. + NgffError + When the ``labels`` group of a recorded OME-Zarr image cannot be read (the message + names the image). """ recorded = {entry.relative_path for entry in manifest.files} discovery = discover_experiment(experiment_root, output_folder) @@ -99,18 +106,19 @@ def _folder_mask_paths( def _label_paths(experiment_root: Path, recorded: Collection[str]) -> set[str]: - """Return the label images of the recorded OME-Zarr images, as their mask paths.""" + """Return the label images of the recorded OME-Zarr images, as their mask paths. + + Without the ``zarr`` extra, :class:`.zarr_input.ZarrUnavailableError` is raised, and an + unreadable ``labels`` group is an :class:`.zarr_input.NgffError` naming its image: + neither is skipped, or a label image added since the run would pass unseen. + """ paths = set() - for image in recorded: + for image in sorted(recorded): if not is_zarr_name(image): continue try: names = ngff_label_names(experiment_root / image) - except ZarrUnavailableError as exc: # the image itself cannot be read either - logger.info("No label images listed for %s: %s", image, exc) - continue - except NgffError: - paths.add(f"{image}/labels") - continue + except NgffError as exc: + raise NgffError(f"the label images of {image} cannot be read: {exc}") from exc paths.update(f"{image}/labels/{name}" for name in names) return paths diff --git a/tests/test_brick_checks.py b/tests/test_brick_checks.py index f5c097b7..dacd7285 100644 --- a/tests/test_brick_checks.py +++ b/tests/test_brick_checks.py @@ -2,6 +2,7 @@ import json import pickle +import sys import numpy as np import pytest @@ -157,3 +158,57 @@ def test_a_label_image_added_after_the_run_is_named(tmp_path): with pytest.raises(UnrecordedFilesError) as raised: RunInputs.open(result.output_folder) assert raised.value.files.masks == ("ctrl/a.ome.zarr/labels/nuclei",) + + +# 3. Label images are never skipped (owner, 2 November 2026) --------------------------- + + +@pytest.fixture +def labelled(tmp_path): + pytest.importorskip("zarr") + image = np.arange(40, dtype=np.uint8).reshape(2, 4, 5) + path = write_ngff(tmp_path / "exp/ctrl/a.ome.zarr", image, "zyx") + write_labels(path, {"cells": _labels((2, 4, 5))}, "zyx") + result = run_experiment(tmp_path / "exp", prefill_mapping("ZYX")) + assert result.written, result.issues + RunInputs.open(result.output_folder) + return result.output_folder, path + + +def test_ome_zarr_images_without_the_zarr_extra_are_refused(labelled, monkeypatch): + output, _ = labelled + monkeypatch.setitem(sys.modules, "zarr", None) # import zarr raises ImportError + with pytest.raises(RunInputsError) as raised: + RunInputs.open(output) + assert not isinstance(raised.value, UnrecordedFilesError) + assert "morphobricks-preprocessing[zarr]" in str(raised.value) + assert "label images cannot be checked" in str(raised.value) + + +def test_a_labels_group_that_cannot_be_read_is_refused(labelled): + output, path = labelled + attrs = path / "labels" / ".zattrs" + if not attrs.exists(): # Zarr format 3 keeps the attributes in zarr.json + attrs = path / "labels" / "zarr.json" + meta = json.loads(attrs.read_text()) + meta["attributes"] = {"labels": "cells"} + attrs.write_text(json.dumps(meta)) + else: + attrs.write_text(json.dumps({"labels": "cells"})) + with pytest.raises(RunInputsError) as raised: + RunInputs.open(output) + message = str(raised.value) + assert not isinstance(raised.value, UnrecordedFilesError) + assert "the label images of ctrl/a.ome.zarr cannot be read" in message + assert "run preprocessing again" in message + + +def test_masks_given_to_a_run_without_masks_ask_for_a_run_with_them(tmp_path): + clean = make_clean_run(tmp_path) + write_data(tmp_path / "masks/Cells/ctrl/rep1/a.tif", _labels((16, 20)), "YX") + with pytest.raises(UnrecordedFilesError) as raised: + RunInputs.open(clean.output, masks_root=tmp_path / "masks") + assert raised.value.files.masks == ("Cells/ctrl/rep1/a.tif",) + message = str(raised.value) + assert "with its masks folder (--masks " in message + assert "--exclude" not in message