From 5924b74e59166f43b335bffb981c2ee8e7c30d80 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 19:38:52 +0000 Subject: [PATCH 01/11] Accept a folder named single as a one-member level In the MorphoBricks convention (CellPatcher's trees, MorphoRelate's data laid out as /single/) a level written "single" means "this level exists with one member". Folder discovery refused any folder named single (reserved_folder_name), so such experiments could not be read. A folder named single in lower case is now that level's value "single" when it is the only folder in its place (system folders aside): /single/ gives biological_repeat and technical_repeat "single", the same labels as /. The reservation came with the initial plan and had no recorded reason beyond keeping "single" for missing levels. What it protects is that "single" keeps one meaning, so two cases stay errors under the same code: another spelling ("Single" would be a label different from the "single" written for missing levels), and a single folder beside other folders (its level would have several members, one of them called single). Decision recorded as plan section 12 question 21; contract section 1 and 8, the user guide, AGENTS.md and the notebook's label help updated. The CSV, the manifest and their versions are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 7 +- docs/contract.md | 4 +- docs/plan.md | 20 ++++- docs/user_guide.md | 10 ++- src/morphobricks_preprocessing/discovery.py | 40 ++++++++-- src/morphobricks_preprocessing/notebook_ui.py | 3 +- tests/test_discovery.py | 80 ++++++++++++++++++- tests/test_run.py | 21 +++++ 8 files changed, 166 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d432e85c..149ef6fa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -148,8 +148,10 @@ functions the tests use. - **Hierarchy:** `////.tif`. - **Leaf folders:** a leaf folder has images and no subfolders. Only TIFFs directly in leaf folders are analysed. -- **Missing levels:** a branch may stop early; missing labels are the reserved value - `single`. A folder *named* `single` (any case) is an error. +- **Missing levels:** a branch may stop early; missing labels are the value `single`. A + folder *named* `single` (lower case) that is the only folder in its place is that level's + value `single` (a level with one member, plan §12 question 21); spelled in another case, + or beside other folders, it is the error `reserved_folder_name`. - **Uniqueness:** labels are unique only among siblings, compared case-insensitively; siblings differing only in case are an error. Groups are identified by the full label path. @@ -313,6 +315,7 @@ Technical details go to `logging` (shown in the notebook's "Technical details" p | OME-Zarr (Q12) | Input may be TIFF and/or OME-Zarr (folders ending in `.zarr`), mixed. OME-Zarr axes come from their metadata and are matched by role; level 0 of a single image only, several images per store is `multiple_series`. TIFF choice once per experiment (`convert_tiffs`): read directly (default) or convert to `/ome_zarr` (OME-NGFF 0.4) and compute rows from the copies; switching recomputes TIFF rows and removes unused copies. Reads OME-NGFF 0.4 and 0.5 (0.5 needs zarr 3). In the notebook: its own step-3 card "(Optional) Image conversion" after Settings (Q13): radio buttons (read directly = recommended), a disk-space warning (amber, red when the estimate exceeds free space, never blocking), and conversion settings (chunk size, compression + level, pyramid levels) shown only when converting. CLI `--convert-tiff` with `--chunk-size`, `--compression`, `--compression-level`, `--pyramid-levels`. No overlap setting: chunks never overlap | | System files (Q5) | Ignored silently; `single` check and sibling uniqueness are case-insensitive | | Too-deep folders (Q7) | Error | +| Folders named `single` (Q21) | Accepted as the level's value `single` when lower case and the only folder in its place (MorphoBricks one-member level, MorphoRelate M3); another case or other folders beside it stay `reserved_folder_name` | | Channel names (Q1) | Manifest only (from OME), warning `channel_names_differ`; CSV stays at schema 1 | | Plane coordinates (Q17) | `z`, `c`, `t` always written (0 for absent axes), `plane_id` always `::z=…,c=…,t=…`; CSV schema 2, version-1 tables recomputed on the next run | | OME-NGFF labels (Q18) | Read-only helpers `ngff_label_names`/`open_ngff_labels` for later bricks (CellPatcher masks); label images are never analysed and the outputs do not change | diff --git a/docs/contract.md b/docs/contract.md index ceabb82a..51556957 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -23,7 +23,7 @@ whole experiment; any change to them goes through the normalization brick. | Leaf folder | A folder that contains images and no subfolders. Only images directly in leaf folders are analysed. | | Missing levels | A branch may stop early; the missing labels are `single`. If the root itself is the leaf, all three labels are `single`. | | Skipped levels | Not possible: a level only exists below its parent level. | -| Reserved name | A folder named `single` (in any upper/lower case) is an error. | +| Folders named `single` | `single` is the MorphoBricks value of a level with one member, the value missing levels get. A folder named `single` in lower case is accepted as that value when it is the only folder in its place (system folders aside): `/single/.tif` gives the biological repeat and the technical repeat `single`, the labels of `/.tif`. It stays an error (`reserved_folder_name`) when it is spelled in another case (`Single`, a different label) or has other folders beside it (its level would then have several members, one called `single`). | | Uniqueness | Folder names need only be unique among siblings, ignoring case: siblings whose names differ only in case are an error. | | Depth | At most three folder levels below the root. A folder below the technical-repeat level is an error. | | System files | Dot-files and dot-folders (`.DS_Store`, `._*`, `.ipynb_checkpoints`), `Thumbs.db`, `desktop.ini` and `__MACOSX` are ignored without a warning. | @@ -92,7 +92,7 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Mixed dtypes; unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images, or converting TIFF images to OME-Zarr, 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; 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`; sibling folders differing only in case; images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; output folder inside the experiment; no usable planes | Processing is blocked | +| Error | Mixed dtypes; unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images, or converting TIFF images to OME-Zarr, 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; 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); sibling folders differing only in case; images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; output folder inside the experiment; no usable planes | Processing is blocked | | Warning | Files that are not images; empty folders; images outside leaf folders; images excluded by the user, or excluded paths that match no image; channel names differing between files; non-finite pixels; different image sizes with a valid mapping; planes without finite pixels while others remain; planes over the memory limit that are read in blocks | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and diff --git a/docs/plan.md b/docs/plan.md index 3725af9a..d0c52dda 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -65,7 +65,9 @@ inside leaf folders are analysed. If the experiment root itself contains images subfolders, it is the leaf folder and every label is `single`. During preflight, show a tree/summary of the inferred labels and ask the user to confirm -or correct it. The folder name `single` is reserved; report it as an error. Report +or correct it. A folder may be named `single` (lower case) when it is the only folder in +its place: it is then a level with one member, labelled `single` like a missing level +(question 21). Spelled in another case, or beside other folders, it is an error. Report warnings for files that are not images, empty folders and images outside leaf folders, and explain that they will not be analysed. @@ -213,7 +215,8 @@ Block processing for: mixed dtypes; unreadable TIFF files or OME-Zarr folders in folders (or OME-Zarr images without the `zarr` extra); OME-Zarr images whose declared axes do not fit the experiment's roles; unsupported sample formats or axes; missing/duplicate X or Y, duplicate roles or inconsistent mapping; a plane -that cannot be read within the memory limit, not even in blocks; a folder named `single`; sibling folders whose names differ only in +that cannot be read within the memory limit, not even in blocks; a folder named `single` +in another case or beside other folders; sibling folders whose names differ only in case; a folder below the technical-repeat level; an output folder inside the experiment; no usable planes. System files (dot-files, `Thumbs.db`, `desktop.ini`, `__MACOSX`) are ignored silently. @@ -519,3 +522,16 @@ Raised during review; to be decided before the milestones that need them: table with blank `position_id` is filled on the next run from the paths, without reading pixels. The normalization contract's note "never inferred from file names" (§8.2) predates D02 and is to be aligned in that repository.* +21. A folder named `single` was an error, but in the MorphoBricks convention (CellPatcher's + trees, MorphoRelate's data such as `/single/`) a level written + `single` means "this level exists with one member". Can such a folder be read? + *Decided (October 2026, for MorphoRelate milestone M3): yes. A folder named `single` in + lower case is accepted as its level's value `single` when it is the only folder in its + place (system folders such as `.ipynb_checkpoints` and `__MACOSX` aside), so + `/single/` gives `biological_repeat` and `technical_repeat` + `single`, the labels of `/`. The reservation had no recorded reason + beyond keeping `single` for missing levels; what it protects is that `single` keeps one + meaning, so two cases stay errors (`reserved_folder_name`): another spelling (`Single` + would be a label different from the `single` written for missing levels), and a + `single` folder with other folders beside it (the level would have several members, + one called `single`). The CSV, the manifest and their versions are unchanged.* diff --git a/docs/user_guide.md b/docs/user_guide.md index b277ba6a..e7fe38cf 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -74,8 +74,12 @@ experiment/ it where you would put a TIFF file. TIFF and OME-Zarr images can be mixed. - Only images in folders **without subfolders** are analysed. Images next to subfolders are reported and skipped. -- Do not name a folder `single`, and do not use two neighbouring folder names that differ - only in upper/lower case (`Control` and `control`). Both are errors. +- A folder may be named `single` (in lower case) when it is the only folder in its place: + like a missing level, it means the level has one member, so `treated/single/img_01.tif` + has the biological repeat `single`. A folder named `Single` or `SINGLE`, or a `single` + folder next to other folders, is an error. +- Do not use two neighbouring folder names that differ only in upper/lower case + (`Control` and `control`). This is an error. - Each image is one position (field of view), named after its file without the extension: `img_01.tif` and `img_01.ome.zarr` are both position `img_01`. The same name may be used in different folders (`tech_rep1/img_01.tif` and `tech_rep2/img_01.tif` are two @@ -312,7 +316,7 @@ continues. |---|---|---| | `experiment_not_found` | Error | The folder does not exist. Select an existing folder. | | `unreadable_folder` | Error | A folder could not be opened. Check permissions. | -| `reserved_folder_name` | Error | A folder is named `single`. Rename it. | +| `reserved_folder_name` | Error | A folder is named `single` in another case (`Single`), or a folder named `single` has other folders beside it. Rename it to `single` in lower case, or give it a name of its own. | | `case_conflict` | Error | Neighbouring folders differ only in case. Rename or merge them. | | `duplicate_position_id` | Error | Two images in one folder have the same name apart from the extension or upper/lower case (`img_01.tif` and `img_01.ome.zarr`), so their positions cannot be told apart. Rename one, or move it out of the experiment. | | `folder_too_deep` | Error | Folders below the technical repeat. Move the images up, or select a lower folder as the experiment. | diff --git a/src/morphobricks_preprocessing/discovery.py b/src/morphobricks_preprocessing/discovery.py index fcc7e375..38c425a2 100644 --- a/src/morphobricks_preprocessing/discovery.py +++ b/src/morphobricks_preprocessing/discovery.py @@ -3,7 +3,10 @@ The experiment root holds up to three levels of folders (condition, biological repeat, technical repeat). A folder with images and no subfolders is a leaf; its labels are the folder names on the way down, with ``single`` for the levels -the branch does not have. The rules are in ``docs/contract.md`` §1. +the branch does not have. A folder may itself be named ``single`` (in lower +case) when it is the only folder in its place: like a missing level, it is a +level with one member (the MorphoBricks convention). The rules are in +``docs/contract.md`` §1. Images are TIFF files (``.tif``, ``.tiff``) and OME-Zarr images, which are folders whose names end in ``.zarr`` (usually ``.ome.zarr``). An OME-Zarr folder @@ -116,7 +119,8 @@ class LeafFolder: Path relative to the experiment root with ``/`` separators; ``"."`` when the experiment root itself is the leaf. condition, biological_repeat, technical_repeat - Labels from the folder names; ``"single"`` for missing levels. + Labels from the folder names; ``"single"`` for missing levels and for + folders named ``single``. images Relative paths of the images (TIFF files and OME-Zarr folders) directly in this folder, sorted. @@ -247,6 +251,7 @@ def __init__(self, root: Path): self.leaves: list[LeafFolder] = [] self.issues: list[ValidationIssue] = [] self.reserved: list[str] = [] + self.single_with_siblings: list[str] = [] self.case_conflicts: list[str] = [] self.too_deep: list[str] = [] self.unreadable: list[str] = [] @@ -307,8 +312,17 @@ def _check_names(self, subfolders: list[Path]) -> None: by_key: dict[str, list[Path]] = {} for path in subfolders: by_key.setdefault(path.name.casefold(), []).append(path) - if path.name.casefold() == SINGLE: + if path.name.casefold() != SINGLE: + continue + # A folder named "single" gives its level the value a missing level gets: + # one member. Two cases would make "single" ambiguous, so they stay errors: + # another spelling ("Single" would be a different label from the "single" + # written for missing levels), and other folders beside it (the level + # would then have several members, one of them called "single"). + if path.name != SINGLE: self.reserved.append(self.relative(path)) + elif len(subfolders) > 1: + self.single_with_siblings.append(self.relative(path)) for paths in by_key.values(): if len(paths) > 1: self.case_conflicts.extend(self.relative(path) for path in paths) @@ -340,10 +354,24 @@ def finish(self) -> tuple[ValidationIssue, ...]: issues.append( error( "reserved_folder_name", - f'"{SINGLE}" is reserved for missing hierarchy levels and cannot be ' - "used as a folder name.", + f'A folder named "{SINGLE}" marks a level with one member, and its label ' + f'is always written "{SINGLE}" in lower case. Written in another case it ' + "would be a different label.", self.reserved, - f'Rename these folders to something other than "{SINGLE}".', + f'Rename these folders to "{SINGLE}" in lower case, or give them a name of ' + "their own.", + ) + ) + if self.single_with_siblings: + issues.append( + error( + "reserved_folder_name", + f'A folder named "{SINGLE}" marks a level with one member, so it must be ' + "the only folder in its place. These folders have other folders beside " + "them.", + self.single_with_siblings, + "Give these folders a name of their own (for example rep1), or move the " + "other folders out.", ) ) if self.case_conflicts: diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index ef98e33f..aeb65463 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -356,7 +356,8 @@ def _build_folder_step(self, folder: str, output: str) -> None: [ ui.help_text( "Each row is a folder of images and the labels its images get. " - "single means the folder level is missing. To change a label, " + "single means the level has one member: the folder level is " + "missing, or the folder is named single. To change a label, " "rename the folder and press Check folder again." ), self.labels_table, diff --git a/tests/test_discovery.py b/tests/test_discovery.py index c99d8735..ecc80018 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -220,18 +220,92 @@ def test_is_tiff_name(name, expected): assert is_tiff_name(name) is expected +# Folders named single (the MorphoBricks one-member level) ----------------------------- + + +def test_folder_named_single_is_a_level_with_one_member(tmp_path): + # Shaped like an experiment whose conditions have one biological repeat, written + # as a folder named single: /single/. + make_tree( + tmp_path, + [ + "control/single/img_01.tif", + "control/single/img_02.tif", + "treated/single/img_01.tif", + "treated/single/img_02.tif", + ], + ) + result = discover_experiment(tmp_path) + assert result.ok and result.issues == () + assert leaf_labels(result) == { + "control/single": ("control", SINGLE, SINGLE), + "treated/single": ("treated", SINGLE, SINGLE), + } + assert result.labels_for("treated/single/img_02.tif") == ("treated", SINGLE, SINGLE) + + +def test_folder_named_single_gives_the_labels_of_a_missing_level(tmp_path): + # The same labels as images placed directly in the condition folder. + named = discover_experiment(make_tree(tmp_path / "named", ["control/single/a.tif"])) + missing = discover_experiment(make_tree(tmp_path / "missing", ["control/a.tif"])) + assert named.ok and missing.ok + assert named.leaves[0].labels == missing.leaves[0].labels == ("control", SINGLE, SINGLE) + + +@pytest.mark.parametrize( + ("tree", "labels"), + [ + (["single/rep1/a.tif"], ("single", "rep1", SINGLE)), + (["control/single/tech_1/a.tif"], ("control", SINGLE, "tech_1")), + (["control/rep1/single/a.tif"], ("control", "rep1", SINGLE)), + (["single/single/single/a.tif"], (SINGLE, SINGLE, SINGLE)), + ], +) +def test_folder_named_single_at_any_level(tmp_path, tree, labels): + result = discover_experiment(make_tree(tmp_path, tree)) + assert result.ok and result.issues == () + assert [leaf.labels for leaf in result.leaves] == [labels] + + +def test_folder_named_single_ignores_system_folders_beside_it(tmp_path): + make_tree(tmp_path, ["control/single/a.tif", "control/.ipynb_checkpoints/x.txt"]) + (tmp_path / "control" / "__MACOSX").mkdir() + result = discover_experiment(tmp_path) + assert result.ok and result.issues == () + assert leaf_labels(result) == {"control/single": ("control", SINGLE, SINGLE)} + + # Errors ------------------------------------------------------------------------------- -@pytest.mark.parametrize("name", ["single", "Single", "SINGLE"]) -def test_folder_named_single_is_an_error(tmp_path, name): +@pytest.mark.parametrize("name", ["Single", "SINGLE"]) +def test_folder_named_single_in_another_case_is_an_error(tmp_path, name): + # "Single" would be a different label from the "single" written for one-member + # levels, so only the lower-case spelling stands for that value. make_tree(tmp_path, [f"control/{name}/a.tif", "treated/a.tif"]) result = discover_experiment(tmp_path) assert not result.ok found = issue(result, "reserved_folder_name") assert found.severity is Severity.ERROR assert found.paths == (f"control/{name}",) - assert found.action + assert "lower case" in found.action + + +def test_folder_named_single_beside_other_folders_is_an_error(tmp_path): + # "single" says the level has one member; beside rep2 it would have two. + make_tree(tmp_path, ["control/single/a.tif", "control/rep2/a.tif", "treated/a.tif"]) + result = discover_experiment(tmp_path) + assert not result.ok + found = issue(result, "reserved_folder_name") + assert found.severity is Severity.ERROR + assert found.paths == ("control/single",) + assert "only folder" in found.message + + +def test_folder_named_single_with_an_empty_sibling_is_an_error(tmp_path): + make_tree(tmp_path, ["control/single/a.tif", "control/rep2/"]) + result = discover_experiment(tmp_path) + assert issue(result, "reserved_folder_name").paths == ("control/single",) def test_folders_differing_only_in_case_are_an_error(tmp_path): diff --git a/tests/test_run.py b/tests/test_run.py index a9a7d59b..501b3ea5 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -227,6 +227,27 @@ def test_every_row_has_its_position_id(experiment): assert all("position_id" in f for f in data["files"]) +def test_folder_named_single_runs_as_a_one_member_level(tmp_path): + # /single/: the repeat level exists with one member. + root = tmp_path / "exp" + for seed, rel in enumerate(["ctrl/single/a.tif", "ctrl/single/b.tif", "drug/single/a.tif"]): + write_data(root / rel, stack(seed), "ZYX") + assert preflight(root).ok + records = read_plane_histograms(run_experiment(root, ZYX).csv_path) + assert { + (r.condition, r.biological_repeat, r.technical_repeat, r.position_id) for r in records + } == { + ("ctrl", SINGLE, SINGLE, "a"), + ("ctrl", SINGLE, SINGLE, "b"), + ("drug", SINGLE, SINGLE, "a"), + } + assert {r.source_relative_path for r in records} == { + "ctrl/single/a.tif", + "ctrl/single/b.tif", + "drug/single/a.tif", + } + + def test_position_ids_that_are_text_stay_text(tmp_path): root = tmp_path / "exp" for seed, rel in enumerate(["ctrl/001.tif", "ctrl/NA.tif", "drug/TRUE.tif"]): From 561b3179ffa82222c78bf0e3b8749350616da646 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 19:43:09 +0000 Subject: [PATCH 02/11] Read each axis's own unit and carry physical sizes in the header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_physical_sizes applied the X unit to Y and Z: an OME-TIFF with PhysicalSizeXUnit="nm" and PhysicalSizeYUnit="µm" reported Y in nanometres, and a Z size without PhysicalSizeZUnit took X's unit instead of the OME default (micrometres). PhysicalSizes now has x_unit, y_unit and z_unit, and unit(role) returns each axis's own unit: OME PhysicalSize{X,Y,Z}Unit (micrometres when left out), ImageJ unit, yunit and zunit (X's unit when left out, as ImageJ writes them). space_unit stays the unit of X, and sizes built with space_unit alone keep it for every space axis, so existing callers see no change. OME-Zarr copies now write each axis's own unit. inspect_image / TiffMetadata gains the field physical_sizes, read from the header that is already open, so callers such as MorphoRelate's `inspect` need not open each file twice. Existing fields are unchanged; the new field is last and defaults to empty sizes. PhysicalSizes and read_physical_sizes move to the new module physical_sizes (zarr_input needs them next without importing zarr_export); they stay importable from zarr_export and the package. Decision recorded as plan section 12 question 22; contract section 10 and the AGENTS.md map updated. The CSV, the manifest and their versions are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 4 +- docs/contract.md | 2 +- docs/plan.md | 14 ++ src/morphobricks_preprocessing/__init__.py | 3 +- src/morphobricks_preprocessing/metadata.py | 8 + .../physical_sizes.py | 223 ++++++++++++++++++ src/morphobricks_preprocessing/zarr_export.py | 143 +---------- tests/test_metadata.py | 41 ++++ tests/test_physical_sizes.py | 118 +++++++++ tests/test_zarr_export.py | 36 ++- 10 files changed, 448 insertions(+), 144 deletions(-) create mode 100644 src/morphobricks_preprocessing/physical_sizes.py create mode 100644 tests/test_physical_sizes.py diff --git a/AGENTS.md b/AGENTS.md index 149ef6fa..cd300bb5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,9 @@ src/morphobricks_preprocessing/ discovery.py folder walk, leaf folders, labels (condition / biological_repeat / technical_repeat), position_id per image and the duplicate check metadata.py TIFF header inspection (tifffile, no pixel reads), dtype/dimension/series checks, - OME channel names + significant bits + OME channel names + significant bits, physical sizes (TiffMetadata.physical_sizes) + physical_sizes.py PhysicalSizes (pixel sizes, z-step, time step, a unit per axis) and + read_physical_sizes (OME / ImageJ TIFF metadata); also importable from zarr_export validation.py ValidationIssue (severity, code, message, paths, action), error()/warning() builders axes.py AxisMapping (role per dimension), prefill, check_mapping, review_axes (one mapping applied to every file, missing axes as singletons only when confirmed) diff --git a/docs/contract.md b/docs/contract.md index 51556957..e6077ffe 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -158,7 +158,7 @@ presentation does not change the conversion rules below. | Level 0 | The source values, unchanged, in the source pixel type; chunks of one plane along Z, C and T and at most the chunk size (16 to 8,192 px, default 1,024) in X and Y | | Further levels | 2 x 2 block means rounded to the pixel type; automatic (default: until a plane fits in 512 x 512, at most 8 levels in all), none, or 1 to 7 levels; for viewing only | | Large planes | A plane read in blocks is written block by block; its further levels are then built from the stored level above, in regions of even size, and equal those of a plane written whole | -| Scale | Physical pixel sizes, z-step and time step from OME or ImageJ metadata, with OME-NGFF units; 1 when unknown | +| Scale | Physical pixel sizes, z-step and time step from OME or ImageJ metadata, with OME-NGFF units, each axis with its own unit (OME `PhysicalSizeXUnit`, `PhysicalSizeYUnit`, `PhysicalSizeZUnit`, micrometres when left out; ImageJ `unit`, `yunit`, `zunit`); 1 when unknown | | Channels | `omero` block with channel names from metadata (or `Channel `) and each channel's observed minimum and maximum as display window | | Updates | Copies of unchanged images are kept, of new or changed images rewritten, and of images no longer analysed removed; a failed run leaves the earlier copies as they were (see §9 for publication) | | Switching | Turning conversion on or off recomputes every TIFF row; turning it off removes the copies the manifest recorded | diff --git a/docs/plan.md b/docs/plan.md index d0c52dda..0baa2db5 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -535,3 +535,17 @@ Raised during review; to be decided before the milestones that need them: would be a label different from the `single` written for missing levels), and a `single` folder with other folders beside it (the level would have several members, one called `single`). The CSV, the manifest and their versions are unchanged.* +22. Physical pixel sizes: `read_physical_sizes` applied the X unit to Y and Z, and callers + (MorphoRelate milestone M3's `inspect`) had to open each file a second time to get + them. Should each axis keep its own unit, and should the header inspection carry them? + *Decided (October 2026): yes to both. `PhysicalSizes` gains `x_unit`, `y_unit` and + `z_unit`; `unit(role)` returns each axis's own unit, from OME `PhysicalSizeYUnit` and + `PhysicalSizeZUnit` (the OME default, micrometres, when left out, as for X) or ImageJ + `yunit` and `zunit` (the X unit when left out, as ImageJ writes them). `space_unit` + stays the unit of X, and sizes made with `space_unit` alone keep it for every space + axis, so existing callers are unchanged. `inspect_image` (`TiffMetadata`) gains the + field `physical_sizes`, read from the header already open, for information only; the + existing fields are unchanged. OME-Zarr copies of images whose Y or Z unit differs + from X now write that axis's own unit. `PhysicalSizes` and `read_physical_sizes` moved + to the module `physical_sizes` and stay importable from `zarr_export` and the package. + The CSV, the manifest and their versions are unchanged.* diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index aa766962..27c45799 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -50,6 +50,7 @@ inspect_ome_zarr, inspect_tiff, ) +from .physical_sizes import PhysicalSizes, read_physical_sizes from .planes import ( DEFAULT_MEMORY_LIMIT, Block, @@ -71,10 +72,8 @@ ConversionEstimate, ConversionSettings, OmeZarrWriter, - PhysicalSizes, estimate_conversion, ome_zarr_relative_path, - read_physical_sizes, ) from .zarr_input import ( NgffError, diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index f63e4bd7..8af877a1 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -25,6 +25,7 @@ from morphobricks_normalization.dtypes import UnsupportedDtypeError, check_supported from .discovery import DiscoveryResult +from .physical_sizes import PhysicalSizes, _tiff_sizes from .validation import ValidationIssue, error, has_errors, plural, warning from .zarr_input import NgffError, ZarrUnavailableError, is_zarr_name, open_ngff @@ -70,6 +71,11 @@ class TiffMetadata: files), from OME metadata; None when not given. For information only. kind ``"tiff"`` or ``"ome-zarr"``. + physical_sizes + Pixel sizes, z-step and time step with their units, from OME or ImageJ + metadata (TIFF) or the OME-NGFF scale (OME-Zarr), as + :func:`.physical_sizes.read_physical_sizes` gives them; all None when + not given or the image does not open. For information only. """ relative_path: str @@ -83,6 +89,7 @@ class TiffMetadata: channel_names: tuple[str | None, ...] = () significant_bits: int | None = None kind: str = TIFF + physical_sizes: PhysicalSizes = PhysicalSizes() @property def ndim(self) -> int: @@ -170,6 +177,7 @@ def inspect_tiff(path: str | os.PathLike, relative_path: str | None = None) -> T series_count=len(series), page_count=len(tif.pages), **_ome_details(tif), + physical_sizes=_tiff_sizes(tif), ) except Exception as exc: # any failure to parse means the file cannot be used logger.debug("Cannot read the TIFF header of %s", path, exc_info=True) diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py new file mode 100644 index 00000000..eae06cc0 --- /dev/null +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -0,0 +1,223 @@ +"""Physical pixel sizes from image metadata, without reading pixels. + +The size of one pixel along X and Y, the z-step and the time step come from OME +or ImageJ metadata in TIFF files. Units are given as OME-NGFF (UDUNITS-2) names +such as ``"micrometer"`` and ``"second"``, one per axis, so that an image whose +Y pixels are measured in another unit than its X pixels is described as it is. +Sizes are never guessed: what the metadata does not give is None. + +They are recorded with an image's header (:func:`.metadata.inspect_image`) and +written as the scale of OME-Zarr copies (``docs/contract.md`` §10). The sizes of +OME-Zarr images are read from their OME-NGFF metadata by :mod:`.zarr_input`. +""" + +from __future__ import annotations + +import logging +import math +import os +from dataclasses import dataclass +from xml.etree import ElementTree + +import tifffile + +logger = logging.getLogger(__name__) + +# Units in OME and ImageJ metadata, and their OME-NGFF (UDUNITS-2) names. +_SPACE_UNITS = { + "pm": "picometer", + "nm": "nanometer", + "µm": "micrometer", + "μm": "micrometer", + "um": "micrometer", + "micron": "micrometer", + "microns": "micrometer", + "micrometer": "micrometer", + "mm": "millimeter", + "cm": "centimeter", + "m": "meter", +} +_TIME_UNITS = { + "ns": "nanosecond", + "µs": "microsecond", + "μs": "microsecond", + "us": "microsecond", + "ms": "millisecond", + "s": "second", + "sec": "second", + "min": "minute", + "h": "hour", +} + +# The OME schema's unit when a PhysicalSize*Unit attribute is left out. +_OME_DEFAULT_SPACE_UNIT = "µm" +_OME_DEFAULT_TIME_UNIT = "s" + + +@dataclass(frozen=True) +class PhysicalSizes: + """Pixel sizes from metadata; None where unknown. + + Attributes + ---------- + x, y, z + Pixel size and z-step, each in its own unit (``x_unit``, ``y_unit``, + ``z_unit``). + t + Time between frames in ``time_unit``. + space_unit + The unit of X. Kept for callers written when one unit served every space + axis; :meth:`unit` gives each axis's own unit. Given alone, it is the unit + of X, Y and Z. + time_unit + The unit of T. + x_unit, y_unit, z_unit + OME-NGFF unit names per space axis, e.g. ``"micrometer"``; None when the + metadata gives none. + + Raises + ------ + ValueError + If ``space_unit`` and ``x_unit`` are both given and differ. + """ + + x: float | None = None + y: float | None = None + z: float | None = None + t: float | None = None + space_unit: str | None = None + time_unit: str | None = None + x_unit: str | None = None + y_unit: str | None = None + z_unit: str | None = None + + def __post_init__(self) -> None: + if self.x_unit is None and self.y_unit is None and self.z_unit is None: + # Sizes made with one space unit (before per-axis units) keep meaning it + # for every space axis. + for name in ("x_unit", "y_unit", "z_unit"): + object.__setattr__(self, name, self.space_unit) + elif self.x_unit is None: + object.__setattr__(self, "x_unit", self.space_unit) + elif self.space_unit is None: + object.__setattr__(self, "space_unit", self.x_unit) + elif self.space_unit != self.x_unit: + raise ValueError( + f"space_unit is the unit of X: {self.space_unit!r} differs from x_unit " + f"{self.x_unit!r}" + ) + + def scale(self, role: str) -> float: + """Return the size of one step along ``role`` (1 when unknown).""" + value = {"X": self.x, "Y": self.y, "Z": self.z, "T": self.t}.get(role) + return 1.0 if value is None or not value > 0 else float(value) + + def unit(self, role: str) -> str | None: + """Return the unit of ``role``, or None when unknown or for channels.""" + if role == "T": + return self.time_unit if self.t is not None else None + if role == "C": + return None + size, unit = { + "X": (self.x, self.x_unit), + "Y": (self.y, self.y_unit), + "Z": (self.z, self.z_unit), + }[role] + return unit if size is not None else None + + +def read_physical_sizes(path: str | os.PathLike) -> PhysicalSizes: + """Read pixel sizes from OME or ImageJ metadata, without reading pixels. + + Each space axis has its own unit: an OME ``PhysicalSizeYUnit`` or + ``PhysicalSizeZUnit`` is used for Y or Z (the OME default, micrometres, when + left out), and ImageJ's ``yunit`` and ``zunit`` when given. + + Returns all-None sizes when the file has no such metadata or it cannot be + parsed; sizes are never guessed. + """ + try: + with tifffile.TiffFile(path) as tif: + return _tiff_sizes(tif) + except Exception: # metadata is optional; a damaged header is reported elsewhere + logger.debug("Cannot read physical sizes of %s", path, exc_info=True) + return PhysicalSizes() + + +def _tiff_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: + """Return the physical sizes of an open TIFF file (all None when not given).""" + try: + if tif.is_ome and tif.ome_metadata: + return _ome_sizes(tif.ome_metadata) + if tif.is_imagej: + return _imagej_sizes(tif) + except Exception: # metadata is optional; a damaged header is reported elsewhere + logger.debug("Cannot read physical sizes of %s", tif.filename, exc_info=True) + return PhysicalSizes() + + +def _ome_sizes(xml: str) -> PhysicalSizes: + root = ElementTree.fromstring(xml) + pixels = next((e for e in root.iter() if e.tag.rsplit("}", 1)[-1] == "Pixels"), None) + if pixels is None: + return PhysicalSizes() + units = { + axis: _SPACE_UNITS.get(pixels.get(f"PhysicalSize{axis}Unit", _OME_DEFAULT_SPACE_UNIT)) + for axis in "XYZ" + } + time_unit = _TIME_UNITS.get(pixels.get("TimeIncrementUnit", _OME_DEFAULT_TIME_UNIT)) + return PhysicalSizes( + x=_number(pixels.get("PhysicalSizeX")), + y=_number(pixels.get("PhysicalSizeY")), + z=_number(pixels.get("PhysicalSizeZ")), + t=_number(pixels.get("TimeIncrement")) if time_unit else None, + space_unit=units["X"], + time_unit=time_unit, + x_unit=units["X"], + y_unit=units["Y"], + z_unit=units["Z"], + ) + + +def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: + meta = tif.imagej_metadata or {} + unit = _imagej_unit(meta.get("unit")) + # ImageJ writes yunit and zunit only when they differ from the X unit. + y_unit = _imagej_unit(meta["yunit"]) if "yunit" in meta else unit + z_unit = _imagej_unit(meta["zunit"]) if "zunit" in meta else unit + page = tif.pages.first + time_unit = _TIME_UNITS.get(str(meta.get("tunit", "sec")).strip().lower()) + # Without a unit the resolution tags are a default (often 1 pixel per inch). + return PhysicalSizes( + x=_resolution(page.tags.get("XResolution")) if unit else None, + y=_resolution(page.tags.get("YResolution")) if y_unit else None, + z=_number(meta.get("spacing")) if z_unit else None, + t=_number(meta.get("finterval")) if time_unit else None, + space_unit=unit, + time_unit=time_unit, + x_unit=unit, + y_unit=y_unit, + z_unit=z_unit, + ) + + +def _imagej_unit(value) -> str | None: + return _SPACE_UNITS.get(str(value or "").strip().lower()) + + +def _resolution(tag) -> float | None: + if tag is None: + return None + try: + numerator, denominator = tag.value + return denominator / numerator if numerator else None + except (TypeError, ValueError): + return None + + +def _number(value) -> float | None: + try: + number = float(value) + except (TypeError, ValueError): + return None + return number if math.isfinite(number) and number > 0 else None diff --git a/src/morphobricks_preprocessing/zarr_export.py b/src/morphobricks_preprocessing/zarr_export.py index da6c0714..aa5aaa43 100644 --- a/src/morphobricks_preprocessing/zarr_export.py +++ b/src/morphobricks_preprocessing/zarr_export.py @@ -28,13 +28,15 @@ from collections.abc import Iterable, Sequence from dataclasses import dataclass, field from pathlib import Path, PurePosixPath -from xml.etree import ElementTree import numpy as np -import tifffile from morphobricks_normalization.dtypes import is_integer from .axes import AxisMapping +from .physical_sizes import ( # noqa: F401 - moved there, still importable from here + PhysicalSizes, + read_physical_sizes, +) from .planes import DEFAULT_MEMORY_LIMIT, Plane, PlaneLayout from .zarr_input import ( # noqa: F401 - re-exported for callers of this module ZarrUnavailableError, @@ -59,32 +61,6 @@ MAX_LEVELS = 8 CHUNK_SIDE = 1024 -# Units in OME and ImageJ metadata, and their OME-NGFF (UDUNITS-2) names. -_SPACE_UNITS = { - "pm": "picometer", - "nm": "nanometer", - "µm": "micrometer", - "μm": "micrometer", - "um": "micrometer", - "micron": "micrometer", - "microns": "micrometer", - "micrometer": "micrometer", - "mm": "millimeter", - "cm": "centimeter", - "m": "meter", -} -_TIME_UNITS = { - "ns": "nanosecond", - "µs": "microsecond", - "μs": "microsecond", - "us": "microsecond", - "ms": "millisecond", - "s": "second", - "sec": "second", - "min": "minute", - "h": "hour", -} - def ome_zarr_relative_path(source_relative_path: str) -> str: """Return where the copy of an image goes, relative to the output folder. @@ -101,59 +77,6 @@ def ome_zarr_relative_path(source_relative_path: str) -> str: return (PurePosixPath(OME_ZARR_FOLDER) / path.parent / (name + OME_ZARR_SUFFIX)).as_posix() -@dataclass(frozen=True) -class PhysicalSizes: - """Pixel sizes from metadata; None where unknown. - - Attributes - ---------- - x, y, z - Pixel size and z-step in ``space_unit``. - t - Time between frames in ``time_unit``. - space_unit, time_unit - OME-NGFF unit names, e.g. ``"micrometer"`` and ``"second"``. - """ - - x: float | None = None - y: float | None = None - z: float | None = None - t: float | None = None - space_unit: str | None = None - time_unit: str | None = None - - def scale(self, role: str) -> float: - """Return the size of one step along ``role`` (1 when unknown).""" - value = {"X": self.x, "Y": self.y, "Z": self.z, "T": self.t}.get(role) - return 1.0 if value is None or not value > 0 else float(value) - - def unit(self, role: str) -> str | None: - """Return the unit of ``role``, or None when unknown or for channels.""" - if role == "T": - return self.time_unit if self.t is not None else None - if role == "C": - return None - sizes = {"X": self.x, "Y": self.y, "Z": self.z} - return self.space_unit if sizes[role] is not None else None - - -def read_physical_sizes(path: str | os.PathLike) -> PhysicalSizes: - """Read pixel sizes from OME or ImageJ metadata, without reading pixels. - - Returns all-None sizes when the file has no such metadata or it cannot be - parsed; sizes are never guessed. - """ - try: - with tifffile.TiffFile(path) as tif: - if tif.is_ome and tif.ome_metadata: - return _ome_sizes(tif.ome_metadata) - if tif.is_imagej: - return _imagej_sizes(tif) - except Exception: # metadata is optional; a damaged header is reported elsewhere - logger.debug("Cannot read physical sizes of %s", path, exc_info=True) - return PhysicalSizes() - - COMPRESSIONS = { "lz4": "Blosc LZ4 (fast)", "zstd": "Blosc Zstandard (smaller, slower)", @@ -618,61 +541,3 @@ def _create_array(group, name: str, shape, chunks, dtype, compressor): compressor=compressor, dimension_separator="/", ) - - -# Metadata ----------------------------------------------------------------------------- - - -def _ome_sizes(xml: str) -> PhysicalSizes: - root = ElementTree.fromstring(xml) - pixels = next((e for e in root.iter() if e.tag.rsplit("}", 1)[-1] == "Pixels"), None) - if pixels is None: - return PhysicalSizes() - unit = _SPACE_UNITS.get(pixels.get("PhysicalSizeXUnit", "µm")) - time_unit = _TIME_UNITS.get(pixels.get("TimeIncrementUnit", "s")) - return PhysicalSizes( - x=_number(pixels.get("PhysicalSizeX")), - y=_number(pixels.get("PhysicalSizeY")), - z=_number(pixels.get("PhysicalSizeZ")), - t=_number(pixels.get("TimeIncrement")) if time_unit else None, - space_unit=unit, - time_unit=time_unit, - ) - - -def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: - meta = tif.imagej_metadata or {} - unit = _SPACE_UNITS.get(str(meta.get("unit", "")).strip().lower()) - page = tif.pages.first - x = _resolution(page.tags.get("XResolution")) - y = _resolution(page.tags.get("YResolution")) - time_unit = _TIME_UNITS.get(str(meta.get("tunit", "sec")).strip().lower()) - if unit is None: - # Without a unit the resolution tags are a default (often 1 pixel per inch). - x = y = None - return PhysicalSizes( - x=x, - y=y, - z=_number(meta.get("spacing")) if unit else None, - t=_number(meta.get("finterval")) if time_unit else None, - space_unit=unit, - time_unit=time_unit, - ) - - -def _resolution(tag) -> float | None: - if tag is None: - return None - try: - numerator, denominator = tag.value - return denominator / numerator if numerator else None - except (TypeError, ValueError): - return None - - -def _number(value) -> float | None: - try: - number = float(value) - except (TypeError, ValueError): - return None - return number if math.isfinite(number) and number > 0 else None diff --git a/tests/test_metadata.py b/tests/test_metadata.py index d01f2932..cf3dbafa 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -5,13 +5,16 @@ import tifffile from morphobricks_preprocessing import ( + PhysicalSizes, Severity, TiffMetadata, check_metadata, discover_experiment, inspect_experiment, + inspect_image, inspect_images, inspect_tiff, + read_physical_sizes, ) from tiff_tree import make_tree, write_file, write_tiff @@ -124,6 +127,44 @@ def test_ome_channel_names_and_significant_bits(tmp_path, no_pixel_reads): assert info.significant_bits == 12 +def test_header_carries_the_physical_sizes(tmp_path, no_pixel_reads): + ome = tmp_path / "a.ome.tif" + tifffile.imwrite( + ome, + np.zeros((3, 8, 8), np.uint16), + ome=True, + photometric="minisblack", + metadata={ + "axes": "ZYX", + "PhysicalSizeX": 0.2, + "PhysicalSizeXUnit": "µm", + "PhysicalSizeY": 0.3, + "PhysicalSizeYUnit": "nm", + "PhysicalSizeZ": 1.5, + }, + ) + info = inspect_image(ome) + assert info.physical_sizes == read_physical_sizes(ome) + assert (info.physical_sizes.x, info.physical_sizes.y, info.physical_sizes.z) == (0.2, 0.3, 1.5) + assert info.physical_sizes.unit("Y") == "nanometer" + imagej = tmp_path / "b.tif" + tifffile.imwrite( + imagej, + np.zeros((8, 8), np.uint16), + imagej=True, + resolution=(1 / 0.25, 1 / 0.25), + metadata={"unit": "um"}, + ) + sizes = inspect_image(imagej).physical_sizes + assert (sizes.x, sizes.y, sizes.unit("X")) == (0.25, 0.25, "micrometer") + + +def test_header_without_sizes_has_empty_physical_sizes(tmp_path): + assert inspect_tiff(write_tiff(tmp_path / "a.tif")).physical_sizes == PhysicalSizes() + bad = inspect_tiff(write_file(tmp_path / "bad.tif", b"not a tiff")) + assert not bad.readable and bad.physical_sizes == PhysicalSizes() + + def test_no_channel_names_without_ome_metadata(tmp_path): info = inspect_tiff(write_tiff(tmp_path / "a.tif", (2, 6, 7), axes="CYX", imagej=True)) assert info.channel_names == () and info.significant_bits is None diff --git a/tests/test_physical_sizes.py b/tests/test_physical_sizes.py new file mode 100644 index 00000000..8aadfe17 --- /dev/null +++ b/tests/test_physical_sizes.py @@ -0,0 +1,118 @@ +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import PhysicalSizes, read_physical_sizes, zarr_export +from morphobricks_preprocessing import physical_sizes as physical_sizes_module + + +def write_ome(path, **sizes): + tifffile.imwrite( + path, + np.zeros((3, 8, 8), np.uint16), + ome=True, + photometric="minisblack", + metadata={"axes": "ZYX", **sizes}, + ) + return path + + +# Units per axis ------------------------------------------------------------------------ + + +def test_ome_units_are_read_per_axis(tmp_path): + path = write_ome( + tmp_path / "a.ome.tif", + PhysicalSizeX=120.0, + PhysicalSizeXUnit="nm", + PhysicalSizeY=0.2, + PhysicalSizeYUnit="µm", + PhysicalSizeZ=2.0, + PhysicalSizeZUnit="mm", + ) + sizes = read_physical_sizes(path) + assert (sizes.x, sizes.y, sizes.z) == (120.0, 0.2, 2.0) + assert [sizes.unit(role) for role in "XYZ"] == ["nanometer", "micrometer", "millimeter"] + assert (sizes.x_unit, sizes.y_unit, sizes.z_unit) == ("nanometer", "micrometer", "millimeter") + assert sizes.space_unit == "nanometer" # the unit of X, as before + + +def test_ome_units_left_out_are_micrometres(tmp_path): + # The OME schema's default unit: Y and Z do not take X's unit. + path = write_ome( + tmp_path / "a.ome.tif", + PhysicalSizeX=120.0, + PhysicalSizeXUnit="nm", + PhysicalSizeY=0.5, + PhysicalSizeZ=2.0, + ) + sizes = read_physical_sizes(path) + assert [sizes.unit(role) for role in "XYZ"] == ["nanometer", "micrometer", "micrometer"] + + +def test_imagej_y_and_z_units(tmp_path): + path = tmp_path / "a.tif" + tifffile.imwrite( + path, + np.zeros((3, 8, 8), np.uint16), + imagej=True, + resolution=(1 / 0.25, 1 / 0.5), + metadata={"axes": "ZYX", "unit": "um", "yunit": "nm", "zunit": "mm", "spacing": 1.5}, + ) + sizes = read_physical_sizes(path) + assert (sizes.x, sizes.y, sizes.z) == (0.25, 0.5, 1.5) + assert [sizes.unit(role) for role in "XYZ"] == ["micrometer", "nanometer", "millimeter"] + + +def test_imagej_without_y_and_z_units_uses_the_unit_for_all(tmp_path): + path = tmp_path / "a.tif" + tifffile.imwrite( + path, + np.zeros((3, 8, 8), np.uint16), + imagej=True, + resolution=(1 / 0.25, 1 / 0.25), + metadata={"axes": "ZYX", "unit": "um", "spacing": 1.5}, + ) + sizes = read_physical_sizes(path) + assert [sizes.unit(role) for role in "XYZ"] == ["micrometer"] * 3 + + +# The dataclass, for callers written before per-axis units ------------------------------- + + +def test_space_unit_alone_is_the_unit_of_every_space_axis(): + sizes = PhysicalSizes(0.5, 0.5, 2.0, None, "micrometer") + assert [sizes.unit(role) for role in "XYZ"] == ["micrometer"] * 3 + assert sizes == PhysicalSizes( + x=0.5, + y=0.5, + z=2.0, + space_unit="micrometer", + x_unit="micrometer", + y_unit="micrometer", + z_unit="micrometer", + ) + + +def test_per_axis_units_set_space_unit_to_the_unit_of_x(): + sizes = PhysicalSizes(x=1.0, y=2.0, x_unit="nanometer", y_unit="micrometer") + assert sizes.space_unit == "nanometer" + assert sizes.unit("Y") == "micrometer" + # An axis given no unit has none, even when X has one. + assert PhysicalSizes(x=1.0, y=2.0, x_unit="nanometer", z_unit="meter").unit("Y") is None + + +def test_space_unit_and_x_unit_must_agree(): + with pytest.raises(ValueError, match="unit of X"): + PhysicalSizes(x=1.0, space_unit="micrometer", x_unit="nanometer") + + +def test_units_of_unknown_sizes_and_channels_are_none(): + sizes = PhysicalSizes(x=1.0, space_unit="micrometer", time_unit="second") + assert sizes.unit("Y") is None and sizes.unit("T") is None and sizes.unit("C") is None + assert sizes.scale("Y") == 1.0 + + +def test_still_importable_from_zarr_export(): + assert zarr_export.PhysicalSizes is physical_sizes_module.PhysicalSizes is PhysicalSizes + assert zarr_export.read_physical_sizes is read_physical_sizes diff --git a/tests/test_zarr_export.py b/tests/test_zarr_export.py index 652ab094..f601bae9 100644 --- a/tests/test_zarr_export.py +++ b/tests/test_zarr_export.py @@ -193,8 +193,17 @@ def test_physical_sizes_from_ome(tmp_path): "TimeIncrementUnit": "s", }, ) + # PhysicalSizeZUnit is left out, so Z is in the OME default, micrometres. assert read_physical_sizes(path) == PhysicalSizes( - x=120.0, y=120.0, z=500.0, t=30.0, space_unit="nanometer", time_unit="second" + x=120.0, + y=120.0, + z=500.0, + t=30.0, + space_unit="nanometer", + time_unit="second", + x_unit="nanometer", + y_unit="nanometer", + z_unit="micrometer", ) @@ -304,6 +313,31 @@ def test_every_dtype_is_copied_unchanged(tmp_path, dtype): np.testing.assert_array_equal(open_copy(result.output_folder, "a.tif")["0"][:], data) +@needs_zarr +def test_copies_give_each_axis_its_own_unit(tmp_path): + path = tmp_path / "exp/a.ome.tif" + path.parent.mkdir(parents=True) + tifffile.imwrite( + path, + random_data((2, 8, 8), "uint16"), + ome=True, + photometric="minisblack", + metadata={ + "axes": "ZYX", + "PhysicalSizeX": 120.0, + "PhysicalSizeXUnit": "nm", + "PhysicalSizeY": 0.25, + "PhysicalSizeYUnit": "µm", + "PhysicalSizeZ": 2.0, + "PhysicalSizeZUnit": "mm", + }, + ) + result = run_experiment(tmp_path / "exp", ZYX, convert_tiffs=True) + meta = multiscale(open_copy(result.output_folder, "a.ome.tif")) + assert [axis["unit"] for axis in meta["axes"]] == ["millimeter", "micrometer", "nanometer"] + assert meta["datasets"][0]["coordinateTransformations"][0]["scale"] == [2.0, 0.25, 120.0] + + @needs_zarr def test_pyramid_levels_and_scales(tmp_path): data = random_data((2, 1100, 700), "uint16") From 9ceb28bae712a3ed638b90c769dab5297bc04bc6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 19:46:43 +0000 Subject: [PATCH 03/11] Read the physical sizes of OME-Zarr images from their scale open_ngff read the axes of an OME-Zarr image but never its OME-NGFF coordinateTransformations or axis units, so the physical size of its pixels was not available. MorphoRelate milestone M3 reads OME-Zarr input and needs them as it gets them for TIFF files. NgffImage gains physical_sizes, in the PhysicalSizes form read_physical_sizes returns for TIFF files: the scale of level 0's coordinateTransformations, multiplied by the multiscales entry's own scale when it has one (OME-NGFF applies them in turn; translations do not change sizes), for X, Y, Z and T, with each axis's unit as written. OME-NGFF 0.4 and 0.5 are read alike. inspect_image records them for OME-Zarr images as for TIFF files, and label images opened with open_ngff_labels carry their own. Nothing is guessed: without a level-0 scale, with a scale stored in a separate file (path) or not one number per axis, every size is None; an axis without a unit has the unit None and a non-positive scale is None. A scale without a unit is kept with the unit None, since only the caller can tell a calibration from a placeholder (this brick's own copies write 1 without a unit when the size is unknown). The test helper write_ngff gains scale, units, dataset_transforms and multiscale_transforms; its default output is unchanged. Decision recorded as plan section 12 question 23; contract section 2 and AGENTS.md updated. Information only: the CSV, the manifest and their versions are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 10 +- docs/contract.md | 1 + docs/plan.md | 15 ++ src/morphobricks_preprocessing/metadata.py | 1 + .../physical_sizes.py | 102 ++++++++++- src/morphobricks_preprocessing/zarr_input.py | 13 ++ tests/test_zarr_input.py | 164 ++++++++++++++++++ tests/zarr_tree.py | 28 ++- 8 files changed, 321 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cd300bb5..fa4cb1c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,8 +61,9 @@ src/morphobricks_preprocessing/ position_id per image and the duplicate check metadata.py TIFF header inspection (tifffile, no pixel reads), dtype/dimension/series checks, OME channel names + significant bits, physical sizes (TiffMetadata.physical_sizes) - physical_sizes.py PhysicalSizes (pixel sizes, z-step, time step, a unit per axis) and - read_physical_sizes (OME / ImageJ TIFF metadata); also importable from zarr_export + physical_sizes.py PhysicalSizes (pixel sizes, z-step, time step, a unit per axis), + read_physical_sizes (OME / ImageJ TIFF metadata; also importable from + zarr_export) and ngff_physical_sizes (OME-NGFF scale and axis units) validation.py ValidationIssue (severity, code, message, paths, action), error()/warning() builders axes.py AxisMapping (role per dimension), prefill, check_mapping, review_axes (one mapping applied to every file, missing axes as singletons only when confirmed) @@ -78,8 +79,9 @@ src/morphobricks_preprocessing/ run.py preflight(), run_experiment() (incremental), sample_planes() for previews report.py plain-language summaries shared by CLI and notebook, group statistics tables cli.py, __main__ `morphobricks-preprocessing check|run` (also `python -m morphobricks_preprocessing`) - zarr_input.py OME-Zarr input: open_ngff (OME-NGFF 0.4/0.5 metadata -> level-0 array and axis - letters), ngff_label_names/open_ngff_labels (label images, for later bricks), + zarr_input.py OME-Zarr input: open_ngff (OME-NGFF 0.4/0.5 metadata -> level-0 array, axis + letters and physical sizes from the scale and axis units), + ngff_label_names/open_ngff_labels (label images, for later bricks), zarr 2/3 helpers, canonical_dtype; needs the `zarr` extra zarr_export.py TIFF -> OME-Zarr conversion (OmeZarrWriter, OME-NGFF 0.4, plane by plane or block by block), diff --git a/docs/contract.md b/docs/contract.md index e6077ffe..24e1f039 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -51,6 +51,7 @@ length 1 and no role, has coordinate 0. | 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 a warning. | +| Physical sizes | Read with the header, for information only (not part of the CSV or the manifest): `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`. 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. | ## 3. Pixel types diff --git a/docs/plan.md b/docs/plan.md index 0baa2db5..bb6ed508 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -549,3 +549,18 @@ Raised during review; to be decided before the milestones that need them: from X now write that axis's own unit. `PhysicalSizes` and `read_physical_sizes` moved to the module `physical_sizes` and stay importable from `zarr_export` and the package. The CSV, the manifest and their versions are unchanged.* +23. OME-Zarr physical sizes: `open_ngff` read the axes but not the OME-NGFF scale, so the + physical size of an OME-Zarr image's pixels was not available (MorphoRelate milestone + M3 reads OME-Zarr input). Should it be? + *Decided (October 2026): yes, in the form of question 22. `NgffImage` gains + `physical_sizes`: the `scale` of level 0's `coordinateTransformations`, multiplied by + the multiscales entry's own `coordinateTransformations` scale when it has one (OME-NGFF + applies them in turn; translations are ignored), for X, Y, Z and T, with each axis's + `unit` as written (OME-NGFF 0.4 and 0.5). `inspect_image` records them for OME-Zarr + images as for TIFF files, and label images opened with `open_ngff_labels` carry their + own. Nothing is guessed: without a level-0 scale, with a scale stored in a separate + file (`path`) or not one number per axis, every size is None; an axis without a unit + has the unit None, and a scale that is not a positive number is None. A scale given + without a unit is kept, since only the caller can tell a real calibration from a + placeholder (this brick's own copies write 1 without a unit when the size is not + known). Information only: the CSV, the manifest and their versions are unchanged.* diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index 8af877a1..11375095 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -210,6 +210,7 @@ def inspect_ome_zarr(path: str | os.PathLike, relative_path: str | None = None) page_count=0, channel_names=image.channel_names, kind=OME_ZARR, + physical_sizes=image.physical_sizes, ) except (NgffError, ZarrUnavailableError) as exc: logger.debug("Cannot read the OME-Zarr image %s", path, exc_info=True) diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index eae06cc0..3fdce523 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -6,9 +6,13 @@ Y pixels are measured in another unit than its X pixels is described as it is. Sizes are never guessed: what the metadata does not give is None. +OME-Zarr images give the same sizes in their OME-NGFF metadata (0.4 and 0.5): +the ``scale`` of the level-0 dataset's ``coordinateTransformations``, times the +``scale`` of the image's own ``coordinateTransformations`` when it has them, with +each axis's ``unit`` (:func:`ngff_physical_sizes`, used by :mod:`.zarr_input`). + They are recorded with an image's header (:func:`.metadata.inspect_image`) and -written as the scale of OME-Zarr copies (``docs/contract.md`` §10). The sizes of -OME-Zarr images are read from their OME-NGFF metadata by :mod:`.zarr_input`. +written as the scale of OME-Zarr copies (``docs/contract.md`` §2 and §10). """ from __future__ import annotations @@ -16,6 +20,7 @@ import logging import math import os +from collections.abc import Sequence from dataclasses import dataclass from xml.etree import ElementTree @@ -144,6 +149,99 @@ def read_physical_sizes(path: str | os.PathLike) -> PhysicalSizes: return PhysicalSizes() +def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: + """Return the level-0 physical sizes of an OME-NGFF image (0.4 or 0.5). + + The size of one step along each axis is the ``scale`` of the first dataset's + ``coordinateTransformations`` (level 0), multiplied by the ``scale`` of the + multiscales entry's own ``coordinateTransformations`` when it has them, as + OME-NGFF applies them in turn; translations do not change sizes. Units are + each axis's ``unit``, as written (OME-NGFF uses UDUNITS-2 names such as + ``"micrometer"``, the names :func:`read_physical_sizes` gives). + + 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 + kept: it may be a placeholder (some writers, this brick's OME-Zarr copies + included, write 1 when the size is not known). + + Parameters + ---------- + multiscale + One entry of the image's ``multiscales`` metadata. + axes + One letter per dimension, as :func:`.zarr_input.ngff_axes` gives them; + X, Y, Z and T are read, the others (C, Q) ignored. + + Returns + ------- + PhysicalSizes + Sizes and units of X, Y, Z and T. + """ + 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() + if overall is not None: + scale = [ + a * b if a is not None and b is not None else None + for a, b in zip(scale, overall, strict=True) + ] + declared = multiscale.get("axes") + units: list[str | None] = [None] * len(axes) + if isinstance(declared, list) and len(declared) == len(axes): + units = [ + axis["unit"] + if isinstance(axis, dict) and isinstance(axis.get("unit"), str) and axis["unit"] + else None + for axis in declared + ] + found: dict[str, tuple[float | None, str | None]] = {} + for letter, size, unit in zip(axes, scale, units, strict=True): + if letter in "XYZT": + found.setdefault(letter, (_number(size), unit)) + sizes = {letter: found.get(letter, (None, None)) for letter in "XYZT"} + return PhysicalSizes( + x=sizes["X"][0], + y=sizes["Y"][0], + z=sizes["Z"][0], + t=sizes["T"][0], + space_unit=sizes["X"][1], + time_unit=sizes["T"][1], + x_unit=sizes["X"][1], + y_unit=sizes["Y"][1], + z_unit=sizes["Z"][1], + ) + + +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 it + has one that cannot be used (stored in a file, or not one number per axis). + """ + transforms = entry.get("coordinateTransformations") if isinstance(entry, dict) else None + if not isinstance(transforms, list): + return None + 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 [ + float(v) if isinstance(v, int | float) and not isinstance(v, bool) else None + for v in values + ] + return None + + def _tiff_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: """Return the physical sizes of an open TIFF file (all None when not given).""" try: diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index 3a403e56..dfb1f821 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -15,6 +15,9 @@ several fields) is described by its first image and reports how many images it holds, so that the preflight can block it like a multi-series TIFF. +The physical size of a pixel along each axis, with its unit, comes from the +OME-NGFF scale of level 0 (:attr:`NgffImage.physical_sizes`). + The label images of an image (segmentations in its OME-NGFF ``labels`` group, such as ``cells`` or ``nuclei``) can be listed with :func:`ngff_label_names` and opened with :func:`open_ngff_labels`, for later bricks that use them as @@ -33,6 +36,8 @@ import numpy as np +from .physical_sizes import PhysicalSizes, ngff_physical_sizes + ZARR_SUFFIX = ".zarr" @@ -92,6 +97,12 @@ class NgffImage: Channel labels from the ``omero`` metadata; empty when not given. dataset_path Path of the level-0 array inside the store, e.g. ``"0"`` or ``"0/0"``. + physical_sizes + Level-0 size of one step along X, Y, Z and T with each axis's unit, from + 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. """ array: Any @@ -100,6 +111,7 @@ class NgffImage: image_count: int channel_names: tuple[str | None, ...] = () dataset_path: str = "" + physical_sizes: PhysicalSizes = PhysicalSizes() @property def shape(self) -> tuple[int, ...]: @@ -270,6 +282,7 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: image_count=image_count, channel_names=labels if any(labels) else (), dataset_path=f"{prefix}{dataset_path}", + physical_sizes=ngff_physical_sizes(first, axes), ) diff --git a/tests/test_zarr_input.py b/tests/test_zarr_input.py index 51606bf2..f76ca5a4 100644 --- a/tests/test_zarr_input.py +++ b/tests/test_zarr_input.py @@ -7,6 +7,7 @@ from morphobricks_preprocessing import ( AxisMapping, NgffError, + PhysicalSizes, Severity, TiffMetadata, declared_mapping, @@ -15,11 +16,13 @@ inspect_image, inspect_images, iter_planes, + ome_zarr_relative_path, open_ngff, plane_layout, prefill_mapping, preflight, read_manifest, + read_physical_sizes, read_plane, read_plane_histograms, read_sample_plane, @@ -108,6 +111,167 @@ def test_open_ngff(tmp_path, version): assert image.chunks == (1, 1, 8, 9) +# Physical sizes (OME-NGFF coordinateTransformations and axis units) -------------------- + +MICRONS = {"z": "micrometer", "y": "micrometer", "x": "micrometer"} + + +@pytest.mark.parametrize("version", ["0.4", "0.5"]) +def test_physical_sizes_with_units(tmp_path, version): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((2, 3, 8, 8), np.uint8), + "czyx", + version=version, + scale=[1.0, 2.0, 0.5, 0.25], + units={**MICRONS, "y": "nanometer"}, + ) + sizes = open_ngff(path).physical_sizes + assert sizes == PhysicalSizes( + x=0.25, + y=0.5, + z=2.0, + space_unit="micrometer", + x_unit="micrometer", + y_unit="nanometer", + z_unit="micrometer", + ) + assert [sizes.unit(role) for role in "XYZCT"] == [ + "micrometer", + "nanometer", + "micrometer", + None, + None, + ] + + +@pytest.mark.parametrize("version", ["0.4", "0.5"]) +def test_physical_sizes_without_units(tmp_path, version): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((3, 8, 8), np.uint8), + "zyx", + version=version, + scale=[2.0, 0.5, 0.25], + ) + sizes = open_ngff(path).physical_sizes + assert (sizes.x, sizes.y, sizes.z, sizes.t) == (0.25, 0.5, 2.0, None) + assert [sizes.unit(role) for role in "XYZ"] == [None, None, None] + assert sizes.space_unit is None + + +@pytest.mark.parametrize("version", ["0.4", "0.5"]) +def test_physical_sizes_with_a_multiscale_level_scale(tmp_path, version): + # OME-NGFF applies the multiscales entry's transformations after the dataset's. + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((4, 3, 8, 8), np.uint8), + "tzyx", + version=version, + scale=[1.0, 1.5, 0.2, 0.25], + units={**MICRONS, "t": "second"}, + multiscale_transforms=[{"type": "scale", "scale": [30.0, 2.0, 3.0, 4.0]}], + ) + sizes = open_ngff(path).physical_sizes + assert (sizes.t, sizes.z, sizes.y, sizes.x) == pytest.approx((30.0, 3.0, 0.6, 1.0)) + assert (sizes.unit("T"), sizes.unit("X")) == ("second", "micrometer") + + +def test_multiscale_level_translation_does_not_change_sizes(tmp_path): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((8, 8), np.uint8), + "yx", + scale=[0.5, 0.25], + multiscale_transforms=[{"type": "translation", "translation": [10.0, 20.0]}], + ) + sizes = open_ngff(path).physical_sizes + assert (sizes.x, sizes.y) == (0.25, 0.5) + + +@pytest.mark.parametrize( + "transforms", + [ + [], # no coordinateTransformations at all (OME-NGFF before 0.4) + [{"type": "scale", "path": "scale.bin"}], # stored in a file: not read + [{"type": "scale", "scale": [0.5, 0.25]}], # not one number per axis + [{"type": "translation", "translation": [0.0, 0.0, 0.0]}], + ], +) +def test_physical_sizes_absent_when_not_given(tmp_path, transforms): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((3, 8, 8), np.uint8), + "zyx", + dataset_transforms=transforms, + units=MICRONS, + ) + assert open_ngff(path).physical_sizes == PhysicalSizes() + + +def test_unusable_multiscale_level_scale_makes_sizes_unknown(tmp_path): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((8, 8), np.uint8), + "yx", + scale=[0.5, 0.25], + multiscale_transforms=[{"type": "scale", "path": "scale.bin"}], + ) + assert open_ngff(path).physical_sizes == PhysicalSizes() + + +def test_scales_that_are_not_positive_numbers_are_unknown(tmp_path): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((3, 8, 8), np.uint8), + "zyx", + scale=[0.0, -1.0, 0.25], + units=MICRONS, + ) + sizes = open_ngff(path).physical_sizes + assert (sizes.x, sizes.y, sizes.z) == (0.25, None, None) + assert (sizes.unit("X"), sizes.unit("Y")) == ("micrometer", None) + + +@pytest.mark.parametrize("version", ["0.4", "0.5"]) +def test_header_of_a_store_carries_its_physical_sizes(tmp_path, version): + path = write_ngff( + tmp_path / "a.ome.zarr", + np.zeros((3, 8, 8), np.uint8), + "zyx", + version=version, + scale=[2.0, 0.5, 0.25], + units=MICRONS, + ) + info = inspect_image(path) + assert info.is_ome_zarr and info.physical_sizes == open_ngff(path).physical_sizes + assert (info.physical_sizes.x, info.physical_sizes.unit("Z")) == (0.25, "micrometer") + + +def test_converted_copies_give_the_sizes_of_their_tiff(tmp_path): + source = tmp_path / "exp/a.ome.tif" + source.parent.mkdir(parents=True) + tifffile.imwrite( + source, + np.zeros((2, 3, 8, 8), np.uint16), + ome=True, + photometric="minisblack", + metadata={ + "axes": "TZYX", + "PhysicalSizeX": 120.0, + "PhysicalSizeXUnit": "nm", + "PhysicalSizeY": 0.25, + "PhysicalSizeYUnit": "µm", + "PhysicalSizeZ": 2.0, + "TimeIncrement": 30.0, + "TimeIncrementUnit": "s", + }, + ) + result = run_experiment(tmp_path / "exp", prefill_mapping("TZYX"), convert_tiffs=True) + copy = result.output_folder / ome_zarr_relative_path("a.ome.tif") + assert open_ngff(copy).physical_sizes == read_physical_sizes(source) + + def test_bioformats2raw_layout(tmp_path): one = write_bioformats2raw(tmp_path / "one.ome.zarr", [random_data((2, 8, 8))]) image = open_ngff(one) diff --git a/tests/zarr_tree.py b/tests/zarr_tree.py index dbb8b79c..0cf0d7c0 100644 --- a/tests/zarr_tree.py +++ b/tests/zarr_tree.py @@ -25,11 +25,19 @@ def write_ngff( channel_names=None, axis_types=None, extra_multiscales=0, + scale=None, + units=None, + dataset_transforms=None, + multiscale_transforms=None, ): """Write ``data`` as a one-level OME-Zarr image (OME-NGFF 0.4 or 0.5) at ``path``. ``axes`` gives one OME-NGFF axis name per dimension; ``axis_types`` overrides - their types. Returns the path. + their types. ``scale`` is the level-0 scale (1 per axis by default), + ``dataset_transforms`` replaces the level-0 ``coordinateTransformations`` + whole (``[]`` leaves them out), ``units`` maps axis names to their ``unit``, + and ``multiscale_transforms`` adds ``coordinateTransformations`` to the + multiscales entry. Returns the path. """ zarr = pytest.importorskip("zarr") if version == "0.5" and zarr_major() < 3: @@ -41,15 +49,21 @@ def write_ngff( chunks = chunks or (*(1,) * (data.ndim - 2), *data.shape[-2:]) _array(group, "0", data, chunks) types = axis_types or [AXIS_TYPES.get(a, "space") for a in axes] + units = units or {} + if dataset_transforms is None: + dataset_transforms = [{"type": "scale", "scale": scale or [1.0] * data.ndim}] + dataset = {"path": "0"} + if dataset_transforms: + dataset["coordinateTransformations"] = dataset_transforms multiscale = { - "axes": [{"name": a, "type": t} for a, t in zip(axes, types, strict=True)], - "datasets": [ - { - "path": "0", - "coordinateTransformations": [{"type": "scale", "scale": [1.0] * data.ndim}], - } + "axes": [ + {"name": a, "type": t, **({"unit": units[a]} if a in units else {})} + for a, t in zip(axes, types, strict=True) ], + "datasets": [dataset], } + if multiscale_transforms is not None: + multiscale["coordinateTransformations"] = multiscale_transforms attrs: dict = {"multiscales": [multiscale] * (1 + extra_multiscales)} if channel_names: attrs["omero"] = {"channels": [{"label": name} for name in channel_names]} From 35c3b1e82ae10da3a03fd1f71a64944f169b3db1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 19:49:23 +0000 Subject: [PATCH 04/11] Let other bricks embed the folder and axis notebook steps PreprocessingNotebook.folder_step and axis_step had fixed titles, purpose sentences and intro boxes that speak of this brick's histograms; step 1 always showed the card for this brick's output folder, and the "What next?" boxes send the user to this notebook's steps (step 2's to compute the histograms). MorphoRelate milestone M3's "your data" notebook reuses the folder check and the axis review, so it needs its own words. Both steps take an optional title and purpose, a keyword intro (HTML; an empty string leaves the box out) and show_next (False hides the "What next?" box); folder_step also takes show_output (False leaves out the "Output folder (optional)" card, and the check then uses the default output folder, as with the box left empty). The defaults give the pages as before, and this notebook still calls them without arguments; the widgets, checks and gating are unchanged. Tested headless (pages built, driven by clicks); the embedded pages were rendered with embed_minimal_html but not looked at in a browser or a real JupyterLab. Decision recorded as plan section 12 question 24; AGENTS.md updated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 9 +- docs/plan.md | 11 ++ src/morphobricks_preprocessing/notebook_ui.py | 124 ++++++++++++++---- tests/test_notebook_ui.py | 88 +++++++++++++ 4 files changed, 207 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fa4cb1c3..c6e9c7b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,7 +87,9 @@ src/morphobricks_preprocessing/ block by block), ConversionSettings (chunk size, compression, pyramid levels), estimate_conversion notebook_style.py notebook look and feel: cards, status cards, tiles, tables, ConfirmToggle, Browse dialogs - notebook_ui.py PreprocessingNotebook: the four notebook steps (needs the `notebook` extra) + notebook_ui.py PreprocessingNotebook: the four notebook steps (needs the `notebook` extra); + folder_step/axis_step take title, purpose, intro and show_next (and + show_output for step 1) so other bricks can embed them (plan §12 Q24) notebooks/MorphoBricks_Preprocessing.ipynb thin notebook: every code cell calls notebook_ui only tests/ pytest with simulated TIFFs (tests/tiff_tree.py, tests/simulation.py) experiments/binning_study/ the study behind the binning decisions (not part of the package) @@ -448,6 +450,11 @@ plainer version "not interactive enough". Descriptive, guided and forgiving is t ### Page layout (`notebook_style`) +- **Embedding steps 1 and 2 (Q24):** `folder_step` and `axis_step` keep this notebook's + words by default; another brick (MorphoRelate) passes its own `title`, `purpose` and + `intro` (HTML; `""` leaves the box out), `show_output=False` to leave out the output + folder card, and `show_next=False` to hide the "What next?" box naming this notebook's + steps. The widgets and their behaviour are the same either way. - **Page and cards:** `ui.page(title, subtitle, children)` wraps a step in a grey page (`mb-root`) with a title and a one-sentence purpose. `ui.card(header, children)` gives a white card with an upper-case header. `ui.intro(html)` adds a blue "what happens here" diff --git a/docs/plan.md b/docs/plan.md index bb6ed508..33ac0aef 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -564,3 +564,14 @@ Raised during review; to be decided before the milestones that need them: without a unit is kept, since only the caller can tell a real calibration from a placeholder (this brick's own copies write 1 without a unit when the size is not known). Information only: the CSV, the manifest and their versions are unchanged.* +24. Can another brick's notebook reuse steps 1 and 2 (folder check and axis review)? + MorphoRelate milestone M3's "your data" notebook needs them, but their title, purpose + and intro speak of this brick's histograms, step 1 has the card for this brick's output + folder, and step 2's "What next?" box sends the user to compute the histograms. + *Decided (October 2026): yes. `PreprocessingNotebook.folder_step` and `axis_step` take + an optional `title` and `purpose` (the sentence under the title), a keyword `intro` + (the blue box; an empty string leaves it out) and `show_next` (False hides the "What + next?" box), and `folder_step` takes `show_output` (False leaves out the "Output + folder (optional)" card; the check then uses the default output folder, as with the + box left empty). The defaults give the pages as before, and this notebook calls them + without arguments; the widgets, the checks and the gating are the same either way.* diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index aeb65463..c07a3f88 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -189,20 +189,63 @@ def welcome(self) -> widgets.Widget: ], ) - def folder_step(self) -> widgets.Widget: - """Step 1: choose and check the experiment folder, confirm the labels.""" + def folder_step( + self, + title: str | None = None, + purpose: str | None = None, + *, + intro: str | None = None, + show_output: bool = True, + show_next: bool = True, + ) -> widgets.Widget: + """Step 1: choose and check the experiment folder, confirm the labels. + + The defaults give this notebook's step. Another brick embedding the step + (it checks the folder and the image headers, nothing more) can give its + own words and leave out the output folder of this brick's histograms. + + Parameters + ---------- + title + Page title; None gives "Step 1 · Check the experiment folder". + purpose + One-sentence purpose under the title (HTML); None gives this + notebook's. + intro + Text of the blue "what happens here" box (HTML); None gives this + notebook's, an empty string leaves the box out. + show_output + False leaves out the "Output folder (optional)" card, which sets where + this brick writes its histogram table. The check then uses the default + output folder, as it does when the box is left empty. + show_next + False hides the "What next?" box, which names this notebook's next + steps. + + Returns + ------- + ipywidgets.Widget + The page. + """ + ui.set_visible(self.folder_next, show_next) + if intro is None: + intro = ( + "What happens here: the folders are scanned, the labels are taken " + "from the folder names (condition / biological repeat / technical repeat) " + "and every image header is checked. Problems are listed with what to do." + ) return ui.page( - "Step 1 · Check the experiment folder", - "Choose the folder with your images. It is checked without reading any pixels " - "and without changing anything.", + "Step 1 · Check the experiment folder" if title is None else title, + ( + "Choose the folder with your images. It is checked without reading any pixels " + "and without changing anything." + ) + if purpose is None + else purpose, [ - ui.intro( - "What happens here: the folders are scanned, the labels are taken " - "from the folder names (condition / biological repeat / technical repeat) " - "and every image header is checked. Problems are listed with what to do." - ), + *([ui.intro(intro)] if intro else []), self.folder_card, - self.output_card, + *([self.output_card] if show_output else []), self.exclude_box, self.check_button, self.folder_status, @@ -213,20 +256,53 @@ def folder_step(self) -> widgets.Widget: ], ) - def axis_step(self) -> widgets.Widget: - """Step 2: axis roles, preview and confirmation.""" + def axis_step( + self, + title: str | None = None, + purpose: str | None = None, + *, + intro: str | None = None, + show_next: bool = True, + ) -> widgets.Widget: + """Step 2: axis roles, preview and confirmation. + + The defaults give this notebook's step; another brick embedding it can give + its own words, since this notebook's explain the roles by the planes its + histograms are computed on. + + Parameters + ---------- + title + Page title; None gives "Step 2 · Review the axis roles". + purpose + One-sentence purpose under the title (HTML); None gives this + notebook's. + intro + Text of the blue "why this matters" box (HTML); None gives this + notebook's, an empty string leaves the box out. + show_next + False hides the "What next?" box, which sends the user to this + notebook's step 3 to compute the histograms. + + Returns + ------- + ipywidgets.Widget + The page. + """ + ui.set_visible(self.axis_next, show_next) + if intro is None: + intro = ( + "Why this matters: every image is cut into flat 2D planes, one per " + "depth slice, channel and time point. The roles below are suggested from " + "the image metadata, which is not always right, so check them with the " + "preview before confirming." + ) return ui.page( - "Step 2 · Review the axis roles", - "Tell the brick which image dimension is width, height, depth, channel and time.", - [ - ui.intro( - "Why this matters: every image is cut into flat 2D planes, one per " - "depth slice, channel and time point. The roles below are suggested from " - "the image metadata, which is not always right, so check them with the " - "preview before confirming." - ), - self.axis_box, - ], + "Step 2 · Review the axis roles" if title is None else title, + "Tell the brick which image dimension is width, height, depth, channel and time." + if purpose is None + else purpose, + [*([ui.intro(intro)] if intro else []), self.axis_box], ) def run_step(self) -> widgets.Widget: diff --git a/tests/test_notebook_ui.py b/tests/test_notebook_ui.py index 08e8d3ce..6a78f241 100644 --- a/tests/test_notebook_ui.py +++ b/tests/test_notebook_ui.py @@ -402,6 +402,94 @@ def test_conflicting_axes_explain_why_no_roles_are_offered(tmp_path): assert "check the experiment folder and fix any errors" in nb.readiness.value +# Steps 1 and 2 embedded in another brick --------------------------------------------------- + + +def page_parts(page): + """Return the title, the purpose and every widget below them in a step's page.""" + root = page.children[1] + found = [] + + def walk(widget): + found.append(widget) + for child in getattr(widget, "children", ()): + walk(child) + + for child in root.children[2:]: + walk(child) + return root.children[0].value, root.children[1].value, found + + +def intros(widgets_found): + return [w.value for w in widgets_found if "mb-intro" in str(getattr(w, "value", ""))] + + +def test_steps_keep_their_words_by_default(): + nb = PreprocessingNotebook() + title, purpose, found = page_parts(nb.folder_step()) + assert "Step 1 · Check the experiment folder" in title + assert "Choose the folder with your images." in purpose + assert len(intros(found)) == 1 and "What happens here" in intros(found)[0] + assert nb.output_card in found and nb.folder_next in found + assert ui.is_visible(nb.folder_next) + title, purpose, found = page_parts(nb.axis_step()) + assert "Step 2 · Review the axis roles" in title + assert "Tell the brick which image dimension" in purpose + assert len(intros(found)) == 1 and "flat 2D planes" in intros(found)[0] + assert nb.axis_box in found and ui.is_visible(nb.axis_next) + + +def test_folder_step_with_its_own_words_and_no_output_card(experiment): + nb = PreprocessingNotebook(str(experiment)) + title, purpose, found = page_parts( + nb.folder_step( + "Step 1 · Your data", + "Choose the folder with your images.", + intro="Here your folders are read.", + show_output=False, + show_next=False, + ) + ) + assert "Step 1 · Your data" in title and "Step 1 · Check" not in title + assert purpose == "
Choose the folder with your images.
" + assert intros(found) == ["
Here your folders are read.
"] + assert nb.output_card not in found and nb.output not in found + assert nb.folder_card in found and nb.check_button in found and nb.labels_card in found + # The check works without the card, with the default output folder. + nb.check_button.click() + assert nb.preflight is not None and nb.preflight.ok + nb.labels_confirmed.button.click() + assert "Go to Step 2" in nb.folder_next.value and not ui.is_visible(nb.folder_next) + + +def test_steps_without_an_intro_box(): + nb = PreprocessingNotebook() + assert intros(page_parts(nb.folder_step(intro=""))[2]) == [] + assert intros(page_parts(nb.axis_step(intro=""))[2]) == [] + + +def test_axis_step_with_its_own_words(experiment): + nb = PreprocessingNotebook(str(experiment)) + nb.folder_step() + title, purpose, found = page_parts( + nb.axis_step("Step 2 · Image dimensions", "Say which dimension is which.", show_next=False) + ) + assert "Step 2 · Image dimensions" in title and purpose.endswith( + "Say which dimension is which." + ) + assert len(intros(found)) == 1 # the default intro is kept + nb.check_button.click() + nb.labels_confirmed.button.click() + nb.preview_button.click() + nb.preview_checked.button.click() + nb.confirm_button.click() + assert nb.confirmed_mapping is not None + assert "compute the histograms" in nb.axis_next.value and not ui.is_visible(nb.axis_next) + # Building the default page again shows the box again. + nb.axis_step() + assert ui.is_visible(nb.axis_next) + + # Step 3 ----------------------------------------------------------------------------------- From 7087ad459d0bfd25a9fbf9463d2cc00286a128b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 19:50:46 +0000 Subject: [PATCH 05/11] Record the M3 input changes as unreleased in the status The repository has no changelog file: changes are recorded as plan section 12 decisions with the milestone status in plan section 10 and AGENTS.md section 11. Both now list questions 21 to 24 as unreleased work for MorphoRelate milestone M3. No version bump or tag. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 5 +++++ docs/plan.md | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c6e9c7b4..ba0f6852 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -610,6 +610,11 @@ jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb v0.1.0 with deprecated re-exports, its version in the manifest, Step 0 checking for it, and contract §3–§7 moved to the normalization contract (PRs #16 and #17, question 19), and `position_id` filled per cross-brick decision D02 (question 20). +- **Unreleased (for MorphoRelate milestone M3):** folders named `single` read as a + one-member level (question 21), a unit per axis in `PhysicalSizes` and physical sizes in + `inspect_image` (question 22), OME-Zarr physical sizes from the OME-NGFF scale (question + 23), and notebook steps 1 and 2 embeddable with their own words (question 24). No + version bump or tag. - **Next (not started):** follow-ups P2–P5 of the normalization plan (modality, references, colour statistics, previews). - **Not yet verified:** diff --git a/docs/plan.md b/docs/plan.md index 33ac0aef..afca26b4 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -312,6 +312,11 @@ API, CLI). The notebook and CLI only call the API. entry carries the image's `position_id`, so `condition`, `biological_repeat`, `technical_repeat` and `position_id` identify a position in every brick; images in one leaf folder with the same identifier are refused. *Implemented, see question 20.* +- **Input support for MorphoRelate milestone M3 (unreleased):** a folder named `single` + read as a one-member level, physical sizes with a unit per axis in the image header + (TIFF and OME-Zarr, from the OME-NGFF scale), and steps 1 and 2 of the notebook + embeddable by another brick. Outputs are unchanged. *Implemented, see questions 21 to + 24.* ## 11. Tests and acceptance criteria From 5cb4781009b660c107e0b416bb7edc9d70b1cf48 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:11:48 +0000 Subject: [PATCH 06/11] Never let OME-Zarr scale metadata make an image unreadable Physical sizes are information only, but open_ngff computed them without a guard, and a scale value too large for a float (an integer such as 10**400) raised OverflowError, so preflight reported the image as unreadable_ome_zarr; on main it was fine. Such a value now gives an unknown size for its axis, and any error while reading the sizes is logged and gives empty sizes, as for TIFF files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- .../physical_sizes.py | 15 +++++++--- src/morphobricks_preprocessing/zarr_input.py | 18 +++++++++++- tests/test_zarr_input.py | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 3fdce523..9cd172d8 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -235,13 +235,20 @@ def _ngff_scale(entry: dict, ndim: int) -> list[float | None] | None: 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 [ - float(v) if isinstance(v, int | float) and not isinstance(v, bool) else None - for v in values - ] + return [_scale_value(v) for v in values] return None +def _scale_value(value) -> float | None: + """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 + + def _tiff_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: """Return the physical sizes of an open TIFF file (all None when not given).""" try: diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index dfb1f821..0abec6c6 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -29,6 +29,7 @@ from __future__ import annotations +import logging import os from dataclasses import dataclass from pathlib import Path @@ -38,6 +39,8 @@ from .physical_sizes import PhysicalSizes, ngff_physical_sizes +logger = logging.getLogger(__name__) + ZARR_SUFFIX = ".zarr" @@ -282,10 +285,23 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: image_count=image_count, channel_names=labels if any(labels) else (), dataset_path=f"{prefix}{dataset_path}", - physical_sizes=ngff_physical_sizes(first, axes), + physical_sizes=_physical_sizes(first, axes, dataset_path), ) +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): """Return the ``labels`` group next to an image, or None when it has none.""" try: diff --git a/tests/test_zarr_input.py b/tests/test_zarr_input.py index f76ca5a4..7a5748d5 100644 --- a/tests/test_zarr_input.py +++ b/tests/test_zarr_input.py @@ -759,3 +759,31 @@ def test_tiff_reading_is_unchanged_next_to_stores(tmp_path): path = write_data(tmp_path / "a.tif", random_data((2, 8, 8)), axes="ZYX") assert tifffile.imread(path).shape == (2, 8, 8) assert len(list(iter_planes(path, prefill_mapping("ZYX")))) == 2 + + +# Physical sizes never block an image ----------------------------------------------------- + + +def test_scale_too_large_for_a_float_is_unknown_and_does_not_block(tmp_path): + root = tmp_path / "exp" + path = write_ngff( + root / "ctrl/a.ome.zarr", + np.zeros((2, 8, 8), np.uint8), + "zyx", + scale=[10**400, 0.5, 0.25], + units={"z": "micrometer", "y": "micrometer", "x": "micrometer"}, + ) + sizes = open_ngff(path).physical_sizes + assert (sizes.z, sizes.y, sizes.x) == (None, 0.5, 0.25) + 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 From 3b5313505715527fcd176e2eacb60f6557389a74 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 20:13:45 +0000 Subject: [PATCH 07/11] Resolve per-axis units when read, so replace and pickles stay consistent PhysicalSizes filled x_unit, y_unit and z_unit from space_unit in __post_init__. dataclasses.replace then raised (space_unit no longer matched the filled x_unit) or kept the old unit (space_unit=None), a pickle written before per-axis units loaded with x_unit None so unit("X") gave None, and PhysicalSizes(z=1, space_unit="micrometer", y_unit="nanometer").unit("Z") gave None. The per-axis units are now stored as given, None meaning space_unit, and unit(role) resolves them. Equality and hashing compare the sizes, space_unit and the resolved units, so a size given with space_unit alone equals the same size spelled out per axis. The readers set space_unit to the unit of X as before, or None when a space axis with a size has no unit (it would otherwise fall back to X's unit). The ValueError for differing space_unit and x_unit is gone: an axis's own unit wins. Plan section 12 question 22 updated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- docs/plan.md | 10 +- .../physical_sizes.py | 145 ++++++++++-------- tests/test_physical_sizes.py | 53 ++++++- 3 files changed, 134 insertions(+), 74 deletions(-) diff --git a/docs/plan.md b/docs/plan.md index afca26b4..ac18807d 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -546,9 +546,13 @@ Raised during review; to be decided before the milestones that need them: *Decided (October 2026): yes to both. `PhysicalSizes` gains `x_unit`, `y_unit` and `z_unit`; `unit(role)` returns each axis's own unit, from OME `PhysicalSizeYUnit` and `PhysicalSizeZUnit` (the OME default, micrometres, when left out, as for X) or ImageJ - `yunit` and `zunit` (the X unit when left out, as ImageJ writes them). `space_unit` - stays the unit of X, and sizes made with `space_unit` alone keep it for every space - axis, so existing callers are unchanged. `inspect_image` (`TiffMetadata`) gains the + `yunit` and `zunit` (the X unit when left out, as ImageJ writes them). The per-axis + units are kept as given and resolved when read: an axis without its own unit has + `space_unit`, so sizes made with `space_unit` alone keep it for every space axis and + stay consistent under `dataclasses.replace` and pickles written before; the readers + give `space_unit` the unit of X, as before (None when a space axis with a size has no + unit). Equality and hashing compare the resolved units, so existing callers are + unchanged. `inspect_image` (`TiffMetadata`) gains the field `physical_sizes`, read from the header already open, for information only; the existing fields are unchanged. OME-Zarr copies of images whose Y or Z unit differs from X now write that axis's own unit. `PhysicalSizes` and `read_physical_sizes` moved diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 9cd172d8..0ff00a10 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -59,31 +59,28 @@ _OME_DEFAULT_TIME_UNIT = "s" -@dataclass(frozen=True) +@dataclass(frozen=True, eq=False) class PhysicalSizes: """Pixel sizes from metadata; None where unknown. Attributes ---------- x, y, z - Pixel size and z-step, each in its own unit (``x_unit``, ``y_unit``, - ``z_unit``). + Pixel size and z-step, each in its own unit (see :meth:`unit`). t Time between frames in ``time_unit``. space_unit - The unit of X. Kept for callers written when one unit served every space - axis; :meth:`unit` gives each axis's own unit. Given alone, it is the unit - of X, Y and Z. + The unit of the space axes that have no unit of their own. Given alone, as + before per-axis units, it is the unit of X, Y and Z; the readers give it + the unit of X (None when a space axis with a size has no unit). time_unit The unit of T. x_unit, y_unit, z_unit - OME-NGFF unit names per space axis, e.g. ``"micrometer"``; None when the - metadata gives none. + An axis's own unit, as an OME-NGFF unit name such as ``"micrometer"``; + None means ``space_unit``. :meth:`unit` resolves them, so they stay + consistent under :func:`dataclasses.replace` and pickling. - Raises - ------ - ValueError - If ``space_unit`` and ``x_unit`` are both given and differ. + Two sizes are equal when their sizes, ``space_unit`` and resolved units are. """ x: float | None = None @@ -96,21 +93,25 @@ class PhysicalSizes: y_unit: str | None = None z_unit: str | None = None - def __post_init__(self) -> None: - if self.x_unit is None and self.y_unit is None and self.z_unit is None: - # Sizes made with one space unit (before per-axis units) keep meaning it - # for every space axis. - for name in ("x_unit", "y_unit", "z_unit"): - object.__setattr__(self, name, self.space_unit) - elif self.x_unit is None: - object.__setattr__(self, "x_unit", self.space_unit) - elif self.space_unit is None: - object.__setattr__(self, "space_unit", self.x_unit) - elif self.space_unit != self.x_unit: - raise ValueError( - f"space_unit is the unit of X: {self.space_unit!r} differs from x_unit " - f"{self.x_unit!r}" - ) + 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``.""" + return tuple( + own if own is not None else self.space_unit + for own in (self.x_unit, self.y_unit, self.z_unit) + ) + + def _key(self) -> tuple: + return (self.x, self.y, self.z, self.t, self.space_unit, self.time_unit) + ( + self._space_units() + ) + + def __eq__(self, other: object) -> bool: + if other.__class__ is not self.__class__: + return NotImplemented + return self._key() == other._key() + + def __hash__(self) -> int: + return hash(self._key()) def scale(self, role: str) -> float: """Return the size of one step along ``role`` (1 when unknown).""" @@ -123,12 +124,39 @@ def unit(self, role: str) -> str | None: return self.time_unit if self.t is not None else None if role == "C": return None - size, unit = { - "X": (self.x, self.x_unit), - "Y": (self.y, self.y_unit), - "Z": (self.z, self.z_unit), - }[role] - return unit if size is not None else None + index = "XYZ".index(role) + size = (self.x, self.y, self.z)[index] + return self._space_units()[index] if size is not None else None + + +def _sizes( + x: float | None, + y: float | None, + z: float | None, + t: float | None, + units: dict[str, str | None], + time_unit: str | None, +) -> PhysicalSizes: + """Build sizes from metadata with each space axis's own unit (``units`` by letter). + + ``space_unit`` is the unit of X, as before per-axis units, unless a space axis + with a size has no unit: it would then fall back to X's unit, so ``space_unit`` + is None. + """ + unitless = any( + size is not None and units[a] is None for a, size in zip("XYZ", (x, y, z), strict=True) + ) + return PhysicalSizes( + x=x, + y=y, + z=z, + t=t, + space_unit=None if unitless else units["X"], + time_unit=time_unit, + x_unit=units["X"], + y_unit=units["Y"], + z_unit=units["Z"], + ) def read_physical_sizes(path: str | os.PathLike) -> PhysicalSizes: @@ -207,16 +235,13 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: if letter in "XYZT": found.setdefault(letter, (_number(size), unit)) sizes = {letter: found.get(letter, (None, None)) for letter in "XYZT"} - return PhysicalSizes( - x=sizes["X"][0], - y=sizes["Y"][0], - z=sizes["Z"][0], - t=sizes["T"][0], - space_unit=sizes["X"][1], - time_unit=sizes["T"][1], - x_unit=sizes["X"][1], - y_unit=sizes["Y"][1], - z_unit=sizes["Z"][1], + return _sizes( + sizes["X"][0], + sizes["Y"][0], + sizes["Z"][0], + sizes["T"][0], + {letter: sizes[letter][1] for letter in "XYZ"}, + sizes["T"][1], ) @@ -271,16 +296,13 @@ def _ome_sizes(xml: str) -> PhysicalSizes: for axis in "XYZ" } time_unit = _TIME_UNITS.get(pixels.get("TimeIncrementUnit", _OME_DEFAULT_TIME_UNIT)) - return PhysicalSizes( - x=_number(pixels.get("PhysicalSizeX")), - y=_number(pixels.get("PhysicalSizeY")), - z=_number(pixels.get("PhysicalSizeZ")), - t=_number(pixels.get("TimeIncrement")) if time_unit else None, - space_unit=units["X"], - time_unit=time_unit, - x_unit=units["X"], - y_unit=units["Y"], - z_unit=units["Z"], + return _sizes( + _number(pixels.get("PhysicalSizeX")), + _number(pixels.get("PhysicalSizeY")), + _number(pixels.get("PhysicalSizeZ")), + _number(pixels.get("TimeIncrement")) if time_unit else None, + units, + time_unit, ) @@ -293,16 +315,13 @@ def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: page = tif.pages.first time_unit = _TIME_UNITS.get(str(meta.get("tunit", "sec")).strip().lower()) # Without a unit the resolution tags are a default (often 1 pixel per inch). - return PhysicalSizes( - x=_resolution(page.tags.get("XResolution")) if unit else None, - y=_resolution(page.tags.get("YResolution")) if y_unit else None, - z=_number(meta.get("spacing")) if z_unit else None, - t=_number(meta.get("finterval")) if time_unit else None, - space_unit=unit, - time_unit=time_unit, - x_unit=unit, - y_unit=y_unit, - z_unit=z_unit, + return _sizes( + _resolution(page.tags.get("XResolution")) if unit else None, + _resolution(page.tags.get("YResolution")) if y_unit else None, + _number(meta.get("spacing")) if z_unit else None, + _number(meta.get("finterval")) if time_unit else None, + {"X": unit, "Y": y_unit, "Z": z_unit}, + time_unit, ) diff --git a/tests/test_physical_sizes.py b/tests/test_physical_sizes.py index 8aadfe17..264a893c 100644 --- a/tests/test_physical_sizes.py +++ b/tests/test_physical_sizes.py @@ -1,10 +1,20 @@ +import pickle +from dataclasses import replace + import numpy as np -import pytest import tifffile from morphobricks_preprocessing import PhysicalSizes, read_physical_sizes, zarr_export from morphobricks_preprocessing import physical_sizes as physical_sizes_module +# PhysicalSizes(x=0.1, y=0.2, z=0.5, space_unit="micrometer") pickled before per-axis units. +OLD_PICKLE = bytes.fromhex( + "80049597000000000000008c266d6f7270686f627269636b735f70726570726f63657373696e672e7a617272" + "5f6578706f7274948c0d506879736963616c53697a65739493942981947d94288c017894473fb99999999999" + "9a8c017994473fc999999999999a8c017a94473fe00000000000008c0174944e8c0a73706163655f756e6974" + "948c0a6d6963726f6d65746572948c0974696d655f756e6974944e75622e" +) + def write_ome(path, **sizes): tifffile.imwrite( @@ -94,17 +104,44 @@ def test_space_unit_alone_is_the_unit_of_every_space_axis(): ) -def test_per_axis_units_set_space_unit_to_the_unit_of_x(): +def test_per_axis_units_fall_back_to_space_unit(): sizes = PhysicalSizes(x=1.0, y=2.0, x_unit="nanometer", y_unit="micrometer") - assert sizes.space_unit == "nanometer" - assert sizes.unit("Y") == "micrometer" - # An axis given no unit has none, even when X has one. + assert (sizes.unit("X"), sizes.unit("Y")) == ("nanometer", "micrometer") + # An axis given no unit has space_unit, here none, even when X has one. assert PhysicalSizes(x=1.0, y=2.0, x_unit="nanometer", z_unit="meter").unit("Y") is None + partial = PhysicalSizes(y=1.0, z=1.0, space_unit="micrometer", y_unit="nanometer") + assert (partial.unit("Y"), partial.unit("Z")) == ("nanometer", "micrometer") + # An axis's own unit wins over space_unit. + assert PhysicalSizes(x=1.0, space_unit="micrometer", x_unit="nanometer").unit("X") == ( + "nanometer" + ) + + +def test_replace_keeps_units_consistent(): + sizes = PhysicalSizes(x=0.1, y=0.1, space_unit="micrometer") + assert replace(sizes, space_unit="nanometer").unit("Y") == "nanometer" + assert replace(sizes, x_unit="nanometer").unit("X") == "nanometer" + assert replace(sizes, x_unit="nanometer").unit("Y") == "micrometer" + assert replace(sizes, space_unit=None).unit("X") is None -def test_space_unit_and_x_unit_must_agree(): - with pytest.raises(ValueError, match="unit of X"): - PhysicalSizes(x=1.0, space_unit="micrometer", x_unit="nanometer") +def test_sizes_pickled_before_per_axis_units_keep_their_unit(): + # PhysicalSizes(x=0.1, y=0.2, z=0.5, space_unit="micrometer") pickled by the version + # before per-axis units (as morphobricks_preprocessing.zarr_export.PhysicalSizes). + sizes = pickle.loads(OLD_PICKLE) + assert [sizes.unit(role) for role in "XYZ"] == ["micrometer"] * 3 + assert sizes == PhysicalSizes(x=0.1, y=0.2, z=0.5, space_unit="micrometer") + assert pickle.loads(pickle.dumps(sizes)) == sizes + + +def test_equal_sizes_hash_alike(): + given = PhysicalSizes(x=0.1, space_unit="micrometer") + spelled_out = PhysicalSizes( + x=0.1, space_unit="micrometer", x_unit="micrometer", y_unit="micrometer" + ) + assert given == spelled_out and len({given, spelled_out}) == 1 + assert given != PhysicalSizes(x=0.1, space_unit="micrometer", y_unit="nanometer") + assert given != "not sizes" def test_units_of_unknown_sizes_and_channels_are_none(): From ef274a3d0b187ae49d35ed4d2cb57a635134b9a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:53:36 +0000 Subject: [PATCH 08/11] Convert again copies written with outdated axis units OME-Zarr copies written before each axis had its own unit carry X's unit on Y and Z, and reruns kept them, so one output folder could mix the old and the new units. A converted TIFF whose copy's axis units (read from the copy's .zattrs only) differ from those its header gives now counts as changed and is converted again through the usual staged publication; its rows do not change. A copy whose metadata cannot be read is written again rather than trusted. Copies with matching units are kept as before. Contract section 10, plan section 12 question 22 and the user guide updated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- docs/contract.md | 1 + docs/plan.md | 5 ++- docs/user_guide.md | 6 ++- src/morphobricks_preprocessing/run.py | 13 ++++++- src/morphobricks_preprocessing/zarr_export.py | 25 ++++++++++++ tests/test_zarr_export.py | 38 +++++++++++++++++++ 6 files changed, 83 insertions(+), 5 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index 24e1f039..8b24076a 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -164,4 +164,5 @@ presentation does not change the conversion rules below. | Updates | Copies of unchanged images are kept, of new or changed images rewritten, and of images no longer analysed removed; a failed run leaves the earlier copies as they were (see §9 for publication) | | Switching | Turning conversion on or off recomputes every TIFF row; turning it off removes the copies the manifest recorded | | Missing copy | A converted TIFF whose copy was deleted counts as changed and is converted again | +| Outdated units | A copy whose axis units differ from those its TIFF's metadata gives now (copies written before each axis had its own unit, plan §12 question 22) counts as changed and is converted again; its rows do not change. Only the copy's `.zattrs` is read to tell | | Settings change | Copies written with other conversion settings (recorded in the manifest) are written again; their rows do not change | diff --git a/docs/plan.md b/docs/plan.md index ac18807d..d34a6a49 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -555,7 +555,10 @@ Raised during review; to be decided before the milestones that need them: unchanged. `inspect_image` (`TiffMetadata`) gains the field `physical_sizes`, read from the header already open, for information only; the existing fields are unchanged. OME-Zarr copies of images whose Y or Z unit differs - from X now write that axis's own unit. `PhysicalSizes` and `read_physical_sizes` moved + from X now write that axis's own unit; on the next run, a copy written before with + other axis units (read from its `.zattrs`) counts as changed and is converted again + through the usual staged publication, so one output folder never mixes the two (its + rows do not change). `PhysicalSizes` and `read_physical_sizes` moved to the module `physical_sizes` and stay importable from `zarr_export` and the package. The CSV, the manifest and their versions are unchanged.* 23. OME-Zarr physical sizes: `open_ngff` read the axes but not the OME-NGFF scale, so the diff --git a/docs/user_guide.md b/docs/user_guide.md index e7fe38cf..e16ce7c3 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -278,8 +278,10 @@ The default output folder is next to the experiment folder: `ome_zarr/control/bio_rep1/tech_rep1/img_01.ome.zarr`. The full-resolution level holds exactly the source pixel values; smaller levels are averages for fast viewing (for example in napari with the `napari-ome-zarr` plugin). Pixel sizes and channel names are - copied from the metadata when present. Copies of unchanged images are kept on later - runs, copies of deleted or excluded images are removed. Switching back to reading the + copied from the metadata when present, each axis with its own unit. Copies of unchanged + images are kept on later runs, copies of deleted or excluded images are removed. Copies + written by an earlier version with the X unit on every axis are converted again once, + with their own units; the table does not change. Switching back to reading the TIFF images directly removes the copies and recomputes their rows. Details in [contract.md](contract.md) §10. diff --git a/src/morphobricks_preprocessing/run.py b/src/morphobricks_preprocessing/run.py index 6f3d6c7c..2e381427 100644 --- a/src/morphobricks_preprocessing/run.py +++ b/src/morphobricks_preprocessing/run.py @@ -64,6 +64,7 @@ from .zarr_export import ( ConversionSettings, OmeZarrWriter, + copy_units_current, is_copy_path, ome_zarr_relative_path, read_physical_sizes, @@ -380,8 +381,16 @@ def stop(**counts) -> RunResult: # During a rebuild there are no rows to check; otherwise the rows must be complete. and (reason is not None or len(previous_records.get(rel, ())) == before.plane_count) # Rows of a converted TIFF are only kept while its copy is still there, as - # written with the same settings. - and (copy is None or ((output / copy).is_dir() and same_conversion)) + # written with the same settings and with the axis units read now (copies + # written before each axis had its own unit are written again). + and ( + copy is None + or ( + (output / copy).is_dir() + and same_conversion + and copy_units_current(output / copy, header.physical_sizes) + ) + ) ): unchanged.append(rel) else: diff --git a/src/morphobricks_preprocessing/zarr_export.py b/src/morphobricks_preprocessing/zarr_export.py index aa5aaa43..eb8077ea 100644 --- a/src/morphobricks_preprocessing/zarr_export.py +++ b/src/morphobricks_preprocessing/zarr_export.py @@ -21,6 +21,7 @@ from __future__ import annotations +import json import logging import math import os @@ -498,6 +499,30 @@ def is_copy_path(output_folder: str | os.PathLike, relative_path: str) -> bool: return base in target.parents and target.name.endswith(OME_ZARR_SUFFIX) +def copy_units_current(copy: str | os.PathLike, sizes: PhysicalSizes) -> bool: + """Return True if a copy's axis units are those its source's sizes give now. + + Copies written before each axis had its own unit (plan §12 question 22) may + carry X's unit on Y or Z; such copies are written again on the next run. Only + the copy's metadata file is read. A copy whose metadata cannot be read counts + as out of date, so it is written again rather than trusted. + + Parameters + ---------- + copy + The copy's folder (``/ome_zarr/....ome.zarr``). + sizes + The source image's physical sizes, as :func:`read_physical_sizes` gives them. + """ + try: + attrs = json.loads((Path(copy) / ".zattrs").read_text(encoding="utf-8")) + axes = attrs["multiscales"][0]["axes"] + return all(axis.get("unit") == sizes.unit(str(axis["name"]).upper()) for axis in axes) + except (OSError, ValueError, KeyError, IndexError, TypeError, AttributeError): + logger.debug("Cannot read the axis units of the copy %s", copy, exc_info=True) + return False + + # zarr 2 / 3 --------------------------------------------------------------------------- diff --git a/tests/test_zarr_export.py b/tests/test_zarr_export.py index f601bae9..6e76b268 100644 --- a/tests/test_zarr_export.py +++ b/tests/test_zarr_export.py @@ -727,6 +727,44 @@ def test_copies_follow_the_conversion_settings(tmp_path, settings, compressor): assert read_manifest(result.manifest_path).settings.conversion == custom +@needs_zarr +def test_copies_with_outdated_axis_units_are_written_again(tmp_path): + root = tmp_path / "exp" + for name in ("a.ome.tif", "b.ome.tif"): + (root / "ctrl").mkdir(parents=True, exist_ok=True) + tifffile.imwrite( + root / "ctrl" / name, + random_data((2, 8, 8), "uint8"), + ome=True, + photometric="minisblack", + metadata={ + "axes": "ZYX", + "PhysicalSizeX": 120.0, + "PhysicalSizeXUnit": "nm", + "PhysicalSizeY": 0.25, + "PhysicalSizeYUnit": "µm", + "PhysicalSizeZ": 2.0, + }, + ) + first = run_experiment(root, ZYX, convert_tiffs=True) + table = first.csv_path.read_bytes() + # Make a.ome.tif's copy look like one written before per-axis units: X's unit + # on every space axis. + attrs_path = first.output_folder / ome_zarr_relative_path("ctrl/a.ome.tif") / ".zattrs" + attrs = json.loads(attrs_path.read_text(encoding="utf-8")) + for axis in attrs["multiscales"][0]["axes"]: + axis["unit"] = "nanometer" + attrs_path.write_text(json.dumps(attrs), encoding="utf-8") + second = run_experiment(root, ZYX, convert_tiffs=True) + assert second.converted == ("ctrl/a.ome.tif",) + assert second.csv_path.read_bytes() == table # same pixels, same rows + units = [ + a["unit"] for a in multiscale(open_copy(second.output_folder, "ctrl/a.ome.tif"))["axes"] + ] + assert units == ["micrometer", "micrometer", "nanometer"] + assert run_experiment(root, ZYX, convert_tiffs=True).converted == () + + @needs_zarr def test_changed_conversion_settings_rewrite_the_copies(experiment): first = run_experiment(experiment, ZYX, convert_tiffs=True) From bff743f75ab8df01b0f4f480a839e3abbfeb9ac3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:55:28 +0000 Subject: [PATCH 09/11] Normalise unit spellings: OME-Zarr units, angstroms, ImageJ escapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OME-Zarr axis units were kept exactly as written, so "µm", "um" or "micron" reached callers that expect UDUNITS-2 names (MorphoRelate rejects them as "not a length unit"). They now map through the same tables as TIFF units ("micrometer", "second", ...); other units are kept as written, and spellings of two letters or fewer are matched exactly ("Mm" is not "mm"). The space-unit table gains the angstrom (OME's "Å", the angstrom sign and "angstrom"). ImageJ writes the micro sign as the text µ in the TIFF description and tifffile returns it as written, so unit, yunit, zunit and tunit of "µm" gave no unit; the escapes are now decoded. Contract section 2 and plan section 12 question 23 updated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- docs/contract.md | 2 +- docs/plan.md | 3 +- .../physical_sizes.py | 48 +++++++++++---- tests/test_physical_sizes.py | 60 +++++++++++++++++++ 4 files changed, 100 insertions(+), 13 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index 8b24076a..08304cc6 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -51,7 +51,7 @@ length 1 and no role, has coordinate 0. | 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 a warning. | -| Physical sizes | Read with the header, for information only (not part of the CSV or the manifest): `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`. 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. | +| Physical sizes | Read with the header, for information only (not part of the CSV or the manifest): `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). Ångström (`Å`) and ImageJ's escaped micro sign (`\u00B5m`) are recognised. 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. | ## 3. Pixel types diff --git a/docs/plan.md b/docs/plan.md index d34a6a49..73d035d0 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -568,7 +568,8 @@ Raised during review; to be decided before the milestones that need them: `physical_sizes`: the `scale` of level 0's `coordinateTransformations`, multiplied by the multiscales entry's own `coordinateTransformations` scale when it has one (OME-NGFF applies them in turn; translations are ignored), for X, Y, Z and T, with each axis's - `unit` as written (OME-NGFF 0.4 and 0.5). `inspect_image` records them for OME-Zarr + `unit` (OME-NGFF 0.4 and 0.5), common spellings (`µm`, `um`, `micron`, `s`) given as + the UDUNITS-2 names TIFF units get, other units as written. `inspect_image` records them for OME-Zarr images as for TIFF files, and label images opened with `open_ngff_labels` carry their own. Nothing is guessed: without a level-0 scale, with a scale stored in a separate file (`path`) or not one number per axis, every size is None; an axis without a unit diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 0ff00a10..444875bb 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -20,6 +20,7 @@ import logging import math import os +import re from collections.abc import Sequence from dataclasses import dataclass from xml.etree import ElementTree @@ -31,6 +32,10 @@ # Units in OME and ImageJ metadata, and their OME-NGFF (UDUNITS-2) names. _SPACE_UNITS = { "pm": "picometer", + "Å": "angstrom", # the OME symbol, LATIN CAPITAL LETTER A WITH RING ABOVE + "\u212b": "angstrom", # ANGSTROM SIGN + "å": "angstrom", # ImageJ units are compared in lower case + "angstrom": "angstrom", "nm": "nanometer", "µm": "micrometer", "μm": "micrometer", @@ -184,8 +189,9 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: ``coordinateTransformations`` (level 0), multiplied by the ``scale`` of the multiscales entry's own ``coordinateTransformations`` when it has them, as OME-NGFF applies them in turn; translations do not change sizes. Units are - each axis's ``unit``, as written (OME-NGFF uses UDUNITS-2 names such as - ``"micrometer"``, the names :func:`read_physical_sizes` gives). + each axis's ``unit`` as UDUNITS-2 names, the names OME-NGFF uses and + :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 @@ -224,16 +230,12 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: declared = multiscale.get("axes") units: list[str | None] = [None] * len(axes) if isinstance(declared, list) and len(declared) == len(axes): - units = [ - axis["unit"] - if isinstance(axis, dict) and isinstance(axis.get("unit"), str) and axis["unit"] - else None - for axis in declared - ] + units = [axis.get("unit") if isinstance(axis, dict) else None for axis in declared] found: dict[str, tuple[float | None, str | None]] = {} for letter, size, unit in zip(axes, scale, units, strict=True): if letter in "XYZT": - found.setdefault(letter, (_number(size), unit)) + table = _TIME_UNITS if letter == "T" else _SPACE_UNITS + found.setdefault(letter, (_number(size), _ngff_unit(unit, table))) sizes = {letter: found.get(letter, (None, None)) for letter in "XYZT"} return _sizes( sizes["X"][0], @@ -313,7 +315,7 @@ def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: y_unit = _imagej_unit(meta["yunit"]) if "yunit" in meta else unit z_unit = _imagej_unit(meta["zunit"]) if "zunit" in meta else unit page = tif.pages.first - time_unit = _TIME_UNITS.get(str(meta.get("tunit", "sec")).strip().lower()) + time_unit = _TIME_UNITS.get(_imagej_text(meta.get("tunit", "sec"))) # Without a unit the resolution tags are a default (often 1 pixel per inch). return _sizes( _resolution(page.tags.get("XResolution")) if unit else None, @@ -325,8 +327,32 @@ def _imagej_sizes(tif: tifffile.TiffFile) -> PhysicalSizes: ) +def _ngff_unit(value, table: dict[str, str]) -> str | None: + """Return an OME-NGFF axis unit with common spellings as UDUNITS-2 names. + + ``"µm"``, ``"um"`` and ``"micron"`` become ``"micrometer"``, ``"s"`` + becomes ``"second"``; other units are kept as written, and spellings of two + letters or fewer are matched exactly (``"Mm"`` is not ``"mm"``). + """ + if not isinstance(value, str) or not value.strip(): + return None + text = value.strip() + known = table.get(text) or (table.get(text.lower()) if len(text) > 2 else None) + return known or text + + def _imagej_unit(value) -> str | None: - return _SPACE_UNITS.get(str(value or "").strip().lower()) + return _SPACE_UNITS.get(_imagej_text(value)) + + +def _imagej_text(value) -> str: + r"""Return an ImageJ unit in lower case, with its ``\uXXXX`` escapes decoded. + + ImageJ writes non-ASCII characters as ``\u00B5`` in the TIFF description, and + tifffile returns them as written, so ``\u00B5m`` is a micrometre. + """ + text = re.sub(r"\\u([0-9a-fA-F]{4})", lambda m: chr(int(m.group(1), 16)), str(value or "")) + return text.strip().lower() def _resolution(tag) -> float | None: diff --git a/tests/test_physical_sizes.py b/tests/test_physical_sizes.py index 264a893c..c3d970ad 100644 --- a/tests/test_physical_sizes.py +++ b/tests/test_physical_sizes.py @@ -2,6 +2,7 @@ from dataclasses import replace import numpy as np +import pytest import tifffile from morphobricks_preprocessing import PhysicalSizes, read_physical_sizes, zarr_export @@ -153,3 +154,62 @@ def test_units_of_unknown_sizes_and_channels_are_none(): def test_still_importable_from_zarr_export(): assert zarr_export.PhysicalSizes is physical_sizes_module.PhysicalSizes is PhysicalSizes assert zarr_export.read_physical_sizes is read_physical_sizes + + +# Unit spellings ------------------------------------------------------------------------ + + +@pytest.mark.parametrize("symbol", ["Å", "Å"]) +def test_ome_angstroms(tmp_path, symbol): + path = write_ome( + tmp_path / "a.ome.tif", + PhysicalSizeX=5.0, + PhysicalSizeXUnit=symbol, + PhysicalSizeY=5.0, + PhysicalSizeYUnit=symbol, + ) + sizes = read_physical_sizes(path) + assert (sizes.unit("X"), sizes.unit("Y")) == ("angstrom", "angstrom") + + +def test_imagej_escaped_micro_sign(tmp_path): + # ImageJ writes the micro sign as µ in the description; tifffile keeps the text. + path = tmp_path / "a.tif" + tifffile.imwrite( + path, + np.zeros((3, 8, 8), np.uint16), + imagej=True, + resolution=(1 / 0.25, 1 / 0.5), + metadata={ + "axes": "ZYX", + "unit": "\\u00B5m", + "yunit": "\\u00B5m", + "zunit": "\\u00B5m", + "spacing": 1.5, + }, + ) + assert tifffile.TiffFile(path).imagej_metadata["unit"] == "\\u00B5m" + sizes = read_physical_sizes(path) + assert (sizes.x, sizes.y, sizes.z) == (0.25, 0.5, 1.5) + assert [sizes.unit(role) for role in "XYZ"] == ["micrometer"] * 3 + + +def test_ngff_unit_spellings_become_udunits_names(): + multiscale = { + "axes": [ + {"name": "t", "type": "time", "unit": "s"}, + {"name": "z", "type": "space", "unit": "µm"}, + {"name": "y", "type": "space", "unit": "Micron"}, + {"name": "x", "type": "space", "unit": "um"}, + ], + "datasets": [ + {"path": "0", "coordinateTransformations": [{"type": "scale", "scale": [1, 2, 3, 4]}]} + ], + } + sizes = physical_sizes_module.ngff_physical_sizes(multiscale, "TZYX") + assert [sizes.unit(role) for role in "TZYX"] == ["second"] + ["micrometer"] * 3 + # Other units are kept as written; short spellings are matched exactly. + multiscale["axes"][1]["unit"] = "kilometer" + multiscale["axes"][2]["unit"] = "Mm" + sizes = physical_sizes_module.ngff_physical_sizes(multiscale, "TZYX") + assert (sizes.unit("Z"), sizes.unit("Y")) == ("kilometer", "Mm") From 3cb920033bcc10a07cecea4c2010e4e772d94086 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:57:03 +0000 Subject: [PATCH 10/11] Give no sizes for malformed transformations; reject boolean sizes A multiscales entry whose coordinateTransformations is not a list (a dict, say) was skipped, so the level-0 scale was used as if the image had no multiscale-level transformations. It now gives no sizes, as the docs say for a scale that cannot be used; the same holds at the dataset level. _number turned True into a size of 1: tifffile reads ImageJ's "spacing=true" as True. Booleans are no longer sizes (OME-NGFF scales already rejected them; a test now says so). Also adds a test that the sizes come from level 0 of a multi-level pyramid (it fails if the last dataset is read instead). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- .../physical_sizes.py | 11 ++++-- tests/test_physical_sizes.py | 26 +++++++++++++ tests/test_zarr_input.py | 37 +++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/morphobricks_preprocessing/physical_sizes.py b/src/morphobricks_preprocessing/physical_sizes.py index 444875bb..cb6aae4a 100644 --- a/src/morphobricks_preprocessing/physical_sizes.py +++ b/src/morphobricks_preprocessing/physical_sizes.py @@ -250,12 +250,15 @@ def ngff_physical_sizes(multiscale: dict, axes: str) -> PhysicalSizes: 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 it - has one that cannot be used (stored in a file, or not one number per axis). + 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 not isinstance(transforms, list): + 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 @@ -366,6 +369,8 @@ def _resolution(tag) -> float | None: def _number(value) -> float | None: + if isinstance(value, bool): # True would otherwise read as a size of 1 + return None try: number = float(value) except (TypeError, ValueError): diff --git a/tests/test_physical_sizes.py b/tests/test_physical_sizes.py index c3d970ad..1d0c2110 100644 --- a/tests/test_physical_sizes.py +++ b/tests/test_physical_sizes.py @@ -213,3 +213,29 @@ def test_ngff_unit_spellings_become_udunits_names(): multiscale["axes"][2]["unit"] = "Mm" sizes = physical_sizes_module.ngff_physical_sizes(multiscale, "TZYX") assert (sizes.unit("Z"), sizes.unit("Y")) == ("kilometer", "Mm") + + +def test_imagej_boolean_values_are_not_sizes(tmp_path): + # tifffile reads "spacing=true" as True, which float() would turn into 1. + path = tmp_path / "a.tif" + tifffile.imwrite( + path, + np.zeros((3, 8, 8), np.uint16), + imagej=True, + resolution=(1 / 0.25, 1 / 0.25), + metadata={"axes": "ZYX", "unit": "um", "spacing": True, "finterval": True}, + ) + assert tifffile.TiffFile(path).imagej_metadata["spacing"] is True + sizes = read_physical_sizes(path) + assert (sizes.x, sizes.z, sizes.t) == (0.25, None, None) + + +def test_ngff_boolean_scales_are_not_sizes(): + multiscale = { + "axes": [{"name": "y", "type": "space"}, {"name": "x", "type": "space"}], + "datasets": [ + {"path": "0", "coordinateTransformations": [{"type": "scale", "scale": [True, 0.5]}]} + ], + } + sizes = physical_sizes_module.ngff_physical_sizes(multiscale, "YX") + assert (sizes.y, sizes.x) == (None, 0.5) diff --git a/tests/test_zarr_input.py b/tests/test_zarr_input.py index 7a5748d5..78fdf875 100644 --- a/tests/test_zarr_input.py +++ b/tests/test_zarr_input.py @@ -34,6 +34,7 @@ from morphobricks_preprocessing import run as run_module from morphobricks_preprocessing.cli import main from morphobricks_preprocessing.histogram import compute_plane_histogram +from morphobricks_preprocessing.zarr_export import ConversionSettings from morphobricks_preprocessing.zarr_input import ngff_axes from tiff_tree import write_data, write_file from zarr_tree import write_bioformats2raw, write_ngff, write_plate, zarr_major @@ -787,3 +788,39 @@ def broken(*args, **kwargs): 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]} + kwargs = ( + {"dataset_transforms": malformed} + if where == "dataset" + else {"scale": [2.0, 0.5, 0.25], "multiscale_transforms": malformed} + ) + path = write_ngff(tmp_path / "a.ome.zarr", np.zeros((3, 8, 8), np.uint8), "zyx", **kwargs) + assert open_ngff(path).physical_sizes == PhysicalSizes() + + +def test_sizes_come_from_level_0_of_a_pyramid(tmp_path): + # The brick's own copy has three levels with scales 0.5, 1 and 2 along X and Y. + source = tmp_path / "exp/a.ome.tif" + source.parent.mkdir(parents=True) + tifffile.imwrite( + source, + np.zeros((2, 64, 64), np.uint16), + ome=True, + photometric="minisblack", + metadata={"axes": "ZYX", "PhysicalSizeX": 0.5, "PhysicalSizeY": 0.5, "PhysicalSizeZ": 2}, + ) + result = run_experiment( + tmp_path / "exp", + prefill_mapping("ZYX"), + convert_tiffs=True, + conversion=ConversionSettings(chunk_size=16, pyramid_levels=2), + ) + copy = result.output_folder / ome_zarr_relative_path("a.ome.tif") + image = open_ngff(copy) + assert image.dataset_path == "0" and image.shape == (2, 64, 64) + sizes = image.physical_sizes + assert (sizes.x, sizes.y, sizes.z) == (0.5, 0.5, 2.0) From cf4ef2161de8b6e0387cd495f4e28362465368d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 05:59:00 +0000 Subject: [PATCH 11/11] Show the hidden output card when its folder must be fixed folder_step(show_output=False) left the output folder card out of the page, so a notebook started with an output folder inside the experiment reported output_inside_experiment with no box to fix it. The card is now always on the page and only hidden; the check shows it again when it finds that error, and hides it once the folder is fixed. The default page is unchanged. Also rewords plan section 2, which still called single "the reserved value", and updates plan section 12 question 24 and AGENTS.md. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017TP44aBCGzyksTpPS2FFmQ --- AGENTS.md | 4 ++-- docs/plan.md | 14 ++++++++------ src/morphobricks_preprocessing/notebook_ui.py | 17 +++++++++++++---- tests/test_notebook_ui.py | 16 +++++++++++++++- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ba0f6852..bd3e0fe9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -452,8 +452,8 @@ plainer version "not interactive enough". Descriptive, guided and forgiving is t - **Embedding steps 1 and 2 (Q24):** `folder_step` and `axis_step` keep this notebook's words by default; another brick (MorphoRelate) passes its own `title`, `purpose` and - `intro` (HTML; `""` leaves the box out), `show_output=False` to leave out the output - folder card, and `show_next=False` to hide the "What next?" box naming this notebook's + `intro` (HTML; `""` leaves the box out), `show_output=False` to hide the output folder + card (shown again when the check reports `output_inside_experiment`), and `show_next=False` to hide the "What next?" box naming this notebook's steps. The widgets and their behaviour are the same either way. - **Page and cards:** `ui.page(title, subtitle, children)` wraps a step in a grey page (`mb-root`) with a title and a one-sentence purpose. `ui.card(header, children)` gives a diff --git a/docs/plan.md b/docs/plan.md index 73d035d0..67996d39 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -56,9 +56,10 @@ Images are TIFF files (`.tif`, `.tiff`) or OME-Zarr images: folders whose names TIFF and OME-Zarr images may be mixed in one experiment. Folder names are arbitrary labels and need only be unique among siblings. Each branch may -stop at any level; omitted levels receive the reserved value `single`. Levels cannot be -skipped. Labels are scoped to their parents: `bio_rep1` under two conditions are two -different samples, so groups are always identified by the full label path. +stop at any level; omitted levels receive the value `single` (a level with one member, +question 21). Levels cannot be skipped. Labels are scoped to their parents: `bio_rep1` +under two conditions are two different samples, so groups are always identified by the +full label path. A folder containing images and no subfolders is a leaf folder. Only images directly inside leaf folders are analysed. If the experiment root itself contains images and has no @@ -584,7 +585,8 @@ Raised during review; to be decided before the milestones that need them: *Decided (October 2026): yes. `PreprocessingNotebook.folder_step` and `axis_step` take an optional `title` and `purpose` (the sentence under the title), a keyword `intro` (the blue box; an empty string leaves it out) and `show_next` (False hides the "What - next?" box), and `folder_step` takes `show_output` (False leaves out the "Output - folder (optional)" card; the check then uses the default output folder, as with the - box left empty). The defaults give the pages as before, and this notebook calls them + next?" box), and `folder_step` takes `show_output` (False hides the "Output folder + (optional)" card; the check then uses the folder in its box, the default when empty, + and the card is shown again when the check finds that folder inside the experiment, + so the box to fix it is never hidden). The defaults give the pages as before, and this notebook calls them without arguments; the widgets, the checks and the gating are the same either way.* diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index c07a3f88..81c2d731 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -156,6 +156,7 @@ def __init__(self, experiment_folder: str = "", output_folder: str = ""): self.log = _LogCapture() self._checked_folder = "" self._preview_shown = False + self._show_output = True self._layouts_cache: tuple[tuple, dict[str, PlaneLayout]] | None = None self._build_folder_step(str(experiment_folder), str(output_folder)) self._build_axis_step() @@ -215,9 +216,10 @@ def folder_step( Text of the blue "what happens here" box (HTML); None gives this notebook's, an empty string leaves the box out. show_output - False leaves out the "Output folder (optional)" card, which sets where - this brick writes its histogram table. The check then uses the default - output folder, as it does when the box is left empty. + False hides the "Output folder (optional)" card, which sets where this + brick writes its histogram table; the check then uses the folder in its + box (the default when empty). The card is shown again when the check + finds that folder inside the experiment, so it can be fixed. show_next False hides the "What next?" box, which names this notebook's next steps. @@ -228,6 +230,8 @@ def folder_step( The page. """ ui.set_visible(self.folder_next, show_next) + self._show_output = show_output + ui.set_visible(self.output_card, show_output) if intro is None: intro = ( "What happens here: the folders are scanned, the labels are taken " @@ -245,7 +249,7 @@ def folder_step( [ *([ui.intro(intro)] if intro else []), self.folder_card, - *([self.output_card] if show_output else []), + self.output_card, self.exclude_box, self.check_button, self.folder_status, @@ -529,6 +533,11 @@ def _show_folder_results(self, pre: Preflight) -> None: ) self.issue_details.selected_index = None ui.set_visible(self.issues_card, bool(pre.issues)) + # A hidden output card comes back when its folder is the problem to fix. + ui.set_visible( + self.output_card, + self._show_output or any(i.code == "output_inside_experiment" for i in pre.issues), + ) blocking = any(i.code in EXCLUDABLE_CODES for i in pre.issues) ui.set_visible(self.exclude_button, blocking) self._refresh_folder_next() diff --git a/tests/test_notebook_ui.py b/tests/test_notebook_ui.py index 6a78f241..e7f65bd0 100644 --- a/tests/test_notebook_ui.py +++ b/tests/test_notebook_ui.py @@ -453,7 +453,7 @@ def test_folder_step_with_its_own_words_and_no_output_card(experiment): assert "Step 1 · Your data" in title and "Step 1 · Check" not in title assert purpose == "
Choose the folder with your images.
" assert intros(found) == ["
Here your folders are read.
"] - assert nb.output_card not in found and nb.output not in found + assert not ui.is_visible(nb.output_card) assert nb.folder_card in found and nb.check_button in found and nb.labels_card in found # The check works without the card, with the default output folder. nb.check_button.click() @@ -462,6 +462,20 @@ def test_folder_step_with_its_own_words_and_no_output_card(experiment): assert "Go to Step 2" in nb.folder_next.value and not ui.is_visible(nb.folder_next) +def test_hidden_output_card_comes_back_when_its_folder_is_inside_the_experiment(experiment): + nb = PreprocessingNotebook(str(experiment), output_folder=str(experiment / "out")) + page = nb.folder_step(show_output=False) + assert nb.output_card in page_parts(page)[2] and not ui.is_visible(nb.output_card) + pre = nb.check_folder() + assert "output_inside_experiment" in [i.code for i in pre.issues] + assert ui.is_visible(nb.output_card) # the box to fix it is shown + nb.output.value = "" + assert nb.check_folder().ok + assert not ui.is_visible(nb.output_card) + nb.folder_step() # the default page shows it again + assert ui.is_visible(nb.output_card) + + def test_steps_without_an_intro_box(): nb = PreprocessingNotebook() assert intros(page_parts(nb.folder_step(intro=""))[2]) == []