From e0e6c2098b20b70ee6cd764865fbab3bb36664aa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:52:16 +0000 Subject: [PATCH 1/3] Check OME-Zarr axes and calibration: nothing guessed, nothing silent (#72, #73, #94) - #72: every OME-Zarr image is matched by the roles its OME-NGFF axes declare, also when its axis letters are the mapping's (axes.ome_zarr_mapping, used by review_axes and file_mapping_for). NGFF ZYX confirmed TYX or ZXY, and QYX confirmed CYX, are ome_zarr_axes_mismatch, naming each image with its declared axes and confirmed roles; the image is never read under them. - #73: missing axes are never guessed. Only OME-NGFF 0.1/0.2 (whose spec fixes 5-D t,c,z,y,x) keep TCZYX, by the stated version; 0.3-0.5 without axes, and a store without axes stating no or another version, are unreadable_ome_zarr. - #94: a malformed coordinateTransformations calibration (ngff_scale.read_scale) gives unknown sizes with the reason in PhysicalSizes.invalid and the warning invalid_physical_calibration naming every image; the blanket except with a debug log is gone. Contract, user guide, plan (question 60) and AGENTS.md updated; the code-rules baseline shrinks (review_axes, physical_sizes.py, test_zarr_input.py, ANN noqa). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 19 +- docs/contract.md | 11 +- docs/plan.md | 29 ++ docs/user_guide.md | 10 +- pyproject.toml | 1 + src/morphobricks_preprocessing/axes.py | 46 ++- src/morphobricks_preprocessing/axis_check.py | 6 +- src/morphobricks_preprocessing/axis_review.py | 149 ++++++--- src/morphobricks_preprocessing/dataset.py | 1 + src/morphobricks_preprocessing/ngff_scale.py | 113 +++++++ .../physical_sizes.py | 76 ++--- .../pixel_size_checks.py | 55 +++- src/morphobricks_preprocessing/zarr_input.py | 83 +++-- tests/test_ngff_axes_calibration.py | 284 ++++++++++++++++++ tests/test_zarr_input.py | 17 -- tools/code_rules_baseline.json | 8 +- 16 files changed, 737 insertions(+), 171 deletions(-) create mode 100644 src/morphobricks_preprocessing/ngff_scale.py create mode 100644 tests/test_ngff_axes_calibration.py diff --git a/AGENTS.md b/AGENTS.md index 2fae5804..5e5cfe0e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,6 +72,8 @@ src/morphobricks_preprocessing/ for comparing), space_unit/to_micrometers (the shared unit rule, contract §16), read_physical_sizes (OME / ImageJ TIFF metadata; also importable ) and ngff_physical_sizes (OME-NGFF scale and axis units) + ngff_scale.py read_scale: an OME-NGFF coordinateTransformations scale, checked; what + is wrong with a malformed one (PhysicalSizes.invalid, issue #94) dataset.py dataset consistency from the folders: biological repeats per condition, balance table, unbalanced_images, repeat_missing_conditions, CONSISTENCY_CODES (P2) design.py experiment design (P3): ExperimentDesign/DesignRow, default pairing, reconcile with @@ -300,7 +302,7 @@ functions the tests use. "confirmed"`). `AxisReview.axis_error_files` lists them; mask sizes and planes are checked by the confirmed roles and skip those images (`run.mask_issues`). OME-Zarr images never have - these errors. + these errors: their roles are always those their OME-NGFF axes declare (next bullets). - **Position-like axes** (`R` tile, `M` mosaic, `J` column, `K` row) cannot get a role. If they hold more than one plane: `unsupported_axis` error. - **Missing axes:** a file lacking some of the mapping's axes (e.g. `ZYX` next to `ZCYX`) is @@ -320,10 +322,17 @@ functions the tests use. (`axes._SUPERSEDED_METADATA_CODES`). Shared builder `metadata.channel_count_issues`. - **OME-Zarr axes:** translated from OME-NGFF metadata (`zarr_input.ngff_axes`: time -> T, channel -> C, space z/y/x -> Z/Y/X, anything else -> Q). The experiment mapping is - defined on the TIFF arrangement if there are TIFFs; OME-Zarr images with another order - are matched by role (`axes.declared_mapping`, `file_mapping_for`); a misfit is - `ome_zarr_axes_mismatch`. Use `file_mapping_for`, never `mapping.for_axes`, to get an - image's mapping. + defined on the TIFF arrangement if there are TIFFs; every OME-Zarr image, also one with + the mapping's letters, is matched by role (`axes.ome_zarr_mapping`, `declared_mapping`, + `file_mapping_for`; issue #72): a misfit, or confirmed roles that contradict the declared + axes (`ZYX` confirmed `TYX`), is `ome_zarr_axes_mismatch`. Use `file_mapping_for`, never + `mapping.for_axes`, to get an image's mapping. Missing axes are never guessed (issue + #73): only OME-NGFF 0.1/0.2 fix them (`t, c, z, y, x`, 5-D); any other store without + axes is `unreadable_ome_zarr`. +- **OME-Zarr calibration (issue #94):** a malformed `coordinateTransformations` + (`ngff_scale.read_scale`) gives unknown sizes with the reason in + `PhysicalSizes.invalid`, named by the warning `invalid_physical_calibration`; never a + silent fallback, never a substituted size. ### Pixel types and binning (normalization contract §1.2.1–§1.2.2) diff --git a/docs/contract.md b/docs/contract.md index 6284a5c4..70b86308 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -45,18 +45,18 @@ channel order; an image without a C axis has one channel. | Rule | Detail | |---|---| | Prefill | Axis labels from metadata: `X`, `Y`, `Z`, `C`, `T` keep their role; the RGB samples axis `S` becomes C when there is no other `C`. Every other label is left unassigned for the user to choose. | -| Axis check (v0.4.0) | Every TIFF's confirmed roles are compared with the axes its own header names (`named_axes`, §16), position by position (`header_axis_issue`, `axis_disagreements`): a named axis keeps its letter as role, except that the RGB samples `S` may be C when the header has no C, and a length-1 axis may have no role. Confirmed roles that give the named Y and X the other way round are the **error** `axes_yx_swapped`; any other role for a named axis (`ZYX` confirmed `TYX`, a named angle or lambda axis given a role) is the **error** `axes_contradict_header` (a named axis longer than 1 without a role is `unassigned_axis`, a position axis `unsupported_axis`). Roles confirmed for a plain stack that put X before Y, such as `TXY`, are the **error** `axes_x_before_y` (owner decision of 1 November 2026): nothing is ever read transposed, so the images are saved again with Y before X, or as OME-TIFF copies with Y before X. Each error names every file with its header axes and confirmed roles and the likely cause (`likely_axis_cause`: tifffile labels a 3-D stack saved with `imagej=True` or `ome=True` `CYX`; an ImageJ stack of slices reads as Z; rows and columns exchanged), and offers two fixes: correct the axes, or save named-axis copies and run preprocessing on them. Such an image is never read: not transposed, not under a contradicted declaration. An axis the metadata does not name (a plain stack `IYX`, `QYX`, `QQYX`, or shaped metadata with a placeholder `QYX`; a plain 2D `YX` page and RGB `YXS` are named, decision D-d) takes the role the user confirmed: the image is read with it, with the **warning** `axes_not_named` recommending named copies before segmenting in other tools, and recorded as `axes_from: "confirmed"` (`AxisReview.axes_from`; `"header"` for every other image). A confirmation covers only the files it was given for, as they were then (`axis_check.ConfirmedAxes`, made by `fingerprint.confirm_axes` with each file's fingerprint; audit 2 B-1): a plain stack outside it, new or changed since (another fingerprint), is the **error** `plain_stack_not_confirmed` and must be confirmed again; it is never read with roles confirmed for other images. An `AxisMapping` given to `run_experiment` confirms every file the run finds. OME-Zarr images take their roles from their OME-NGFF axes and never have these issues. Since one mapping covers each TIFF arrangement, all files of one arrangement pass or fail together. The notebook shows these codes as one card; when the roles agree with every header, the notebook and `run` say so ("Axes checked"). Before v0.4.0 a swap was read transposed and a contradiction under the declaration, silently. | +| Axis check (v0.4.0) | Every TIFF's confirmed roles are compared with the axes its own header names (`named_axes`, §16), position by position (`header_axis_issue`, `axis_disagreements`): a named axis keeps its letter as role, except that the RGB samples `S` may be C when the header has no C, and a length-1 axis may have no role. Confirmed roles that give the named Y and X the other way round are the **error** `axes_yx_swapped`; any other role for a named axis (`ZYX` confirmed `TYX`, a named angle or lambda axis given a role) is the **error** `axes_contradict_header` (a named axis longer than 1 without a role is `unassigned_axis`, a position axis `unsupported_axis`). Roles confirmed for a plain stack that put X before Y, such as `TXY`, are the **error** `axes_x_before_y` (owner decision of 1 November 2026): nothing is ever read transposed, so the images are saved again with Y before X, or as OME-TIFF copies with Y before X. Each error names every file with its header axes and confirmed roles and the likely cause (`likely_axis_cause`: tifffile labels a 3-D stack saved with `imagej=True` or `ome=True` `CYX`; an ImageJ stack of slices reads as Z; rows and columns exchanged), and offers two fixes: correct the axes, or save named-axis copies and run preprocessing on them. Such an image is never read: not transposed, not under a contradicted declaration. An axis the metadata does not name (a plain stack `IYX`, `QYX`, `QQYX`, or shaped metadata with a placeholder `QYX`; a plain 2D `YX` page and RGB `YXS` are named, decision D-d) takes the role the user confirmed: the image is read with it, with the **warning** `axes_not_named` recommending named copies before segmenting in other tools, and recorded as `axes_from: "confirmed"` (`AxisReview.axes_from`; `"header"` for every other image). A confirmation covers only the files it was given for, as they were then (`axis_check.ConfirmedAxes`, made by `fingerprint.confirm_axes` with each file's fingerprint; audit 2 B-1): a plain stack outside it, new or changed since (another fingerprint), is the **error** `plain_stack_not_confirmed` and must be confirmed again; it is never read with roles confirmed for other images. An `AxisMapping` given to `run_experiment` confirms every file the run finds. OME-Zarr images take their roles from their OME-NGFF axes and never have these issues: confirmed roles that contradict those axes are `ome_zarr_axes_mismatch`, whatever the arrangement ("OME-Zarr axes" below, issue #72). Since one mapping covers each TIFF arrangement, all files of one arrangement pass or fail together. The notebook shows these codes as one card; when the roles agree with every header, the notebook and `run` say so ("Axes checked"). Before v0.4.0 a swap was read transposed and a contradiction under the declaration, silently. | | Unsupported axes | Position-like axes (tile `R`, mosaic `M`, row `K`, column `J`) cannot have a role. If they hold more than one plane, the file is an error. | | Singleton axes | An axis of length 1 needs no role; its index is always 0. | | Missing axes | A file lacking axes the mapping has is read with those axes as length 1 only when the user confirmed this and only one arrangement fits; its coordinates for them are 0. | | Notebook readiness | Confirming a valid singleton mapping resolves its missing-axis error for running; folder, exclusion and all other metadata errors still block processing. | | Series | Only the first image series, at full-resolution level 0. A file with more than one series is an error. | -| OME-Zarr axes | Taken from the OME-NGFF metadata (0.4 and 0.5; 0.1–0.3 fixed `t, c, z, y, x` or axis names): a time axis is T, a channel axis C, space axes named z, y, x are Z, Y, X; any other axis has no role. An OME-Zarr image with the experiment mapping's arrangement uses the mapping; others are matched by role: a Z, C or T axis the experiment lacks is allowed only with length 1, an axis without a role only with length 1, and experiment roles the image lacks follow the missing-axes rule. Otherwise `ome_zarr_axes_mismatch` is an error. | +| OME-Zarr axes | Taken from the OME-NGFF metadata (0.4 and 0.5; 0.3 axis names): a time axis is T, a channel axis C, space axes named z, y, x are Z, Y, X; any other axis has no role. **Nothing is guessed** (issue #73): without `axes`, only OME-NGFF 0.1 and 0.2 have axes, because their specification fixes every image array as 5-D `t, c, z, y, x` (by the version the metadata states; a 0.1 or 0.2 array of another dimensionality is refused). A store of OME-NGFF 0.3, 0.4 or 0.5 without `axes` (which those versions require), and a store without `axes` that states no version or another one, is unreadable (`unreadable_ome_zarr`, naming what is missing); so are `axes` that are not a list. **Every OME-Zarr image is matched by the roles it declares** (issue #72), whatever its arrangement: with the experiment mapping's arrangement, a confirmed role that is not the axis's declared letter (`ZYX` confirmed `TYX` or `ZXY`; an axis of no known type, `QYX`, confirmed `CYX`) is the error `ome_zarr_axes_mismatch`, naming each image with its declared axes and confirmed roles, and the image is never read under them (`axes.ome_zarr_mapping`, also used by `file_mapping_for`). Images in another order are matched by role: a Z, C or T axis the experiment lacks is allowed only with length 1, an axis without a role only with length 1, and experiment roles the image lacks follow the missing-axes rule; otherwise `ome_zarr_axes_mismatch` is an error. An OME-Zarr image cannot be converted (§17), so these are not axis-check errors. | | OME-Zarr images | Only level 0 (the first dataset of the first multiscales image) is read. A store with more than one image (several multiscales, a bioformats2raw folder with several series, a plate, a well with several fields) is an error (`multiple_series`). | | OME-Zarr labels | The label images of an image's OME-NGFF `labels` group (0.4 and 0.5) are listed with `ngff_label_names` and opened, level 0, with `open_ngff_labels` (by name; the name may be left out when there is one). They are never analysed; later bricks such as patch extraction use them as masks. | | Experiment mapping | Defined on the TIFF arrangement when the experiment has TIFF files; in an experiment of OME-Zarr images only, on their arrangement with the most axes. | | Channel names, significant bits | Recorded in the run manifest when metadata provides them; not part of the CSV. Channel names that differ at the same channel index are the error `channel_names_differ` (issue #37): channels are matched by their number, so two stains at one number would be pooled as one channel. Only named channels are compared; a channel without a name in some images is not. Names are read without leading and trailing whitespace, for every image (TIFF, OME-Zarr and this brick's own copies), so `DAPI ` and `DAPI` are one name; the manifest records it (`channel_names_trimmed`, §9). Whitespace inside a name is compared exactly (`DA PI` and `DAPI` differ), with no override. An OME-Zarr channel label that is not text is malformed metadata: the image is unreadable (`unreadable_ome_zarr`), never read as an unnamed channel. | -| Physical sizes | Read with the header, not part of the CSV or the manifest, and compared between images in the folder check (§11, in micrometres with each axis's own unit; a size without a known unit cannot be compared): `PhysicalSizes` with the size of one step along X, Y, Z and T and each axis's unit (OME-NGFF unit names). TIFF: OME `PhysicalSize{X,Y,Z}` with their own units (micrometres when left out) and `TimeIncrement`, or ImageJ resolution, `spacing` and `finterval` with `unit`, `yunit`, `zunit`. OME-Zarr (0.4 and 0.5): the `scale` of level 0's `coordinateTransformations`, times the multiscales entry's own `scale` when given, with each axis's `unit` (common spellings such as `µm`, `um`, `micron` and `s` given as UDUNITS-2 names, others as written). Length units are the microscopy units of MorphoBricks decision D-a2 (§16): `pm`, `Å`/`angstrom`, `nm`, `µm` (`um`, `micron(s)`, `micrometer(s)`, `micrometre(s)`), `mm`, `cm`, `m`, and the names of each in US and British spelling (`nanometre`, ...); ImageJ's escaped micro sign (`\u00B5m`) is recognised. A size in any other unit (`inch`, `decimeter`, ...) is not converted to micrometres, so it cannot be compared: that axis is uncalibrated, and the unit as written is kept in `PhysicalSizes.unsupported_units` (axis letter and unit, information only, not part of equality) so that the folder check names it (`unsupported_pixel_size_unit`, §11). A plain TIFF's resolution tags (no OME or ImageJ metadata, for example 300 dpi with the unit inch) are never a pixel size (decision D-c): such an image has no calibration. Nothing is guessed: what the metadata does not give is None; a scale in a separate file (`path`) or not one number per axis gives no sizes; an OME-Zarr scale without a unit is kept with the unit None, except on the axes a multiscales entry's `metadata` lists as `unknown_scale` (copies written by this brick up to v0.3, §10), whose sizes are None. | +| Physical sizes | Read with the header, not part of the CSV or the manifest, and compared between images in the folder check (§11, in micrometres with each axis's own unit; a size without a known unit cannot be compared): `PhysicalSizes` with the size of one step along X, Y, Z and T and each axis's unit (OME-NGFF unit names). TIFF: OME `PhysicalSize{X,Y,Z}` with their own units (micrometres when left out) and `TimeIncrement`, or ImageJ resolution, `spacing` and `finterval` with `unit`, `yunit`, `zunit`. OME-Zarr (0.4 and 0.5): the `scale` of level 0's `coordinateTransformations`, times the multiscales entry's own `scale` when given, with each axis's `unit` (common spellings such as `µm`, `um`, `micron` and `s` given as UDUNITS-2 names, others as written). Length units are the microscopy units of MorphoBricks decision D-a2 (§16): `pm`, `Å`/`angstrom`, `nm`, `µm` (`um`, `micron(s)`, `micrometer(s)`, `micrometre(s)`), `mm`, `cm`, `m`, and the names of each in US and British spelling (`nanometre`, ...); ImageJ's escaped micro sign (`\u00B5m`) is recognised. A size in any other unit (`inch`, `decimeter`, ...) is not converted to micrometres, so it cannot be compared: that axis is uncalibrated, and the unit as written is kept in `PhysicalSizes.unsupported_units` (axis letter and unit, information only, not part of equality) so that the folder check names it (`unsupported_pixel_size_unit`, §11). A plain TIFF's resolution tags (no OME or ImageJ metadata, for example 300 dpi with the unit inch) are never a pixel size (decision D-c): such an image has no calibration. Nothing is guessed: what the metadata does not give is None. Nothing is dropped silently either (issue #94): a calibration given but malformed (`coordinateTransformations` that are not a list, a transformation of a type other than `scale`, `translation` and `identity`, more than one scale, a scale in a separate file (`path`) or not one number per axis) gives no sizes, and a scale value of X, Y, Z or T that is not a positive finite number gives none on that axis; each is recorded with what is wrong (`PhysicalSizes.invalid`, `ngff_scale.read_scale`) and named by the warning `invalid_physical_calibration` (§11); an OME-Zarr scale without a unit is kept with the unit None, except on the axes a multiscales entry's `metadata` lists as `unknown_scale` (copies written by this brick up to v0.3, §10), whose sizes are None. | | Plane counts | Images may have different numbers of depth slices, time points or planes along an unassigned dimension; this is a warning (§11). | | Channel counts | Every image of an experiment has the same number of channels (normalization plan N43): along the C axis, 1 without one (also when a missing C axis is confirmed as length 1). A different number is the error `different_channel_counts`, found before any image is read: in the folder check from the headers (the `C` dimension, or the RGB samples `S` when there is no `C`), and again with the confirmed axis roles, which decide (§11). | @@ -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 copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17); 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 | +| 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 or that the confirmed roles contradict (`ome_zarr_axes_mismatch`, §2); OME-Zarr images that do not name their axes when their OME-NGFF version does not fix them (`unreadable_ome_zarr`, §2); 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 copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17); 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); in a tree of named-axis copies, sources changed or gone since the conversion (`conversion_source_changed`) and copies changed, gone or added since (`copy_changed_after_conversion`, §17); 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 @@ -297,6 +297,7 @@ row (follow-up P3b, normalization plan N43). | `different_pixel_sizes` | Warning | The physical pixel size (X by Y) differs between images, or the z-step differs between images with more than one depth slice. Sizes are compared in micrometres by the shared rule of decision D-a (§16): two sizes are the same when they differ by at most 0.1 % of the larger, on every axis. The warning is given exactly when, on some axis, the smallest and the largest size differ, that is when some two images differ. The images are grouped by single linkage (`pixel_size_groups`) and those outside the largest group are listed. When every image is linked to the others by near-equal sizes (1.0, 1.0009, 1.0018 µm), the sizes drift, no image stands out, and the message says that none is listed for that size. Until D86 sizes were rounded to four significant digits, so 1.0004 and 1.0006 µm differed (1 and 1.001); they are now the same. | | `missing_pixel_size` | Warning | The pixel size (or the z-step of images with more than one depth slice) is known for some images and not for others. Nothing is reported when no image has it. | | `unsupported_pixel_size_unit` | Warning | An image's metadata give a pixel size or z-step (OME `PhysicalSize{X,Y,Z}`, ImageJ resolution or `spacing`, OME-Zarr level-0 scale) in a length unit that is not one of decision D-a2 (§16), for example OME `in` or ImageJ `inch`. The size is not converted and the axis is treated as uncalibrated (as before); the warning names each image, its axes and the unit as written, so the missing calibration is never silent. A unit without a size, and a plain TIFF's resolution tags (decision D-c), are not reported. The run is not refused. | +| `invalid_physical_calibration` | Warning | An image's metadata give a calibration that cannot be used (issue #94): OME-Zarr `coordinateTransformations` that are not a list or of an unsupported type, more than one scale, a scale stored in a separate file or not one number per axis, or a scale value of X, Y, Z or T that is not a positive finite number. The message names each image and what is wrong. Its sizes are unknown on those axes (never a substituted value) and recorded as uncalibrated; the warning is in the `check` and `run` issues and the report (`discovery_report.json`). An image whose X, Y and (with several depth slices) Z are all stated in `calibration.csv` is not named. A calibration that is not given (no transformations, or no scale) is not reported. The run is not refused. | | `different_channel_counts` | Error | The number of channels differs between images: the `C` dimension, or the RGB samples `S` when there is no `C`; once the axis roles are confirmed, the length of the C axis (1 without one), which replaces the header's count. The images not having the most common number are listed, with each number's image count. Every image of an experiment must have the same channels: split the others into another experiment or exclude them. | | `different_plane_counts` | Warning | The number of depth slices (`Z`), time points (`T`) or planes along any other dimension except X, Y, C and S differs between images. A missing dimension counts as 1. Channels are compared by `different_channel_counts`. | | `different_sizes` | Warning | The image width or height differs between images (TIFF and OME-Zarr). The other dimensions are compared by `different_plane_counts`. | @@ -306,7 +307,7 @@ row (follow-up P3b, normalization plan N43). | Counts | Only images to analyse count: excluded images are left out, and a folder whose images are all excluded is not a biological repeat. A biological repeat with images directly in its folder has no technical-repeat folders (count 0). | | Balance table | Conditions as columns, repeat groups as rows (§12; by default the biological-repeat folder name, `single` when there are no biological-repeat folders); each cell holds the number of images and of technical-repeat folders. | | Pixel sizes | TIFF: OME `PhysicalSizeX`, `PhysicalSizeY`, `PhysicalSizeZ` with their units (micrometres when not given), or ImageJ resolution tags and `spacing` with the ImageJ unit (unknown without a unit). OME-Zarr: the level-0 scale (`coordinateTransformations` of the first dataset, times the image's own scale when given) with the axis units; a scale without a unit is unknown. Sizes are never guessed. The checks run on the sizes each image is recorded with: its header's, or those stated in `calibration.csv` (§9.3); a size stated `pixels` counts as unknown, and `missing_pixel_size` says how many such images it lists. | -| Panel | The notebook shows the balance table with the issues `mixed_dtypes`, `inconsistent_dimensions`, `missing_axes`, `inconsistent_mapping`, `different_sizes`, `different_channel_counts`, `different_plane_counts`, `channel_names_differ`, `different_pixel_sizes`, `missing_pixel_size`, `unsupported_pixel_size_unit`, `unbalanced_images` and `repeat_missing_conditions`. The command line `check` prints the balance table and every issue; `run` reports the same warnings, and stops on `different_channel_counts` and `channel_names_differ`. | +| Panel | The notebook shows the balance table with the issues `mixed_dtypes`, `inconsistent_dimensions`, `missing_axes`, `inconsistent_mapping`, `different_sizes`, `different_channel_counts`, `different_plane_counts`, `channel_names_differ`, `different_pixel_sizes`, `missing_pixel_size`, `unsupported_pixel_size_unit`, `invalid_physical_calibration`, `unbalanced_images` and `repeat_missing_conditions`. The command line `check` prints the balance table and every issue; `run` reports the same warnings, and stops on `different_channel_counts` and `channel_names_differ`. | | Outputs | Unchanged: the checks write nothing, and `plane_histograms.csv` and `run_manifest.json` do not change. | ## 12. Experiment design (`experiment_design.csv`, schema version 1) diff --git a/docs/plan.md b/docs/plan.md index 7e63cc48..67dad37f 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1465,4 +1465,33 @@ Raised during review; to be decided before the milestones that need them: disappeared, as for `calibration_sha256` and the design's SHA-256. (15) **Notebook card**: the next PR, PP-11b (§10); the axis-check card names the command meanwhile. +60. How are OME-Zarr axes and calibrations checked (issues #72, #73, #94, from the audit)? + *Decided (2 November 2026, design v0.4 §3: nothing guessed, nothing silent):* + (1) **Same letters, contradictory roles (#72)**: every OME-Zarr image is matched by the + roles its OME-NGFF axes declare, also when its letters are the mapping's + (`axes.ome_zarr_mapping`, in `review_axes` and `file_mapping_for`). A confirmed role + that is not the declared letter (`ZYX` confirmed `TYX` or `ZXY`, `QYX` confirmed `CYX`) + is the error `ome_zarr_axes_mismatch`, naming each image with its declared axes and + the confirmed roles. It is not one of the axis-check errors (`axes_contradict_header` + and others) because those offer conversion, and an OME-Zarr image is never converted; + the fix is to correct the roles. An axis of no known type is therefore never given a + role, in any arrangement. + (2) **Missing axes (#73)**, by the version the metadata states, per the OME-NGFF + specifications: 0.1 and 0.2 ("all image arrays are 5-dimensional with dimension order + (t, c, z, y, x)") keep `TCZYX` for a 5-D array and refuse other dimensionalities; 0.3, + 0.4 and 0.5 require `axes`, so a store without them is `unreadable_ome_zarr`; a store + without `axes` that states no version or another one is `unreadable_ome_zarr` too. + Such an image is refused rather than offered for confirmation as a plain stack: an + OME-Zarr image cannot be converted to a named-axis copy, and the declared-role match + refuses an axis of no known type. The spec-fixed axes of 0.1/0.2 are recorded as + `axes_from: "header"` (the metadata's version fixes them); no new `axes_from` value. + (3) **Malformed calibration (#94)**: a calibration given but unusable (transformations + not a list or of another type than scale, translation and identity, two scales, a + scale in a file or not one number per axis, a scale value of X, Y, Z or T that is not a + positive finite number) gives unknown sizes on those axes, with what is wrong in + `PhysicalSizes.invalid`, and the warning `invalid_physical_calibration` naming every + image (also when every image has it). A warning, not an error: the pixels are readable + and the image is uncalibrated, as design v0.4 §3 treats an image without a value. Only + these metadata problems are caught: any other exception while reading the scale makes + the image unreadable instead of being hidden by a debug log. diff --git a/docs/user_guide.md b/docs/user_guide.md index 65d6f434..31483b4f 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -201,7 +201,10 @@ notebook's step 3 (§4), also tells how they relate. axes you know better than its header is saved as a named-axis copy first. A plain stack of pages without axis metadata is read with the roles you confirm (a warning). - **OME-Zarr images:** OME-NGFF 0.4 and 0.5 (needs the `zarr` extra; 0.5 needs Python 3.11 - or newer). Their axes (time, channel, z, y, x) come from their metadata. Only the + or newer). Their axes (time, channel, z, y, x) come from their metadata, and they are + read only with those roles: roles that give an axis another role (its depth as time, + say) are an error, and an image whose metadata do not name its axes is not read (the + axes are never guessed). Only the full-resolution level of a single image is read; a folder holding several images (a plate, or a bioformats2raw export with several series) is an error: export each image separately or exclude it. @@ -860,8 +863,8 @@ continues. | `output_inside_experiment` | Error | The output folder is inside the experiment. Choose another. | | `no_images` | Error | No images to analyse (or all excluded). Check the folder. | | `unreadable_tiff` | Error | A TIFF file cannot be opened. Export it again, or exclude it. | -| `unreadable_ome_zarr` | Error | A `.zarr` folder is not an OME-Zarr image that can be read. Export it again as OME-Zarr (OME-NGFF 0.4 or 0.5), or exclude it. | -| `ome_zarr_axes_mismatch` | Error | An OME-Zarr image declares an axis with several planes that the experiment has no role for (or has no X/Y). Check the axis roles, or process it separately. | +| `unreadable_ome_zarr` | Error | A `.zarr` folder is not an OME-Zarr image that can be read; the message says why. That includes an image whose metadata do not name its axes (OME-NGFF 0.3 to 0.5 require them; only 0.1 and 0.2 fix them as `t, c, z, y, x`): the axes are never guessed. Export it again as OME-Zarr (OME-NGFF 0.4 or 0.5) with its axes, or exclude it. | +| `ome_zarr_axes_mismatch` | Error | An OME-Zarr image declares an axis with several planes that the experiment has no role for (or has no X/Y), or the confirmed roles give one of its axes another role than its metadata declares (for example its depth `z` confirmed as time `T`); the message names each image with its declared axes and the confirmed roles. An OME-Zarr image is read only with the roles it declares. Check the axis roles, or process it separately. | | `multiple_series` | Error | A file or OME-Zarr folder holds several images (positions, scenes, series, wells or fields). Export them separately, or exclude it. | | `unsupported_dtype` | Error | Unsupported pixel type (e.g. float16). Convert, or exclude it. | | `mixed_dtypes` | Error | Images have different pixel types. Process them separately or convert. | @@ -906,6 +909,7 @@ continues. | `different_pixel_sizes` | Warning | The physical pixel size or z-step (from the metadata or `calibration.csv`) differs between images by more than 0.1 % (for example different objectives). Images outside the largest group of matching sizes are listed; when sizes only change by small steps from image to image, none is listed. Check the microscope settings if this is not intended. | | `missing_pixel_size` | Warning | Some images have a pixel size (from the metadata or `calibration.csv`) and others do not, so it cannot be checked; images stated as measured in pixels count as without one, and the message says how many. Export them again with metadata, or state their sizes with `calibrate`, if needed. | | `unsupported_pixel_size_unit` | Warning | An image gives its pixel size or z-step in a unit that is not converted (for example inches); only pm, Å, nm, µm, mm, cm and m are. The message names each image, axis and unit. Those sizes are not used: the image is treated as uncalibrated on those axes. Export it again with the pixel size in a supported unit, or state it with `calibrate`, if the physical size matters. Not reported for an axis whose size is stated. | +| `invalid_physical_calibration` | Warning | An image's metadata give a pixel size, z-step or time step that cannot be used (for OME-Zarr: `coordinateTransformations` that are malformed, of an unsupported type, stored in a separate file, or a scale that is zero, negative or not a number); the message names each image and what is wrong. The image is treated as uncalibrated on those axes; no size is made up. Correct the metadata, or state the sizes with the `calibrate` command. | | `calibration_axis_absent` | Warning | `calibration.csv` states a size for an axis some images do not have (for example a Z size for 2D images or projections); the message names them. No size is recorded for an axis an image does not have. Give the statement a narrower target, or no action if it is meant for the other images. | | `calibration_differs_from_header` | Warning | `calibration.csv` states a pixel size (or `pixels`) that differs from an image's metadata; the message names each image and axis with both sizes. The stated size is used. Correct or remove the statement if the metadata are right. | | `calibration_target_not_found` | Error | `calibration.csv` names a folder or image that is not in the experiment (names are compared exactly, case included). Correct the target, or remove the line. | diff --git a/pyproject.toml b/pyproject.toml index 3415b659..4edf2209 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -191,6 +191,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_ui" = "ui" "morphobricks_preprocessing.physical_sizes" = "io" "morphobricks_preprocessing.pixel_size_checks" = "pure" +"morphobricks_preprocessing.ngff_scale" = "pure" "morphobricks_preprocessing.calibration" = "pure" "morphobricks_preprocessing.calibration_io" = "io" "morphobricks_preprocessing.run.calibrate" = "io" diff --git a/src/morphobricks_preprocessing/axes.py b/src/morphobricks_preprocessing/axes.py index 3d4da9f1..4e858cef 100644 --- a/src/morphobricks_preprocessing/axes.py +++ b/src/morphobricks_preprocessing/axes.py @@ -10,6 +10,8 @@ contradict the named axes are an error, and such an image is never read (the user corrects the roles, or saves named-axis copies); an image whose metadata does not name its axes (a plain stack) is read with the confirmed roles, with a warning, and recorded as confirmed. +An OME-Zarr image is always matched by the roles its OME-NGFF metadata declares +(:func:`ome_zarr_mapping`), also when its axis letters are the mapping's (issue #72). Nothing here reads pixels; the preview plane is read by :func:`~morphobricks_preprocessing.planes.read_sample_plane`. @@ -322,16 +324,17 @@ def file_mapping_for( ) -> AxisMapping | None: """Resolve the experiment mapping for one image, or None if it does not fit. - An image with the mapping's arrangement gets the mapping itself. A TIFF with - some of its dimensions left out gets :meth:`AxisMapping.for_axes`. An OME-Zarr - image with another arrangement is matched by the roles its metadata declares - (see :func:`declared_mapping`). Experiment roles an image lacks are read as - length 1 only if ``missing_as_singleton``. + A TIFF with the mapping's arrangement gets the mapping itself, and one with some + of its dimensions left out gets :meth:`AxisMapping.for_axes`. An OME-Zarr image, + whatever its arrangement, is matched by the roles its metadata declares + (:func:`ome_zarr_mapping`): with the mapping's arrangement but other roles it + does not fit (issue #72). Experiment roles an image lacks are read as length 1 + only if ``missing_as_singleton``. """ - if f.axes == mapping.axes: - return mapping if f.is_ome_zarr: - resolved, problem = declared_mapping(f, mapping) + resolved, problem = ome_zarr_mapping(f, mapping) + elif f.axes == mapping.axes: + return mapping else: resolved = mapping.for_axes(f.axes) problem = "" if resolved is not None else "mismatch" @@ -342,6 +345,33 @@ def file_mapping_for( return resolved +def ome_zarr_mapping(f: TiffMetadata, mapping: AxisMapping) -> tuple[AxisMapping | None, str]: + """Resolve the experiment mapping for an OME-Zarr image by the roles it declares. + + Every OME-Zarr image is matched by its OME-NGFF axes (:func:`declared_mapping`), + also when its axis letters are the mapping's (issue #72): confirmed roles that + give a declared axis another role (``ZYX`` confirmed ``TYX``, ``ZYX`` confirmed + ``ZXY``) or give an axis of no known type a role (``QYX`` confirmed ``CYX``) + contradict the metadata, and the image is not read under them. + + Returns + ------- + tuple + As :func:`declared_mapping`, and ``(None, "contradicted")`` for an image + with the mapping's arrangement when a confirmed role is not the letter its + metadata declares for that axis. + """ + if f.axes == mapping.axes and any( + role is not None and role != axis for axis, role in zip(f.axes, mapping.roles, strict=True) + ): + return None, CONTRADICTED + return declared_mapping(f, mapping) + + +CONTRADICTED = "contradicted" +""":func:`ome_zarr_mapping`'s problem for roles that contradict an image's OME-NGFF axes.""" + + def declared_mapping(f: TiffMetadata, mapping: AxisMapping) -> tuple[AxisMapping | None, str]: """Match an OME-Zarr image to the experiment mapping by the roles it declares. diff --git a/src/morphobricks_preprocessing/axis_check.py b/src/morphobricks_preprocessing/axis_check.py index ec00a4ad..81b66937 100644 --- a/src/morphobricks_preprocessing/axis_check.py +++ b/src/morphobricks_preprocessing/axis_check.py @@ -79,8 +79,10 @@ def header_axis_issue(meta: TiffMetadata, mapping: AxisMapping) -> str | None: ``unassigned_axis``, and a position axis (``R``, ``M``, ``J``, ``K``) its ``unsupported_axis``, so they are left to those. - OME-Zarr images name their axes in their OME-NGFF metadata, which gives their roles - (:func:`declared_mapping`): they never have one of these problems. + OME-Zarr images name their axes in their OME-NGFF metadata, which gives their roles: + confirmed roles that contradict them are :func:`.axes.ome_zarr_mapping`'s + ``ome_zarr_axes_mismatch`` (issue #72), whatever the arrangement, so they never have + one of these problems (an OME-Zarr image cannot be converted to a named-axis copy). Parameters ---------- diff --git a/src/morphobricks_preprocessing/axis_review.py b/src/morphobricks_preprocessing/axis_review.py index 9a83b858..652dad79 100644 --- a/src/morphobricks_preprocessing/axis_review.py +++ b/src/morphobricks_preprocessing/axis_review.py @@ -8,15 +8,17 @@ from __future__ import annotations from collections.abc import Iterable +from dataclasses import dataclass, field from .axes import ( + CONTRADICTED, ROLES, AxisMapping, AxisReview, _axis_label, _embeddings, _sizes, - declared_mapping, + ome_zarr_mapping, ) from .axis_check import ( POSITION_AXES, @@ -162,7 +164,10 @@ def review_axes( from :func:`check_mapping`, files that lack dimensions without confirmation (``missing_axes``) or in an ambiguous way (``ambiguous_missing_axes``), files the mapping does not fit - (``inconsistent_mapping``), files with different numbers of channels + (``inconsistent_mapping``), OME-Zarr images whose declared axes do not fit + the experiment's roles or that the confirmed roles contradict, whatever + their arrangement (``ome_zarr_axes_mismatch``, issue #72), files with + different numbers of channels along the C axis (``different_channel_counts``; 1 without one), and the axis check of every TIFF against its header (:func:`header_axis_issue`), each naming its files with their header axes and confirmed roles: the @@ -175,36 +180,9 @@ def review_axes( if isinstance(files, MetadataReport): files = files.files readable = [f for f in files if f.readable] - file_mappings: dict[str, AxisMapping] = {} - unconfirmed: list[str] = [] - ambiguous: list[str] = [] - misfits: list[str] = [] - zarr_misfits: list[str] = [] - by_role: list[str] = [] - for f in readable: - if f.axes == mapping.axes: - file_mappings[f.relative_path] = mapping - continue - if f.is_ome_zarr: - by_role.append(f.relative_path) - resolved, problem = declared_mapping(f, mapping) - if resolved is None: - zarr_misfits.append(f.relative_path) - elif problem == "missing" and not missing_as_singleton: - unconfirmed.append(f.relative_path) - else: - file_mappings[f.relative_path] = resolved - continue - resolved = mapping.for_axes(f.axes) - if resolved is None: - if len(_embeddings(f.axes, mapping.axes)) > 1: - ambiguous.append(f.relative_path) - else: - misfits.append(f.relative_path) - elif not missing_as_singleton: - unconfirmed.append(f.relative_path) - else: - file_mappings[f.relative_path] = resolved + found = _resolve_files(readable, mapping, missing_as_singleton) + file_mappings, unconfirmed, misfits = found.mappings, found.unconfirmed, found.misfits + ambiguous, by_role = found.ambiguous, found.by_role issues = check_mapping( mapping, @@ -235,19 +213,7 @@ def review_axes( "process them as a separate experiment.", ) ) - if zarr_misfits: - issues.append( - error( - "ome_zarr_axes_mismatch", - f"The axes declared in {plural(len(zarr_misfits), 'OME-Zarr image')} do not " - f"match the axis roles of the experiment ({mapping.describe()}): they have an " - "axis with several planes that the experiment has no role for, or no X or Y " - "axis.", - zarr_misfits, - "Check the axis roles in the axis review, or process these images as a " - "separate experiment.", - ) - ) + issues.extend(_ome_zarr_issues(found.zarr_misfits, found.contradicted, mapping)) if misfits: issues.append( error( @@ -271,7 +237,7 @@ def review_axes( ) # Each file's confirmed roles against the axes its own header names. Files the mapping # does not fit already have their error; OME-Zarr images take their roles from their - # metadata. + # metadata, checked against the confirmed roles above (ome_zarr_mapping, issue #72). checked = [ (f, file_mappings.get(f.relative_path) or mapping.for_axes(f.axes)) for f in readable @@ -283,6 +249,97 @@ def review_axes( return AxisReview(mapping, file_mappings, (*issues, *axis_issues), axes_from) +@dataclass +class _Resolved: + """Each readable file's mapping, or why it has none (:func:`_resolve_files`).""" + + mappings: dict[str, AxisMapping] = field(default_factory=dict) + unconfirmed: list[str] = field(default_factory=list) + ambiguous: list[str] = field(default_factory=list) + misfits: list[str] = field(default_factory=list) + zarr_misfits: list[str] = field(default_factory=list) + contradicted: list[TiffMetadata] = field(default_factory=list) + by_role: list[str] = field(default_factory=list) + + +def _resolve_files( + readable: list[TiffMetadata], mapping: AxisMapping, missing_as_singleton: bool +) -> _Resolved: + """Resolve the experiment mapping for each file, sorting out those it does not fit. + + Every OME-Zarr image is matched by the roles its metadata declares + (:func:`.axes.ome_zarr_mapping`), also with the mapping's arrangement (issue #72). + """ + found = _Resolved() + for f in readable: + path = f.relative_path + if f.is_ome_zarr: + if f.axes != mapping.axes: + found.by_role.append(path) + resolved, problem = ome_zarr_mapping(f, mapping) + if problem == CONTRADICTED: + found.contradicted.append(f) + elif resolved is None: + found.zarr_misfits.append(path) + elif problem == "missing" and not missing_as_singleton: + found.unconfirmed.append(path) + else: + found.mappings[path] = resolved + continue + if f.axes == mapping.axes: + found.mappings[path] = mapping + continue + resolved = mapping.for_axes(f.axes) + if resolved is None: + several = len(_embeddings(f.axes, mapping.axes)) > 1 + (found.ambiguous if several else found.misfits).append(path) + elif not missing_as_singleton: + found.unconfirmed.append(path) + else: + found.mappings[path] = resolved + return found + + +def _ome_zarr_issues( + misfits: list[str], contradicted: list[TiffMetadata], mapping: AxisMapping +) -> list[ValidationIssue]: + """Return the ``ome_zarr_axes_mismatch`` error naming the OME-Zarr images that do not fit. + + ``misfits`` have an axis the experiment has no role for, or no X or Y; ``contradicted`` + have the mapping's arrangement, but a confirmed role gives an axis another role than its + OME-NGFF metadata declares (issue #72), each named with its declared axes and roles. + """ + if not misfits and not contradicted: + return [] + parts = [] + if misfits: + parts.append( + "they have an axis with several planes that the experiment has no role for, or " + "no X or Y axis" + ) + if contradicted: + details = "; ".join( + f"declared {f.axes}, confirmed {mapping.roles_string}: {f.relative_path}" + for f in sorted(contradicted, key=lambda f: f.relative_path) + ) + parts.append( + "the confirmed roles give an axis another role than their OME-NGFF metadata " + f"declares ({details}), and they are not read under a contradicted declaration" + ) + paths = sorted([*misfits, *(f.relative_path for f in contradicted)]) + return [ + error( + "ome_zarr_axes_mismatch", + f"The axes declared in {plural(len(paths), 'OME-Zarr image')} do not match the " + f"axis roles of the experiment ({mapping.describe()}): {'; '.join(parts)}.", + paths, + "Check the axis roles in the axis review: an OME-Zarr image's roles are those " + "its OME-NGFF metadata declares (time, channel, and the space axes z, y, x). Or " + "process these images as a separate experiment.", + ) + ] + + def channel_count(f: TiffMetadata, mapping: AxisMapping) -> int: """Return the number of channels of ``f`` under its own mapping; 1 without a C axis. diff --git a/src/morphobricks_preprocessing/dataset.py b/src/morphobricks_preprocessing/dataset.py index e48393ae..571a6774 100644 --- a/src/morphobricks_preprocessing/dataset.py +++ b/src/morphobricks_preprocessing/dataset.py @@ -34,6 +34,7 @@ "different_pixel_sizes", "missing_pixel_size", "unsupported_pixel_size_unit", + "invalid_physical_calibration", "unbalanced_images", "repeat_missing_conditions", ) diff --git a/src/morphobricks_preprocessing/ngff_scale.py b/src/morphobricks_preprocessing/ngff_scale.py new file mode 100644 index 00000000..6586c331 --- /dev/null +++ b/src/morphobricks_preprocessing/ngff_scale.py @@ -0,0 +1,113 @@ +"""The ``scale`` of an OME-NGFF ``coordinateTransformations`` list, checked (issue #94). + +A calibration that is not given (no transformations, or none of type ``scale``) is valid: +the image is uncalibrated. A calibration that is given but cannot be used is never dropped +silently: :func:`read_scale` returns what is wrong with it, in plain words, so that +:func:`.physical_sizes.ngff_physical_sizes` records it (:attr:`.PhysicalSizes.invalid`) and +the folder check names the image (``invalid_physical_calibration``, contract §11). Such +sizes are unknown (None); no value is ever substituted. + +Nothing here reads files. +""" + +from __future__ import annotations + +import math +from collections.abc import Sequence + +TRANSFORM_TYPES = ("scale", "translation", "identity") +"""The transformation types a level or image may have; only ``scale`` changes the sizes.""" + +CALIBRATED_AXES = "XYZT" +"""The axes whose scale is a pixel size, z-step or time step: it must be a positive number.""" + + +def read_scale(entry: object, axes: str, where: str) -> tuple[list[float | None] | None, list[str]]: + """Return the ``scale`` of an entry's ``coordinateTransformations`` and its problems. + + Parameters + ---------- + entry + A dataset (a level) or a multiscales entry. + axes + One letter per dimension (:func:`.zarr_input.ngff_axes`). + where + What the entry is, for the problems, e.g. ``"level 0"``. + + Returns + ------- + tuple + ``(scale, problems)``. ``scale`` is None when the entry has no scale, or when its + transformations cannot be used at all (then ``problems`` says why); otherwise one + value per axis, None for a value that is not a usable number (``problems`` names + it). ``problems`` is empty for a usable or absent scale. + """ + transforms = entry.get("coordinateTransformations") if isinstance(entry, dict) else None + if transforms is None: + return None, [] + if not isinstance(transforms, list): + return None, [f"the coordinateTransformations of {where} are not a list"] + scales = [] + for transform in transforms: + kind = transform.get("type") if isinstance(transform, dict) else None + if kind not in TRANSFORM_TYPES: + return None, [ + f"{where} has a coordinate transformation of type {kind!r}, not one of " + f"{', '.join(TRANSFORM_TYPES)}" + ] + if kind == "scale": + scales.append(transform) + if not scales: + return None, [] + if len(scales) > 1: + return None, [f"{where} has {len(scales)} scale transformations, not one"] + return _scale_values(scales[0], axes, where) + + +def _scale_values( + transform: dict, axes: str, where: str +) -> tuple[list[float | None] | None, list[str]]: + """Return the values of one ``scale`` transformation and what is wrong with them.""" + values = transform.get("scale") + if values is None and "path" in transform: + return None, [ + f"the scale of {where} is stored in a separate file ({transform['path']!r}), " + "which is not read" + ] + if not isinstance(values, Sequence) or isinstance(values, str): + return None, [f"the scale of {where} is not a list of numbers"] + if len(values) != len(axes): + return None, [ + f"the scale of {where} has {len(values)} values for {len(axes)} axes, not one " + "number per axis" + ] + scale: list[float | None] = [] + problems = [] + for letter, value in zip(axes, values, strict=True): + number = _scale_value(value) + usable = number is not None and ( + letter not in CALIBRATED_AXES or (math.isfinite(number) and number > 0) + ) + scale.append(number if usable else None) + if not usable: + problems.append( + f"the scale of {where} gives {_shown(value)} for axis {letter}, not a " + "positive number" + ) + return scale, problems + + +def _scale_value(value: object) -> float | None: + """Return one scale value as a float; None when it is not a number a float can hold.""" + if isinstance(value, bool) or not isinstance(value, int | float): + return None + try: + return float(value) + except OverflowError: # an integer too large for a float is not a pixel size + return None + + +def _shown(value: object) -> str: + """Return a scale value as shown in a problem, shortened when it is long.""" + text = repr(value) + return text if len(text) <= 24 else f"{text[:20]}..." diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index ab679903..04906600 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -23,11 +23,13 @@ import os import re from collections.abc import Hashable, Mapping, Sequence -from dataclasses import dataclass +from dataclasses import dataclass, replace from xml.etree import ElementTree import tifffile +from .ngff_scale import read_scale + logger = logging.getLogger(__name__) UNKNOWN_SCALE_KEY = "unknown_scale" @@ -132,6 +134,13 @@ class PhysicalSizes: converted: the axis is uncalibrated (:meth:`micrometers` is None). Recorded so that the unit can be named (``unsupported_pixel_size_unit``); for information only, not part of equality. + invalid + What is wrong with a calibration the metadata give but that cannot be used (a + malformed OME-NGFF ``coordinateTransformations``, :mod:`.ngff_scale`), in plain + words; None when the calibration is usable or not given. The sizes it would give + are None (never substituted), and the folder check names the image + (``invalid_physical_calibration``, issue #94). For information only, not part of + equality. Two sizes are equal when their sizes, ``space_unit`` and resolved units are. """ @@ -146,6 +155,7 @@ class PhysicalSizes: y_unit: str | None = None z_unit: str | None = None unsupported_units: tuple[tuple[str, str], ...] = () + invalid: str | None = None def _space_units(self) -> tuple[str | None, str | None, str | None]: """Return the units of X, Y and Z, each falling back to ``space_unit``.""" @@ -480,10 +490,13 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: :func:`read_physical_sizes` gives: common spellings such as ``"µm"``, ``"um"`` or ``"micron"`` become ``"micrometer"``, other units are kept as written. - Nothing is guessed: without a level-0 scale (OME-NGFF 0.1 to 0.3, or a scale - stored in a separate file through ``path``) or with a malformed one, every - size is None; an axis without a ``unit`` has the unit None, and a scale that - is not a positive finite number is None. A scale given without a unit is + Nothing is guessed: without a level-0 scale (OME-NGFF 0.1 to 0.3) every size + is None. Nothing is dropped silently either (issue #94): with a malformed + scale (transformations that are not a list or of another type, a scale stored + in a separate file through ``path``, not one number per axis) every size is + None, and a scale value that is not a positive finite number is None, each + with what is wrong in :attr:`PhysicalSizes.invalid` (:mod:`.ngff_scale`). An + axis without a ``unit`` has the unit None. A scale given without a unit is kept: it may be a placeholder (some writers write 1 when the size is not known). This brick's OME-Zarr copies name such axes in the multiscales entry's ``metadata`` (``"unknown_scale": ["x", "y"]``); their sizes are None. @@ -503,13 +516,12 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: """ datasets = multiscale.get("datasets") or [] first = datasets[0] if isinstance(datasets, list) and datasets else {} - try: - scale = _ngff_scale(first, len(axes)) - overall = _ngff_scale(multiscale, len(axes)) - except ValueError: - return PhysicalSizes() - if scale is None: - return PhysicalSizes() + scale, problems = read_scale(first, axes, "level 0") + overall, more = read_scale(multiscale, axes, "the image") + problems += more + invalid = "; ".join(problems) or None + if scale is None or (more and overall is None): + return PhysicalSizes(invalid=invalid) if overall is not None: scale = [ a * b if a is not None and b is not None else None @@ -536,47 +548,13 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: for letter, (_, unit) in sizes.items() if letter in "XYZ" } - return _sizes( - sizes["X"][0], - sizes["Y"][0], - sizes["Z"][0], - sizes["T"][0], + found = _sizes( + *(sizes[letter][0] for letter in "XYZT"), {letter: sizes[letter][1] for letter in "XYZ"}, sizes["T"][1], _unsupported({letter: sizes[letter][0] for letter in "XYZ"}, written), ) - - -def _ngff_scale(entry: dict, ndim: int) -> list[float | None] | None: - """Return the ``scale`` in an entry's ``coordinateTransformations``. - - None when the entry has no scale transformation; raises ValueError when its - transformations are not a list, or its scale cannot be used (stored in a - file, or not one number per axis). - """ - transforms = entry.get("coordinateTransformations") if isinstance(entry, dict) else None - if transforms is None: - return None - if not isinstance(transforms, list): - raise ValueError("the coordinate transformations are not a list") - for transform in transforms: - if not isinstance(transform, dict) or transform.get("type") != "scale": - continue - values = transform.get("scale") - if not isinstance(values, Sequence) or isinstance(values, str) or len(values) != ndim: - raise ValueError("the scale is not given as one number per axis") - return [_scale_value(v) for v in values] - return None - - -def _scale_value(value) -> float | None: # noqa: ANN001 - """Return one scale value as a float; None when it is not a usable number.""" - if isinstance(value, bool) or not isinstance(value, int | float): - return None - try: - return float(value) - except OverflowError: # an integer too large for a float is not a pixel size - return None + return replace(found, invalid=invalid) def _tiff_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: diff --git a/src/morphobricks_preprocessing/pixel_size_checks.py b/src/morphobricks_preprocessing/pixel_size_checks.py index 17ab9c1d..f9e7bc20 100644 --- a/src/morphobricks_preprocessing/pixel_size_checks.py +++ b/src/morphobricks_preprocessing/pixel_size_checks.py @@ -2,9 +2,11 @@ Pixel sizes and z-steps that differ between the images (``different_pixel_sizes``), are known for only some of them (``missing_pixel_size``), or are given in a length unit that is not -converted (``unsupported_pixel_size_unit``, decision D-a2). The checks run on the sizes an -image is recorded with: its header's, or those stated in ``calibration.csv`` (design v0.4 §3, -:mod:`.calibration`), so they check the resolved values. Moved from :mod:`.metadata` (PP-8). +converted (``unsupported_pixel_size_unit``, decision D-a2), or come from a calibration the +metadata give but that cannot be used (``invalid_physical_calibration``, issue #94). The +checks run on the sizes an image is recorded with: its header's, or those stated in +``calibration.csv`` (design v0.4 §3, :mod:`.calibration`), so they check the resolved values. +Moved from :mod:`.metadata` (PP-8). """ from __future__ import annotations @@ -20,7 +22,12 @@ if TYPE_CHECKING: from .metadata import TiffMetadata -PIXEL_SIZE_CODES = ("different_pixel_sizes", "missing_pixel_size", "unsupported_pixel_size_unit") +PIXEL_SIZE_CODES = ( + "different_pixel_sizes", + "missing_pixel_size", + "unsupported_pixel_size_unit", + "invalid_physical_calibration", +) """The codes of these checks; :func:`.run.preflight.preflight` gives them on resolved sizes.""" @@ -50,8 +57,8 @@ def pixel_size_issues( Returns ------- list of ValidationIssue - Warnings only: ``different_pixel_sizes``, ``missing_pixel_size`` and - ``unsupported_pixel_size_unit``. + Warnings only: ``different_pixel_sizes``, ``missing_pixel_size``, + ``unsupported_pixel_size_unit`` and ``invalid_physical_calibration``. """ if sizes is None: sizes = {f.relative_path: header_sizes(f.physical_sizes) for f in files} @@ -65,6 +72,42 @@ def pixel_size_issues( *_different_issue(axes, by_role), *_missing_issue(axes, by_role), *_unit_issue(files, by_role), + *_invalid_issue(files, by_role), + ] + + +def _invalid_issue( + files: Sequence[TiffMetadata], by_role: Mapping[str, Mapping[str, RecordedSize]] +) -> list[ValidationIssue]: + """Warn about calibrations the metadata give but that cannot be used (issue #94). + + Such an image is uncalibrated on the axes concerned (never a substituted value); without + this warning it would look merely uncalibrated. Every image is named with what is wrong. + An image whose X, Y and (with several depth slices) Z are all stated in + ``calibration.csv`` does not use the header's sizes, so it is not named. + """ + found = {} + for f in files: + roles = ("X", "Y", "Z") if f.length("Z") > 1 else ("X", "Y") + recorded = by_role[f.relative_path] + uses_header = any( + role not in recorded or recorded[role].source in (None, HEADER) for role in roles + ) + if f.physical_sizes.invalid and uses_header: + found[f.relative_path] = f.physical_sizes.invalid + if not found: + return [] + details = "; ".join(f"{path}: {problem}" for path, problem in found.items()) + return [ + warning( + "invalid_physical_calibration", + f"{plural(len(found), 'image')} give a calibration in their metadata that cannot " + f"be used ({details}). The sizes it gives are not used: these images are " + "uncalibrated on those axes, and no size is substituted.", + list(found), + "Correct the coordinateTransformations in these images' OME-NGFF metadata, or " + "state their pixel sizes in calibration.csv (the 'calibrate' command).", + ) ] diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index bef7aec2..933f42a1 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -29,7 +29,6 @@ from __future__ import annotations -import logging import os from collections.abc import Iterable from dataclasses import dataclass @@ -40,8 +39,6 @@ from .physical_sizes import PhysicalSizes, ngff_physical_sizes -logger = logging.getLogger(__name__) - ZARR_SUFFIX = ".zarr" @@ -130,7 +127,9 @@ class NgffImage: the OME-NGFF ``coordinateTransformations`` scale and axis units (:func:`.physical_sizes.ngff_physical_sizes`), in the form :func:`.physical_sizes.read_physical_sizes` gives for TIFF files; None - where the metadata does not give them. + where the metadata does not give them. A calibration given but malformed + is never dropped silently: its sizes are None and + :attr:`.PhysicalSizes.invalid` says what is wrong (issue #94). """ array: Any @@ -296,7 +295,8 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: # n array = group[dataset_path] except KeyError as exc: raise NgffError(f"the level-0 array {dataset_path!r} is missing") from exc - axes = ngff_axes(first.get("axes"), len(array.shape)) + version = str(meta.get("version") or first.get("version") or "") + axes = ngff_axes(first.get("axes"), len(array.shape), version) if len(axes) != len(array.shape): raise NgffError(f"its metadata names {len(axes)} axes but the array has {len(array.shape)}") omero = meta.get("omero") if isinstance(meta.get("omero"), dict) else {} @@ -313,28 +313,15 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: # n return NgffImage( array=array, axes=axes, - version=str(meta.get("version") or first.get("version") or ""), + version=version, image_count=image_count, channel_names=labels, channel_names_trimmed=trimmed, dataset_path=f"{prefix}{dataset_path}", - physical_sizes=_physical_sizes(first, axes, dataset_path), + physical_sizes=ngff_physical_sizes(first, axes), ) -def _physical_sizes(multiscale: dict, axes: str, where: str) -> PhysicalSizes: - """Return the image's physical sizes; empty when its scale metadata cannot be used. - - The sizes are for information only, so metadata that cannot be understood - never makes the image unreadable. - """ - try: - return ngff_physical_sizes(multiscale, axes) - except Exception: # sizes are optional; never block an image over them - logger.debug("Cannot read the physical sizes of %s", where, exc_info=True) - return PhysicalSizes() - - def _labels_group(group): # noqa: ANN001, ANN202 """Return the ``labels`` group next to an image, or None when it has none.""" try: @@ -344,17 +331,28 @@ def _labels_group(group): # noqa: ANN001, ANN202 return labels if hasattr(labels, "group_keys") else None -def ngff_axes(axes: Any, ndim: int) -> str: # noqa: ANN401 +def ngff_axes(axes: Any, ndim: int, version: str = "") -> str: # noqa: ANN401 """Translate OME-NGFF axis metadata into one-letter codes. ``axes`` is the ``multiscales[0]["axes"]`` entry: a list of objects with - ``name`` and ``type`` (OME-NGFF 0.4 and 0.5) or of names (0.3). Without axes - metadata (OME-NGFF 0.1 and 0.2) the order is fixed to ``t, c, z, y, x``. + ``name`` and ``type`` (OME-NGFF 0.4 and 0.5) or of names (0.3). Nothing is + guessed when it is absent or empty (issue #73): only OME-NGFF 0.1 and 0.2, + whose specification fixes every image array as 5-D in the order ``t, c, z, + y, x``, have axes without naming them (:data:`SPEC_FIXED_AXES`, by the + ``version`` the metadata states). OME-NGFF 0.3 to 0.5 require the axes, and + a store that states another version or none does not say what its axes are: + both are refused. + + Raises + ------ + NgffError + If the axes are not given and the stated version does not fix them, or + are not a list. """ if not axes: - if ndim != 5: - raise NgffError("it does not say what its axes are") - return "TCZYX" + return _spec_fixed_axes(ndim, version) + if not isinstance(axes, list): + raise NgffError("its axes metadata is not a list of axes") letters = [] for axis in axes: name, kind = ( @@ -373,6 +371,39 @@ def ngff_axes(axes: Any, ndim: int) -> str: # noqa: ANN401 return "".join(letters) +SPEC_FIXED_AXES = {"0.1": "TCZYX", "0.2": "TCZYX"} +"""The OME-NGFF versions whose specification fixes the axes without naming them: "All image +arrays are 5-dimensional with dimension order (t, c, z, y, x)".""" + +AXES_REQUIRED_VERSIONS = ("0.3", "0.4", "0.5") +"""The OME-NGFF versions whose specification requires the multiscales ``axes`` field.""" + + +def _spec_fixed_axes(ndim: int, version: str) -> str: + """Return the axes the stated OME-NGFF version fixes for a store that names none. + + Raises :class:`NgffError` naming what is wrong when the version does not fix them. + """ + fixed = SPEC_FIXED_AXES.get(version) + if fixed is not None: + if ndim != len(fixed): + raise NgffError( + f"it does not name its axes, and OME-NGFF {version} fixes them only for " + f"5-D arrays (t, c, z, y, x); its array has {ndim} dimensions" + ) + return fixed + if version in AXES_REQUIRED_VERSIONS: + raise NgffError( + f"its OME-NGFF {version} multiscales metadata has no axes, which that version " + "requires; add the axes (name and type of each dimension) to its metadata" + ) + stated = f"states OME-NGFF version {version!r}" if version else "states no OME-NGFF version" + raise NgffError( + f"it does not name its axes and {stated}, so what its axes are is not known; add " + "the axes (name and type of each dimension) to its metadata" + ) + + def read_selection(array: Any, selection: tuple) -> np.ndarray: # noqa: ANN401 """Read part of a zarr array as a NumPy array in native byte order.""" data = np.asarray(array[selection]) diff --git a/tests/test_ngff_axes_calibration.py b/tests/test_ngff_axes_calibration.py new file mode 100644 index 00000000..21774797 --- /dev/null +++ b/tests/test_ngff_axes_calibration.py @@ -0,0 +1,284 @@ +"""OME-Zarr axes and calibration: nothing guessed, nothing silent (issues #72, #73, #94).""" + +import json + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import ( + AxisMapping, + NgffError, + PhysicalSizes, + file_mapping_for, + inspect_image, + open_ngff, + preflight, + review_axes, + run_experiment, + zarr_input, +) +from morphobricks_preprocessing.zarr_input import ngff_axes +from tiff_tree import write_data +from zarr_tree import write_ngff + +zarr = pytest.importorskip("zarr") + +VERSIONS = ["0.4", "0.5"] + + +def codes(issues): + return [issue.code for issue in issues] + + +def issue(issues, code): + (found,) = [i for i in issues if i.code == code] + return found + + +def data(shape): + return np.arange(int(np.prod(shape)), dtype=np.uint8).reshape(shape) + + +def edit_multiscale(path, change): + """Apply ``change`` to the first multiscales entry of a 0.4 or 0.5 store.""" + group = zarr.open_group(str(path), mode="r+") + attrs = dict(group.attrs) + ome = attrs.get("ome") + meta = dict(ome) if isinstance(ome, dict) else attrs + multiscales = [dict(m) for m in meta["multiscales"]] + change(multiscales[0], meta) + meta["multiscales"] = multiscales + if isinstance(ome, dict): + group.attrs["ome"] = meta + else: + group.attrs.update(meta) + return path + + +# Issue #72: same axis letters, contradictory confirmed roles -------------------------------- + + +@pytest.fixture +def zyx_store(tmp_path): + return write_ngff(tmp_path / "exp/ctrl/a.ome.zarr", data((3, 6, 8)), "zyx") + + +@pytest.mark.parametrize("roles", ["TYX", "ZXY", "CYX"]) +def test_confirmed_roles_that_contradict_ngff_axes_are_refused(zyx_store, roles): + meta = inspect_image(zyx_store, "ctrl/a.ome.zarr") + assert meta.axes == "ZYX" + mapping = AxisMapping("ZYX", roles) + review = review_axes([meta], mapping) + found = issue(review.issues, "ome_zarr_axes_mismatch") + assert found.is_error and found.paths == ("ctrl/a.ome.zarr",) + assert f"declared ZYX, confirmed {roles}: ctrl/a.ome.zarr" in found.message + assert "ctrl/a.ome.zarr" not in review.file_mappings + assert file_mapping_for(meta, mapping) is None + + +def test_a_role_for_an_axis_of_no_known_type_is_refused(tmp_path): + path = write_ngff( + tmp_path / "a.ome.zarr", data((3, 6, 8)), "qyx", axis_types=["other", "space", "space"] + ) + meta = inspect_image(path, "a.ome.zarr") + assert meta.axes == "QYX" + mapping = AxisMapping("QYX", "CYX") + review = review_axes([meta], mapping) + assert "declared QYX, confirmed CYX" in issue(review.issues, "ome_zarr_axes_mismatch").message + assert review.file_mappings == {} + assert file_mapping_for(meta, mapping) is None + + +def test_a_run_refuses_ngff_depth_confirmed_as_time(zyx_store, tmp_path): + result = run_experiment( + tmp_path / "exp", AxisMapping("ZYX", "TYX"), output_folder=tmp_path / "o" + ) + assert not result.written + assert "ome_zarr_axes_mismatch" in codes(result.issues) + assert not (tmp_path / "o" / "run_manifest.json").exists() + + +def test_roles_that_agree_with_ngff_axes_are_read_as_declared(zyx_store, tmp_path): + write_data(tmp_path / "exp/ctrl/b.tif", data((3, 6, 8)), axes="ZYX", imagej=True) + mapping = AxisMapping("ZYX", "ZYX") + result = run_experiment(tmp_path / "exp", mapping, output_folder=tmp_path / "o") + assert result.written, result.issues + assert "ome_zarr_axes_mismatch" not in codes(result.issues) + meta = inspect_image(zyx_store, "ctrl/a.ome.zarr") + assert file_mapping_for(meta, mapping) == mapping + + +def test_another_arrangement_is_still_matched_by_role(tmp_path): + tiff = tmp_path / "exp/ctrl/a.tif" + write_data(tiff, data((3, 2, 6, 8)), axes="ZCYX", imagej=True) + store = write_ngff(tmp_path / "exp/ctrl/b.ome.zarr", data((2, 3, 6, 8)), "czyx") + mapping = AxisMapping("ZCYX", "ZCYX") + review = review_axes( + [inspect_image(tiff, "a.tif"), inspect_image(store, "b.ome.zarr")], mapping + ) + assert not review.issues + assert review.file_mappings["b.ome.zarr"].roles == ("C", "Z", "Y", "X") + + +# Issue #73: missing axes are never guessed ------------------------------------------------- + + +def drop_axes(multiscale, meta): + multiscale.pop("axes") + + +@pytest.mark.parametrize("version", VERSIONS) +def test_modern_ngff_without_axes_is_unreadable(tmp_path, version): + # A 5-D array in a non-TCZYX order whose required axes were removed. + path = write_ngff( + tmp_path / "exp/ctrl/a.ome.zarr", data((2, 3, 4, 6, 8)), "zctyx", version=version + ) + edit_multiscale(path, drop_axes) + with pytest.raises(NgffError, match=f"OME-NGFF {version} .*no axes"): + open_ngff(path) + pre = preflight(tmp_path / "exp", tmp_path / "o") + found = issue(pre.issues, "unreadable_ome_zarr") + assert found.is_error and found.paths == ("ctrl/a.ome.zarr",) + + +def test_ngff_without_version_or_axes_is_unreadable(tmp_path): + def unversioned(multiscale, meta): + multiscale.pop("axes") + multiscale.pop("version", None) + + path = edit_multiscale( + write_ngff(tmp_path / "a.ome.zarr", data((2, 3, 4, 6, 8)), "tczyx"), unversioned + ) + with pytest.raises(NgffError, match="states no OME-NGFF version"): + open_ngff(path) + assert not inspect_image(path, "a.ome.zarr").readable + + +@pytest.mark.parametrize("version", ["0.1", "0.2"]) +def test_early_ngff_axes_are_those_its_specification_fixes(tmp_path, version): + def early(multiscale, meta): + multiscale.pop("axes") + multiscale.pop("coordinateTransformations", None) + multiscale["datasets"] = [{"path": "0"}] + multiscale["version"] = version + + path = edit_multiscale( + write_ngff(tmp_path / "a.ome.zarr", data((1, 2, 3, 6, 8)), "tczyx"), early + ) + image = open_ngff(path) + assert (image.axes, image.version) == ("TCZYX", version) + + +def test_ngff_axes_without_metadata_follow_the_version(): + assert ngff_axes(None, 5, "0.1") == ngff_axes([], 5, "0.2") == "TCZYX" + with pytest.raises(NgffError, match="5-D"): + ngff_axes(None, 3, "0.2") + for version in ("", "0.3", "0.4", "0.5", "0.6"): + with pytest.raises(NgffError): + ngff_axes(None, 5, version) + with pytest.raises(NgffError, match="not a list"): + ngff_axes("tczyx", 5, "0.4") + + +# Issue #94: a malformed calibration is never dropped silently ------------------------------- + + +MALFORMED = { + "not a list": ({"type": "scale", "scale": [2.0, 0.5, 0.25]}, "are not a list"), + "in a file": ([{"type": "scale", "path": "scale.bin"}], "stored in a separate file"), + "wrong length": ([{"type": "scale", "scale": [0.5, 0.25]}], "2 values for 3 axes"), + "other type": ([{"type": "affine", "affine": [[1, 0]]}], "type 'affine'"), + "two scales": ([{"type": "scale", "scale": [1, 1, 1]}] * 2, "2 scale transformations"), + "not numbers": ([{"type": "scale", "scale": "1,1,1"}], "not a list of numbers"), + "zero and negative": ([{"type": "scale", "scale": [0.0, -1.0, 0.25]}], "gives 0.0 for axis Z"), + "too large": ([{"type": "scale", "scale": [10**400, 0.5, 0.25]}], "for axis Z"), +} + + +@pytest.mark.parametrize("case", sorted(MALFORMED)) +def test_a_malformed_calibration_says_what_is_wrong(tmp_path, case): + transforms, reason = MALFORMED[case] + path = write_ngff( + tmp_path / "a.ome.zarr", data((3, 6, 8)), "zyx", dataset_transforms=transforms + ) + sizes = open_ngff(path).physical_sizes + assert reason in sizes.invalid + assert sizes.z is None + assert inspect_image(path, "a.ome.zarr").physical_sizes.invalid == sizes.invalid + + +def test_a_malformed_image_scale_is_named(tmp_path): + path = write_ngff( + tmp_path / "a.ome.zarr", + data((6, 8)), + "yx", + scale=[0.5, 0.25], + multiscale_transforms=[{"type": "scale", "path": "scale.bin"}], + ) + sizes = open_ngff(path).physical_sizes + assert sizes == PhysicalSizes() + assert "the scale of the image is stored in a separate file" in sizes.invalid + + +@pytest.mark.parametrize( + "transforms", + [None, [], [{"type": "translation", "translation": [0.0, 0.0, 0.0]}]], +) +def test_a_usable_or_absent_calibration_is_not_invalid(tmp_path, transforms): + path = write_ngff( + tmp_path / "a.ome.zarr", data((3, 6, 8)), "zyx", dataset_transforms=transforms + ) + assert open_ngff(path).physical_sizes.invalid is None + + +def test_every_image_with_a_malformed_calibration_is_named_and_kept_in_the_report(tmp_path): + root = tmp_path / "exp" + malformed = {"type": "scale", "scale": [2.0, 0.5, 0.25]} # a dict, not a list + for name in ("a", "b"): + write_ngff( + root / f"ctrl/{name}.ome.zarr", data((3, 6, 8)), "zyx", dataset_transforms=malformed + ) + pre = preflight(root, tmp_path / "o") + assert pre.ok + found = issue(pre.issues, "invalid_physical_calibration") + assert not found.is_error + assert found.paths == ("ctrl/a.ome.zarr", "ctrl/b.ome.zarr") + assert ( + "ctrl/a.ome.zarr: the coordinateTransformations of level 0 are not a list" in found.message + ) + result = run_experiment(root, AxisMapping("ZYX", "ZYX"), output_folder=tmp_path / "o") + assert result.written, result.issues + assert "invalid_physical_calibration" in codes(result.issues) + report = json.loads((tmp_path / "o/discovery_report.json").read_text(encoding="utf-8")) + assert "invalid_physical_calibration" in [i["code"] for i in report["issues"]] + + +def test_a_stated_calibration_replaces_the_malformed_one(tmp_path): + root = tmp_path / "exp" + malformed = {"type": "scale", "scale": [2.0, 0.5, 0.25]} + write_ngff(root / "ctrl/a.ome.zarr", data((1, 6, 8)), "zyx", dataset_transforms=malformed) + out = tmp_path / "o" + out.mkdir() + (out / "calibration.csv").write_text( + "target,axis,size,unit\n.,X,0.5,um\n.,Y,0.5,um\n", encoding="utf-8" + ) + assert "invalid_physical_calibration" not in codes(preflight(root, out).issues) + + +def test_unexpected_errors_reading_the_scale_are_not_swallowed(tmp_path, monkeypatch): + # Only malformed metadata is recorded as such; a failure of the reader is not hidden. + def broken(*args, **kwargs): + raise RuntimeError("not understood") + + monkeypatch.setattr(zarr_input, "ngff_physical_sizes", broken) + path = write_ngff(tmp_path / "a.ome.zarr", data((2, 6, 8)), "zyx") + with pytest.raises(RuntimeError): + open_ngff(path) + meta = inspect_image(path, "a.ome.zarr") + assert not meta.readable and "not understood" in meta.error + + +def test_tiff_sizes_have_no_calibration_problem(tmp_path): + tifffile.imwrite(tmp_path / "a.tif", data((6, 8))) + assert inspect_image(tmp_path / "a.tif", "a.tif").physical_sizes.invalid is None diff --git a/tests/test_zarr_input.py b/tests/test_zarr_input.py index 1a9fd307..7e98cad8 100644 --- a/tests/test_zarr_input.py +++ b/tests/test_zarr_input.py @@ -77,7 +77,6 @@ def meta(path, shape, axes, kind="ome-zarr"): "CZYX", ), (["t", "c", "z", "y", "x"], 5, "TCZYX"), # OME-NGFF 0.3 names only - (None, 5, "TCZYX"), # 0.1 and 0.2: fixed order ([{"name": "angle", "type": "angle"}, {"name": "y"}, {"name": "x"}], 3, "QYX"), ([{"name": "frames", "type": "time"}, {"name": "y"}, {"name": "x"}], 3, "TYX"), ([{"name": "lambda", "type": "space"}, {"name": "y"}, {"name": "x"}], 3, "QYX"), @@ -87,11 +86,6 @@ def test_ngff_axes_become_letters(axes, ndim, letters): assert ngff_axes(axes, ndim) == letters -def test_ngff_axes_without_metadata_need_five_dimensions(): - with pytest.raises(NgffError): - ngff_axes(None, 3) - - # Opening stores ------------------------------------------------------------------------- @@ -791,17 +785,6 @@ def test_scale_too_large_for_a_float_is_unknown_and_does_not_block(tmp_path): assert preflight(root, tmp_path / "out").ok -def test_unexpected_scale_metadata_does_not_block_the_image(tmp_path, monkeypatch): - def broken(*args, **kwargs): - raise RuntimeError("not understood") - - monkeypatch.setattr(zarr_input, "ngff_physical_sizes", broken) - root = tmp_path / "exp" - path = write_ngff(root / "ctrl/a.ome.zarr", np.zeros((2, 8, 8), np.uint8), "zyx") - assert open_ngff(path).physical_sizes == PhysicalSizes() - assert preflight(root, tmp_path / "out").ok - - @pytest.mark.parametrize("where", ["dataset", "multiscale"]) def test_transformations_that_are_not_a_list_give_no_sizes(tmp_path, where): malformed = {"type": "scale", "scale": [2.0, 0.5, 0.25]} diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index d96a4100..61b1d6cb 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -1,6 +1,6 @@ { "noqa": { - "ANN": 54, + "ANN": 53, "C901": 0, "PLR0913": 3, "PLR0915": 0 @@ -10,7 +10,7 @@ "identical bodies: tests/test_axis_check.py:# plain, tests/test_header_axes.py:# plain": 0, "identical bodies: tests/test_cli.py:# bright_tree, tests/test_notebook_ui.py:# bright_tree": 0, "src/morphobricks_preprocessing/axis_review.py check_mapping: # executable lines (max #)": 72, - "src/morphobricks_preprocessing/axis_review.py review_axes: # executable lines (max #)": 106, + "src/morphobricks_preprocessing/axis_review.py review_axes: # executable lines (max #)": 67, "src/morphobricks_preprocessing/dataset.py _missing_issues: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/dataset.py _unbalanced_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/design.py _check_strict_values: private function of # lines without a docstring": 19, @@ -32,7 +32,7 @@ "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 779, "src/morphobricks_preprocessing/physical_sizes.py _imagej_sizes: private function of # lines without a docstring": 27, "src/morphobricks_preprocessing/physical_sizes.py _ome_sizes: private function of # lines without a docstring": 20, - "src/morphobricks_preprocessing/physical_sizes.py: # lines (max #)": 706, + "src/morphobricks_preprocessing/physical_sizes.py: # lines (max #)": 684, "src/morphobricks_preprocessing/planes.py _layout: private function of # lines without a docstring": 45, "src/morphobricks_preprocessing/planes.py _tiff_block_plane: private function of # lines without a docstring": 14, "src/morphobricks_preprocessing/planes.py _zarr_layout: private function of # lines without a docstring": 40, @@ -48,7 +48,7 @@ "tests/test_notebook_ui.py: # lines (max #)": 1537, "tests/test_planes.py: # lines (max #)": 818, "tests/test_run.py: # lines (max #)": 982, - "tests/test_zarr_input.py: # lines (max #)": 869, + "tests/test_zarr_input.py: # lines (max #)": 852, "tests/zarr_tree.py write_ngff: # parameters (max #)": 12 } } From 718e8b5bce8533dad3fade70a9317d28ce02145c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 16:53:37 +0000 Subject: [PATCH 2/3] Review #129: refuse OME-NGFF 0.1/0.2 stores; keep the calibration warning for a lost time step Owner decision (2 November 2026): no spec-default t, c, z, y, x. An OME-NGFF 0.1 or 0.2 store is unreadable_ome_zarr whatever it contains, with the message to re-save it as OME-NGFF 0.4 or later. ngff_axes loses its unused ndim. Review finding: invalid_physical_calibration disappeared once X and Y (and Z) were stated in calibration.csv, also when the malformed scale had lost the time step of a time series, which cannot be stated there: that loss became silent. The warning now stays until every size the malformed calibration leaves unknown is stated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 5 +- docs/contract.md | 4 +- docs/plan.md | 25 ++++--- docs/user_guide.md | 4 +- .../physical_sizes.py | 2 +- .../pixel_size_checks.py | 29 +++++--- src/morphobricks_preprocessing/zarr_input.py | 54 +++++++-------- tests/test_ngff_axes_calibration.py | 69 +++++++++++++++---- tests/test_zarr_input.py | 2 +- 9 files changed, 126 insertions(+), 68 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5e5cfe0e..9f92956e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -327,8 +327,9 @@ functions the tests use. `file_mapping_for`; issue #72): a misfit, or confirmed roles that contradict the declared axes (`ZYX` confirmed `TYX`), is `ome_zarr_axes_mismatch`. Use `file_mapping_for`, never `mapping.for_axes`, to get an image's mapping. Missing axes are never guessed (issue - #73): only OME-NGFF 0.1/0.2 fix them (`t, c, z, y, x`, 5-D); any other store without - axes is `unreadable_ome_zarr`. + #73): the version is read first, and OME-NGFF 0.1/0.2 stores are refused whatever they + contain (re-save as 0.4 or later; owner, 2 November 2026); any other store without axes + is `unreadable_ome_zarr` too, with no confirmation path. - **OME-Zarr calibration (issue #94):** a malformed `coordinateTransformations` (`ngff_scale.read_scale`) gives unknown sizes with the reason in `PhysicalSizes.invalid`, named by the warning `invalid_physical_calibration`; never a diff --git a/docs/contract.md b/docs/contract.md index 70b86308..ffb73cd2 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -51,7 +51,7 @@ channel order; an image without a C axis has one channel. | Missing axes | A file lacking axes the mapping has is read with those axes as length 1 only when the user confirmed this and only one arrangement fits; its coordinates for them are 0. | | Notebook readiness | Confirming a valid singleton mapping resolves its missing-axis error for running; folder, exclusion and all other metadata errors still block processing. | | Series | Only the first image series, at full-resolution level 0. A file with more than one series is an error. | -| OME-Zarr axes | Taken from the OME-NGFF metadata (0.4 and 0.5; 0.3 axis names): a time axis is T, a channel axis C, space axes named z, y, x are Z, Y, X; any other axis has no role. **Nothing is guessed** (issue #73): without `axes`, only OME-NGFF 0.1 and 0.2 have axes, because their specification fixes every image array as 5-D `t, c, z, y, x` (by the version the metadata states; a 0.1 or 0.2 array of another dimensionality is refused). A store of OME-NGFF 0.3, 0.4 or 0.5 without `axes` (which those versions require), and a store without `axes` that states no version or another one, is unreadable (`unreadable_ome_zarr`, naming what is missing); so are `axes` that are not a list. **Every OME-Zarr image is matched by the roles it declares** (issue #72), whatever its arrangement: with the experiment mapping's arrangement, a confirmed role that is not the axis's declared letter (`ZYX` confirmed `TYX` or `ZXY`; an axis of no known type, `QYX`, confirmed `CYX`) is the error `ome_zarr_axes_mismatch`, naming each image with its declared axes and confirmed roles, and the image is never read under them (`axes.ome_zarr_mapping`, also used by `file_mapping_for`). Images in another order are matched by role: a Z, C or T axis the experiment lacks is allowed only with length 1, an axis without a role only with length 1, and experiment roles the image lacks follow the missing-axes rule; otherwise `ome_zarr_axes_mismatch` is an error. An OME-Zarr image cannot be converted (§17), so these are not axis-check errors. | +| OME-Zarr axes | Taken from the OME-NGFF metadata (0.4 and 0.5; 0.3 axis names): a time axis is T, a channel axis C, space axes named z, y, x are Z, Y, X; any other axis has no role. **Nothing is guessed** (issue #73): the version the metadata state is read before the axes. An OME-NGFF 0.1 or 0.2 store is unreadable (`unreadable_ome_zarr`) whatever it contains: its metadata do not name its axes, and the order its specification fixes (`t, c, z, y, x`) is never assumed; the message says to re-save it as OME-NGFF 0.4 or later (owner decision, 2 November 2026). A store of OME-NGFF 0.3, 0.4 or 0.5 without `axes` (which those versions require), and a store without `axes` that states no version or another one, is unreadable too (`unreadable_ome_zarr`, naming what is missing; there is no confirmation path); so are `axes` that are not a list. **Every OME-Zarr image is matched by the roles it declares** (issue #72), whatever its arrangement: with the experiment mapping's arrangement, a confirmed role that is not the axis's declared letter (`ZYX` confirmed `TYX` or `ZXY`; an axis of no known type, `QYX`, confirmed `CYX`) is the error `ome_zarr_axes_mismatch`, naming each image with its declared axes and confirmed roles, and the image is never read under them (`axes.ome_zarr_mapping`, also used by `file_mapping_for`). Images in another order are matched by role: a Z, C or T axis the experiment lacks is allowed only with length 1, an axis without a role only with length 1, and experiment roles the image lacks follow the missing-axes rule; otherwise `ome_zarr_axes_mismatch` is an error. An OME-Zarr image cannot be converted (§17), so these are not axis-check errors. | | OME-Zarr images | Only level 0 (the first dataset of the first multiscales image) is read. A store with more than one image (several multiscales, a bioformats2raw folder with several series, a plate, a well with several fields) is an error (`multiple_series`). | | OME-Zarr labels | The label images of an image's OME-NGFF `labels` group (0.4 and 0.5) are listed with `ngff_label_names` and opened, level 0, with `open_ngff_labels` (by name; the name may be left out when there is one). They are never analysed; later bricks such as patch extraction use them as masks. | | Experiment mapping | Defined on the TIFF arrangement when the experiment has TIFF files; in an experiment of OME-Zarr images only, on their arrangement with the most axes. | @@ -297,7 +297,7 @@ row (follow-up P3b, normalization plan N43). | `different_pixel_sizes` | Warning | The physical pixel size (X by Y) differs between images, or the z-step differs between images with more than one depth slice. Sizes are compared in micrometres by the shared rule of decision D-a (§16): two sizes are the same when they differ by at most 0.1 % of the larger, on every axis. The warning is given exactly when, on some axis, the smallest and the largest size differ, that is when some two images differ. The images are grouped by single linkage (`pixel_size_groups`) and those outside the largest group are listed. When every image is linked to the others by near-equal sizes (1.0, 1.0009, 1.0018 µm), the sizes drift, no image stands out, and the message says that none is listed for that size. Until D86 sizes were rounded to four significant digits, so 1.0004 and 1.0006 µm differed (1 and 1.001); they are now the same. | | `missing_pixel_size` | Warning | The pixel size (or the z-step of images with more than one depth slice) is known for some images and not for others. Nothing is reported when no image has it. | | `unsupported_pixel_size_unit` | Warning | An image's metadata give a pixel size or z-step (OME `PhysicalSize{X,Y,Z}`, ImageJ resolution or `spacing`, OME-Zarr level-0 scale) in a length unit that is not one of decision D-a2 (§16), for example OME `in` or ImageJ `inch`. The size is not converted and the axis is treated as uncalibrated (as before); the warning names each image, its axes and the unit as written, so the missing calibration is never silent. A unit without a size, and a plain TIFF's resolution tags (decision D-c), are not reported. The run is not refused. | -| `invalid_physical_calibration` | Warning | An image's metadata give a calibration that cannot be used (issue #94): OME-Zarr `coordinateTransformations` that are not a list or of an unsupported type, more than one scale, a scale stored in a separate file or not one number per axis, or a scale value of X, Y, Z or T that is not a positive finite number. The message names each image and what is wrong. Its sizes are unknown on those axes (never a substituted value) and recorded as uncalibrated; the warning is in the `check` and `run` issues and the report (`discovery_report.json`). An image whose X, Y and (with several depth slices) Z are all stated in `calibration.csv` is not named. A calibration that is not given (no transformations, or no scale) is not reported. The run is not refused. | +| `invalid_physical_calibration` | Warning | An image's metadata give a calibration that cannot be used (issue #94): OME-Zarr `coordinateTransformations` that are not a list or of an unsupported type, more than one scale, a scale stored in a separate file or not one number per axis, or a scale value of X, Y, Z or T that is not a positive finite number. The message names each image and what is wrong. Its sizes are unknown on those axes (never a substituted value) and recorded as uncalibrated; the warning is in the `check` and `run` issues and the report (`discovery_report.json`). An image is not named once every size the malformed calibration leaves unknown (X and Y, and Z or T with more than one plane) is stated in `calibration.csv`; a time step cannot be stated there, so a lost time step of a time series is always named. A calibration that is not given (no transformations, or no scale) is not reported. The run is not refused. | | `different_channel_counts` | Error | The number of channels differs between images: the `C` dimension, or the RGB samples `S` when there is no `C`; once the axis roles are confirmed, the length of the C axis (1 without one), which replaces the header's count. The images not having the most common number are listed, with each number's image count. Every image of an experiment must have the same channels: split the others into another experiment or exclude them. | | `different_plane_counts` | Warning | The number of depth slices (`Z`), time points (`T`) or planes along any other dimension except X, Y, C and S differs between images. A missing dimension counts as 1. Channels are compared by `different_channel_counts`. | | `different_sizes` | Warning | The image width or height differs between images (TIFF and OME-Zarr). The other dimensions are compared by `different_plane_counts`. | diff --git a/docs/plan.md b/docs/plan.md index 67dad37f..086fb3a1 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1476,22 +1476,27 @@ Raised during review; to be decided before the milestones that need them: and others) because those offer conversion, and an OME-Zarr image is never converted; the fix is to correct the roles. An axis of no known type is therefore never given a role, in any arrangement. - (2) **Missing axes (#73)**, by the version the metadata states, per the OME-NGFF - specifications: 0.1 and 0.2 ("all image arrays are 5-dimensional with dimension order - (t, c, z, y, x)") keep `TCZYX` for a 5-D array and refuse other dimensionalities; 0.3, - 0.4 and 0.5 require `axes`, so a store without them is `unreadable_ome_zarr`; a store - without `axes` that states no version or another one is `unreadable_ome_zarr` too. - Such an image is refused rather than offered for confirmation as a plain stack: an - OME-Zarr image cannot be converted to a named-axis copy, and the declared-role match - refuses an axis of no known type. The spec-fixed axes of 0.1/0.2 are recorded as - `axes_from: "header"` (the metadata's version fixes them); no new `axes_from` value. + (2) **Missing axes (#73)**, by the version the metadata states, read before the axes: + an OME-NGFF 0.1 or 0.2 store is `unreadable_ome_zarr` whatever it contains, with the + message to re-save it as OME-NGFF 0.4 or later: no spec-default `t, c, z, y, x` is + applied (owner, 2 November 2026; the PR had kept it for 5-D arrays). 0.3, 0.4 and 0.5 + require `axes`, so a store without them is `unreadable_ome_zarr`; a store without + `axes` that states no version or another one is `unreadable_ome_zarr` too. Such an + image is refused, with no confirmation path, rather than offered for confirmation as a + plain stack (owner, 2 November 2026): an OME-Zarr image cannot be converted to a + named-axis copy, and the declared-role match refuses an axis of no known type. No new + `axes_from` value. (3) **Malformed calibration (#94)**: a calibration given but unusable (transformations not a list or of another type than scale, translation and identity, two scales, a scale in a file or not one number per axis, a scale value of X, Y, Z or T that is not a positive finite number) gives unknown sizes on those axes, with what is wrong in `PhysicalSizes.invalid`, and the warning `invalid_physical_calibration` naming every image (also when every image has it). A warning, not an error: the pixels are readable - and the image is uncalibrated, as design v0.4 §3 treats an image without a value. Only + and the image is uncalibrated, as design v0.4 §3 treats an image without a value + (owner, 2 November 2026: stays a warning). It disappears once every size the malformed + calibration leaves unknown is stated in `calibration.csv` (owner, 2 November 2026); a + time step cannot be stated there, so a lost time step of a time series stays named + (review finding: the PR dropped the warning when only X and Y were stated). Only these metadata problems are caught: any other exception while reading the scale makes the image unreadable instead of being hidden by a debug log. diff --git a/docs/user_guide.md b/docs/user_guide.md index 31483b4f..e0b360fe 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -863,7 +863,7 @@ continues. | `output_inside_experiment` | Error | The output folder is inside the experiment. Choose another. | | `no_images` | Error | No images to analyse (or all excluded). Check the folder. | | `unreadable_tiff` | Error | A TIFF file cannot be opened. Export it again, or exclude it. | -| `unreadable_ome_zarr` | Error | A `.zarr` folder is not an OME-Zarr image that can be read; the message says why. That includes an image whose metadata do not name its axes (OME-NGFF 0.3 to 0.5 require them; only 0.1 and 0.2 fix them as `t, c, z, y, x`): the axes are never guessed. Export it again as OME-Zarr (OME-NGFF 0.4 or 0.5) with its axes, or exclude it. | +| `unreadable_ome_zarr` | Error | A `.zarr` folder is not an OME-Zarr image that can be read; the message says why. That includes an image whose metadata do not name its axes (OME-NGFF 0.3 to 0.5 require them), and every OME-NGFF 0.1 or 0.2 store (their metadata never name the axes): the axes are never guessed. Re-save an 0.1 or 0.2 store as OME-NGFF 0.4 or later. Export it again as OME-Zarr (OME-NGFF 0.4 or 0.5) with its axes, or exclude it. | | `ome_zarr_axes_mismatch` | Error | An OME-Zarr image declares an axis with several planes that the experiment has no role for (or has no X/Y), or the confirmed roles give one of its axes another role than its metadata declares (for example its depth `z` confirmed as time `T`); the message names each image with its declared axes and the confirmed roles. An OME-Zarr image is read only with the roles it declares. Check the axis roles, or process it separately. | | `multiple_series` | Error | A file or OME-Zarr folder holds several images (positions, scenes, series, wells or fields). Export them separately, or exclude it. | | `unsupported_dtype` | Error | Unsupported pixel type (e.g. float16). Convert, or exclude it. | @@ -909,7 +909,7 @@ continues. | `different_pixel_sizes` | Warning | The physical pixel size or z-step (from the metadata or `calibration.csv`) differs between images by more than 0.1 % (for example different objectives). Images outside the largest group of matching sizes are listed; when sizes only change by small steps from image to image, none is listed. Check the microscope settings if this is not intended. | | `missing_pixel_size` | Warning | Some images have a pixel size (from the metadata or `calibration.csv`) and others do not, so it cannot be checked; images stated as measured in pixels count as without one, and the message says how many. Export them again with metadata, or state their sizes with `calibrate`, if needed. | | `unsupported_pixel_size_unit` | Warning | An image gives its pixel size or z-step in a unit that is not converted (for example inches); only pm, Å, nm, µm, mm, cm and m are. The message names each image, axis and unit. Those sizes are not used: the image is treated as uncalibrated on those axes. Export it again with the pixel size in a supported unit, or state it with `calibrate`, if the physical size matters. Not reported for an axis whose size is stated. | -| `invalid_physical_calibration` | Warning | An image's metadata give a pixel size, z-step or time step that cannot be used (for OME-Zarr: `coordinateTransformations` that are malformed, of an unsupported type, stored in a separate file, or a scale that is zero, negative or not a number); the message names each image and what is wrong. The image is treated as uncalibrated on those axes; no size is made up. Correct the metadata, or state the sizes with the `calibrate` command. | +| `invalid_physical_calibration` | Warning | An image's metadata give a pixel size, z-step or time step that cannot be used (for OME-Zarr: `coordinateTransformations` that are malformed, of an unsupported type, stored in a separate file, or a scale that is zero, negative or not a number); the message names each image and what is wrong. The image is treated as uncalibrated on those axes; no size is made up. Correct the metadata, or state the sizes with the `calibrate` command: the warning goes once every lost pixel size and z-step is stated. A lost time step cannot be stated, so its warning stays until the metadata are corrected. | | `calibration_axis_absent` | Warning | `calibration.csv` states a size for an axis some images do not have (for example a Z size for 2D images or projections); the message names them. No size is recorded for an axis an image does not have. Give the statement a narrower target, or no action if it is meant for the other images. | | `calibration_differs_from_header` | Warning | `calibration.csv` states a pixel size (or `pixels`) that differs from an image's metadata; the message names each image and axis with both sizes. The stated size is used. Correct or remove the statement if the metadata are right. | | `calibration_target_not_found` | Error | `calibration.csv` names a folder or image that is not in the experiment (names are compared exactly, case included). Correct the target, or remove the line. | diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 04906600..bbf852b4 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -490,7 +490,7 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: :func:`read_physical_sizes` gives: common spellings such as ``"µm"``, ``"um"`` or ``"micron"`` become ``"micrometer"``, other units are kept as written. - Nothing is guessed: without a level-0 scale (OME-NGFF 0.1 to 0.3) every size + Nothing is guessed: without a level-0 scale (OME-NGFF 0.3) every size is None. Nothing is dropped silently either (issue #94): with a malformed scale (transformations that are not a list or of another type, a scale stored in a separate file through ``path``, not one number per axis) every size is diff --git a/src/morphobricks_preprocessing/pixel_size_checks.py b/src/morphobricks_preprocessing/pixel_size_checks.py index f9e7bc20..dcebdf6f 100644 --- a/src/morphobricks_preprocessing/pixel_size_checks.py +++ b/src/morphobricks_preprocessing/pixel_size_checks.py @@ -76,24 +76,37 @@ def pixel_size_issues( ] +def _lost_roles(f: TiffMetadata) -> tuple[str, ...]: + """Return the roles whose size an image's malformed calibration leaves unknown. + + X and Y, and Z and T with more than one plane, that the header gives no size for; X and + Y when the problem concerns none of them (it is still named unless they are stated). + """ + roles = ("X", "Y", *(role for role in ("Z", "T") if f.length(role) > 1)) + lost = tuple(role for role in roles if not f.physical_sizes.known(role)) + return lost or ("X", "Y") + + def _invalid_issue( files: Sequence[TiffMetadata], by_role: Mapping[str, Mapping[str, RecordedSize]] ) -> list[ValidationIssue]: """Warn about calibrations the metadata give but that cannot be used (issue #94). Such an image is uncalibrated on the axes concerned (never a substituted value); without - this warning it would look merely uncalibrated. Every image is named with what is wrong. - An image whose X, Y and (with several depth slices) Z are all stated in - ``calibration.csv`` does not use the header's sizes, so it is not named. + this warning it would look merely uncalibrated. Every image is named with what is wrong, + unless each axis the malformed calibration leaves without a size (:func:`_lost_roles`) is + stated in ``calibration.csv``. A time step cannot be stated there, so an image with + several time points whose time step is lost is always named. """ found = {} for f in files: - roles = ("X", "Y", "Z") if f.length("Z") > 1 else ("X", "Y") + if not f.physical_sizes.invalid: + continue recorded = by_role[f.relative_path] - uses_header = any( - role not in recorded or recorded[role].source in (None, HEADER) for role in roles - ) - if f.physical_sizes.invalid and uses_header: + if any( + role not in recorded or recorded[role].source in (None, HEADER) + for role in _lost_roles(f) + ): found[f.relative_path] = f.physical_sizes.invalid if not found: return [] diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index 933f42a1..41649ec9 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -296,7 +296,7 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: # n except KeyError as exc: raise NgffError(f"the level-0 array {dataset_path!r} is missing") from exc version = str(meta.get("version") or first.get("version") or "") - axes = ngff_axes(first.get("axes"), len(array.shape), version) + axes = ngff_axes(first.get("axes"), version) if len(axes) != len(array.shape): raise NgffError(f"its metadata names {len(axes)} axes but the array has {len(array.shape)}") omero = meta.get("omero") if isinstance(meta.get("omero"), dict) else {} @@ -331,26 +331,30 @@ def _labels_group(group): # noqa: ANN001, ANN202 return labels if hasattr(labels, "group_keys") else None -def ngff_axes(axes: Any, ndim: int, version: str = "") -> str: # noqa: ANN401 +def ngff_axes(axes: Any, version: str = "") -> str: # noqa: ANN401 """Translate OME-NGFF axis metadata into one-letter codes. ``axes`` is the ``multiscales[0]["axes"]`` entry: a list of objects with - ``name`` and ``type`` (OME-NGFF 0.4 and 0.5) or of names (0.3). Nothing is - guessed when it is absent or empty (issue #73): only OME-NGFF 0.1 and 0.2, - whose specification fixes every image array as 5-D in the order ``t, c, z, - y, x``, have axes without naming them (:data:`SPEC_FIXED_AXES`, by the - ``version`` the metadata states). OME-NGFF 0.3 to 0.5 require the axes, and - a store that states another version or none does not say what its axes are: - both are refused. + ``name`` and ``type`` (OME-NGFF 0.4 and 0.5) or of names (0.3). ``version`` + is the version the metadata state, read before the axes. Nothing is guessed + (issue #73; owner decision, 2 November 2026): OME-NGFF 0.1 and 0.2 stores + (:data:`REFUSED_VERSIONS`), whose metadata never name the axes, are refused, + whatever they contain; OME-NGFF 0.3 to 0.5 require the axes, and a store that + states another version or none does not say what its axes are: a store + without axes is refused in every case. Raises ------ NgffError - If the axes are not given and the stated version does not fix them, or - are not a list. + If the version is 0.1 or 0.2, or the axes are not given, or are not a list. """ + if version in REFUSED_VERSIONS: + raise NgffError( + f"it is an OME-NGFF {version} store, whose metadata do not name its axes, so " + "they are not guessed; re-save it as OME-NGFF 0.4 or later" + ) if not axes: - return _spec_fixed_axes(ndim, version) + raise NgffError(_no_axes_problem(version)) if not isinstance(axes, list): raise NgffError("its axes metadata is not a list of axes") letters = [] @@ -371,34 +375,24 @@ def ngff_axes(axes: Any, ndim: int, version: str = "") -> str: # noqa: ANN401 return "".join(letters) -SPEC_FIXED_AXES = {"0.1": "TCZYX", "0.2": "TCZYX"} -"""The OME-NGFF versions whose specification fixes the axes without naming them: "All image -arrays are 5-dimensional with dimension order (t, c, z, y, x)".""" +REFUSED_VERSIONS = ("0.1", "0.2") +"""The OME-NGFF versions that are refused: their metadata never name the axes (their +specification fixes them as ``t, c, z, y, x``), and no axis order is ever assumed (owner +decision, 2 November 2026).""" AXES_REQUIRED_VERSIONS = ("0.3", "0.4", "0.5") """The OME-NGFF versions whose specification requires the multiscales ``axes`` field.""" -def _spec_fixed_axes(ndim: int, version: str) -> str: - """Return the axes the stated OME-NGFF version fixes for a store that names none. - - Raises :class:`NgffError` naming what is wrong when the version does not fix them. - """ - fixed = SPEC_FIXED_AXES.get(version) - if fixed is not None: - if ndim != len(fixed): - raise NgffError( - f"it does not name its axes, and OME-NGFF {version} fixes them only for " - f"5-D arrays (t, c, z, y, x); its array has {ndim} dimensions" - ) - return fixed +def _no_axes_problem(version: str) -> str: + """Return why a store whose metadata name no axes is refused, by the version it states.""" if version in AXES_REQUIRED_VERSIONS: - raise NgffError( + return ( f"its OME-NGFF {version} multiscales metadata has no axes, which that version " "requires; add the axes (name and type of each dimension) to its metadata" ) stated = f"states OME-NGFF version {version!r}" if version else "states no OME-NGFF version" - raise NgffError( + return ( f"it does not name its axes and {stated}, so what its axes are is not known; add " "the axes (name and type of each dimension) to its metadata" ) diff --git a/tests/test_ngff_axes_calibration.py b/tests/test_ngff_axes_calibration.py index 21774797..0af18ae4 100644 --- a/tests/test_ngff_axes_calibration.py +++ b/tests/test_ngff_axes_calibration.py @@ -155,30 +155,43 @@ def unversioned(multiscale, meta): assert not inspect_image(path, "a.ome.zarr").readable +@pytest.mark.parametrize("keep_axes", [False, True]) @pytest.mark.parametrize("version", ["0.1", "0.2"]) -def test_early_ngff_axes_are_those_its_specification_fixes(tmp_path, version): +def test_early_ngff_stores_are_refused_whatever_they_contain(tmp_path, version, keep_axes): + # Owner decision (2 November 2026): no spec-default t, c, z, y, x is ever applied; an + # OME-NGFF 0.1 or 0.2 store is refused, also a 5-D one and one that lists axes. def early(multiscale, meta): - multiscale.pop("axes") + if not keep_axes: + multiscale.pop("axes") multiscale.pop("coordinateTransformations", None) multiscale["datasets"] = [{"path": "0"}] multiscale["version"] = version + root = tmp_path / "exp" path = edit_multiscale( - write_ngff(tmp_path / "a.ome.zarr", data((1, 2, 3, 6, 8)), "tczyx"), early + write_ngff(root / "ctrl" / "a.ome.zarr", data((1, 2, 3, 6, 8)), "tczyx"), early ) - image = open_ngff(path) - assert (image.axes, image.version) == ("TCZYX", version) + with pytest.raises(NgffError, match=r"OME-NGFF 0\.[12] store.*re-save it as OME-NGFF 0\.4"): + open_ngff(path) + assert not inspect_image(path, "ctrl/a.ome.zarr").readable + found = issue(preflight(root, tmp_path / "o").issues, "unreadable_ome_zarr") + assert found.is_error and found.paths == ("ctrl/a.ome.zarr",) + assert "re-save it as OME-NGFF 0.4 or later" in found.message -def test_ngff_axes_without_metadata_follow_the_version(): - assert ngff_axes(None, 5, "0.1") == ngff_axes([], 5, "0.2") == "TCZYX" - with pytest.raises(NgffError, match="5-D"): - ngff_axes(None, 3, "0.2") +def test_ngff_axes_without_metadata_are_never_guessed(): + for version in ("0.1", "0.2"): + with pytest.raises(NgffError, match="re-save it as OME-NGFF 0.4"): + ngff_axes(None, version) + with pytest.raises(NgffError, match="re-save it as OME-NGFF 0.4"): + ngff_axes(["t", "c", "z", "y", "x"], version) for version in ("", "0.3", "0.4", "0.5", "0.6"): - with pytest.raises(NgffError): - ngff_axes(None, 5, version) + with pytest.raises(NgffError, match="add the axes"): + ngff_axes(None, version) + with pytest.raises(NgffError, match="add the axes"): + ngff_axes([], version) with pytest.raises(NgffError, match="not a list"): - ngff_axes("tczyx", 5, "0.4") + ngff_axes("tczyx", "0.4") # Issue #94: a malformed calibration is never dropped silently ------------------------------- @@ -266,6 +279,38 @@ def test_a_stated_calibration_replaces_the_malformed_one(tmp_path): assert "invalid_physical_calibration" not in codes(preflight(root, out).issues) +@pytest.mark.parametrize( + ("axes", "shape", "scale", "stated", "named"), + [ + # A time step cannot be stated in calibration.csv: a lost one is always named. + ("tyx", (3, 6, 8), [0.0, 0.5, 0.5], "XY", True), + # Only X is lost; stating X and Y replaces everything the malformed scale lost. + ("tyx", (3, 6, 8), [60.0, -1.0, 0.5], "XY", False), + # The whole scale is lost on a z-stack: X and Y stated still leave Z unknown. + ("zyx", (3, 6, 8), "dict", "XY", True), + ("zyx", (3, 6, 8), "dict", "XYZ", False), + ], +) +def test_the_warning_stays_until_every_lost_size_is_stated( + tmp_path, axes, shape, scale, stated, named +): + root = tmp_path / "exp" + transforms = ( + {"type": "scale", "scale": [2.0, 0.5, 0.25]} + if scale == "dict" + else [{"type": "scale", "scale": scale}] + ) + write_ngff(root / "ctrl/a.ome.zarr", data(shape), axes, dataset_transforms=transforms) + out = tmp_path / "o" + out.mkdir() + rows = "".join(f".,{axis},0.5,um\n" for axis in stated) + (out / "calibration.csv").write_text(f"target,axis,size,unit\n{rows}", encoding="utf-8") + found = [i for i in preflight(root, out).issues if i.code == "invalid_physical_calibration"] + assert bool(found) is named + if named: + assert found[0].paths == ("ctrl/a.ome.zarr",) + + def test_unexpected_errors_reading_the_scale_are_not_swallowed(tmp_path, monkeypatch): # Only malformed metadata is recorded as such; a failure of the reader is not hidden. def broken(*args, **kwargs): diff --git a/tests/test_zarr_input.py b/tests/test_zarr_input.py index 7e98cad8..b4959c00 100644 --- a/tests/test_zarr_input.py +++ b/tests/test_zarr_input.py @@ -83,7 +83,7 @@ def meta(path, shape, axes, kind="ome-zarr"): ], ) def test_ngff_axes_become_letters(axes, ndim, letters): - assert ngff_axes(axes, ndim) == letters + assert (ngff_axes(axes), len(letters)) == (letters, ndim) # Opening stores ------------------------------------------------------------------------- From eeae9d0ae29d476015441ea4b88dad9fd35f4295 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 17:03:22 +0000 Subject: [PATCH 3/3] Review #129: give the OME-Zarr store of a PP-11c reuse test its own arrangement test_an_ome_zarr_copy_changed_in_place_is_not_reused (from #126) wrote a 'zyx' store into an experiment confirmed ZYX -> ZXY: the same letters under contradicted roles, which issue #72 refuses (ome_zarr_axes_mismatch). The store is now 'tzyx' with one time point, matched by its declared roles, so the test checks the reuse of a changed chunk as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_convert_integrity.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_convert_integrity.py b/tests/test_convert_integrity.py index 569d068e..3cf36be6 100644 --- a/tests/test_convert_integrity.py +++ b/tests/test_convert_integrity.py @@ -155,7 +155,10 @@ def test_an_ome_zarr_copy_changed_in_place_is_not_reused(tmp_path): from zarr_tree import write_ngff exp, copies = swapped_experiment(tmp_path / "exp", names=("c/a.tif",)), tmp_path / "copies" - write_ngff(exp / "c" / "z.zarr", np.arange(2 * 9 * 6, dtype=np.uint8).reshape(2, 9, 6), "zyx") + # Another arrangement than the TIFFs', so it is matched by its declared roles: a "zyx" + # store under the experiment's swapped roles contradicts its metadata (issue #72). + pixels = np.arange(2 * 9 * 6, dtype=np.uint8).reshape(1, 2, 9, 6) + write_ngff(exp / "c" / "z.zarr", pixels, "tzyx") assert convert_experiment(exp, SWAPPED, copies).written chunk = max( (p for p in (copies / "c" / "z.zarr").rglob("*") if p.is_file()),