diff --git a/AGENTS.md b/AGENTS.md index 4b23b8e5..db2625ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -86,8 +86,16 @@ src/morphobricks_preprocessing/ 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) + axis_review.py review_axes and check_mapping (moved from axes.py in PP-4) + fingerprint.py Fingerprint (size, mtime) and RowKey: when a file's rows are reused (D5); + confirm_axes/unchanged_confirmation (a confirmation of the files as they are) + recorded_sizes.py RecordedSize: pixel sizes by role with unit and source (manifest v2) + narrow_ranges.py NarrowRange (moved from manifest.py) + sources.py check_sources (moved from manifest.py in PP-4) + mask_record.py the manifest's masks entry with fingerprints and roles (manifest v2) axis_check.py the axis check (contract §2): header_axis_issue, axis_disagreements, - likely_axis_cause, check_header_axes; AXIS_ERROR_CODES, AXIS_CHECK_CODES + likely_axis_cause, check_header_axes, ConfirmedAxes (a confirmation covers + only its files as fingerprinted, plain_stack_not_confirmed); AXIS_ERROR_CODES, AXIS_CHECK_CODES planes.py plane-by-plane reader (iter_planes, read_plane, read_sample_plane), table rows as XYC planes whose channels are read one at a time (iter_xyc_planes, XycPlane), memory estimate, @@ -100,6 +108,7 @@ src/morphobricks_preprocessing/ run/ the run (a package since PP-3; `morphobricks_preprocessing.run` re-exports its public names): preflight.py (Preflight, preflight, sample_planes), pipeline.py (run_experiment, incremental), result.py (RunResult), + entries.py (manifest entries, new/changed/unchanged by RowKey), reuse.py (which earlier rows are reused), rows.py (a file's rows and their pixel checks), publish.py (staged outputs, published together) report.py plain-language summaries shared by CLI and notebook, group statistics tables diff --git a/docs/contract.md b/docs/contract.md index d15bece4..d477055c 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -45,7 +45,7 @@ channel order; an image without a C axis has one channel. | Rule | Detail | |---|---| | Prefill | Axis labels from metadata: `X`, `Y`, `Z`, `C`, `T` keep their role; the RGB samples axis `S` becomes C when there is no other `C`. Every other label is left unassigned for the user to choose. | -| Axis check (v0.4.0) | Every TIFF's confirmed roles are compared with the axes its own header names (`named_axes`, §16), position by position (`header_axis_issue`, `axis_disagreements`): a named axis keeps its letter as role, except that the RGB samples `S` may be C when the header has no C, and a length-1 axis may have no role. Confirmed roles that give the named Y and X the other way round are the **error** `axes_yx_swapped`; any other role for a named axis (`ZYX` confirmed `TYX`, a named angle or lambda axis given a role) is the **error** `axes_contradict_header` (a named axis longer than 1 without a role is `unassigned_axis`, a position axis `unsupported_axis`). Each error names every file with its header axes and confirmed roles and the likely cause (`likely_axis_cause`: tifffile labels a 3-D stack saved with `imagej=True` or `ome=True` `CYX`; an ImageJ stack of slices reads as Z; rows and columns exchanged), and offers two fixes: correct the axes, or save named-axis copies and run preprocessing on them. Such an image is never read: not transposed, not under a contradicted declaration. An axis the metadata does not name (a plain stack `IYX`, `QYX`, `QQYX`, or shaped metadata with a placeholder `QYX`; a plain 2D `YX` page and RGB `YXS` are named, decision D-d) takes the role the user confirmed: the image is read with it, with the **warning** `axes_not_named` recommending named copies before segmenting in other tools, and recorded as `axes_from: "confirmed"` (`AxisReview.axes_from`; `"header"` for every other image). OME-Zarr images take their roles from their OME-NGFF axes and never have these issues. Since one mapping covers each TIFF arrangement, all files of one arrangement pass or fail together. The notebook shows the three codes as one card; when the roles agree with every header, the notebook and `run` say so ("Axes checked"). Before v0.4.0 a swap was read transposed and a contradiction under the declaration, silently. | +| Axis check (v0.4.0) | Every TIFF's confirmed roles are compared with the axes its own header names (`named_axes`, §16), position by position (`header_axis_issue`, `axis_disagreements`): a named axis keeps its letter as role, except that the RGB samples `S` may be C when the header has no C, and a length-1 axis may have no role. Confirmed roles that give the named Y and X the other way round are the **error** `axes_yx_swapped`; any other role for a named axis (`ZYX` confirmed `TYX`, a named angle or lambda axis given a role) is the **error** `axes_contradict_header` (a named axis longer than 1 without a role is `unassigned_axis`, a position axis `unsupported_axis`). Roles confirmed for a plain stack that put X before Y, such as `TXY`, are the **error** `axes_x_before_y` (owner decision of 1 November 2026): nothing is ever read transposed, so the images are saved again with Y before X, or as OME-TIFF copies with Y before X. Each error names every file with its header axes and confirmed roles and the likely cause (`likely_axis_cause`: tifffile labels a 3-D stack saved with `imagej=True` or `ome=True` `CYX`; an ImageJ stack of slices reads as Z; rows and columns exchanged), and offers two fixes: correct the axes, or save named-axis copies and run preprocessing on them. Such an image is never read: not transposed, not under a contradicted declaration. An axis the metadata does not name (a plain stack `IYX`, `QYX`, `QQYX`, or shaped metadata with a placeholder `QYX`; a plain 2D `YX` page and RGB `YXS` are named, decision D-d) takes the role the user confirmed: the image is read with it, with the **warning** `axes_not_named` recommending named copies before segmenting in other tools, and recorded as `axes_from: "confirmed"` (`AxisReview.axes_from`; `"header"` for every other image). A confirmation covers only the files it was given for, as they were then (`axis_check.ConfirmedAxes`, made by `fingerprint.confirm_axes` with each file's fingerprint; audit 2 B-1): a plain stack outside it, new or changed since (another fingerprint), is the **error** `plain_stack_not_confirmed` and must be confirmed again; it is never read with roles confirmed for other images. An `AxisMapping` given to `run_experiment` confirms every file the run finds. OME-Zarr images take their roles from their OME-NGFF axes and never have these issues. Since one mapping covers each TIFF arrangement, all files of one arrangement pass or fail together. The notebook shows these codes as one card; when the roles agree with every header, the notebook and `run` say so ("Axes checked"). Before v0.4.0 a swap was read transposed and a contradiction under the declaration, silently. | | Unsupported axes | Position-like axes (tile `R`, mosaic `M`, row `K`, column `J`) cannot have a role. If they hold more than one plane, the file is an error. | | Singleton axes | An axis of length 1 needs no role; its index is always 0. | | Missing axes | A file lacking axes the mapping has is read with those axes as length 1 only when the user confirmed this and only one arrangement fits; its coordinates for them are 0. | @@ -121,7 +121,7 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X or contradict its header (`axes_yx_swapped`, `axes_contradict_header`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment; no usable planes; when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named Z/T (`mask_not_2d`), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | +| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment; no usable planes; when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named Z/T (`mask_not_2d`), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | | Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); images named as masks of other images, or folders named like a masks folder holding them (`image_named_as_mask`, `mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no analysed image, files outside the mask-type folders (§15) | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and @@ -133,7 +133,7 @@ whether processing is blocked. The consistency issues of §11 are shown in their **Dataset consistency** panel, next to the balance table, and not repeated under the other messages. -## 9. Run manifest (`run_manifest.json`, manifest version 1) +## 9. Run manifest (`run_manifest.json`, manifest version 2) UTF-8 JSON next to `plane_histograms.csv`, written after the table and only when the run succeeded. @@ -151,21 +151,24 @@ file's schema or statistical interpretation. | Key | Meaning | |---|---| -| `manifest_version`, `schema_version` | Manifest format (`"1"`) and CSV format (`"4"`), written as JSON **strings**, not numbers. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | +| `manifest_version`, `schema_version` | Manifest format (`"2"` since v0.4.0) and CSV format (`"4"`), written as JSON **strings**, not numbers. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | | `written_at` | UTC time of writing, ISO 8601. | | `experiment_root` | Absolute path of the experiment folder, for information. | | `dtype` | Pixel type of the experiment. | | `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6). | | `axis_mapping` | `axes` (file axis labels) and `roles` (`X`, `Y`, `Z`, `C`, `T` or null per axis). | | `labels` | One entry per leaf folder: `folder` (relative path, `.` for the root) and its three labels. | -| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1; null when absent, in manifests written before it), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files and `mtime_ns` the time of its newest file. | +| `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1; null when absent, in manifests written before it), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files and `mtime_ns` the time of its newest file. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source` (`header` for now; `stated` and `pixels` with stated calibration), `value` and `source` null for an uncalibrated axis (never guessed). The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. | | `csv_sha256` | SHA-256 of the table written with this manifest. | | `experiment_design_sha256` | SHA-256 of the `experiment_design.csv` written with this run (§12), so later bricks can check that the design they read is the one used; null when the run had no design, or absent in manifests written before it. Optional key: the manifest stays version 1. | -| `masks` | The masks paired with the images (§15), when the run was given a masks folder: `folder` (absolute path of the masks folder, null when only OME-Zarr label images were used), `types` (sorted mask-type names) and `files` (one entry per image and mask type, sorted by image then type: `image`, `type`, `path`, `source` `folder` or `ome-zarr`, `axes` and `shape`: the mask's axes and lengths without its length-1 axes other than Y and X, `YX` for a 2D mask (`[1, 512]` for a mask one pixel high), with `Z` and/or `T` for a mask with depth slices or time points, e.g. `TYX` and `[50, 512, 512]`; §15). Absent when the run had no masks. Optional key: the manifest stays version 1, and the table does not depend on the masks. `axes` and `shape` are absent in manifests written before them, whose masks are all 2D; a later brick must read `axes` before using a mask and never apply a 2D mask to several planes. | +| `masks` | The masks paired with the images (§15), when the run was given a masks folder: `folder` (absolute path of the masks folder, null when only OME-Zarr label images were used), `types` (sorted mask-type names) and `files` (one entry per image and mask type, sorted by image then type: `image`, `type`, `path`, `source` `folder` or `ome-zarr`, `axes` and `shape`: the mask's axes and lengths without its length-1 axes other than Y and X, `YX` for a 2D mask (`[1, 512]` for a mask one pixel high), with `Z` and/or `T` for a mask with depth slices or time points, e.g. `TYX` and `[50, 512, 512]`; §15). Absent when the run had no masks. Optional key: the manifest stays version 1, and the table does not depend on the masks. `axes` and `shape` are absent in manifests written before them, whose masks are all 2D; a later brick must read `axes` before using a mask and never apply a 2D mask to several planes. **Version 2:** each mask entry also has `size` and `mtime_ns` (its fingerprint, as `source_fingerprint` gives it: the mask file, or the label image's folder), `roles` (one per axis of `axes`), `axes_from` (`header`) and `shared` (`false`; a 2-D mask stated as shared by every plane of its image comes with the mask-axis statements). | | `image_type` | `fluorescence` or `rgb`, the image type the user confirmed (§13); null when the run was given none, or absent in manifests written before it. Optional key: the manifest stays version 1. Changing it recomputes nothing. | | `channels_from_rgb_samples` | `true` when the C role is the RGB samples axis of images stored as RGB colour (§13), `false` otherwise; absent in manifests written before it. Optional key. | | `intensity_checks` | The limits of the intensity checks run on the table (§14): `IntensityCheckSettings.to_dict()` of the normalization brick (`library_version`, `outlier_limit`, `saturation_limit`, `min_images`); absent in manifests written before it. Optional key. Changing the limits recomputes nothing. | | `narrow_histogram_ranges` | The channel planes whose range check found the central range too narrow (§4), sorted by plane and channel: `relative_path`, `plane_id`, `channel` and the fields of the normalization brick's `RangeCheck` (`histogram_range`, `central_percentiles`, `central_bins`, `robust_quantiles`, `robust_limits`, `outside_robust_count`, `robust_central_bins`, `min_central_bins`). Empty when there are none; absent in manifests written before follow-up P6. Optional key: the manifest stays version 1. | +| `conversion_record_sha256` | SHA-256 of the conversion record in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5); null otherwise. | +| Version 1 | A manifest written before v0.4.0 (`"1"`) is still read, for reporting, but no row of it is reused: its axes were never checked. The next run recomputes every row ("the earlier run was written before v0.4.0, without the axis check"). Bricks refuse it. | +| Row reuse | A file's rows are reused only when its row key is the earlier run's (`fingerprint.RowKey`, one rule for "the same file, read the same way", audit D5): its fingerprint (`fingerprint.Fingerprint`, size and modification time), labels, axes, roles, `singleton_roles`, `axes_from` and the path it is read from (so moving or converting the experiment recomputes it), and the settings that change rows (bins, histogram range, robust quantiles, range-check limit). | | Removed keys | `settings.convert_tiffs`, `settings.conversion` and `files[].ome_zarr_path` were written by v0.3 runs that converted TIFF images to OME-Zarr in the output folder (removed in v0.4.0, §10); a manifest that holds them is still read, and they are ignored. The copies such a run left in `/ome_zarr` are not touched. | | Flags | `missing_as_singleton`, `read_in_blocks`, `channel_names_trimmed` and `channels_from_rgb_samples` are JSON booleans (`true`/`false`); any other value (`"false"`, `0`) makes the manifest malformed, never read as true or false, and `write_manifest` refuses to write one (`ValueError`), so a written manifest always reads back. | | `versions` | Versions of `morphobricks-preprocessing`, `morphobricks-normalization` (which computes the histograms and writes the CSV; absent in manifests written before it), `numpy`, `tifffile` and Python. Information only: a different version never causes a rebuild. | diff --git a/docs/plan.md b/docs/plan.md index d08d0610..43a22364 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1071,3 +1071,14 @@ Raised during review; to be decided before the milestones that need them: `pooling` and `records` modules); and `check_declared_axes` with `declared_axes_transposed`, removed rather than deprecated because they read a Y/X swap transposed, which v0.4 forbids. Breaking for v0.4.0.* +47. What does manifest version 2 record, and when is a row reused (design v0.4 §1, §3, §6; + PP-4)? + *Decided (1 November 2026, owner, design revision 4): `manifest_version` "2"; per file + `roles` and `axes_from` (`header` or `confirmed`) are required, with `pixel_sizes` by role + (value, unit, source); per mask `size`, `mtime_ns`, `roles`, `axes_from` and `shared`; an + optional `conversion_record_sha256`. A version 1 manifest gives no reusable row. A row is + reused only when its `RowKey` (fingerprint, labels, axes, roles, `axes_from`, read path and + the row-changing settings) is the earlier run's. A plain stack's confirmation covers only + the files it was given for (`ConfirmedAxes`); a new or changed plain stack is + `plain_stack_not_confirmed` until confirmed again (audit 2 B-1). Breaking: every v1 run is + recomputed.* diff --git a/docs/user_guide.md b/docs/user_guide.md index 56a96840..62ef677f 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -542,6 +542,8 @@ continues. | `axes_not_named` | Warning | The metadata of some images does not name their axes (a plain stack of pages, `IYX` or `QYX`), so which dimension is depth, time or channel comes only from the roles you confirm. The images are read with them, and the roles are recorded as confirmed by you. Check the roles with the preview. Before segmenting these images in other tools, consider saving named-axis copies (OME-TIFF), so every tool reads the same axes. | | `axes_yx_swapped` | Error | The roles you confirmed give the Y and X the image header names the other way round, so the images are not read (never transposed). The message names the likely cause. Two fixes: correct the axes (if the header is right, choose its roles), or convert (save named-axis copies with your roles, then run preprocessing on the copies). | | `axes_contradict_header` | Error | The roles you confirmed contradict the axes the image header names (for example the header says channels, C, and you chose time, T). The images are not read under a contradicted declaration. The message names the likely cause, often the tool that saved the files: tifffile labels a 3-D stack saved with `imagej=True` or `ome=True` as `CYX`, and an ImageJ stack of slices reads as depth even for a time-lapse. The same two fixes as `axes_yx_swapped`. | +| `axes_x_before_y` | Error | The roles you confirmed put X before Y (for example a plain stack confirmed `TXY`). Reading them would transpose every plane, and nothing is ever read transposed. Save the images again with Y before X, or as OME-TIFF copies with Y before X; if the roles are wrong, correct them. | +| `plain_stack_not_confirmed` | Error | Some images that do not name their axes (plain stacks) are new or changed since you confirmed the axis roles, so the confirmation does not cover them: a plain stack's roles are your statement about the images you checked, and are never applied to others. Check their roles with the preview and confirm them again (on the command line: give `--axes`). | | `missing_xy_axis`, `duplicate_axis_role` | Error | X/Y missing or a role used twice. Correct the axis roles. | | `unassigned_axis` | Error | A dimension with several planes has no role. Choose Z, C or T. | | `unsupported_axis` | Error | A position or tile dimension. Export each position separately. | diff --git a/pyproject.toml b/pyproject.toml index 44756d3c..f58c4e0a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,7 +125,9 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing._io" = "io" "morphobricks_preprocessing.axes" = "pure" "morphobricks_preprocessing.axis_check" = "pure" +"morphobricks_preprocessing.axis_review" = "pure" "morphobricks_preprocessing.mask_checks" = "pure" +"morphobricks_preprocessing.mask_record" = "io" "morphobricks_preprocessing.notebook_axes" = "ui" "morphobricks_preprocessing.cli" = "ui" "morphobricks_preprocessing.dataset" = "pure" @@ -134,6 +136,9 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" "morphobricks_preprocessing.manifest" = "io" +"morphobricks_preprocessing.fingerprint" = "io" +"morphobricks_preprocessing.narrow_ranges" = "pure" +"morphobricks_preprocessing.recorded_sizes" = "pure" "morphobricks_preprocessing.mask_scan" = "io" "morphobricks_preprocessing.masks" = "io" "morphobricks_preprocessing.metadata" = "io" @@ -142,8 +147,10 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.physical_sizes" = "io" "morphobricks_preprocessing.planes" = "io" "morphobricks_preprocessing.report" = "pure" +"morphobricks_preprocessing.sources" = "io" "morphobricks_preprocessing.run" = "orchestration" "morphobricks_preprocessing.run.pipeline" = "orchestration" +"morphobricks_preprocessing.run.entries" = "io" "morphobricks_preprocessing.run.preflight" = "io" "morphobricks_preprocessing.run.publish" = "io" "morphobricks_preprocessing.run.result" = "pure" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index e8f993a2..31dd877f 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -14,13 +14,11 @@ AxisMapping, AxisReview, axis_length_ranges, - check_mapping, declared_mapping, file_mapping_for, prefill_experiment_mapping, prefill_mapping, reference_axes, - review_axes, ) from .axis_check import ( AXIS_CHECK_CODES, @@ -29,6 +27,7 @@ header_axis_issue, likely_axis_cause, ) +from .axis_review import check_mapping, review_axes from .dataset import ( CONSISTENCY_CODES, BalanceCell, @@ -100,7 +99,6 @@ ManifestFormatError, RunManifest, RunSettings, - check_sources, file_sha256, read_manifest, source_fingerprint, @@ -174,6 +172,7 @@ ) from .report import GroupSummary, group_summaries from .run import Preflight, RunResult, SamplePlane, preflight, run_experiment, sample_planes +from .sources import check_sources from .validation import Severity, ValidationIssue, has_errors from .zarr_input import ( NgffError, diff --git a/src/morphobricks_preprocessing/axes.py b/src/morphobricks_preprocessing/axes.py index 8ad0cbd5..3d4da9f1 100644 --- a/src/morphobricks_preprocessing/axes.py +++ b/src/morphobricks_preprocessing/axes.py @@ -26,15 +26,12 @@ AXES_FROM_CONFIRMED, AXIS_CHECK_CODES, AXIS_ERROR_CODES, - POSITION_AXES, - check_header_axes, ) from .metadata import ( MetadataReport, TiffMetadata, - channel_count_issues, ) -from .validation import ValidationIssue, error, has_errors, plural +from .validation import ValidationIssue, has_errors ROLES = ("X", "Y", "Z", "C", "T") """Supported axis roles.""" @@ -401,265 +398,6 @@ def axis_name(axis: str) -> str: return _axis_label(axis) -def check_mapping( - mapping: AxisMapping, files: Iterable[TiffMetadata] = () -) -> list[ValidationIssue]: - """Check a mapping against the axis rules of ``docs/contract.md`` §2. - - Parameters - ---------- - mapping - The experiment mapping. - files - Headers of the files it applies to. A dimension counts as length 1 only - if it is length 1 in every one of these files that has it; with no files, - every dimension is assumed to be longer than 1. - - Returns - ------- - list of ValidationIssue - Errors: X or Y missing (``missing_xy_axis``), a role on more than one - dimension (``duplicate_axis_role``), a position-like axis with a role or - with more than one plane (``unsupported_axis``), and a dimension longer - than 1 without a role (``unassigned_axis``). - """ - files = [f for f in files if f.readable] - paths = [f.relative_path for f in files] - lengths = _dimension_lengths(mapping, files) - issues: list[ValidationIssue] = [] - - missing = [role for role in ("X", "Y") if role not in mapping.roles] - if missing: - issues.append( - error( - "missing_xy_axis", - f"The axis mapping has no {' or '.join(missing)} axis. Every image needs " - "exactly one X (width) and one Y (height) axis to be split into planes.", - paths, - f"Assign the {' and '.join(missing)} role to the image's width and height axes.", - ) - ) - - duplicates = sorted( - {r for r in mapping.roles if r is not None and mapping.roles.count(r) > 1}, - key=ROLES.index, - ) - if duplicates: - issues.append( - error( - "duplicate_axis_role", - f"More than one axis has the role {', '.join(duplicates)}. Each role can " - f"be given to one axis only ({mapping.describe()}).", - paths, - "Give each of these roles to a single axis.", - ) - ) - - positions = [i for i, axis in enumerate(mapping.axes) if axis in POSITION_AXES] - several = [i for i in positions if lengths[i] > 1] - if several: - names = ", ".join(_axis_label(mapping.axes[i]) for i in several) - issues.append( - error( - "unsupported_axis", - f"The images have a position axis ({names}): they hold images of several " - "positions or tiles. Images with several positions are not supported yet.", - _paths_with_long(files, mapping, several), - "Export each position as a separate image file, then run the check again.", - ) - ) - with_role = [i for i in positions if i not in several and mapping.roles[i] is not None] - if with_role: - names = ", ".join(_axis_label(mapping.axes[i]) for i in with_role) - issues.append( - error( - "unsupported_axis", - f"The position axis {names} cannot be used as Z, C or T: it identifies " - "where an image was taken, not a dimension of the image.", - paths, - "Remove the role from this axis in the axis review.", - ) - ) - - unassigned = [ - i - for i, role in enumerate(mapping.roles) - if role is None and lengths[i] > 1 and mapping.axes[i] not in POSITION_AXES - ] - if unassigned: - names = ", ".join(_axis_label(mapping.axes[i]) for i in unassigned) - issues.append( - error( - "unassigned_axis", - f"No role is chosen for the axis {names}, which has more than one plane. " - "The images cannot be split into planes until every such axis is Z " - "(depth), C (channel) or T (time).", - _paths_with_long(files, mapping, unassigned), - "Choose Z, C or T for this axis in the axis review.", - ) - ) - return issues - - -def review_axes( - files: MetadataReport | Iterable[TiffMetadata], - mapping: AxisMapping, - *, - missing_as_singleton: bool = False, -) -> AxisReview: - """Check a confirmed experiment mapping and resolve it for every file. - - Parameters - ---------- - files - The metadata report or the file headers. - mapping - The mapping the user confirmed, defined on the experiment's full - arrangement of axes (see :func:`reference_axes`). - missing_as_singleton - The user confirmed that files lacking some of the mapping's dimensions - simply have one plane along them (for example one channel). Without this - confirmation such files are an error. - - Returns - ------- - AxisReview - Per-file mappings for the files that fit and the issues found: those - from :func:`check_mapping`, files that lack dimensions without - confirmation (``missing_axes``) or in an ambiguous way - (``ambiguous_missing_axes``), files the mapping does not fit - (``inconsistent_mapping``), files with different numbers of channels - along the C axis (``different_channel_counts``; 1 without one), and the - axis check of every TIFF against its header (:func:`header_axis_issue`), - each naming its files with their header axes and confirmed roles: the - errors ``axes_yx_swapped`` and ``axes_contradict_header`` (with the likely - cause; such an image is never read transposed or under a contradicted - declaration) and the warning ``axes_not_named`` (a plain stack, read with - the confirmed roles; :attr:`AxisReview.axes_from` records it). - """ - if isinstance(files, MetadataReport): - files = files.files - readable = [f for f in files if f.readable] - file_mappings: dict[str, AxisMapping] = {} - unconfirmed: list[str] = [] - ambiguous: list[str] = [] - misfits: list[str] = [] - zarr_misfits: list[str] = [] - by_role: list[str] = [] - for f in readable: - if f.axes == mapping.axes: - file_mappings[f.relative_path] = mapping - continue - if f.is_ome_zarr: - by_role.append(f.relative_path) - resolved, problem = declared_mapping(f, mapping) - if resolved is None: - zarr_misfits.append(f.relative_path) - elif problem == "missing" and not missing_as_singleton: - unconfirmed.append(f.relative_path) - else: - file_mappings[f.relative_path] = resolved - continue - resolved = mapping.for_axes(f.axes) - if resolved is None: - if len(_embeddings(f.axes, mapping.axes)) > 1: - ambiguous.append(f.relative_path) - else: - misfits.append(f.relative_path) - elif not missing_as_singleton: - unconfirmed.append(f.relative_path) - else: - file_mappings[f.relative_path] = resolved - - issues = check_mapping( - mapping, - [f for f in readable if f.relative_path not in misfits and f.relative_path not in by_role], - ) - if unconfirmed: - issues.append( - error( - "missing_axes", - f"{plural(len(unconfirmed), 'image')} lack some of the dimensions of the " - f"axis mapping ({mapping.axes}). They can be read as having one plane along " - "each missing dimension, but only once this is confirmed.", - unconfirmed, - "If the missing dimensions simply have one plane in these files (for " - "example a single channel), confirm this in the axis review. Otherwise " - "export the images again with the same dimensions.", - ) - ) - if ambiguous: - issues.append( - error( - "ambiguous_missing_axes", - f"{plural(len(ambiguous), 'image')} lack some of the dimensions of the " - f"axis mapping ({mapping.axes}), and it is not clear which ones are " - "missing.", - ambiguous, - "Export the images again with the same dimensions as the others, or " - "process them as a separate experiment.", - ) - ) - if zarr_misfits: - issues.append( - error( - "ome_zarr_axes_mismatch", - f"The axes declared in {plural(len(zarr_misfits), 'OME-Zarr image')} do not " - f"match the axis roles of the experiment ({mapping.describe()}): they have an " - "axis with several planes that the experiment has no role for, or no X or Y " - "axis.", - zarr_misfits, - "Check the axis roles in the axis review, or process these images as a " - "separate experiment.", - ) - ) - if misfits: - issues.append( - error( - "inconsistent_mapping", - f"The axis mapping ({mapping.axes}) does not fit " - f"{plural(len(misfits), 'image')}: their dimensions are arranged " - "differently.", - misfits, - "Export the images again with the same dimension order, or process the " - "differently arranged images as a separate experiment.", - ) - ) - issues.extend( - channel_count_issues( - { - f.relative_path: channel_count(f, file_mappings[f.relative_path]) - for f in readable - if f.relative_path in file_mappings - } - ) - ) - # Each file's confirmed roles against the axes its own header names. Files the mapping - # does not fit already have their error; OME-Zarr images take their roles from their - # metadata. - checked = [ - (f, file_mappings.get(f.relative_path) or mapping.for_axes(f.axes)) - for f in readable - if f.relative_path in file_mappings or f.relative_path in unconfirmed - ] - axis_issues, axes_from = check_header_axes( - [(f, m) for f, m in checked if m is not None], analysed=file_mappings - ) - issues.extend(axis_issues) - return AxisReview( - mapping=mapping, file_mappings=file_mappings, issues=tuple(issues), axes_from=axes_from - ) - - -def channel_count(f: TiffMetadata, mapping: AxisMapping) -> int: - """Return the number of channels of ``f`` under its own mapping; 1 without a C axis. - - ``mapping`` is the file's own mapping (:attr:`AxisReview.file_mappings`). - """ - c = mapping.index_of("C") - return 1 if c is None else f.shape[c] - - def _embeddings(short: str, long: str) -> list[tuple[int, ...]]: """Every way ``short`` is ``long`` with some characters left out (as kept indices).""" results: list[tuple[int, ...]] = [] @@ -691,27 +429,6 @@ def _sizes(f: TiffMetadata, mapping: AxisMapping) -> dict[int, int] | None: return dict(zip(matches[0], f.shape, strict=True)) -def _dimension_lengths(mapping: AxisMapping, files: list[TiffMetadata]) -> list[int]: - """Largest length of each mapping dimension across the files (2 if unknown).""" - if not files: - return [2] * mapping.ndim - lengths = [1] * mapping.ndim - for f in files: - for dim, n in (_sizes(f, mapping) or {}).items(): - lengths[dim] = max(lengths[dim], n) - return lengths - - -def _paths_with_long(files: list[TiffMetadata], mapping: AxisMapping, dims: list[int]) -> list[str]: - """Paths of the files in which any of ``dims`` is longer than 1.""" - paths = [] - for f in files: - sizes = _sizes(f, mapping) or {} - if any(sizes.get(d, 1) > 1 for d in dims): - paths.append(f.relative_path) - return paths - - def _axis_label(axis: str) -> str: name = _AXIS_NAMES.get(axis) return f"{axis} ({name})" if name else axis diff --git a/src/morphobricks_preprocessing/axis_check.py b/src/morphobricks_preprocessing/axis_check.py index 617f4fc1..f39d596b 100644 --- a/src/morphobricks_preprocessing/axis_check.py +++ b/src/morphobricks_preprocessing/axis_check.py @@ -13,6 +13,7 @@ from __future__ import annotations from collections.abc import Iterable, Mapping +from dataclasses import dataclass, field from typing import TYPE_CHECKING from .metadata import OPEN_AXIS, TiffMetadata, named_axes @@ -20,6 +21,7 @@ if TYPE_CHECKING: # the mapping class lives in .axes, which imports this module from .axes import AxisMapping + from .fingerprint import Fingerprint POSITION_AXES = frozenset("RMKJ") """``tifffile`` letters for positions in a sample (tile, mosaic, row, column): they identify @@ -35,7 +37,11 @@ AXES_CONTRADICT_HEADER = "axes_contradict_header" """An error: confirmed roles that give a named axis another role than the header's.""" -AXIS_ERROR_CODES = (AXES_YX_SWAPPED, AXES_CONTRADICT_HEADER) +AXES_X_BEFORE_Y = "axes_x_before_y" +"""An error: confirmed roles that put X before Y, for example a plain stack confirmed +``TXY``. Reading it would transpose every plane, and nothing is ever read transposed.""" + +AXIS_ERROR_CODES = (AXES_YX_SWAPPED, AXES_CONTRADICT_HEADER, AXES_X_BEFORE_Y) """The errors of an image whose confirmed roles disagree with the axes its header names (contract §2). Such an image is never read, not transposed and not under a contradicted declaration: the user corrects the roles, or saves the image as a named-axis copy.""" @@ -64,6 +70,8 @@ def header_axis_issue(meta: TiffMetadata, mapping: AxisMapping) -> str | None: - a named axis with its own letter as role, the RGB samples ``S`` as C when the header has no C, and a length-1 axis without a role agree; - the named Y given role X and the named X role Y: ``axes_yx_swapped``; + - roles confirmed for a plain stack's unnamed axes that put X before Y (``TXY``): + ``axes_x_before_y``, as nothing is ever read transposed; - any other role for a named axis: ``axes_contradict_header``. That includes a Z read as T, and a named axis that is not a role (angle ``A``, lambda ``E``, ...) given one. A named axis longer than 1 without a role is :func:`check_mapping`'s @@ -83,12 +91,13 @@ def header_axis_issue(meta: TiffMetadata, mapping: AxisMapping) -> str | None: Returns ------- str or None - ``"axes_contradict_header"``, ``"axes_yx_swapped"`` or ``"axes_not_named"`` (the - first of these when several apply), or None when the roles agree with the header. + ``"axes_contradict_header"``, ``"axes_yx_swapped"``, ``"axes_x_before_y"`` or + ``"axes_not_named"`` (the first of these when several apply), or None when the roles + agree with the header. """ found = set(axis_disagreements(meta, mapping).values()) # Errors first (a contradiction is more than a swap); then the plain stack's warning. - for code in (AXES_CONTRADICT_HEADER, AXES_YX_SWAPPED, AXES_NOT_NAMED): + for code in (AXES_CONTRADICT_HEADER, AXES_YX_SWAPPED, AXES_X_BEFORE_Y, AXES_NOT_NAMED): if code in found: return code return None @@ -110,9 +119,9 @@ def axis_disagreements(meta: TiffMetadata, mapping: AxisMapping) -> dict[int, st Returns ------- dict - ``{dimension: code}``, with ``"axes_not_named"``, ``"axes_yx_swapped"`` or - ``"axes_contradict_header"``; empty for an OME-Zarr image, an unreadable one, or - roles that agree with the header. + ``{dimension: code}``, with ``"axes_not_named"``, ``"axes_yx_swapped"``, + ``"axes_x_before_y"`` or ``"axes_contradict_header"``; empty for an OME-Zarr + image, an unreadable one, or roles that agree with the header. """ if not meta.readable or meta.is_ome_zarr or len(mapping.roles) != len(meta.axes): return {} @@ -132,6 +141,23 @@ def axis_disagreements(meta: TiffMetadata, mapping: AxisMapping) -> dict[int, st found[index] = ( AXES_YX_SWAPPED if letter in "YX" and role in "YX" else AXES_CONTRADICT_HEADER ) + return _x_before_y(mapping.roles, found) + + +def _x_before_y(roles: tuple[str | None, ...], found: dict[int, str]) -> dict[int, str]: + """Mark the Y and X a plain stack's roles put X before Y, unless already an error. + + Only roles the user confirmed for axes the metadata does not name; a header that names + X before Y is read as it names them (unchanged in v0.4.0). + """ + if "X" not in roles or "Y" not in roles or roles.index("X") > roles.index("Y"): + return found + yx = (roles.index("Y"), roles.index("X")) + if AXES_NOT_NAMED not in (found.get(index) for index in yx): + return found + for index in yx: + if found.get(index, AXES_NOT_NAMED) == AXES_NOT_NAMED: + found[index] = AXES_X_BEFORE_Y return found @@ -185,6 +211,9 @@ def axis_check_issues( ) ) continue + if code == AXES_X_BEFORE_Y: + issues.append(error(code, _x_before_y_text(n, details), paths, _SAVE_Y_FIRST)) + continue if code == AXES_YX_SWAPPED: message = ( f"The axes confirmed for {n} swap the Y and X their header names ({details}). " @@ -241,6 +270,20 @@ def likely_axis_cause(meta: TiffMetadata, mapping: AxisMapping, code: str) -> st return "the tool that saved the file wrote a default or wrong axis order in its metadata" +def _x_before_y_text(n: str, details: str) -> str: + """Say that the roles confirmed for ``n`` images put X before Y.""" + return ( + f"The axes confirmed for {n} put X before Y ({details}). Reading them would " + "transpose every plane, and nothing is ever read transposed." + ) + + +_SAVE_Y_FIRST = ( + "Save these images again with Y before X (rows, then columns), or as named-axis copies " + "(OME-TIFF) with Y before X, then run preprocessing on them. If the roles are wrong, " + "correct them." +) + _TWO_FIXES = ( "Two fixes. Correct the axes: if the header is right, choose its roles. Or convert: if " "your roles are right, save these images as named-axis copies (OME-TIFF) with them, " @@ -269,7 +312,9 @@ def _some(paths: Iterable[str], shown: int = 3) -> str: def check_header_axes( - files: Iterable[tuple[TiffMetadata, AxisMapping]], analysed: Mapping[str, AxisMapping] + files: Iterable[tuple[TiffMetadata, AxisMapping]], + analysed: Mapping[str, AxisMapping], + confirmed: frozenset[str] | None = None, ) -> tuple[list[ValidationIssue], dict[str, str]]: """Run the axis check on every file and say where each analysed file's roles come from. @@ -280,6 +325,9 @@ def check_header_axes( analysed The files that are read (:attr:`.axes.AxisReview.file_mappings`); only these get an ``axes_from`` entry. + confirmed + The files the user's confirmation covers (:class:`ConfirmedAxes`), or None for every + file given. A plain stack outside them is the error ``plain_stack_not_confirmed``. Returns ------- @@ -296,4 +344,78 @@ def check_header_axes( if meta.relative_path in analysed: unnamed = AXES_NOT_NAMED in axis_disagreements(meta, mapping).values() axes_from[meta.relative_path] = AXES_FROM_CONFIRMED if unnamed else AXES_FROM_HEADER - return axis_check_issues(found), axes_from + unconfirmed = [ + path + for path, how in sorted(axes_from.items()) + if how == AXES_FROM_CONFIRMED and confirmed is not None and path not in confirmed + ] + return [*axis_check_issues(found), *_unconfirmed_issues(unconfirmed)], axes_from + + +@dataclass(frozen=True) +class ConfirmedAxes: + """The user's confirmation of an axis mapping, for the files it was given for. + + A plain stack names no axes, so its roles are only the user's statement about the files + the user looked at, as they were then (audit 2 B-1): a plain stack added later, or + changed since (another :class:`.fingerprint.Fingerprint`), is not covered and must be + confirmed again, never read with roles confirmed for other files or other contents. + :func:`.fingerprint.confirm_axes` records the files with their fingerprints. + + Attributes + ---------- + mapping + The experiment mapping confirmed. + files + ``{relative path: Fingerprint}``: the images it was confirmed for, relative to the + experiment root, each with its fingerprint when the user confirmed it. + """ + + mapping: AxisMapping + files: Mapping[str, Fingerprint] = field(hash=False) + + def __post_init__(self) -> None: + if not isinstance(self.files, Mapping): + raise TypeError( + "ConfirmedAxes.files maps each confirmed image to its fingerprint when it was " + "confirmed (fingerprint.confirm_axes), so a changed image is not covered" + ) + object.__setattr__(self, "files", dict(sorted(self.files.items()))) + + +def confirmation_of( + mapping: AxisMapping | ConfirmedAxes, +) -> tuple[AxisMapping, frozenset[str] | None]: + """Split a confirmation into its mapping and the files it covers (None for every file). + + Parameters + ---------- + mapping + An :class:`.axes.AxisMapping`, confirmed for every file given, or a + :class:`ConfirmedAxes`. + + Returns + ------- + tuple + ``(mapping, files)``. + """ + if isinstance(mapping, ConfirmedAxes): + return mapping.mapping, frozenset(mapping.files) + return mapping, None + + +def _unconfirmed_issues(paths: list[str]) -> list[ValidationIssue]: + """Return the error naming plain stacks the user's confirmation does not cover.""" + if not paths: + return [] + return [ + error( + "plain_stack_not_confirmed", + f"{plural(len(paths), 'image')} do not name their axes, and the roles were not " + f"confirmed for them ({_some(paths)}): they are new or changed since the roles " + "were confirmed. A plain stack's roles cover only the files they were confirmed " + "for, so they are not read with roles confirmed for other images.", + paths, + "Check their roles with the preview and confirm them again (the CLI: give --axes).", + ) + ] diff --git a/src/morphobricks_preprocessing/axis_review.py b/src/morphobricks_preprocessing/axis_review.py new file mode 100644 index 00000000..9a83b858 --- /dev/null +++ b/src/morphobricks_preprocessing/axis_review.py @@ -0,0 +1,313 @@ +"""Check a confirmed axis mapping and resolve it for every file (contract §2). + +:func:`review_axes` applies the experiment mapping to each image, checks the mapping's rules +(:func:`check_mapping`) and runs the axis check (:mod:`.axis_check`). Moved from ``axes.py`` +(PP-4). +""" + +from __future__ import annotations + +from collections.abc import Iterable + +from .axes import ( + ROLES, + AxisMapping, + AxisReview, + _axis_label, + _embeddings, + _sizes, + declared_mapping, +) +from .axis_check import ( + POSITION_AXES, + ConfirmedAxes, + check_header_axes, + confirmation_of, +) +from .metadata import ( + MetadataReport, + TiffMetadata, + channel_count_issues, +) +from .validation import ValidationIssue, error, plural + + +def check_mapping( + mapping: AxisMapping, files: Iterable[TiffMetadata] = () +) -> list[ValidationIssue]: + """Check a mapping against the axis rules of ``docs/contract.md`` §2. + + Parameters + ---------- + mapping + The experiment mapping. + files + Headers of the files it applies to. A dimension counts as length 1 only + if it is length 1 in every one of these files that has it; with no files, + every dimension is assumed to be longer than 1. + + Returns + ------- + list of ValidationIssue + Errors: X or Y missing (``missing_xy_axis``), a role on more than one + dimension (``duplicate_axis_role``), a position-like axis with a role or + with more than one plane (``unsupported_axis``), and a dimension longer + than 1 without a role (``unassigned_axis``). + """ + files = [f for f in files if f.readable] + paths = [f.relative_path for f in files] + lengths = _dimension_lengths(mapping, files) + issues: list[ValidationIssue] = [] + + missing = [role for role in ("X", "Y") if role not in mapping.roles] + if missing: + issues.append( + error( + "missing_xy_axis", + f"The axis mapping has no {' or '.join(missing)} axis. Every image needs " + "exactly one X (width) and one Y (height) axis to be split into planes.", + paths, + f"Assign the {' and '.join(missing)} role to the image's width and height axes.", + ) + ) + + duplicates = sorted( + {r for r in mapping.roles if r is not None and mapping.roles.count(r) > 1}, + key=ROLES.index, + ) + if duplicates: + issues.append( + error( + "duplicate_axis_role", + f"More than one axis has the role {', '.join(duplicates)}. Each role can " + f"be given to one axis only ({mapping.describe()}).", + paths, + "Give each of these roles to a single axis.", + ) + ) + + positions = [i for i, axis in enumerate(mapping.axes) if axis in POSITION_AXES] + several = [i for i in positions if lengths[i] > 1] + if several: + names = ", ".join(_axis_label(mapping.axes[i]) for i in several) + issues.append( + error( + "unsupported_axis", + f"The images have a position axis ({names}): they hold images of several " + "positions or tiles. Images with several positions are not supported yet.", + _paths_with_long(files, mapping, several), + "Export each position as a separate image file, then run the check again.", + ) + ) + with_role = [i for i in positions if i not in several and mapping.roles[i] is not None] + if with_role: + names = ", ".join(_axis_label(mapping.axes[i]) for i in with_role) + issues.append( + error( + "unsupported_axis", + f"The position axis {names} cannot be used as Z, C or T: it identifies " + "where an image was taken, not a dimension of the image.", + paths, + "Remove the role from this axis in the axis review.", + ) + ) + + unassigned = [ + i + for i, role in enumerate(mapping.roles) + if role is None and lengths[i] > 1 and mapping.axes[i] not in POSITION_AXES + ] + if unassigned: + names = ", ".join(_axis_label(mapping.axes[i]) for i in unassigned) + issues.append( + error( + "unassigned_axis", + f"No role is chosen for the axis {names}, which has more than one plane. " + "The images cannot be split into planes until every such axis is Z " + "(depth), C (channel) or T (time).", + _paths_with_long(files, mapping, unassigned), + "Choose Z, C or T for this axis in the axis review.", + ) + ) + return issues + + +def review_axes( + files: MetadataReport | Iterable[TiffMetadata], + mapping: AxisMapping | ConfirmedAxes, + *, + missing_as_singleton: bool = False, +) -> AxisReview: + """Check a confirmed experiment mapping and resolve it for every file. + + Parameters + ---------- + files + The metadata report or the file headers. + mapping + The mapping the user confirmed, defined on the experiment's full + arrangement of axes (see :func:`reference_axes`). A plain mapping is a + confirmation for every file given; a :class:`.axis_check.ConfirmedAxes` + covers only the files it names, and a plain stack outside them is the + error ``plain_stack_not_confirmed`` (design v0.4 §3, audit 2 B-1). + missing_as_singleton + The user confirmed that files lacking some of the mapping's dimensions + simply have one plane along them (for example one channel). Without this + confirmation such files are an error. + + Returns + ------- + AxisReview + Per-file mappings for the files that fit and the issues found: those + from :func:`check_mapping`, files that lack dimensions without + confirmation (``missing_axes``) or in an ambiguous way + (``ambiguous_missing_axes``), files the mapping does not fit + (``inconsistent_mapping``), files with different numbers of channels + along the C axis (``different_channel_counts``; 1 without one), and the + axis check of every TIFF against its header (:func:`header_axis_issue`), + each naming its files with their header axes and confirmed roles: the + errors ``axes_yx_swapped`` and ``axes_contradict_header`` (with the likely + cause; such an image is never read transposed or under a contradicted + declaration) and the warning ``axes_not_named`` (a plain stack, read with + the confirmed roles; :attr:`AxisReview.axes_from` records it). + """ + mapping, confirmed = confirmation_of(mapping) + if isinstance(files, MetadataReport): + files = files.files + readable = [f for f in files if f.readable] + file_mappings: dict[str, AxisMapping] = {} + unconfirmed: list[str] = [] + ambiguous: list[str] = [] + misfits: list[str] = [] + zarr_misfits: list[str] = [] + by_role: list[str] = [] + for f in readable: + if f.axes == mapping.axes: + file_mappings[f.relative_path] = mapping + continue + if f.is_ome_zarr: + by_role.append(f.relative_path) + resolved, problem = declared_mapping(f, mapping) + if resolved is None: + zarr_misfits.append(f.relative_path) + elif problem == "missing" and not missing_as_singleton: + unconfirmed.append(f.relative_path) + else: + file_mappings[f.relative_path] = resolved + continue + resolved = mapping.for_axes(f.axes) + if resolved is None: + if len(_embeddings(f.axes, mapping.axes)) > 1: + ambiguous.append(f.relative_path) + else: + misfits.append(f.relative_path) + elif not missing_as_singleton: + unconfirmed.append(f.relative_path) + else: + file_mappings[f.relative_path] = resolved + + issues = check_mapping( + mapping, + [f for f in readable if f.relative_path not in misfits and f.relative_path not in by_role], + ) + if unconfirmed: + issues.append( + error( + "missing_axes", + f"{plural(len(unconfirmed), 'image')} lack some of the dimensions of the " + f"axis mapping ({mapping.axes}). They can be read as having one plane along " + "each missing dimension, but only once this is confirmed.", + unconfirmed, + "If the missing dimensions simply have one plane in these files (for " + "example a single channel), confirm this in the axis review. Otherwise " + "export the images again with the same dimensions.", + ) + ) + if ambiguous: + issues.append( + error( + "ambiguous_missing_axes", + f"{plural(len(ambiguous), 'image')} lack some of the dimensions of the " + f"axis mapping ({mapping.axes}), and it is not clear which ones are " + "missing.", + ambiguous, + "Export the images again with the same dimensions as the others, or " + "process them as a separate experiment.", + ) + ) + if zarr_misfits: + issues.append( + error( + "ome_zarr_axes_mismatch", + f"The axes declared in {plural(len(zarr_misfits), 'OME-Zarr image')} do not " + f"match the axis roles of the experiment ({mapping.describe()}): they have an " + "axis with several planes that the experiment has no role for, or no X or Y " + "axis.", + zarr_misfits, + "Check the axis roles in the axis review, or process these images as a " + "separate experiment.", + ) + ) + if misfits: + issues.append( + error( + "inconsistent_mapping", + f"The axis mapping ({mapping.axes}) does not fit " + f"{plural(len(misfits), 'image')}: their dimensions are arranged " + "differently.", + misfits, + "Export the images again with the same dimension order, or process the " + "differently arranged images as a separate experiment.", + ) + ) + issues.extend( + channel_count_issues( + { + f.relative_path: channel_count(f, file_mappings[f.relative_path]) + for f in readable + if f.relative_path in file_mappings + } + ) + ) + # Each file's confirmed roles against the axes its own header names. Files the mapping + # does not fit already have their error; OME-Zarr images take their roles from their + # metadata. + checked = [ + (f, file_mappings.get(f.relative_path) or mapping.for_axes(f.axes)) + for f in readable + if f.relative_path in file_mappings or f.relative_path in unconfirmed + ] + axis_issues, axes_from = check_header_axes( + [(f, m) for f, m in checked if m is not None], file_mappings, confirmed + ) + return AxisReview(mapping, file_mappings, (*issues, *axis_issues), axes_from) + + +def channel_count(f: TiffMetadata, mapping: AxisMapping) -> int: + """Return the number of channels of ``f`` under its own mapping; 1 without a C axis. + + ``mapping`` is the file's own mapping (:attr:`AxisReview.file_mappings`). + """ + c = mapping.index_of("C") + return 1 if c is None else f.shape[c] + + +def _dimension_lengths(mapping: AxisMapping, files: list[TiffMetadata]) -> list[int]: + """Largest length of each mapping dimension across the files (2 if unknown).""" + if not files: + return [2] * mapping.ndim + lengths = [1] * mapping.ndim + for f in files: + for dim, n in (_sizes(f, mapping) or {}).items(): + lengths[dim] = max(lengths[dim], n) + return lengths + + +def _paths_with_long(files: list[TiffMetadata], mapping: AxisMapping, dims: list[int]) -> list[str]: + """Paths of the files in which any of ``dims`` is longer than 1.""" + paths = [] + for f in files: + sizes = _sizes(f, mapping) or {} + if any(sizes.get(d, 1) > 1 for d in dims): + paths.append(f.relative_path) + return paths diff --git a/src/morphobricks_preprocessing/cli.py b/src/morphobricks_preprocessing/cli.py index 95cd01a2..1991abd8 100644 --- a/src/morphobricks_preprocessing/cli.py +++ b/src/morphobricks_preprocessing/cli.py @@ -33,7 +33,8 @@ HistogramSettings, ) -from .axes import AxisMapping, reference_axes, review_axes +from .axes import AxisMapping, reference_axes +from .axis_review import review_axes from .design import ( DESIGN_FILENAME, DesignCheckError, diff --git a/src/morphobricks_preprocessing/fingerprint.py b/src/morphobricks_preprocessing/fingerprint.py new file mode 100644 index 00000000..1a7461fc --- /dev/null +++ b/src/morphobricks_preprocessing/fingerprint.py @@ -0,0 +1,200 @@ +"""When a file is unchanged, and when its rows can be reused (contract §9, design v0.4 §1). + +One rule for "the same file, read the same way" (plan-structure audit D5), used by the run +when it decides which rows to reuse, and by the bricks' checks: + +- :class:`Fingerprint`: a file's size and modification time (for an OME-Zarr folder, the + total size of its files and the newest time), as :func:`.manifest.source_fingerprint` + gives them; +- :class:`RowKey`: everything a row depends on. A row is reused only when its file is + unchanged and its roles, ``axes_from`` and every setting that changes rows are unchanged + (bins, histogram range, robust quantiles, range-check limit, the path it is read from). +""" + +from __future__ import annotations + +import os +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING + +from .axis_check import ConfirmedAxes +from .manifest import FileEntry, RunSettings, source_fingerprint + +if TYPE_CHECKING: + from collections.abc import Iterable + + from .axes import AxisMapping + + +@dataclass(frozen=True) +class Fingerprint: + """A file's size and modification time, which tell whether it changed. + + Attributes + ---------- + size + Size in bytes; for an OME-Zarr folder, the total size of its files. + mtime_ns + Modification time in nanoseconds; for an OME-Zarr folder, its newest file's. + """ + + size: int + mtime_ns: int + + @classmethod + def of(cls, path: str | os.PathLike) -> Fingerprint: + """Return the fingerprint of a file or OME-Zarr folder as it is on disk now. + + Raises + ------ + FileNotFoundError + If the file does not exist. + """ + size, mtime_ns = source_fingerprint(path) + return cls(size, mtime_ns) + + @classmethod + def recorded(cls, entry: FileEntry | Mapping) -> Fingerprint: + """Return the fingerprint recorded for a file (a manifest entry or its data).""" + if isinstance(entry, Mapping): + return cls(int(entry["size"]), int(entry["mtime_ns"])) + return cls(entry.size, entry.mtime_ns) + + +@dataclass(frozen=True) +class RowKey: + """Everything one file's rows depend on: equal keys mean the rows can be reused. + + Attributes + ---------- + relative_path + The file, relative to the experiment root. + fingerprint + The file's :class:`Fingerprint`. + labels + ``(condition, biological_repeat, technical_repeat)``. + axes, roles, singleton_roles + The file's axes and the roles it is read with. + axes_from + Where the roles come from (``"header"`` or ``"confirmed"``). + read_path + The absolute path the rows are read from: moving or converting the experiment + recomputes them. + bin_count + Bins per plane. + histogram + The histogram settings (range, robust quantiles, range-check limit), as data. + """ + + relative_path: str + fingerprint: Fingerprint + labels: tuple[str, str, str] + axes: str + roles: tuple[str | None, ...] | None + singleton_roles: tuple[str, ...] + axes_from: str | None + read_path: str + bin_count: int + histogram: tuple + + @classmethod + def of(cls, entry: FileEntry, settings: RunSettings, experiment_root: str) -> RowKey: + """Return the key of a file's rows from its manifest entry and the run's settings. + + Parameters + ---------- + entry + The file's entry (as recorded, or as it is now). + settings + The run's settings. + experiment_root + The experiment folder the file is read from (absolute). + """ + return cls( + relative_path=entry.relative_path, + fingerprint=Fingerprint.recorded(entry), + labels=tuple(entry.labels), + axes=entry.axes, + roles=None if entry.roles is None else tuple(entry.roles), + singleton_roles=tuple(entry.singleton_roles), + axes_from=entry.axes_from, + read_path=(Path(experiment_root) / entry.relative_path).as_posix(), + bin_count=settings.bin_count, + histogram=tuple(sorted(_frozen(settings.histogram.to_dict()).items())), + ) + + +def confirm_axes( + mapping: AxisMapping, experiment_root: str | os.PathLike, files: Iterable[str] +) -> ConfirmedAxes: + """Record the user's confirmation of ``mapping`` for ``files`` as they are now. + + Parameters + ---------- + mapping + The experiment mapping the user confirmed. + experiment_root + The experiment folder. + files + The images the user confirmed it for, relative to the experiment root. + + Returns + ------- + ConfirmedAxes + The confirmation, with each file's :class:`Fingerprint`. + + Raises + ------ + FileNotFoundError + If a file does not exist. + """ + root = Path(experiment_root) + return ConfirmedAxes(mapping, {rel: Fingerprint.of(root / rel) for rel in files}) + + +def unchanged_confirmation( + mapping: AxisMapping | ConfirmedAxes, experiment_root: str | os.PathLike +) -> AxisMapping | ConfirmedAxes: + """Keep only the confirmed files that are unchanged since they were confirmed. + + A file that changed or is gone is no longer covered, so a plain stack among them is + ``plain_stack_not_confirmed`` (design v0.4 §3, audit 2 B-1). A plain mapping is + returned as it is. + + Parameters + ---------- + mapping + The confirmation given to the run. + experiment_root + The experiment folder. + + Returns + ------- + AxisMapping or ConfirmedAxes + """ + if not isinstance(mapping, ConfirmedAxes): + return mapping + root = Path(experiment_root) + kept = { + rel: recorded for rel, recorded in mapping.files.items() if _current(root / rel) == recorded + } + return ConfirmedAxes(mapping.mapping, kept) + + +def _current(path: Path) -> Fingerprint | None: + """Return a file's fingerprint now, or None when it is gone.""" + try: + return Fingerprint.of(path) + except FileNotFoundError: + return None + + +def _frozen(value: object) -> object: + """Return ``value`` with lists and dicts made hashable (tuples), for comparing keys.""" + if isinstance(value, Mapping): + return {key: _frozen(item) for key, item in value.items()} + if isinstance(value, list | tuple): + return tuple(_frozen(item) for item in value) + return value diff --git a/src/morphobricks_preprocessing/image_type.py b/src/morphobricks_preprocessing/image_type.py index 26bdab03..5b5b5b0b 100644 --- a/src/morphobricks_preprocessing/image_type.py +++ b/src/morphobricks_preprocessing/image_type.py @@ -20,7 +20,8 @@ from morphobricks_normalization.rgb import check_rgb -from .axes import AxisReview, channel_count +from .axes import AxisReview +from .axis_review import channel_count from .metadata import MetadataReport from .validation import ValidationIssue, error diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index ddce8e1b..2da3dd0b 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -23,8 +23,7 @@ import json import os import platform -from collections.abc import Iterable, Mapping, Sequence -from dataclasses import asdict, dataclass, field +from dataclasses import dataclass, field from importlib.metadata import PackageNotFoundError, version from pathlib import Path @@ -33,16 +32,20 @@ import tifffile from morphobricks_normalization.checks import IntensityCheckSettings from morphobricks_normalization.csv_io import SCHEMA_VERSION -from morphobricks_normalization.ranges import HistogramSettings, RangeCheck +from morphobricks_normalization.ranges import HistogramSettings from ._io import write_text_atomic -from .axes import AxisMapping, file_mapping_for +from .axes import AxisMapping from .discovery import position_id -from .metadata import inspect_image -from .validation import ValidationIssue, error, plural +from .narrow_ranges import NarrowRange +from .recorded_sizes import RecordedSize, sizes_from_data, sizes_to_data MANIFEST_FILENAME = "run_manifest.json" -MANIFEST_VERSION = "1" +AXES_FROM = ("header", "confirmed") +"""Where a file's roles come from (:attr:`FileEntry.axes_from`).""" +MANIFEST_VERSION = "2" +LEGACY_MANIFEST_VERSION = "1" +"""Manifests written before v0.4.0: read for reporting, never trusted (no row is reused).""" class ManifestFormatError(ValueError): @@ -121,8 +124,15 @@ class FileEntry: The role of each of the file's axes (``"X"``, ``"Y"``, ``"Z"``, ``"C"``, ``"T"`` or None), from its own mapping: for an OME-Zarr image whose declared order differs from the experiment mapping's, these are its - roles, not the experiment's. None in manifests written before they were - recorded. + roles, not the experiment's. Required in manifest version 2; None in older + manifests. + axes_from + Where the roles come from: ``"header"`` (the metadata names every axis and + the roles agree) or ``"confirmed"`` (a plain stack: the user confirmed them). + Required in manifest version 2; None in older manifests. + pixel_sizes + The size of one step along X, Y, Z and T, each with its unit and source + (:class:`.recorded_sizes.RecordedSize`); empty in older manifests. """ relative_path: str @@ -138,6 +148,8 @@ class FileEntry: position_id: str | None = None channel_names_trimmed: bool = False roles: tuple[str | None, ...] | None = None + axes_from: str | None = None + pixel_sizes: tuple[RecordedSize, ...] = () def same_source(self, other: FileEntry) -> bool: """Return True if ``other`` describes the same unchanged file, read the same way.""" @@ -151,57 +163,6 @@ def same_source(self, other: FileEntry) -> bool: ) -@dataclass(frozen=True) -class NarrowRange: - """A channel plane whose central intensity range covers too few histogram bins. - - Attributes - ---------- - relative_path - The image the plane belongs to. - plane_id - The plane's ``plane_id`` in the table. - channel - The zero-based channel. - check - The normalization brick's ``RangeCheck`` of the plane's channel. - """ - - relative_path: str - plane_id: str - channel: int - check: RangeCheck - - def to_dict(self) -> dict: - """Return the entry as JSON-compatible data.""" - check = {name: _plain(value) for name, value in asdict(self.check).items()} - return { - "relative_path": self.relative_path, - "plane_id": self.plane_id, - "channel": self.channel, - **check, - } - - @classmethod - def from_dict(cls, data: dict) -> NarrowRange: - """Read an entry written by :meth:`to_dict`.""" - return cls( - relative_path=str(data["relative_path"]), - plane_id=str(data["plane_id"]), - channel=int(data["channel"]), - check=RangeCheck( - histogram_range=str(data["histogram_range"]), - central_percentiles=tuple(float(v) for v in data["central_percentiles"]), - central_bins=int(data["central_bins"]), - robust_quantiles=tuple(float(v) for v in data["robust_quantiles"]), - robust_limits=tuple(data["robust_limits"]), - outside_robust_count=int(data["outside_robust_count"]), - robust_central_bins=int(data["robust_central_bins"]), - min_central_bins=int(data["min_central_bins"]), - ), - ) - - @dataclass(frozen=True) class RunManifest: """Everything a run records about its inputs and output. @@ -248,10 +209,14 @@ class RunManifest: than the range check's limit (``narrow_histogram_range``), sorted by plane and channel; empty when there were none (and in manifests written before they were recorded). + conversion_record_sha256 + SHA-256 of the conversion record in the experiment root, when the experiment is + a copy tree of named-axis copies (design v0.4 §5); None otherwise. schema_version Version of the CSV format. manifest_version - Version of the manifest format. + Version of the manifest format: ``"2"`` since v0.4.0. A ``"1"`` manifest (older + runs) is read for reporting only; no row of such a run is reused. """ experiment_root: str @@ -268,6 +233,7 @@ class RunManifest: channels_from_rgb_samples: bool | None = None intensity_checks: IntensityCheckSettings | None = None narrow_ranges: tuple[NarrowRange, ...] = () + conversion_record_sha256: str | None = None schema_version: str = SCHEMA_VERSION manifest_version: str = MANIFEST_VERSION @@ -310,7 +276,7 @@ def to_dict(self) -> dict: } for folder, labels in self.leaves.items() ], - "files": [_file_dict(f) for f in self.files], + "files": [_file_dict(f, strict=self.manifest_version != "1") for f in self.files], "csv_sha256": self.csv_sha256, "experiment_design_sha256": self.experiment_design_sha256, "image_type": self.image_type, @@ -323,6 +289,7 @@ def to_dict(self) -> dict: None if self.intensity_checks is None else self.intensity_checks.to_dict() ), "narrow_histogram_ranges": [entry.to_dict() for entry in self.narrow_ranges], + "conversion_record_sha256": self.conversion_record_sha256, "versions": dict(self.versions), } if self.masks is not None: @@ -339,9 +306,10 @@ def from_dict(cls, data: dict) -> RunManifest: If the data is not a manifest of this version. """ try: - if data["manifest_version"] != MANIFEST_VERSION: + version = data["manifest_version"] + if version not in (MANIFEST_VERSION, LEGACY_MANIFEST_VERSION): raise ManifestFormatError( - f"Manifest version {data['manifest_version']!r} is not supported " + f"Manifest version {version!r} is not supported " f"(this version reads {MANIFEST_VERSION!r})." ) settings = data["settings"] @@ -361,7 +329,7 @@ def from_dict(cls, data: dict) -> RunManifest: ), ), mapping=AxisMapping(str(mapping["axes"]), tuple(mapping["roles"])), - files=tuple(_file_entry(f) for f in data["files"]), + files=tuple(_file_entry(f, strict=version != "1") for f in data["files"]), csv_sha256=str(data["csv_sha256"]), versions={str(k): str(v) for k, v in data.get("versions", {}).items()}, written_at=str(data.get("written_at", "")), @@ -386,7 +354,9 @@ def from_dict(cls, data: dict) -> RunManifest: NarrowRange.from_dict(entry) for entry in data.get("narrow_histogram_ranges") or () ), + conversion_record_sha256=_text(data.get("conversion_record_sha256")), schema_version=str(data["schema_version"]), + manifest_version=str(version), ) except ManifestFormatError: raise @@ -445,157 +415,6 @@ def source_fingerprint(path: str | os.PathLike) -> tuple[int, int]: return size, newest -def check_sources( # noqa: C901 - manifest: RunManifest, - experiment_root: str | os.PathLike, - relative_paths: Iterable[str], - *, - image_axes: Mapping[str, str] | None = None, - identities: Mapping[str, Sequence[str]] | None = None, -) -> list[ValidationIssue]: - """Check that images are still the ones a preprocessing run read, before using its table. - - The per-image checks every brick that uses ``plane_histograms.csv`` makes (MorphoBricks - D86; formerly CellPatcher ``statistics.check_images`` and MorphoFeature - ``histogram_fit``): each image is in the manifest; it is unchanged, by the size and - modification time :func:`source_fingerprint` gives (an OME-Zarr folder: its files' total - size and newest time); its recorded ``position_id`` is the one its name gives; with - ``identities``, its labels and ``position_id`` are the caller's; with ``image_axes``, - the caller's axes give every axis the role the run confirmed for that image. - - Parameters - ---------- - manifest - The run's manifest (:func:`read_manifest`). - experiment_root - The experiment folder the paths are relative to (the run's, possibly moved). - relative_paths - The images to check, relative to the experiment root, with ``/``. - image_axes - The axes the caller reads each image with, one letter per dimension (``"ZYX"``), - by path; every path must have one. A dimension the run gave no role (length 1) - fits any letter; the others must have the run's role. The roles are the file's - own as recorded (:attr:`FileEntry.roles`), or for manifests written before they - were recorded, the run's mapping resolved for the image's header. - identities - The caller's ``(condition, biological_repeat, technical_repeat, position_id)`` of - each image (:data:`.discovery.FOV_KEY` order), by path; every path must have one. - - Returns - ------- - list of ValidationIssue - Errors, each naming its images: ``images_not_in_manifest``, ``images_missing`` - (no longer on disk), ``images_changed``, ``position_id_not_recorded`` (a manifest - from before ``position_id`` was recorded, when ``identities`` are checked), - ``identity_differs`` and ``axis_mapping_mismatch``. Empty when all images pass. - - Raises - ------ - ValueError - If ``image_axes`` or ``identities`` is given without an entry for every path. - """ - root = Path(experiment_root) - paths = list(dict.fromkeys(relative_paths)) - for name, given in (("image_axes", image_axes), ("identities", identities)): - absent = [path for path in paths if given is not None and path not in given] - if absent: - raise ValueError(f"{name} has no entry for {', '.join(absent)}") - found: dict[str, list[tuple[str, str]]] = {} - - def report(code: str, path: str, detail: str = "") -> None: - found.setdefault(code, []).append((path, detail)) - - for path in paths: - entry = manifest.file(path) - if entry is None: - report("images_not_in_manifest", path) - continue - try: - fingerprint = source_fingerprint(root / path) - except FileNotFoundError: - report("images_missing", path) - continue - if fingerprint != (entry.size, entry.mtime_ns): - report("images_changed", path) - recorded = (*entry.labels, entry.position_id) - if entry.position_id is not None and entry.position_id != position_id(path): - report( - "identity_differs", - path, - f"recorded position_id {entry.position_id!r}, its name gives {position_id(path)!r}", - ) - elif identities is not None and entry.position_id is None: - report("position_id_not_recorded", path) - elif identities is not None and tuple(identities[path]) != recorded: - report( - "identity_differs", path, f"given {tuple(identities[path])}, recorded {recorded}" - ) - if image_axes is not None: - roles = entry.roles - if roles is None: - mapping = file_mapping_for( - inspect_image(root / path, path), - manifest.mapping, - missing_as_singleton=manifest.settings.missing_as_singleton, - ) - roles = None if mapping is None else mapping.roles - axes = str(image_axes[path]).upper() - if not _roles_agree(axes, roles): - described = "none fit" if roles is None else "".join(r or "-" for r in roles) - report("axis_mapping_mismatch", path, f"given {axes}, preprocessing {described}") - issues = [] - for code, items in found.items(): - message, action = _SOURCE_TEXTS[code] - message = message.format(n=plural(len(items), "image")) - details = [f"{path}: {detail}" for path, detail in items if detail] - if details: - message += " " + "; ".join(details) + "." - issues.append(error(code, message, sorted(path for path, _ in items), action)) - return issues - - -def _roles_agree(axes: str, roles: Sequence[str | None] | None) -> bool: - """Return True when every axis has the run's role; one without a role (length 1) fits any.""" - if roles is None or len(roles) != len(axes): - return False - return all(role is None or role == axis for role, axis in zip(roles, axes, strict=True)) - - -_RERUN = "Run the preprocessing brick again on this experiment, or use the table and manifest of " -_SOURCE_TEXTS = { - "images_not_in_manifest": ( - "{n} not analysed by the preprocessing run of this manifest: added after it ran, " - "excluded, or from another experiment.", - _RERUN + "the run that analysed these images.", - ), - "images_missing": ( - "{n} recorded in the manifest no longer exist in the experiment folder (moved, " - "renamed or deleted, or the experiment folder given is not the run's).", - "Give the experiment folder the preprocessing run read, or run it again.", - ), - "images_changed": ( - "{n} changed after the preprocessing brick read them: the size or modification " - "time differs from the manifest, so their rows may not describe them.", - _RERUN + "the images as they were.", - ), - "position_id_not_recorded": ( - "The manifest does not record the position of {n}; it was written by a version " - "of the preprocessing brick before positions were recorded.", - "Run the preprocessing brick again on this experiment.", - ), - "identity_differs": ( - "The labels or position of {n} differ from those the preprocessing run recorded.", - "Use the labels the preprocessing run confirmed, or run it again with the folders as " - "they are now.", - ), - "axis_mapping_mismatch": ( - "The axes given for {n} give some axes another role than the axis mapping confirmed " - "in the preprocessing brick.", - "Use the axis roles confirmed in the preprocessing brick.", - ), -} - - def package_versions() -> dict[str, str]: """Versions of the packages that determine the output.""" try: @@ -653,7 +472,18 @@ def _masks(data: object) -> dict | None: return data -def _file_dict(entry: FileEntry) -> dict: +def _text(value: object) -> str | None: + """Return an optional text value as text, or None.""" + return None if value is None else str(value) + + +def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: + """Return a file entry as manifest data; ``strict`` (version 2) requires roles and source. + + The writer refuses what the reader would refuse, so a written manifest always reads back. + """ + if strict and (entry.roles is None or entry.axes_from not in AXES_FROM): + raise ValueError(f"the entry of {entry.relative_path!r} has no roles or axes_from") return { "relative_path": entry.relative_path, "size": entry.size, @@ -670,11 +500,16 @@ def _file_dict(entry: FileEntry) -> dict: "position_id": entry.position_id, "channel_names_trimmed": _flag(entry.channel_names_trimmed, "channel_names_trimmed"), "roles": None if entry.roles is None else list(entry.roles), + "axes_from": entry.axes_from, + "pixel_sizes": sizes_to_data(entry.pixel_sizes), } -def _file_entry(data: dict) -> FileEntry: +def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: + """Read a file entry; ``strict`` (manifest version 2) requires its roles and their source.""" bits = data.get("significant_bits") + if strict and (data.get("roles") is None or data.get("axes_from") not in AXES_FROM): + raise ValueError(f"the entry of {data.get('relative_path')!r} has no roles or axes_from") return FileEntry( relative_path=str(data["relative_path"]), size=int(data["size"]), @@ -693,6 +528,8 @@ def _file_entry(data: dict) -> FileEntry: position_id=None if data.get("position_id") is None else str(data["position_id"]), channel_names_trimmed=_flag(data.get("channel_names_trimmed", False)), roles=_roles(data.get("roles"), str(data["axes"])), + axes_from=_text(data.get("axes_from")), + pixel_sizes=sizes_from_data(data.get("pixel_sizes")), ) @@ -719,14 +556,5 @@ def _flag(value: object, key: str | None = None) -> bool: return value -def _plain(value): # noqa: ANN001, ANN202 - """Return a range check value as JSON data: Python numbers, lists for tuples.""" - if isinstance(value, tuple): - return [_plain(item) for item in value] - if isinstance(value, np.generic): - return value.item() - return value - - def _parent(relative_path: str) -> str: return relative_path.rsplit("/", 1)[0] if "/" in relative_path else "." diff --git a/src/morphobricks_preprocessing/mask_record.py b/src/morphobricks_preprocessing/mask_record.py new file mode 100644 index 00000000..d4f93a7c --- /dev/null +++ b/src/morphobricks_preprocessing/mask_record.py @@ -0,0 +1,52 @@ +"""The masks as the run manifest records them (manifest version 2, contract §9, §15). + +Every mask entry adds to what :meth:`.masks.MaskInventory.to_manifest` gives its +fingerprint (``size``, ``mtime_ns``), so a brick can tell a mask changed since the run, and +how it is read: ``roles`` (one per axis of ``axes``), ``axes_from`` (``"header"``: the +mask's metadata names them) and ``shared`` (a 2-D mask stated as shared by every plane of +its image; never without a statement). +""" + +from __future__ import annotations + +import os +from pathlib import Path + +from .fingerprint import Fingerprint +from .masks import FOLDER, MaskFile, MaskInventory + + +def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) -> dict: + """Return the manifest's ``masks`` entry, with each mask's fingerprint and roles. + + Parameters + ---------- + inventory + The masks found (:func:`.masks.find_masks`). + experiment_root + The experiment folder; the label images of OME-Zarr images are inside it. + + Returns + ------- + dict + :meth:`.masks.MaskInventory.to_manifest` with, per file, ``size``, ``mtime_ns``, + ``roles``, ``axes_from`` and ``shared``. + """ + data = inventory.to_manifest() + by_path = {mask.path: mask for mask in inventory.files} + for entry in data["files"]: + mask = by_path[entry["path"]] + fingerprint = Fingerprint.of(_mask_path(mask, inventory.folder, Path(experiment_root))) + entry.update( + size=fingerprint.size, + mtime_ns=fingerprint.mtime_ns, + roles=mask.axes, + axes_from="header", + shared=False, + ) + return data + + +def _mask_path(mask: MaskFile, folder: Path | None, root: Path) -> Path: + """Return where a mask is on disk: in the masks folder, or inside its OME-Zarr image.""" + return (folder / mask.path) if mask.source == FOLDER else (root / mask.path) diff --git a/src/morphobricks_preprocessing/narrow_ranges.py b/src/morphobricks_preprocessing/narrow_ranges.py new file mode 100644 index 00000000..32a09d06 --- /dev/null +++ b/src/morphobricks_preprocessing/narrow_ranges.py @@ -0,0 +1,72 @@ +"""The planes whose central intensity range covered too few bins (contract §4, §9). + +Recorded in the manifest so that a later run keeping their rows still reports them. Moved +from ``manifest.py`` (PP-4). +""" + +from __future__ import annotations + +from dataclasses import asdict, dataclass + +import numpy as np +from morphobricks_normalization.ranges import RangeCheck + + +@dataclass(frozen=True) +class NarrowRange: + """A channel plane whose central intensity range covers too few histogram bins. + + Attributes + ---------- + relative_path + The image the plane belongs to. + plane_id + The plane's ``plane_id`` in the table. + channel + The zero-based channel. + check + The normalization brick's ``RangeCheck`` of the plane's channel. + """ + + relative_path: str + plane_id: str + channel: int + check: RangeCheck + + def to_dict(self) -> dict: + """Return the entry as JSON-compatible data.""" + check = {name: _plain(value) for name, value in asdict(self.check).items()} + return { + "relative_path": self.relative_path, + "plane_id": self.plane_id, + "channel": self.channel, + **check, + } + + @classmethod + def from_dict(cls, data: dict) -> NarrowRange: + """Read an entry written by :meth:`to_dict`.""" + return cls( + relative_path=str(data["relative_path"]), + plane_id=str(data["plane_id"]), + channel=int(data["channel"]), + check=RangeCheck( + histogram_range=str(data["histogram_range"]), + central_percentiles=tuple(float(v) for v in data["central_percentiles"]), + central_bins=int(data["central_bins"]), + robust_quantiles=tuple(float(v) for v in data["robust_quantiles"]), + robust_limits=tuple(data["robust_limits"]), + outside_robust_count=int(data["outside_robust_count"]), + robust_central_bins=int(data["robust_central_bins"]), + min_central_bins=int(data["min_central_bins"]), + ), + ) + + +def _plain(value): # noqa: ANN001, ANN202 + """Return a range check value as JSON data: Python numbers, lists for tuples.""" + if isinstance(value, tuple): + return [_plain(item) for item in value] + if isinstance(value, np.generic): + return value.item() + return value diff --git a/src/morphobricks_preprocessing/notebook_axes.py b/src/morphobricks_preprocessing/notebook_axes.py index a671ddd7..a76b4724 100644 --- a/src/morphobricks_preprocessing/notebook_axes.py +++ b/src/morphobricks_preprocessing/notebook_axes.py @@ -18,7 +18,8 @@ def axis_check_html(review: AxisReview) -> str: """Show the axis check (contract §2) as one card: errors or plain stacks. - The three codes (``axes_yx_swapped``, ``axes_contradict_header``, ``axes_not_named``) + The four codes (``axes_yx_swapped``, ``axes_contradict_header``, ``axes_x_before_y``, + ``axes_not_named``) share one card; each group of files is listed with its header and confirmed axes, the errors with the likely cause and the two fixes. The codes stay in small print, for logs. @@ -89,6 +90,13 @@ def header_marks(mapping: AxisMapping, files: Iterable) -> dict[int, str]: ) continue codes = codes - {"axes_not_named"} + if codes == {"axes_x_before_y"}: + marks[index] = ( + "
⚠ These roles put X before Y, and " + "nothing is read transposed. Choose Y before X, or save the images again with " + "Y before X.
" + ) + continue named = ", ".join(axis_name(letter) for letter in sorted(letters[index]) if letter) what = "swaps the Y and X it names" if codes == {"axes_yx_swapped"} else "contradicts it" text = ( diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index baa12081..370433ef 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -67,9 +67,9 @@ axis_length_ranges, axis_name, file_mapping_for, - review_axes, ) from .axis_check import AXIS_CHECK_CODES +from .axis_review import review_axes from .dataset import CONSISTENCY_CODES, BalanceTable, DatasetReport, check_dataset from .design import ( DESIGN_FILENAME, diff --git a/src/morphobricks_preprocessing/recorded_sizes.py b/src/morphobricks_preprocessing/recorded_sizes.py new file mode 100644 index 00000000..9abb83d6 --- /dev/null +++ b/src/morphobricks_preprocessing/recorded_sizes.py @@ -0,0 +1,110 @@ +"""Pixel sizes as a run records them: a value with its unit and source (manifest v2, §9). + +Each analysed file's manifest entry gives the size of one step along X, Y and Z (and the +time step along T), by role, each with its unit and where it comes from: ``"header"`` (the +image's own metadata) or, once stated calibration exists (design v0.4 §3), ``"stated"`` or +``"pixels"``. An axis without a size records no value and no source: it is uncalibrated, +never guessed. Bricks read these, never the headers. +""" + +from __future__ import annotations + +from collections.abc import Iterable, Mapping +from dataclasses import dataclass + +from .physical_sizes import PhysicalSizes + +SIZE_ROLES = ("X", "Y", "Z", "T") +"""The roles a size is recorded for, in this order.""" + +SOURCES = ("header", "stated", "pixels") +"""Where a recorded size comes from.""" + + +@dataclass(frozen=True) +class RecordedSize: + """The size of one step along one role, as recorded. + + Attributes + ---------- + role + ``"X"``, ``"Y"``, ``"Z"`` or ``"T"``. + value + The size, or None when unknown (uncalibrated). + unit + Its unit as an OME-NGFF name (``"micrometer"``, ``"second"``), or None. + source + ``"header"``, ``"stated"`` or ``"pixels"``; None when the size is unknown. + """ + + role: str + value: float | None = None + unit: str | None = None + source: str | None = None + + def to_dict(self) -> dict: + """Return the size as JSON-compatible data.""" + return {"role": self.role, "value": self.value, "unit": self.unit, "source": self.source} + + @classmethod + def from_dict(cls, data: Mapping) -> RecordedSize: + """Read a size written by :meth:`to_dict`. + + Raises + ------ + ValueError + For an unknown role or source, or a source without a value. + """ + role, source = str(data["role"]), data.get("source") + value = data.get("value") + if role not in SIZE_ROLES or (source is not None and source not in SOURCES): + raise ValueError(f"the pixel size {dict(data)!r} has an unknown role or source") + if (value is None) != (source is None): + raise ValueError(f"the pixel size {dict(data)!r} needs both a value and a source") + unit = data.get("unit") + return cls(role, None if value is None else float(value), unit and str(unit), source) + + +def header_sizes(sizes: PhysicalSizes) -> tuple[RecordedSize, ...]: + """Record the sizes an image's header gives, by role, with the source ``"header"``. + + The header's sizes are by its own axis letters, which are the roles of every image + that passes the axis check (contract §2): named axes agree with their roles, and a + plain stack's header gives no size. + + Parameters + ---------- + sizes + The image's physical sizes (:attr:`.metadata.TiffMetadata.physical_sizes`). + + Returns + ------- + tuple of RecordedSize + One entry per role of :data:`SIZE_ROLES`; unknown sizes have no value. + """ + return tuple( + RecordedSize(role, sizes.scale(role), sizes.unit(role), "header") + if sizes.known(role) + else RecordedSize(role) + for role in SIZE_ROLES + ) + + +def sizes_to_data(sizes: Iterable[RecordedSize]) -> list[dict]: + """Return recorded sizes as JSON-compatible data.""" + return [size.to_dict() for size in sizes] + + +def sizes_from_data(data: object) -> tuple[RecordedSize, ...]: + """Read recorded sizes (absent in v1 manifests, which gives none). + + Raises + ------ + ValueError + If the data is not a list of sizes. + """ + if data is None: + return () + if not isinstance(data, list): + raise ValueError(f"the pixel sizes {data!r} are not a list") + return tuple(RecordedSize.from_dict(item) for item in data) diff --git a/src/morphobricks_preprocessing/run/entries.py b/src/morphobricks_preprocessing/run/entries.py new file mode 100644 index 00000000..39ab8760 --- /dev/null +++ b/src/morphobricks_preprocessing/run/entries.py @@ -0,0 +1,143 @@ +"""The manifest entry of every analysed file, and which of them changed (contract §9). + +Each entry records the file's fingerprint, labels, axes, roles, where the roles come from +(``axes_from``) and its pixel sizes with their source (manifest version 2). A file is +unchanged when its :class:`.fingerprint.RowKey` is the earlier run's: the same file, read +the same way from the same place. +""" + +from __future__ import annotations + +import logging +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, replace +from pathlib import Path + +from morphobricks_normalization.records import PlaneRecord + +from ..axes import AxisReview +from ..fingerprint import RowKey +from ..manifest import FileEntry, RunManifest, RunSettings, file_entry_for +from ..recorded_sizes import header_sizes +from .preflight import Preflight + +logger = logging.getLogger(__name__) + + +def file_entries( + pre: Preflight, review: AxisReview, root: Path, in_blocks: set[str] +) -> dict[str, FileEntry]: + """Describe every analysed file as it is now, with its roles and pixel sizes. + + Parameters + ---------- + pre + The run's preflight. + review + The axis review of the confirmed mapping. + root + The experiment folder. + in_blocks + Files whose planes are read in blocks. + + Returns + ------- + dict + ``{relative path: FileEntry}``, sorted by path. + """ + labels = {image: leaf.labels for leaf in pre.discovery.leaves for image in leaf.images} + headers = {f.relative_path: f for f in pre.metadata.files} + entries: dict[str, FileEntry] = {} + for rel, file_mapping in sorted(review.file_mappings.items()): + header = headers[rel] + entry = file_entry_for( + root, + rel, + labels[rel], + file_mapping, + channel_names=header.channel_names, + significant_bits=header.significant_bits, + read_in_blocks=rel in in_blocks, + channel_names_trimmed=header.channel_names_trimmed, + ) + entries[rel] = replace( + entry, + axes_from=review.axes_from.get(rel), + pixel_sizes=header_sizes(header.physical_sizes), + ) + if header.channel_names_trimmed: + logger.info( + "Leading or trailing whitespace was removed from the channel names of %s", rel + ) + return entries + + +@dataclass(frozen=True) +class Comparison: + """The files of a run compared with the earlier run's. + + Attributes + ---------- + new, changed, unchanged + Files not in the earlier run, changed since (another fingerprint, labels, roles, + source of the roles or place), and as they were. + removed + Files of the earlier run no longer analysed. + """ + + new: tuple[str, ...] + changed: tuple[str, ...] + unchanged: tuple[str, ...] + removed: tuple[str, ...] + + +def compare_with_previous( + entries: Mapping[str, FileEntry], + previous: RunManifest | None, + previous_records: Mapping[str, Sequence[PlaneRecord]], + rebuild: bool, + settings: RunSettings, + root: Path, +) -> Comparison: + """Sort the files into new, changed, unchanged and removed, by their row keys. + + The settings that change every row are compared once, by the earlier run's reason for + a rebuild; the keys are compared with the current settings on both sides, so they + differ only in what the file is and how it is read. + + Parameters + ---------- + entries + The files as they are now (:func:`file_entries`). + previous + The earlier run's manifest, or None. + previous_records + Its rows by file (empty during a rebuild). + rebuild + Whether every row is recomputed anyway; the earlier rows need not be complete then. + settings + The run's settings. + root + The experiment folder. + + Returns + ------- + Comparison + """ + new, changed, unchanged = [], [], [] + for rel, entry in entries.items(): + before = previous.file(rel) if previous is not None else None + if before is None: + new.append(rel) + continue + same = RowKey.of(before, settings, previous.experiment_root) == RowKey.of( + entry, settings, root.as_posix() + ) + complete = rebuild or len(previous_records.get(rel, ())) == before.plane_count + (unchanged if same and complete else changed).append(rel) + removed = sorted( + f.relative_path + for f in (previous.files if previous else ()) + if f.relative_path not in entries + ) + return Comparison(tuple(new), tuple(changed), tuple(unchanged), tuple(removed)) diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 51fb3b20..eb90fd29 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -8,7 +8,7 @@ import logging import os from collections.abc import Callable, Iterable -from dataclasses import replace +from dataclasses import asdict, replace from datetime import UTC, datetime from pathlib import Path @@ -22,8 +22,9 @@ from ..axes import ( AxisMapping, - review_axes, ) +from ..axis_check import ConfirmedAxes +from ..axis_review import review_axes from ..dataset import check_dataset from ..design import ( DESIGN_FILENAME, @@ -33,26 +34,27 @@ write_experiment_design, ) from ..discovery import default_output_folder +from ..fingerprint import unchanged_confirmation from ..image_type import check_image_type, image_type_issues, review_image_type from ..intensity import check_intensities from ..manifest import ( MANIFEST_FILENAME, - FileEntry, NarrowRange, RunManifest, RunSettings, - file_entry_for, file_sha256, package_versions, write_manifest, ) from ..mask_checks import mask_issues +from ..mask_record import mask_manifest from ..planes import ( DEFAULT_MEMORY_LIMIT, PlaneReadError, check_planes, ) from ..validation import ValidationIssue, error, has_errors, plural +from .entries import compare_with_previous, file_entries from .preflight import preflight from .publish import _staged_outputs from .result import RunResult @@ -64,7 +66,7 @@ def run_experiment( # noqa: C901, PLR0913, PLR0915 experiment_root: str | os.PathLike, - mapping: AxisMapping, + mapping: AxisMapping | ConfirmedAxes, *, output_folder: str | os.PathLike | None = None, bin_count: int | None = None, @@ -87,7 +89,9 @@ def run_experiment( # noqa: C901, PLR0913, PLR0915 experiment_root The experiment folder. It is never written to. mapping - The experiment axis mapping the user confirmed. + The experiment axis mapping the user confirmed: for every file the run finds, or, + as a :class:`.axis_check.ConfirmedAxes`, only for the files it names (a plain stack + outside them is the error ``plain_stack_not_confirmed``). output_folder Where to write; defaults to :func:`default_output_folder`. Created if needed; must not be inside the experiment. @@ -162,6 +166,8 @@ def run_experiment( # noqa: C901, PLR0913, PLR0915 pre = preflight( root, output, exclude=exclude, masks_folder=masks_folder, ome_zarr_labels=ome_zarr_labels ) + # A plain stack changed since its roles were confirmed is not covered (audit 2 B-1). + mapping = unchanged_confirmation(mapping, root) review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) type_review = review_image_type(pre.metadata, review) dataset = pre.dataset @@ -210,53 +216,17 @@ def stop(**counts) -> RunResult: # noqa: ANN003 histogram=histogram_settings or HistogramSettings(), ) previous, previous_records, reason = _load_previous( - output, settings, mapping, dtype.name, force_rebuild + output, settings, review.mapping, dtype.name, force_rebuild ) labels = {image: leaf.labels for leaf in pre.discovery.leaves for image in leaf.images} in_blocks = {path for i in issues if i.code == "planes_read_in_blocks" for path in i.paths} - headers = {f.relative_path: f for f in pre.metadata.files} - entries: dict[str, FileEntry] = {} - new, changed, unchanged = [], [], [] - for rel, file_mapping in sorted(review.file_mappings.items()): - header = headers[rel] - entries[rel] = file_entry_for( - root, - rel, - labels[rel], - file_mapping, - channel_names=header.channel_names, - significant_bits=header.significant_bits, - read_in_blocks=rel in in_blocks, - channel_names_trimmed=header.channel_names_trimmed, - ) - if header.channel_names_trimmed: - logger.info( - "Leading or trailing whitespace was removed from the channel names of %s", rel - ) - before = previous.file(rel) if previous is not None else None - if before is None: - new.append(rel) - elif ( - before.same_source(entries[rel]) - # 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) - ): - unchanged.append(rel) - else: - changed.append(rel) - removed = sorted( - f.relative_path - for f in (previous.files if previous else ()) - if f.relative_path not in entries + entries = file_entries(pre, review, root, in_blocks) + found = compare_with_previous( + entries, previous, previous_records, reason is not None, settings, root ) + new, changed, unchanged = found.new, found.changed, found.unchanged to_read = sorted([*new, *changed, *(unchanged if reason else ())]) - counts = { - "new": tuple(new), - "changed": tuple(changed), - "unchanged": tuple(unchanged), - "removed": tuple(removed), - "rebuild_reason": reason, - } + counts = {**asdict(found), "rebuild_reason": reason} records: list[PlaneRecord] = [] narrow: list[NarrowRange] = [] @@ -324,7 +294,7 @@ def stop(**counts) -> RunResult: # noqa: ANN003 experiment_root=root.as_posix(), dtype=dtype.name, settings=settings, - mapping=mapping, + mapping=review.mapping, files=tuple( replace(entry, plane_count=per_file[rel]) for rel, entry in sorted(entries.items()) ), @@ -332,7 +302,7 @@ def stop(**counts) -> RunResult: # noqa: ANN003 versions=package_versions(), written_at=datetime.now(UTC).isoformat(timespec="seconds"), experiment_design_sha256=design_sha256, - masks=None if pre.masks is None else pre.masks.to_manifest(), + masks=None if pre.masks is None else mask_manifest(pre.masks, root), image_type=image_type, channels_from_rgb_samples=type_review.channels_from_rgb_samples, intensity_checks=intensity.settings, diff --git a/src/morphobricks_preprocessing/run/reuse.py b/src/morphobricks_preprocessing/run/reuse.py index abf088ff..7ac92475 100644 --- a/src/morphobricks_preprocessing/run/reuse.py +++ b/src/morphobricks_preprocessing/run/reuse.py @@ -21,6 +21,7 @@ AxisMapping, ) from ..manifest import ( + LEGACY_MANIFEST_VERSION, MANIFEST_FILENAME, ManifestFormatError, RunManifest, @@ -50,6 +51,8 @@ def _load_previous( return None, {}, "the earlier run manifest could not be read" if force: return manifest, {}, "a full rebuild was requested" + if manifest.manifest_version == LEGACY_MANIFEST_VERSION: + return manifest, {}, "the earlier run was written before v0.4.0, without the axis check" if manifest.schema_version != SCHEMA_VERSION: return manifest, {}, "the earlier table uses a different format version" if manifest.dtype != dtype: diff --git a/src/morphobricks_preprocessing/sources.py b/src/morphobricks_preprocessing/sources.py new file mode 100644 index 00000000..b69c526a --- /dev/null +++ b/src/morphobricks_preprocessing/sources.py @@ -0,0 +1,170 @@ +"""Check that images are still the ones a preprocessing run read (contract §16). + +:func:`check_sources` is the per-image check every brick makes before using a run's table. +Moved from ``manifest.py`` (PP-4). +""" + +from __future__ import annotations + +import os +from collections.abc import Iterable, Mapping, Sequence +from pathlib import Path + +from .axes import file_mapping_for +from .discovery import position_id +from .manifest import RunManifest, source_fingerprint +from .metadata import inspect_image +from .validation import ValidationIssue, error, plural + + +def check_sources( # noqa: C901 + manifest: RunManifest, + experiment_root: str | os.PathLike, + relative_paths: Iterable[str], + *, + image_axes: Mapping[str, str] | None = None, + identities: Mapping[str, Sequence[str]] | None = None, +) -> list[ValidationIssue]: + """Check that images are still the ones a preprocessing run read, before using its table. + + The per-image checks every brick that uses ``plane_histograms.csv`` makes (MorphoBricks + D86; formerly CellPatcher ``statistics.check_images`` and MorphoFeature + ``histogram_fit``): each image is in the manifest; it is unchanged, by the size and + modification time :func:`source_fingerprint` gives (an OME-Zarr folder: its files' total + size and newest time); its recorded ``position_id`` is the one its name gives; with + ``identities``, its labels and ``position_id`` are the caller's; with ``image_axes``, + the caller's axes give every axis the role the run confirmed for that image. + + Parameters + ---------- + manifest + The run's manifest (:func:`read_manifest`). + experiment_root + The experiment folder the paths are relative to (the run's, possibly moved). + relative_paths + The images to check, relative to the experiment root, with ``/``. + image_axes + The axes the caller reads each image with, one letter per dimension (``"ZYX"``), + by path; every path must have one. A dimension the run gave no role (length 1) + fits any letter; the others must have the run's role. The roles are the file's + own as recorded (:attr:`FileEntry.roles`), or for manifests written before they + were recorded, the run's mapping resolved for the image's header. + identities + The caller's ``(condition, biological_repeat, technical_repeat, position_id)`` of + each image (:data:`.discovery.FOV_KEY` order), by path; every path must have one. + + Returns + ------- + list of ValidationIssue + Errors, each naming its images: ``images_not_in_manifest``, ``images_missing`` + (no longer on disk), ``images_changed``, ``position_id_not_recorded`` (a manifest + from before ``position_id`` was recorded, when ``identities`` are checked), + ``identity_differs`` and ``axis_mapping_mismatch``. Empty when all images pass. + + Raises + ------ + ValueError + If ``image_axes`` or ``identities`` is given without an entry for every path. + """ + root = Path(experiment_root) + paths = list(dict.fromkeys(relative_paths)) + for name, given in (("image_axes", image_axes), ("identities", identities)): + absent = [path for path in paths if given is not None and path not in given] + if absent: + raise ValueError(f"{name} has no entry for {', '.join(absent)}") + found: dict[str, list[tuple[str, str]]] = {} + + def report(code: str, path: str, detail: str = "") -> None: + found.setdefault(code, []).append((path, detail)) + + for path in paths: + entry = manifest.file(path) + if entry is None: + report("images_not_in_manifest", path) + continue + try: + fingerprint = source_fingerprint(root / path) + except FileNotFoundError: + report("images_missing", path) + continue + if fingerprint != (entry.size, entry.mtime_ns): + report("images_changed", path) + recorded = (*entry.labels, entry.position_id) + if entry.position_id is not None and entry.position_id != position_id(path): + report( + "identity_differs", + path, + f"recorded position_id {entry.position_id!r}, its name gives {position_id(path)!r}", + ) + elif identities is not None and entry.position_id is None: + report("position_id_not_recorded", path) + elif identities is not None and tuple(identities[path]) != recorded: + report( + "identity_differs", path, f"given {tuple(identities[path])}, recorded {recorded}" + ) + if image_axes is not None: + roles = entry.roles + if roles is None: + mapping = file_mapping_for( + inspect_image(root / path, path), + manifest.mapping, + missing_as_singleton=manifest.settings.missing_as_singleton, + ) + roles = None if mapping is None else mapping.roles + axes = str(image_axes[path]).upper() + if not _roles_agree(axes, roles): + described = "none fit" if roles is None else "".join(r or "-" for r in roles) + report("axis_mapping_mismatch", path, f"given {axes}, preprocessing {described}") + issues = [] + for code, items in found.items(): + message, action = _SOURCE_TEXTS[code] + message = message.format(n=plural(len(items), "image")) + details = [f"{path}: {detail}" for path, detail in items if detail] + if details: + message += " " + "; ".join(details) + "." + issues.append(error(code, message, sorted(path for path, _ in items), action)) + return issues + + +def _roles_agree(axes: str, roles: Sequence[str | None] | None) -> bool: + """Return True when every axis has the run's role; one without a role (length 1) fits any.""" + if roles is None or len(roles) != len(axes): + return False + return all(role is None or role == axis for role, axis in zip(roles, axes, strict=True)) + + +_RERUN = "Run the preprocessing brick again on this experiment, or use the table and manifest of " + + +_SOURCE_TEXTS = { + "images_not_in_manifest": ( + "{n} not analysed by the preprocessing run of this manifest: added after it ran, " + "excluded, or from another experiment.", + _RERUN + "the run that analysed these images.", + ), + "images_missing": ( + "{n} recorded in the manifest no longer exist in the experiment folder (moved, " + "renamed or deleted, or the experiment folder given is not the run's).", + "Give the experiment folder the preprocessing run read, or run it again.", + ), + "images_changed": ( + "{n} changed after the preprocessing brick read them: the size or modification " + "time differs from the manifest, so their rows may not describe them.", + _RERUN + "the images as they were.", + ), + "position_id_not_recorded": ( + "The manifest does not record the position of {n}; it was written by a version " + "of the preprocessing brick before positions were recorded.", + "Run the preprocessing brick again on this experiment.", + ), + "identity_differs": ( + "The labels or position of {n} differ from those the preprocessing run recorded.", + "Use the labels the preprocessing run confirmed, or run it again with the folders as " + "they are now.", + ), + "axis_mapping_mismatch": ( + "The axes given for {n} give some axes another role than the axis mapping confirmed " + "in the preprocessing brick.", + "Use the axis roles confirmed in the preprocessing brick.", + ), +} diff --git a/tests/test_axis_check.py b/tests/test_axis_check.py index b607b103..1dd705a5 100644 --- a/tests/test_axis_check.py +++ b/tests/test_axis_check.py @@ -104,7 +104,7 @@ def test_named_headers(tmp_path, kind, header, roles, expected): assert review.axis_error_files == ("a.tif",) -@pytest.mark.parametrize("roles", ["TYX", "ZYX", "CYX", "TXY"]) +@pytest.mark.parametrize("roles", ["TYX", "ZYX", "CYX"]) @pytest.mark.parametrize("writer", ["plain-IYX", "shaped-QYX"]) def test_plain_stacks_never_name_their_axes(tmp_path, writer, roles): path = tmp_path / "a.tif" @@ -232,8 +232,8 @@ def test_the_likely_cause_of_tifffiles_cyx_default_is_named(tmp_path): def test_the_codes(): - assert AXIS_ERROR_CODES == ("axes_yx_swapped", "axes_contradict_header") - assert AXIS_CHECK_CODES == ("axes_not_named", "axes_yx_swapped", "axes_contradict_header") + assert AXIS_ERROR_CODES == ("axes_yx_swapped", "axes_contradict_header", "axes_x_before_y") + assert AXIS_CHECK_CODES == ("axes_not_named", *AXIS_ERROR_CODES) def test_named_images_are_recorded_as_from_the_header(tmp_path): diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 712d6b5c..d4e091b3 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -27,6 +27,8 @@ def entry(path="cond/rep/a.tif", **kwargs): "labels": ("cond", "rep", "single"), "axes": "ZYX", "plane_count": 3, + "roles": ("Z", "Y", "X"), + "axes_from": "header", } return FileEntry(**{**values, **kwargs}) @@ -64,7 +66,7 @@ def test_written_json_is_readable_and_stable(tmp_path): raw = path.read_bytes() assert b"\r\n" not in raw and raw.endswith(b"\n") data = json.loads(raw) - assert data["manifest_version"] == "1" and data["schema_version"] == "4" + assert data["manifest_version"] == "2" and data["schema_version"] == "4" assert data["axis_mapping"] == {"axes": "ZYX", "roles": ["Z", "Y", "X"]} assert data["settings"]["exclude"] == ["a.tif", "b.tif"] assert data["labels"] == [ @@ -151,7 +153,7 @@ def test_image_type_round_trips_and_is_none_when_absent(tmp_path): assert read_manifest(path) == original data = json.loads(path.read_bytes()) assert (data["image_type"], data["channels_from_rgb_samples"]) == ("rgb", True) - assert data["manifest_version"] == "1" + assert data["manifest_version"] == "2" del data["image_type"], data["channels_from_rgb_samples"] # written before P4 path.write_text(json.dumps(data), encoding="utf-8") read = read_manifest(path) @@ -313,7 +315,11 @@ def test_each_files_axis_roles_round_trip_and_are_none_when_absent(tmp_path): assert read_manifest(path) == original data = json.loads(path.read_bytes()) assert [f["roles"] for f in data["files"]] == [["Z", "Y", "X"], [None, None, None]] - del data["files"][0]["roles"] # written before the roles were recorded + del data["files"][0]["roles"] + path.write_text(json.dumps(data), encoding="utf-8") + with pytest.raises(ManifestFormatError, match="has no roles or axes_from"): + read_manifest(path) # required in manifest version 2 + data["manifest_version"] = "1" # written before the roles were recorded path.write_text(json.dumps(data), encoding="utf-8") assert read_manifest(path).files[0].roles is None data["files"][1]["roles"] = ["Z", "Y"] # not one role per axis diff --git a/tests/test_manifest_v2.py b/tests/test_manifest_v2.py new file mode 100644 index 00000000..2faa1914 --- /dev/null +++ b/tests/test_manifest_v2.py @@ -0,0 +1,235 @@ +"""Manifest version 2, the row key and per-file plain-stack confirmation (design v0.4 PP-4). + +- Every file records its roles, where they come from (``axes_from``) and its pixel sizes + with their source; masks record their fingerprint and roles (B3). +- A version 1 manifest (before the axis check) gives no reusable row. +- A row is reused only when its :class:`RowKey` is the earlier run's (D5). +- A plain stack's confirmation covers only the files it was given for (audit 2 B-1). +""" + +import json +from dataclasses import replace + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import ( + AxisMapping, + prefill_mapping, + read_manifest, + review_axes, + run_experiment, +) +from morphobricks_preprocessing.axis_check import ConfirmedAxes +from morphobricks_preprocessing.fingerprint import Fingerprint, RowKey, confirm_axes +from morphobricks_preprocessing.manifest import ( + MANIFEST_VERSION, + FileEntry, + ManifestFormatError, + RunSettings, + source_fingerprint, + write_manifest, +) +from morphobricks_preprocessing.recorded_sizes import RecordedSize +from tiff_tree import write_data, write_plain + + +def stack(seed=0): + return np.random.default_rng(seed).integers(0, 200, (3, 6, 8), dtype=np.uint8) + + +def codes(issues): + return [i.code for i in issues if i.is_error] + + +# What version 2 records ------------------------------------------------------------------ + + +def test_version_2_records_roles_their_source_and_pixel_sizes(tmp_path): + root = tmp_path / "exp" + (root / "ctrl").mkdir(parents=True) + tifffile.imwrite( + root / "ctrl/a.ome.tif", + stack(), + ome=True, + photometric="minisblack", + metadata={"axes": "ZYX", "PhysicalSizeX": 0.5, "PhysicalSizeY": 0.25}, + ) + write_plain(root / "ctrl/b.tif", stack(1)) + result = run_experiment(root, AxisMapping("ZYX", "ZYX")) + assert not result.written # one arrangement per experiment: ZYX and IYX differ + result = run_experiment(root, prefill_mapping("ZYX"), exclude=["ctrl/b.tif"]) + data = json.loads(result.manifest_path.read_text(encoding="utf-8")) + assert data["manifest_version"] == MANIFEST_VERSION == "2" + (entry,) = data["files"] + assert entry["roles"] == ["Z", "Y", "X"] and entry["axes_from"] == "header" + sizes = {size["role"]: size for size in entry["pixel_sizes"]} + assert sizes["X"] == {"role": "X", "value": 0.5, "unit": "micrometer", "source": "header"} + assert sizes["Y"]["value"] == 0.25 + assert sizes["Z"] == {"role": "Z", "value": None, "unit": None, "source": None} + assert data["conversion_record_sha256"] is None + + +def test_a_plain_stack_is_recorded_as_confirmed_and_uncalibrated(tmp_path): + root = tmp_path / "exp" + write_plain(root / "ctrl/a.tif", stack()) + result = run_experiment(root, AxisMapping("IYX", "TYX")) + (entry,) = read_manifest(result.manifest_path).files + assert (entry.roles, entry.axes_from) == (("T", "Y", "X"), "confirmed") + assert all(size.value is None and size.source is None for size in entry.pixel_sizes) + + +def test_masks_record_their_fingerprint_and_roles(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", np.zeros((6, 8), np.uint8), "YX") + mask = write_data(tmp_path / "masks/Cells/ctrl/a.tif", np.ones((6, 8), np.uint16), "YX") + result = run_experiment(root, prefill_mapping("YX"), masks_folder=tmp_path / "masks") + (recorded,) = read_manifest(result.manifest_path).masks["files"] + size, mtime_ns = source_fingerprint(mask) + assert recorded == { + "image": "ctrl/a.tif", + "type": "Cells", + "path": "Cells/ctrl/a.tif", + "source": "folder", + "axes": "YX", + "shape": [6, 8], + "size": size, + "mtime_ns": mtime_ns, + "roles": "YX", + "axes_from": "header", + "shared": False, + } + + +def test_the_reader_requires_roles_and_their_source_in_version_2(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", stack(), "ZYX") + path = run_experiment(root, prefill_mapping("ZYX")).manifest_path + data = json.loads(path.read_text(encoding="utf-8")) + del data["files"][0]["axes_from"] + path.write_text(json.dumps(data), encoding="utf-8") + with pytest.raises(ManifestFormatError, match="has no roles or axes_from"): + read_manifest(path) + + +def test_the_writer_refuses_an_entry_without_roles(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", stack(), "ZYX") + manifest = read_manifest(run_experiment(root, prefill_mapping("ZYX")).manifest_path) + broken = replace(manifest, files=(replace(manifest.files[0], axes_from=None),)) + with pytest.raises(ValueError, match="has no roles or axes_from"): + write_manifest(broken, tmp_path / "m.json") + + +def test_recorded_sizes_are_checked(): + assert RecordedSize.from_dict({"role": "X", "value": 1, "unit": "nm", "source": "header"}) + with pytest.raises(ValueError, match="both a value and a source"): + RecordedSize.from_dict({"role": "X", "value": 1, "unit": "nm", "source": None}) + with pytest.raises(ValueError, match="unknown role or source"): + RecordedSize.from_dict({"role": "C", "value": None, "source": None}) + + +# Version 1 and the row key ----------------------------------------------------------------- + + +def test_a_version_1_run_gives_no_reusable_row(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", stack(), "ZYX") + first = run_experiment(root, prefill_mapping("ZYX")) + data = json.loads(first.manifest_path.read_text(encoding="utf-8")) + data["manifest_version"] = "1" + for entry in data["files"]: + del entry["axes_from"], entry["pixel_sizes"] + first.manifest_path.write_text(json.dumps(data), encoding="utf-8") + assert read_manifest(first.manifest_path).manifest_version == "1" + again = run_experiment(root, prefill_mapping("ZYX")) + assert ( + again.rebuild_reason == "the earlier run was written before v0.4.0, without the axis check" + ) + assert again.read == ("ctrl/a.tif",) + assert read_manifest(again.manifest_path).manifest_version == "2" + + +def test_rows_are_reused_only_for_the_same_row_key(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", stack(), "ZYX") + first = run_experiment(root, prefill_mapping("ZYX")) + assert run_experiment(root, prefill_mapping("ZYX")).read == () + # The same experiment moved elsewhere: its rows are read again from the new place. + moved = tmp_path / "moved" + root.rename(moved) + again = run_experiment(moved, prefill_mapping("ZYX"), output_folder=first.output_folder) + assert again.changed == ("ctrl/a.tif",) and again.read == ("ctrl/a.tif",) + + +def test_the_row_key(): + settings = RunSettings(256, 2**30) + + entry = FileEntry( + "a.tif", 10, 20, ("c", "b", "t"), "ZYX", roles=("Z", "Y", "X"), axes_from="header" + ) + key = RowKey.of(entry, settings, "/exp") + assert key == RowKey.of(entry, settings, "/exp") + assert key.fingerprint == Fingerprint(10, 20) + for changed in ( + replace(entry, size=11), + replace(entry, roles=("T", "Y", "X")), + replace(entry, axes_from="confirmed"), + replace(entry, labels=("c", "b", "u")), + ): + assert RowKey.of(changed, settings, "/exp") != key + assert RowKey.of(entry, settings, "/moved") != key + assert RowKey.of(entry, RunSettings(512, 2**30), "/exp") != key + + +def test_fingerprint_of_a_file(tmp_path): + path = write_data(tmp_path / "a.tif", stack(), "ZYX") + assert Fingerprint.of(path) == Fingerprint(*source_fingerprint(path)) + + +# A plain stack's confirmation covers only its files (audit 2 B-1) --------------------------- + + +def test_a_confirmation_covers_only_the_files_it_was_given_for(tmp_path): + root = tmp_path / "exp" + write_plain(root / "ctrl/a.tif", stack()) + write_plain(root / "ctrl/b.tif", stack(1)) # added after the roles were confirmed for a.tif + confirmed = confirm_axes(AxisMapping("IYX", "TYX"), root, ["ctrl/a.tif"]) + result = run_experiment(root, confirmed) + assert codes(result.issues) == ["plain_stack_not_confirmed"] + (issue,) = [i for i in result.issues if i.code == "plain_stack_not_confirmed"] + assert issue.paths == ("ctrl/b.tif",) and "confirm them again" in issue.action + both = confirm_axes(AxisMapping("IYX", "TYX"), root, ["ctrl/a.tif", "ctrl/b.tif"]) + assert run_experiment(root, both).written + # A plain mapping confirms every file the run finds, as before. + assert run_experiment(root, AxisMapping("IYX", "TYX")).written + + +def test_named_images_need_no_confirmation(tmp_path): + root = tmp_path / "exp" + write_data(root / "ctrl/a.tif", stack(), "ZYX") + write_data(root / "ctrl/b.tif", stack(1), "ZYX") + assert review_axes([], ConfirmedAxes(prefill_mapping("ZYX"), {})).ok + confirmed = confirm_axes(prefill_mapping("ZYX"), root, ["ctrl/a.tif"]) + assert run_experiment(root, confirmed).written + + +def test_a_plain_stack_changed_since_its_confirmation_is_not_covered(tmp_path): + root = tmp_path / "exp" + write_plain(root / "ctrl/a.tif", stack()) + write_plain(root / "ctrl/b.tif", stack(1)) + confirmed = confirm_axes(AxisMapping("IYX", "TYX"), root, ["ctrl/a.tif", "ctrl/b.tif"]) + assert run_experiment(root, confirmed).written + write_plain(root / "ctrl/b.tif", np.ones((4, 6, 8), np.uint8)) # changed since + result = run_experiment(root, confirmed) + assert not result.written and result.read == () + (issue,) = [i for i in result.issues if i.code == "plain_stack_not_confirmed"] + assert issue.paths == ("ctrl/b.tif",) + again = confirm_axes(AxisMapping("IYX", "TYX"), root, ["ctrl/a.tif", "ctrl/b.tif"]) + assert run_experiment(root, again).read == ("ctrl/b.tif",) + + +def test_a_confirmation_records_fingerprints(): + with pytest.raises(TypeError, match="fingerprint"): + ConfirmedAxes(AxisMapping("IYX", "TYX"), {"ctrl/a.tif"}) diff --git a/tests/test_masks.py b/tests/test_masks.py index d7069839..b848f802 100644 --- a/tests/test_masks.py +++ b/tests/test_masks.py @@ -289,14 +289,8 @@ def test_the_run_records_the_masks_in_the_manifest(experiment, masks, tmp_path): assert result.written recorded = read_manifest(out / "run_manifest.json").masks assert recorded["types"] == ["Cells", "Nuclei"] - assert recorded["files"][0] == { - "image": "ctrl/rep1/a.tif", - "type": "Cells", - "path": "Cells/ctrl/rep1/a.tif", - "source": "folder", - "axes": "YX", - "shape": [16, 20], - } + # Each entry's keys are checked in test_manifest_v2 (manifest version 2). + assert recorded["files"][0]["path"] == "Cells/ctrl/rep1/a.tif" # Without masks the manifest has no masks key, as before. run_experiment(experiment, prefill_mapping("CYX"), output_folder=out) data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) diff --git a/tests/test_rules_for_tests.py b/tests/test_rules_for_tests.py new file mode 100644 index 00000000..38c750e5 --- /dev/null +++ b/tests/test_rules_for_tests.py @@ -0,0 +1,39 @@ +"""Tests are exempt from the annotation and docstring rules (owner decision, 1 Nov 2026).""" + +import sys +import tomllib +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "tools")) + +import check # noqa: E402 + +LONG_PRIVATE = ( + "def _helper(a, b):\n" + + "".join(f" a = a + {i}\n" for i in range(15)) + + (" return a + b\n") +) + + +def test_ruff_ignores_annotations_and_docstrings_in_tests(): + data = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) + ignored = data["tool"]["ruff"]["lint"]["per-file-ignores"]["tests/**"] + assert {"ANN", "D"} <= set(ignored) + + +def test_the_rules_tool_leaves_tests_out_of_the_docstring_rule(tmp_path, monkeypatch): + (tmp_path / "pyproject.toml").write_text( + (ROOT / "pyproject.toml").read_text(encoding="utf-8"), encoding="utf-8" + ) + package = tmp_path / check.config()["package"] + package.mkdir(parents=True) + (package / "__init__.py").write_text('"""Package."""\n', encoding="utf-8") + (tmp_path / "tests").mkdir() + (tmp_path / "tests" / "test_x.py").write_text(LONG_PRIVATE, encoding="utf-8") + (tmp_path / "tools").mkdir() + (tmp_path / "tools" / "helper.py").write_text(LONG_PRIVATE, encoding="utf-8") + monkeypatch.setattr(check, "ROOT", tmp_path) + problems, _ = check.code_rule_problems() + flagged = [p for p in problems if "without a docstring" in p] + assert [p.split(":")[0] for p in flagged] == ["tools/helper.py"] diff --git a/tests/test_run.py b/tests/test_run.py index 3303724c..56dedc63 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -326,7 +326,7 @@ def test_every_row_has_its_position_id(experiment): rel: rel.rsplit("/", 1)[-1].removesuffix(".tif") for rel in IMAGES } data = json.loads(result.manifest_path.read_text(encoding="utf-8")) - assert data["manifest_version"] == "1" and data["schema_version"] == "4" + assert data["manifest_version"] == "2" and data["schema_version"] == "4" assert all("position_id" in f for f in data["files"]) diff --git a/tests/test_schema_v2_equivalence.py b/tests/test_schema_v2_equivalence.py index 0157fe59..a84ec32f 100644 --- a/tests/test_schema_v2_equivalence.py +++ b/tests/test_schema_v2_equivalence.py @@ -179,7 +179,7 @@ def test_a_version_2_table_is_recomputed_on_the_next_run(tmp_path, pixels): assert second.read == tuple(sorted(rel for rel, _, _ in EXPERIMENTS["float32"][2])) assert second.csv_path.read_bytes() == table data = json.loads(second.manifest_path.read_text(encoding="utf-8")) - assert (data["manifest_version"], data["schema_version"]) == ("1", "4") + assert (data["manifest_version"], data["schema_version"]) == ("2", "4") assert read_manifest(second.manifest_path).csv_sha256 == file_sha256(second.csv_path) diff --git a/tests/test_x_before_y.py b/tests/test_x_before_y.py new file mode 100644 index 00000000..9f7bb926 --- /dev/null +++ b/tests/test_x_before_y.py @@ -0,0 +1,62 @@ +"""Roles that put X before Y are refused: nothing is ever read transposed (owner, 1 Nov 2026). + +A plain stack names no axes, so the user confirms its roles; confirming X before Y (``TXY``) +would read every plane transposed. It is the error ``axes_x_before_y``, which recommends +saving the images again with Y before X, or as named-axis OME-TIFF copies. +""" + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + AXIS_ERROR_CODES, + AxisMapping, + header_axis_issue, + inspect_image, + review_axes, + run_experiment, +) +from morphobricks_preprocessing.axis_check import axis_disagreements +from tiff_tree import write_data, write_plain + + +def stack(): + return np.arange(3 * 6 * 8, dtype=np.uint8).reshape(3, 6, 8) + + +@pytest.mark.parametrize("writer", ["plain-IYX", "shaped-QYX"]) +@pytest.mark.parametrize("roles", ["TXY", "XTY", "XYT"]) +def test_a_plain_stack_confirmed_x_before_y_is_refused(tmp_path, writer, roles): + path = tmp_path / "a.tif" + write_plain(path, stack()) if writer == "plain-IYX" else write_data(path, stack()) + meta = inspect_image(path) + mapping = AxisMapping(meta.axes, roles) + assert header_axis_issue(meta, mapping) == "axes_x_before_y" + assert "axes_x_before_y" in AXIS_ERROR_CODES + review = review_axes([meta], mapping) + (issue,) = [i for i in review.issues if i.is_error] + assert issue.code == "axes_x_before_y" and review.axis_error_files == ("a.tif",) + assert "nothing is ever read transposed" in issue.message + assert "Y before X" in issue.action and "OME-TIFF" in issue.action + + +def test_the_run_reads_nothing(tmp_path): + root = tmp_path / "exp" + write_plain(root / "ctrl/a.tif", stack()) + result = run_experiment(root, AxisMapping("IYX", "TXY")) + assert not result.written and result.read == () + assert [i.code for i in result.issues if i.is_error] == ["axes_x_before_y"] + # Y before X is read, with the plain stack's warning. + assert run_experiment(root, AxisMapping("IYX", "TYX")).written + + +def test_only_y_and_x_are_marked(tmp_path): + path = write_plain(tmp_path / "a.tif", stack()) + marks = axis_disagreements(inspect_image(path), AxisMapping("IYX", "TXY")) + assert marks == {0: "axes_not_named", 1: "axes_x_before_y", 2: "axes_x_before_y"} + + +def test_a_named_swap_stays_a_swap(tmp_path): + path = write_data(tmp_path / "a.tif", stack(), "ZYX") + meta = inspect_image(path) + assert header_axis_issue(meta, AxisMapping("ZYX", "ZXY")) == "axes_yx_swapped" diff --git a/tests/tiff_tree.py b/tests/tiff_tree.py index 6303b95c..84a8a9ad 100644 --- a/tests/tiff_tree.py +++ b/tests/tiff_tree.py @@ -24,6 +24,14 @@ def write_data(path, data, axes=None, **kwargs): return path +def write_plain(path, data): + """Write ``data`` as a plain stack of pages, without axis metadata (``IYX``).""" + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite(path, data, photometric="minisblack", metadata=None) + return path + + def write_file(path, content=b"not an image"): path = Path(path) path.parent.mkdir(parents=True, exist_ok=True) diff --git a/tools/check.py b/tools/check.py index 369b6c80..f123f1ec 100644 --- a/tools/check.py +++ b/tools/check.py @@ -39,6 +39,10 @@ ROOT = Path(__file__).resolve().parents[1] LINTED = ("src", "tests", "tools") +TESTS = "tests/" +"""Tests are exempt from the annotation and docstring rules (owner decision, 1 November +2026): Ruff ignores ``ANN`` and ``D`` there (``pyproject.toml``), and the private-docstring +rule leaves them out here. Every other rule applies.""" def config() -> dict: @@ -71,7 +75,8 @@ def code_rule_problems() -> tuple[list[str], dict[str, int]]: problems = [] for name, text in sources: problems += check_source(text, name) - problems += missing_private_docstrings(text, name) + if not name.startswith(TESTS): + problems += missing_private_docstrings(text, name) problems += reserved_name_problems(text, name, settings["owner"]) path = ROOT / name if package_dir in path.parents: diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index b6e2fcf0..da575e7e 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -9,9 +9,8 @@ "identical bodies: tests/test_axis_check.py:# imagej, tests/test_header_axes.py:# imagej": 0, "identical bodies: tests/test_axis_check.py:# plain, tests/test_header_axes.py:# plain": 0, "identical bodies: tests/test_cli.py:# bright_tree, tests/test_notebook_ui.py:# bright_tree": 0, - "src/morphobricks_preprocessing/axes.py check_mapping: # executable lines (max #)": 72, - "src/morphobricks_preprocessing/axes.py review_axes: # executable lines (max #)": 108, - "src/morphobricks_preprocessing/axes.py: # lines (max #)": 717, + "src/morphobricks_preprocessing/axis_review.py check_mapping: # executable lines (max #)": 72, + "src/morphobricks_preprocessing/axis_review.py review_axes: # executable lines (max #)": 106, "src/morphobricks_preprocessing/cli.py _check: private function of # lines without a docstring": 52, "src/morphobricks_preprocessing/cli.py _common: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/cli.py _masks: private function of # lines without a docstring": 39, @@ -28,10 +27,7 @@ "src/morphobricks_preprocessing/discovery.py finish: # executable lines (max #)": 145, "src/morphobricks_preprocessing/discovery.py: # lines (max #)": 847, "src/morphobricks_preprocessing/image_type.py _rgb_problem: private function of # lines without a docstring": 12, - "src/morphobricks_preprocessing/manifest.py _file_dict: private function of # lines without a docstring": 17, - "src/morphobricks_preprocessing/manifest.py _file_entry: private function of # lines without a docstring": 20, "src/morphobricks_preprocessing/manifest.py file_entry_for: # parameters (max #)": 9, - "src/morphobricks_preprocessing/manifest.py: # lines (max #)": 732, "src/morphobricks_preprocessing/masks.py _duplicate_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/masks.py _folder_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/masks.py _name_issues: private function of # lines without a docstring": 15, @@ -91,13 +87,13 @@ "src/morphobricks_preprocessing/planes.py _zarr_layout: private function of # lines without a docstring": 40, "src/morphobricks_preprocessing/planes.py check_planes: # executable lines (max #)": 65, "src/morphobricks_preprocessing/planes.py: # lines (max #)": 1422, - "src/morphobricks_preprocessing/run/pipeline.py run_experiment: # executable lines (max #)": 195, + "src/morphobricks_preprocessing/run/pipeline.py run_experiment: # executable lines (max #)": 161, "src/morphobricks_preprocessing/run/pipeline.py run_experiment: # parameters (max #)": 15, "src/morphobricks_preprocessing/run/preflight.py _exclusion_issues: private function of # lines without a docstring": 34, "src/morphobricks_preprocessing/run/rows.py _bin_count: private function of # lines without a docstring": 12, "src/morphobricks_preprocessing/run/rows.py _pixel_issues: private function of # lines without a docstring": 38, "tests/test_discovery.py: # lines (max #)": 848, - "tests/test_masks.py: # lines (max #)": 618, + "tests/test_masks.py: # lines (max #)": 612, "tests/test_metadata.py: # lines (max #)": 629, "tests/test_notebook_ui.py: # lines (max #)": 1604, "tests/test_planes.py: # lines (max #)": 818,