diff --git a/CHANGELOG.md b/CHANGELOG.md index a3747821..0dc9de87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,23 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Discovery and header additions for the experiment description (Stage 1, PR 6).** + Additive; a run without a description behaves as before. (1) `preflight` and `remasked` + take `with_description=False`; with it, a condition, repeat or technical-repeat folder that + is not Unicode NFC is the error `R_FOLDER_NOT_NFC`, a discovered mask type that breaks the + Label rules (`NA`, ` cells`) is `R_MASK_TYPE_NAME_INVALID` and one that is not NFC is + `R_FOLDER_NOT_NFC` (new module `name_issues`, also used by `find_masks(with_description=)`); + nothing is renamed or normalized (owner decision Q1: without a description these names are + accepted as in v0.4). (2) `TiffMetadata.raw_channel_names` and `FileEntry.raw_channel_names` + hold the header channel names exactly as read, untrimmed; `channel_names` stays trimmed, + `same_source` ignores them, and the manifest file entry gains the optional key + `raw_channel_names`, written only when the header names a channel. The OME header reading + moved from `metadata` to `ome_header`. (3) `preflight(progress=...)` calls + `progress(phase, done, total, path)` for the phases `folders`, `headers` (one call per + image), `masks` and `acquisitions`; `run.phases.progress_text` words them ("2/4 headers + 120/480"). Not used by the notebook or the command line yet. Tests: + `tests/test_spec_discovery.py` (conformance P21, P26 recorded names, P29) and + `tests/test_preflight_progress.py`. - **Content hashing of the sources is on by default (#159; owner decision of 7 November 2026).** A run records each image's content SHA-256 and reuses a row only for the same content, so an image edited in place with its size and modification time kept is read again (manifest diff --git a/pyproject.toml b/pyproject.toml index efd2eec7..9e32ea12 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -129,6 +129,9 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.axis_names" = "pure" "morphobricks_preprocessing.image_names" = "pure" "morphobricks_preprocessing.mask_names" = "pure" +"morphobricks_preprocessing.name_issues" = "pure" +"morphobricks_preprocessing.run.phases" = "pure" +"morphobricks_preprocessing.ome_header" = "io" "morphobricks_preprocessing.axis_review" = "pure" "morphobricks_preprocessing.mask_checks" = "pure" "morphobricks_preprocessing.mask_review" = "pure" diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index f9f2f988..6bd23a2e 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -38,10 +38,12 @@ from .inventory import INVENTORY_KEY, source_signature from .inventory import source_fingerprint as source_fingerprint # re-exported (moved, PP-12) from .manifest_fields import ( + RAW_NAMES_KEY, optional_text, recorded_flag, recorded_masks, recorded_position_id, + recorded_raw_names, recorded_roles, ) from .manifest_versions import ( @@ -130,21 +132,19 @@ class FileEntry: significant_bits Significant bits from metadata; None when not given. channel_names_trimmed - True when leading or trailing whitespace was removed from the image's - channel names as they were read (names are recorded trimmed). + True when whitespace was removed around the channel names (names are recorded trimmed). read_in_blocks - True when the image's planes are too large for the memory limit and are - read in blocks. For information only: the rows are the same either way. + True when the planes are too large for the memory limit and are read in blocks. + For information only: the rows are the same either way. position_id The image's position identifier, its name without the extension (:func:`~morphobricks_preprocessing.image_names.position_id`). Required in manifest version 2, and equal to that rule (round-2 audit A-m5); None in older manifests. roles - 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. Required in manifest version 2; None in older - manifests. + 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. 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). @@ -158,9 +158,10 @@ class FileEntry: TIFF file, and in manifests written before it was recorded (such an OME-Zarr image counts as changed: its fingerprint is not known to be complete). content_sha256 - The SHA-256 of the file's content (an OME-Zarr folder: of its files' paths and - SHA-256s, :func:`.source_content.source_content_sha256`), in a ``verify_content`` - run only; None otherwise. + The SHA-256 of the file's content (an OME-Zarr folder: of its files' paths and SHA-256s, + :func:`.source_content.source_content_sha256`) in a ``verify_content`` run; else None. + raw_channel_names + Header channel names, untrimmed; written only when not empty; not in :meth:`same_source`. """ relative_path: str @@ -180,6 +181,7 @@ class FileEntry: pixel_sizes: tuple[RecordedSize, ...] = () inventory_sha256: str | None = None content_sha256: str | None = None + raw_channel_names: tuple[str | None, ...] = () def same_source(self, other: FileEntry) -> bool: """Return True if ``other`` describes the same unchanged file, read the same way.""" @@ -555,6 +557,8 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: data[INVENTORY_KEY] = entry.inventory_sha256 if entry.content_sha256 is not None: data[CONTENT_KEY] = entry.content_sha256 + if entry.raw_channel_names: + data[RAW_NAMES_KEY] = list(entry.raw_channel_names) return data @@ -588,6 +592,7 @@ def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: pixel_sizes=sizes_from_data(data.get("pixel_sizes")), inventory_sha256=optional_text(data.get(INVENTORY_KEY)), content_sha256=optional_text(data.get(CONTENT_KEY)), + raw_channel_names=recorded_raw_names(data.get(RAW_NAMES_KEY), path), ) diff --git a/src/morphobricks_preprocessing/manifest_fields.py b/src/morphobricks_preprocessing/manifest_fields.py index 9d87639f..9e3b6ab2 100644 --- a/src/morphobricks_preprocessing/manifest_fields.py +++ b/src/morphobricks_preprocessing/manifest_fields.py @@ -14,6 +14,22 @@ from .axes import AxisMapping from .image_names import position_id +RAW_NAMES_KEY = "raw_channel_names" +"""The file entry's key for the channel names as the header gave them (written when not empty).""" + + +def recorded_raw_names(value: object, relative_path: str) -> tuple[str | None, ...]: + """Read a recorded ``raw_channel_names`` (a list of text or null), or refuse it. + + Absent (a manifest written before it was recorded, or an image whose header names no + channel) reads as empty. A name is kept exactly as written, never trimmed. + """ + if value is None: + return () + if not isinstance(value, list) or any(n is not None and not isinstance(n, str) for n in value): + raise ValueError(f"the raw channel names of {relative_path!r} are not text or null") + return tuple(value) + def recorded_position_id(value: object, relative_path: str, *, strict: bool = True) -> str | None: """Check the ``position_id`` of a manifest file entry against its image's name. diff --git a/src/morphobricks_preprocessing/masks/files.py b/src/morphobricks_preprocessing/masks/files.py index 436b0cff..2fcb804b 100644 --- a/src/morphobricks_preprocessing/masks/files.py +++ b/src/morphobricks_preprocessing/masks/files.py @@ -10,11 +10,22 @@ from collections.abc import Collection, Iterable, Mapping from pathlib import Path -from ..discovery import DiscoveryResult, is_system_name, is_tiff_name +from ..discovery import ( + DiscoveryResult, + IdentityError, + check_identity_text, + is_system_name, + is_tiff_name, +) from ..folder_rules import masks_folder_issues from ..image_names import position_id from ..links import LinkGuard from ..metadata import TiffMetadata +from ..name_issues import ( + invalid_mask_types_issue, + is_nfc, + non_nfc_mask_types_issue, +) from ..validation import ValidationIssue, agree, error, has_errors, plural, warning from ..zarr_input import ( NgffError, @@ -35,6 +46,7 @@ def find_masks( mask_axes: Mapping[str, str] | None = None, output_folder: str | os.PathLike | None = None, bind: bool = False, + with_description: bool = False, ) -> MaskInventory: """Pair every image with its masks and check them from their headers, without pixels. @@ -65,6 +77,12 @@ def find_masks( fingerprint with it (:attr:`.model.MaskFile.bound`, issue #160), so a run or the mask step refuses a mask that is no longer the one its header described; a mask that changed while its header was read is the error ``mask_changed_during_check``. + with_description + True when an experiment description is in use: a discovered mask type name that + breaks the Label rules (``NA``, `` cells``) is then the error + ``R_MASK_TYPE_NAME_INVALID``, and one that is not Unicode NFC the error + ``R_FOLDER_NOT_NFC`` (Spec experiment-v1 §6; never normalized or renamed). Without a + description, mask type names are accepted as before (owner decision Q1). Returns ------- @@ -85,6 +103,8 @@ def find_masks( found, unreadable = _label_masks(discovery.root, images) files.extend(found) issues.extend(_name_issues(files)) + if with_description: + issues.extend(_description_name_issues(files)) files.sort(key=lambda f: (f.image, f.mask_type)) issues.extend(_duplicate_issues(files)) stated = dict(mask_axes or {}) @@ -287,6 +307,19 @@ def _name_issues(files: list[MaskFile]) -> list[ValidationIssue]: ] +def _description_name_issues(files: list[MaskFile]) -> list[ValidationIssue]: + """Refuse the discovered mask types that cannot be a ``source_type`` of a description.""" + invalid, not_nfc = [], [] + for name in sorted({f.mask_type for f in files}): + try: + check_identity_text(name, "mask type") + except IdentityError: + invalid.append(name) + if not is_nfc(name): + not_nfc.append(name) + return [*invalid_mask_types_issue(invalid), *non_nfc_mask_types_issue(not_nfc)] + + def _duplicate_issues(files: list[MaskFile]) -> list[ValidationIssue]: seen: dict[tuple[str, str], MaskFile] = {} duplicates = [] diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index bc374454..d1de1937 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -21,7 +21,6 @@ from collections.abc import Callable, Hashable, Iterable, Mapping, Sequence from dataclasses import dataclass from pathlib import Path -from xml.etree import ElementTree import numpy as np import tifffile @@ -29,6 +28,7 @@ from .axis_names import OPEN_AXIS, named_letters from .discovery import DiscoveryResult +from .ome_header import ome_details from .physical_sizes import PhysicalSizes, _tiff_sizes from .pixel_size_checks import pixel_size_issues from .size_checks import ( @@ -48,7 +48,6 @@ ZarrUnavailableError, is_zarr_name, open_ngff, - read_channel_names, ) logger = logging.getLogger(__name__) @@ -93,6 +92,11 @@ class TiffMetadata: channel_names_trimmed True when leading or trailing whitespace was removed from a channel name (recorded in the run manifest). + raw_channel_names + The channel names exactly as the header gives them, untrimmed (a name that is not + text is None); empty when the header names no channel. The evidence an experiment + description's header names are compared with (Spec experiment-v1 §4.3); recorded in + the run manifest as read. significant_bits Bits per pixel that carry data (e.g. 12 for a 12-bit camera in 16-bit files), from OME metadata; None when not given. For information only. @@ -132,6 +136,7 @@ class TiffMetadata: rgb_samples: bool = False channel_names_trimmed: bool = False axes_from_metadata: bool = True + raw_channel_names: tuple[str | None, ...] = () def length(self, axis: str) -> int: """Return the length of the dimension labelled ``axis``, 1 if there is none.""" @@ -281,7 +286,7 @@ def inspect_tiff(path: str | os.PathLike, relative_path: str | None = None) -> T axes=str(first.axes), series_count=len(series), page_count=len(tif.pages), - **_ome_details(tif), + **ome_details(tif), physical_sizes=_tiff_sizes(tif), rgb_samples=_rgb_samples(first), axes_from_metadata=series_axes_from_metadata(first), @@ -317,6 +322,7 @@ def inspect_ome_zarr(path: str | os.PathLike, relative_path: str | None = None) page_count=0, channel_names=image.channel_names, channel_names_trimmed=image.channel_names_trimmed, + raw_channel_names=image.raw_channel_names, kind=OME_ZARR, physical_sizes=image.physical_sizes, ) @@ -511,29 +517,6 @@ def check_metadata(files: Iterable[TiffMetadata]) -> list[ValidationIssue]: return issues -def _ome_details(tif: tifffile.TiffFile) -> dict: - """Channel names and significant bits of the first OME image, if present.""" - if not tif.is_ome: - return {} - try: - root = ElementTree.fromstring(tif.ome_metadata) - except (ElementTree.ParseError, TypeError, ValueError): - logger.debug("Cannot parse the OME metadata of %s", tif.filename, exc_info=True) - return {} - pixels = next((e for e in root.iter() if _local_name(e.tag) == "Pixels"), None) - if pixels is None: - return {} - names, trimmed = read_channel_names( - channel.get("Name") for channel in pixels if _local_name(channel.tag) == "Channel" - ) - bits = pixels.get("SignificantBits") - return { - "channel_names": names, - "channel_names_trimmed": trimmed, - "significant_bits": int(bits) if bits and bits.isdigit() else None, - } - - def _rgb_samples(series: tifffile.TiffPageSeries) -> bool: """Return True if the series stores RGB colour: photometric RGB, three samples per pixel.""" keyframe = series.keyframe @@ -577,10 +560,6 @@ def series_axes_from_metadata(series: tifffile.TiffPageSeries) -> bool: return True -def _local_name(tag: str) -> str: - return tag.rsplit("}", 1)[-1] - - def _check_channel_names(files: list[TiffMetadata]) -> list[ValidationIssue]: named = [f for f in files if f.channel_names] if len(named) < 2: diff --git a/src/morphobricks_preprocessing/name_issues.py b/src/morphobricks_preprocessing/name_issues.py new file mode 100644 index 00000000..84026454 --- /dev/null +++ b/src/morphobricks_preprocessing/name_issues.py @@ -0,0 +1,114 @@ +"""The refusals of folder and mask-type names that apply with an experiment description. + +Spec experiment-v1 §3.3 and §6: a folder label or a discovered mask type name read from disk +must be Unicode NFC (``R_FOLDER_NOT_NFC``), and a mask type name must pass the Label rules +(``R_MASK_TYPE_NAME_INVALID``). Neither is ever normalized, trimmed or renamed: the name is +reported as read. They apply only when a description is in use (owner decision Q1, 7 November +2026); a run without one keeps accepting what v0.4 accepted. +""" + +from __future__ import annotations + +import unicodedata +from collections.abc import Iterable +from pathlib import PurePosixPath +from typing import TYPE_CHECKING + +from .validation import ValidationIssue, error, plural + +if TYPE_CHECKING: + from .discovery import LeafFolder + +FOLDER_NOT_NFC = "R_FOLDER_NOT_NFC" +"""A folder label or discovered mask type name that is not Unicode NFC.""" + +MASK_TYPE_NAME_INVALID = "R_MASK_TYPE_NAME_INVALID" +"""A discovered mask type name that breaks the Label rules, so it cannot be a ``source_type``.""" + +NAME_CODES = frozenset({FOLDER_NOT_NFC, MASK_TYPE_NAME_INVALID}) +"""The codes of this module.""" + + +def is_nfc(name: str) -> bool: + """Return True if ``name`` is Unicode NFC (checked, never converted).""" + return unicodedata.is_normalized("NFC", name) + + +def non_nfc_folders_issue(leaves: Iterable[LeafFolder]) -> list[ValidationIssue]: + """Return ``R_FOLDER_NOT_NFC`` for the label folders whose names are not NFC, or nothing. + + The label folders are the condition, biological-repeat and technical-repeat folders on + the way to each leaf folder (:attr:`.LeafFolder.relative_path`), as read from disk; a + folder that leads to no image is no label and is not checked. Never normalized: the + paths are reported as read. + + Parameters + ---------- + leaves + The experiment's leaf folders (:attr:`.DiscoveryResult.leaves`). + """ + found = set() + for leaf in leaves: + parts = () if leaf.relative_path == "." else PurePosixPath(leaf.relative_path).parts + found.update("/".join(parts[: i + 1]) for i, part in enumerate(parts) if not is_nfc(part)) + found = sorted(found) + if not found: + return [] + return [ + error( + FOLDER_NOT_NFC, + f"{plural(len(found), 'folder')} {'has a name' if len(found) == 1 else 'have names'}" + " that " + f"{'is' if len(found) == 1 else 'are'} not in Unicode NFC form, for example an " + "accented letter stored as a letter plus a separate accent. The name is never " + "normalized or renamed for you, because the experiment description records folder " + "names exactly as they are on disk.", + found, + "Rename these folders so that each accented character is a single composed " + "character (NFC), or use names without accents.", + ) + ] + + +def non_nfc_mask_types_issue(names: Iterable[str]) -> list[ValidationIssue]: + """Return ``R_FOLDER_NOT_NFC`` for mask types whose names are not NFC, or nothing.""" + found = sorted(set(names)) + if not found: + return [] + return [ + error( + FOLDER_NOT_NFC, + f"{plural(len(found), 'mask type')} {'has a name' if len(found) == 1 else 'have names'}" + " that " + f"{'is' if len(found) == 1 else 'are'} not in Unicode NFC form. The name is never " + "normalized or renamed for you.", + found, + "Rename the mask folders (or the mask file endings) so that each accented character " + "is a single composed character (NFC), or use names without accents.", + ) + ] + + +def invalid_mask_types_issue(names: Iterable[str]) -> list[ValidationIssue]: + """Return ``R_MASK_TYPE_NAME_INVALID`` for mask types that break the Label rules, or nothing. + + Parameters + ---------- + names + Mask type names that :func:`.discovery.check_identity_text` refused. + """ + found = sorted(set(names)) + if not found: + return [] + return [ + error( + MASK_TYPE_NAME_INVALID, + f"{plural(len(found), 'mask type')} " + f"{'has a name' if len(found) == 1 else 'have names'} that cannot identify a " + "mask type: the name starts or ends with a space, or is NA, nan, none, null, n/a " + "or (in any case), which tables read as a missing value.", + found, + "Rename these mask folders (or the mask file endings), without spaces at the start " + "or end, and not NA, nan, none, null, n/a or .", + ) + ] diff --git a/src/morphobricks_preprocessing/ome_header.py b/src/morphobricks_preprocessing/ome_header.py new file mode 100644 index 00000000..683d53f9 --- /dev/null +++ b/src/morphobricks_preprocessing/ome_header.py @@ -0,0 +1,43 @@ +"""The OME metadata of a TIFF header: channel names (as read and trimmed) and significant bits. + +Moved from ``metadata.py``; the names are read by :func:`.zarr_input.read_channel_names` +(trimmed, for the checks) and :func:`.zarr_input.raw_channel_names` (as the header gave them). +""" + +from __future__ import annotations + +import logging +from xml.etree import ElementTree + +import tifffile + +from .zarr_input import raw_channel_names, read_channel_names + +logger = logging.getLogger(__name__) + + +def ome_details(tif: tifffile.TiffFile) -> dict: + """Channel names and significant bits of the first OME image, if present.""" + if not tif.is_ome: + return {} + try: + root = ElementTree.fromstring(tif.ome_metadata) + except (ElementTree.ParseError, TypeError, ValueError): + logger.debug("Cannot parse the OME metadata of %s", tif.filename, exc_info=True) + return {} + pixels = next((e for e in root.iter() if _local_name(e.tag) == "Pixels"), None) + if pixels is None: + return {} + raw = [channel.get("Name") for channel in pixels if _local_name(channel.tag) == "Channel"] + names, trimmed = read_channel_names(raw) + bits = pixels.get("SignificantBits") + return { + "channel_names": names, + "channel_names_trimmed": trimmed, + "raw_channel_names": raw_channel_names(raw), + "significant_bits": int(bits) if bits and bits.isdigit() else None, + } + + +def _local_name(tag: str) -> str: + return tag.rsplit("}", 1)[-1] diff --git a/src/morphobricks_preprocessing/run/entries.py b/src/morphobricks_preprocessing/run/entries.py index e163b8f6..db234baf 100644 --- a/src/morphobricks_preprocessing/run/entries.py +++ b/src/morphobricks_preprocessing/run/entries.py @@ -62,6 +62,7 @@ def file_entries( significant_bits=header.significant_bits, read_in_blocks=rel in in_blocks, channel_names_trimmed=header.channel_names_trimmed, + raw_channel_names=tuple(header.raw_channel_names), ) entries[rel] = replace( entry, diff --git a/src/morphobricks_preprocessing/run/phases.py b/src/morphobricks_preprocessing/run/phases.py new file mode 100644 index 00000000..4a19158f --- /dev/null +++ b/src/morphobricks_preprocessing/run/phases.py @@ -0,0 +1,36 @@ +"""The four phases of the folder check and how their progress is told (Stage 1 design, card 2). + +:func:`.preflight` reports ``progress(phase, done, total, path)`` while it works, and +:func:`progress_text` words it as the notebook and the command line show it: +``"1/4 folders"``, ``"2/4 headers 120/480"``, ``"3/4 masks"``, ``"4/4 acquisitions"``. +""" + +from __future__ import annotations + +PHASES = ("folders", "headers", "masks", "acquisitions") +"""The phases of the check, in order: the folder walk, one header per image, the masks found +and paired, and the acquisitions (calibration, dataset consistency, axis suggestion).""" + +COUNTED = "headers" +"""The phase that counts its work (one header per image); the others are one step each.""" + + +def progress_text(phase: str, done: int, total: int) -> str: + """Return the progress line of ``phase`` (``"2/4 headers 120/480"``). + + Parameters + ---------- + phase + One of :data:`PHASES`. + done, total + Work done and to do in that phase; shown only for the counted phase (headers). + + Raises + ------ + ValueError + For a phase that is not in :data:`PHASES`. + """ + if phase not in PHASES: + raise ValueError(f"unknown phase {phase!r}; the phases are {', '.join(PHASES)}") + text = f"{PHASES.index(phase) + 1}/{len(PHASES)} {phase}" + return f"{text} {done}/{total}" if phase == COUNTED else text diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py index c1a41315..ccbe2c88 100644 --- a/src/morphobricks_preprocessing/run/preflight.py +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -9,7 +9,7 @@ import logging import os -from collections.abc import Iterable, Mapping +from collections.abc import Callable, Iterable, Mapping from dataclasses import dataclass, field, replace from pathlib import Path @@ -28,6 +28,7 @@ from ..mask_names import MASK_NAME_CODES, mask_name_issues from ..masks import MaskInventory, find_masks from ..metadata import MetadataReport, check_metadata, inspect_image, inspect_images +from ..name_issues import non_nfc_folders_issue from ..pixel_size_checks import PIXEL_SIZE_CODES, pixel_size_issues from ..planes import ( DEFAULT_MEMORY_LIMIT, @@ -173,6 +174,8 @@ def preflight( masks_folder: str | os.PathLike | None = None, mask_axes: Mapping[str, str] | None = None, bind_headers: bool = False, + with_description: bool = False, + progress: Callable[[str, int, int, str], None] | None = None, ) -> Preflight: """Pass 1: check the folder and the image headers without reading pixels. @@ -197,45 +200,87 @@ def preflight( (:attr:`Preflight.header_fingerprints`), so a caller that writes from the headers (:func:`.conversion_plan.plan_conversion`) never mixes a header with a later file. Each mask is bound the same way (:attr:`.masks.MaskFile.bound`, issue #160). + with_description + True when an experiment description is in use: folder names that are not Unicode NFC + (``R_FOLDER_NOT_NFC``) and mask type names that break the Label rules + (``R_MASK_TYPE_NAME_INVALID``) or are not NFC are then errors. Without one (the + default) they are accepted as in v0.4 (owner decision Q1). + progress + Called as ``progress(phase, done, total, path)`` while the check runs, with a phase of + :data:`.phases.PHASES` (``folders``, ``headers``, ``masks``, ``acquisitions``): a phase + is announced as ``(phase, 0, total, "")`` and ends with ``(phase, total, total, "")``; + the headers phase also reports ``(phase, done, total, path)`` before each image. + :func:`.phases.progress_text` words it. It changes nothing about the result. Returns ------- Preflight """ + report = progress or _no_progress root = Path(experiment_root).resolve() output = default_output_folder(root) if output_folder is None else Path(output_folder) + report("folders", 0, 1, "") discovery = discover_experiment(root, output) + if with_description: + discovery = replace( + discovery, issues=(*discovery.issues, *non_nfc_folders_issue(discovery.leaves)) + ) + report("folders", 1, 1, "") excluded = tuple(sorted({normalise_relative_path(path) for path in exclude})) discovery = _without_excluded_masks(discovery, excluded) images = [path for path in discovery.images if path not in excluded] - inspected, bound = _inspected(root, images, bind_headers) + inspected, bound = _inspected(root, images, bind_headers, progress) metadata, calibration = _calibrated(inspected, discovery.images, output) - return Preflight( + report("masks", 0, 1, "") + masks = _masks( + discovery, metadata, masks_folder, mask_axes, output, bind_headers, with_description + ) + report("masks", 1, 1, "") + report("acquisitions", 0, 1, "") + result = Preflight( discovery=discovery, metadata=metadata, excluded=excluded, exclusion_issues=tuple(_exclusion_issues(discovery, excluded)), suggested_mapping=prefill_experiment_mapping(metadata), dataset=check_dataset(discovery, excluded=excluded), - masks=_masks(discovery, metadata, masks_folder, mask_axes, output, bind_headers), + masks=masks, calibration=calibration, conversion=review_conversion(root, discovery.images), header_fingerprints=bound, ) + report("acquisitions", 1, 1, "") + return result + + +def _no_progress(phase: str, done: int, total: int, path: str) -> None: + """Report nothing (the default of :func:`preflight`'s ``progress``).""" def _inspected( - root: Path, images: list[str], bind: bool + root: Path, + images: list[str], + bind: bool, + progress: Callable[[str, int, int, str], None] | None = None, ) -> tuple[MetadataReport, dict[str, Fingerprint | None]]: - """Inspect the headers; with ``bind``, fingerprint each image before and after.""" - if not bind: + """Inspect the headers; with ``bind``, fingerprint each image before and after. + + With ``progress``, each image is reported before its header is read (phase ``headers``). + """ + if not bind and progress is None: return inspect_images(root, images), {} + report = progress or _no_progress files, bound = [], {} - for rel in sorted(images): - before = current_fingerprint(root / rel) + ordered = sorted(images) + report("headers", 0, len(ordered), "") + for done, rel in enumerate(ordered): + report("headers", done, len(ordered), rel) + before = current_fingerprint(root / rel) if bind else None files.append(inspect_image(root / rel, rel)) - after = current_fingerprint(root / rel) - bound[rel] = before if before is not None and before == after else None + if bind: + after = current_fingerprint(root / rel) + bound[rel] = before if before is not None and before == after else None + report("headers", len(ordered), len(ordered), "") return MetadataReport(files=tuple(files), issues=tuple(check_metadata(files))), bound @@ -244,6 +289,8 @@ def remasked( masks_folder: str | os.PathLike | None, mask_axes: Mapping[str, str] | None, output_folder: str | os.PathLike, + *, + with_description: bool = False, ) -> Preflight: """Return the preflight with its masks paired again, as :func:`preflight` pairs them. @@ -259,13 +306,22 @@ def remasked( As :func:`preflight`. output_folder The output folder (it may not be inside the masks folder). + with_description + As :func:`preflight`: the mask type name refusals of a description. Returns ------- Preflight With :attr:`Preflight.masks` replaced. """ - masks = _masks(pre.discovery, pre.metadata, masks_folder, mask_axes, Path(output_folder)) + masks = _masks( + pre.discovery, + pre.metadata, + masks_folder, + mask_axes, + Path(output_folder), + with_description=with_description, + ) return replace(pre, masks=masks) @@ -276,6 +332,7 @@ def _masks( mask_axes: Mapping[str, str] | None, output: Path, bind: bool = False, + with_description: bool = False, ) -> MaskInventory | None: """Pair the analysed images with their masks; None without masks folder or label image.""" masks = find_masks( @@ -285,6 +342,7 @@ def _masks( mask_axes=mask_axes, output_folder=output, bind=bind, + with_description=with_description, ) if masks_folder is None and not masks.files and not masks.issues: return None # no masks folder and no label images: the run has no masks diff --git a/src/morphobricks_preprocessing/zarr_input.py b/src/morphobricks_preprocessing/zarr_input.py index 41649ec9..31b32e43 100644 --- a/src/morphobricks_preprocessing/zarr_input.py +++ b/src/morphobricks_preprocessing/zarr_input.py @@ -71,6 +71,19 @@ def read_channel_names(raw: Iterable[Any]) -> tuple[tuple[str | None, ...], bool return (tuple(names) if any(names) else ()), trimmed +def raw_channel_names(raw: Iterable[Any]) -> tuple[str | None, ...]: + """Return channel names exactly as the header gives them, untrimmed (Spec experiment-v1 §4.3). + + The counterpart of :func:`read_channel_names` for the record of what the header said: a + name that is text is kept as read, with its outer whitespace (``" DAPI"`` stays), and one + that is not text is None. Empty when no channel has a text name. Nothing is trimmed, + normalized or compared here: the description's header evidence is compared exactly, after + NFC of both sides, by the checks that read this record. + """ + names = tuple(value if isinstance(value, str) else None for value in raw) + return names if any(name is not None for name in names) else () + + def is_zarr_name(name: str) -> bool: """Return True if a folder name marks a Zarr store (``.zarr``, any case).""" return name.lower().endswith(ZARR_SUFFIX) @@ -120,6 +133,8 @@ class NgffImage: whitespace (:func:`read_channel_names`); empty when not given. channel_names_trimmed True when leading or trailing whitespace was removed from a label. + raw_channel_names + The labels exactly as read, untrimmed (:func:`raw_channel_names`); empty when not given. dataset_path Path of the level-0 array inside the store, e.g. ``"0"`` or ``"0/0"``. physical_sizes @@ -140,6 +155,7 @@ class NgffImage: dataset_path: str = "" channel_names_trimmed: bool = False physical_sizes: PhysicalSizes = PhysicalSizes() + raw_channel_names: tuple[str | None, ...] = () @property def shape(self) -> tuple[int, ...]: @@ -317,6 +333,7 @@ def _level0(group, meta: dict, prefix: str, image_count: int) -> NgffImage: # n image_count=image_count, channel_names=labels, channel_names_trimmed=trimmed, + raw_channel_names=raw_channel_names(raw_labels), dataset_path=f"{prefix}{dataset_path}", physical_sizes=ngff_physical_sizes(first, axes), ) diff --git a/tests/test_preflight_progress.py b/tests/test_preflight_progress.py new file mode 100644 index 00000000..6f76da6d --- /dev/null +++ b/tests/test_preflight_progress.py @@ -0,0 +1,161 @@ +"""Phase-aware progress of the folder check, and the raw header names (Stage 1 design, PR 6).""" + +import json +from dataclasses import replace + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import ( + prefill_mapping, + preflight, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing.manifest import FileEntry +from morphobricks_preprocessing.run.phases import PHASES, progress_text +from tiff_tree import write_data, write_tiff +from zarr_tree import write_ngff + + +@pytest.fixture +def experiment(tmp_path): + root = tmp_path / "exp" + for rel in ("ctrl/rep1/a.tif", "ctrl/rep1/b.tif", "drug/rep1/a.tif"): + write_tiff(root / rel) + return root + + +def collect(root, **kwargs): + calls = [] + pre = preflight(root, progress=lambda *call: calls.append(call), **kwargs) + return pre, calls + + +def test_the_phases_are_announced_in_order(experiment): + _, calls = collect(experiment) + order = [phase for phase, *_ in calls] + assert [p for p in PHASES if p in order] == list(PHASES) + assert order == sorted(order, key=PHASES.index) + for phase in PHASES: + mine = [call for call in calls if call[0] == phase] + assert mine[0][1] == 0 and mine[-1][1] == mine[-1][2] + + +def test_the_headers_phase_counts_every_image_with_its_path(experiment): + _, calls = collect(experiment) + headers = [call for call in calls if call[0] == "headers"] + reading = [(done, total, path) for _, done, total, path in headers if path] + assert reading == [ + (0, 3, "ctrl/rep1/a.tif"), + (1, 3, "ctrl/rep1/b.tif"), + (2, 3, "drug/rep1/a.tif"), + ] + assert headers[-1] == ("headers", 3, 3, "") + + +def test_excluded_images_are_not_counted(experiment): + _, calls = collect(experiment, exclude=["drug/rep1/a.tif"]) + assert {total for phase, _, total, _ in calls if phase == "headers"} == {2} + + +def test_progress_text_words_each_phase(): + assert progress_text("folders", 1, 1) == "1/4 folders" + assert progress_text("headers", 120, 480) == "2/4 headers 120/480" + assert progress_text("masks", 0, 1) == "3/4 masks" + assert progress_text("acquisitions", 1, 1) == "4/4 acquisitions" + with pytest.raises(ValueError, match="unknown phase"): + progress_text("pixels", 0, 1) + + +@pytest.mark.parametrize("bind", [False, True]) +def test_progress_changes_nothing_about_the_result(experiment, bind): + plain = preflight(experiment, bind_headers=bind) + with_progress, _ = collect(experiment, bind_headers=bind) + assert with_progress.metadata == plain.metadata + assert with_progress.discovery == plain.discovery + assert with_progress.issues == plain.issues + assert with_progress.header_fingerprints == plain.header_fingerprints + assert with_progress.dataset == plain.dataset + + +def test_the_masks_phase_runs_with_a_masks_folder(experiment, tmp_path): + masks = tmp_path / "masks" + write_data(masks / "Nuclei" / "ctrl" / "rep1" / "a.tif", np.ones((8, 8), np.uint16)) + pre, calls = collect(experiment, masks_folder=masks) + assert pre.masks is not None and [c for c in calls if c[0] == "masks"] + + +# Raw header names --------------------------------------------------------------------- + + +def write_named(path, names): + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite( + path, + np.zeros((len(names), 6, 7), np.uint16), + ome=True, + photometric="minisblack", + metadata={"axes": "CYX", "Channel": {"Name": names}}, + ) + + +def test_an_image_without_names_has_no_raw_names_and_no_manifest_key(experiment, tmp_path): + out = tmp_path / "out" + run_experiment(experiment, prefill_mapping("YX"), output_folder=out) + data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + assert all("raw_channel_names" not in entry for entry in data["files"]) + assert all(f.raw_channel_names == () for f in read_manifest(out / "run_manifest.json").files) + + +def test_raw_names_keep_outer_whitespace_and_unnamed_channels(tmp_path): + root = tmp_path / "exp" + write_named(root / "ctrl" / "a.ome.tif", ["DAPI ", " GFP"]) + pre = preflight(root) + header = pre.metadata.files[0] + assert header.raw_channel_names == ("DAPI ", " GFP") + assert header.channel_names == ("DAPI", "GFP") + + +def test_raw_names_round_trip_through_the_manifest(tmp_path): + root = tmp_path / "exp" + write_named(root / "ctrl" / "a.ome.tif", ["DAPI ", " GFP"]) + out = tmp_path / "out" + run_experiment(root, prefill_mapping("CYX"), output_folder=out) + data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + assert data["files"][0]["raw_channel_names"] == ["DAPI ", " GFP"] + assert data["files"][0]["channel_names"] == ["DAPI", "GFP"] + assert read_manifest(out / "run_manifest.json").files[0].raw_channel_names == ("DAPI ", " GFP") + + +def test_a_malformed_recorded_raw_name_list_is_refused(tmp_path): + root = tmp_path / "exp" + write_named(root / "ctrl" / "a.ome.tif", ["A", "B"]) + out = tmp_path / "out" + run_experiment(root, prefill_mapping("CYX"), output_folder=out) + path = out / "run_manifest.json" + data = json.loads(path.read_text(encoding="utf-8")) + data["files"][0]["raw_channel_names"] = [1, 2] + path.write_text(json.dumps(data), encoding="utf-8") + with pytest.raises(ValueError, match="raw channel names"): + read_manifest(path) + + +def test_raw_names_do_not_decide_whether_a_file_is_unchanged(): + entry = FileEntry("a.tif", 1, 2, ("c", "r", "t"), "YX", raw_channel_names=("A",)) + assert entry.same_source(replace(entry, raw_channel_names=("B",))) + + +def test_ome_zarr_raw_labels_are_untrimmed(tmp_path): + path = write_ngff( + tmp_path / "exp/ctrl/a.ome.zarr", + np.zeros((2, 6, 8), np.uint16), + "cyx", + channel_names=[" DAPI", "GFP"], + ) + from morphobricks_preprocessing.metadata import inspect_image + + header = inspect_image(path, "ctrl/a.ome.zarr") + assert header.raw_channel_names == (" DAPI", "GFP") + assert header.channel_names == ("DAPI", "GFP") diff --git a/tests/test_spec_discovery.py b/tests/test_spec_discovery.py new file mode 100644 index 00000000..04318f57 --- /dev/null +++ b/tests/test_spec_discovery.py @@ -0,0 +1,158 @@ +"""Conformance P21, P26 (recorded header names) and P29: discovery before any description. + +Spec experiment-v1 §3.3 and §6, conformance plan section 4. The refusals apply only when a +description is in use (owner decision Q1): a run without one accepts what v0.4 accepted. +P26's comparison of header names with a description's confirmed names (``R_HEADER_CONTRADICTION``) +belongs to the physical checks (Stage 1, PR 8); here the names are recorded as read, untrimmed. +""" + +import numpy as np +import tifffile + +from morphobricks_preprocessing import ( + prefill_mapping, + preflight, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing.discovery import discover_experiment +from morphobricks_preprocessing.metadata import inspect_tiff +from tiff_tree import write_data, write_tiff + +DECOMPOSED = "Café" +COMPOSED = "Café" + + +def codes(issues): + return [issue.code for issue in issues] + + +def issue(issues, code): + return next(i for i in issues if i.code == code) + + +def masks_tree(tmp_path, *mask_types): + root = tmp_path / "exp" + write_tiff(root / "ctrl" / "rep1" / "a.tif") + masks = tmp_path / "masks" + for name in mask_types: + write_data(masks / name / "ctrl" / "rep1" / "a.tif", np.ones((8, 8), np.uint16)) + return root, masks + + +# P21 ---------------------------------------------------------------------------------- + + +def test_p21_a_decomposed_folder_is_refused_with_a_description(tmp_path): + root = tmp_path / "exp" + write_tiff(root / DECOMPOSED / "rep1" / "a.tif") + write_tiff(root / "ctrl" / DECOMPOSED / "a.tif") + pre = preflight(root, with_description=True) + result = pre.discovery + found = issue(pre.issues, "R_FOLDER_NOT_NFC") + assert found.is_error + assert set(found.paths) == {DECOMPOSED, f"ctrl/{DECOMPOSED}"} + # Nothing is renamed or normalized: the labels and the folders are as read. + assert {leaf.condition for leaf in result.leaves} == {DECOMPOSED, "ctrl"} + assert (root / DECOMPOSED).is_dir() + + +def test_p21_a_composed_folder_is_accepted(tmp_path): + root = tmp_path / "exp" + write_tiff(root / COMPOSED / "rep1" / "a.tif") + assert "R_FOLDER_NOT_NFC" not in codes(preflight(root, with_description=True).issues) + + +def test_p21_without_a_description_discovery_is_as_in_v0_4(tmp_path): + root = tmp_path / "exp" + write_tiff(root / DECOMPOSED / "rep1" / "a.tif") + result = discover_experiment(root) + assert "R_FOLDER_NOT_NFC" not in codes(result.issues) + assert not any(i.is_error for i in result.issues) + assert result.leaves[0].condition == DECOMPOSED + + +def test_p21_preflight_carries_the_refusal(tmp_path): + root = tmp_path / "exp" + write_tiff(root / DECOMPOSED / "rep1" / "a.tif") + assert "R_FOLDER_NOT_NFC" in codes(preflight(root, with_description=True).issues) + assert "R_FOLDER_NOT_NFC" not in codes(preflight(root).issues) + + +# P26 (the part that is recorded here) ------------------------------------------------- + + +def write_named(path, names): + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite( + path, + np.zeros((len(names), 6, 7), np.uint16), + ome=True, + photometric="minisblack", + metadata={"axes": "CYX", "Channel": {"Name": names}}, + ) + return path + + +def test_p26_the_header_names_are_recorded_as_read_untrimmed(tmp_path): + path = write_named(tmp_path / "a.ome.tif", [" DAPI", f"{DECOMPOSED} 488"]) + info = inspect_tiff(path) + assert info.raw_channel_names == (" DAPI", f"{DECOMPOSED} 488") + # The trimmed names the existing checks use are unchanged. + assert info.channel_names == ("DAPI", f"{DECOMPOSED} 488") + assert info.channel_names_trimmed + + +def test_p26_the_manifest_records_the_header_name_as_read(tmp_path): + root = tmp_path / "exp" + write_named(root / "ctrl" / "a.ome.tif", [" DAPI", f"{DECOMPOSED} 488"]) + out = tmp_path / "out" + run_experiment(root, prefill_mapping("CYX"), output_folder=out) + entry = read_manifest(out / "run_manifest.json").files[0] + assert entry.raw_channel_names == (" DAPI", f"{DECOMPOSED} 488") + assert entry.channel_names == ("DAPI", f"{DECOMPOSED} 488") + assert entry.channel_names_trimmed + + +# P29 ---------------------------------------------------------------------------------- + + +def test_p29_na_and_a_leading_space_are_invalid_mask_type_names(tmp_path): + root, masks = masks_tree(tmp_path, "NA", " cells", "Nuclei") + pre = preflight(root, masks_folder=masks, with_description=True) + found = issue(pre.masks.issues, "R_MASK_TYPE_NAME_INVALID") + assert found.is_error + assert set(found.paths) == {"NA", " cells"} + assert "R_FOLDER_NOT_NFC" not in codes(pre.masks.issues) + assert not pre.ok + # Never renamed: the types are as discovered. + assert set(pre.masks.types) == {"NA", " cells", "Nuclei"} + + +def test_p29_a_decomposed_mask_type_is_not_nfc(tmp_path): + root, masks = masks_tree(tmp_path, DECOMPOSED, "Nuclei") + pre = preflight(root, masks_folder=masks, with_description=True) + found = issue(pre.masks.issues, "R_FOLDER_NOT_NFC") + assert found.is_error and found.paths == (DECOMPOSED,) + assert "R_MASK_TYPE_NAME_INVALID" not in codes(pre.masks.issues) + assert DECOMPOSED in pre.masks.types + + +def test_p29_the_three_cases_together(tmp_path): + root, masks = masks_tree(tmp_path, "NA", " cells", DECOMPOSED) + found = preflight(root, masks_folder=masks, with_description=True).masks.issues + assert issue(found, "R_MASK_TYPE_NAME_INVALID").paths == (" cells", "NA") + assert issue(found, "R_FOLDER_NOT_NFC").paths == (DECOMPOSED,) + + +def test_p29_valid_mask_types_are_accepted(tmp_path): + root, masks = masks_tree(tmp_path, COMPOSED, "Nuclei") + pre = preflight(root, masks_folder=masks, with_description=True) + assert not {"R_FOLDER_NOT_NFC", "R_MASK_TYPE_NAME_INVALID"} & set(codes(pre.masks.issues)) + + +def test_p29_without_a_description_mask_types_are_accepted_as_in_v0_4(tmp_path): + root, masks = masks_tree(tmp_path, "NA", " cells", DECOMPOSED) + pre = preflight(root, masks_folder=masks) + assert not {"R_FOLDER_NOT_NFC", "R_MASK_TYPE_NAME_INVALID"} & set(codes(pre.masks.issues)) + assert not any(i.is_error for i in pre.masks.issues) diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 81f916c0..e9b576fd 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -25,7 +25,7 @@ "src/morphobricks_preprocessing/metadata.py _check_channel_names: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 29, "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 96, - "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 721, + "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 700, "src/morphobricks_preprocessing/planes.py _layout: private function of # lines without a docstring": 45, "src/morphobricks_preprocessing/planes.py _tiff_block_plane: private function of # lines without a docstring": 14, "src/morphobricks_preprocessing/planes.py _zarr_layout: private function of # lines without a docstring": 40,