From 907663ff91d5a030e1181be6bbfdee1fb04221e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 13:23:48 +0000 Subject: [PATCH 01/13] PP-11: convert, named-axis OME-TIFF copies of images with axis errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exception path of design v0.4 §5: convert_experiment (conversion) writes a copy tree outside the experiment, every image with an axis error or unnamed axes as a named-axis OME-TIFF with the confirmed roles (copy_axes, ome_tiff_copy: planes read by the run's reader, in blocks over the memory limit), every other image copied byte for byte, .partial files moved into place together, the conversion record written last (conversion_record). A run on the copy tree records conversion_record_sha256 and warns when a source or a copy changed. CLI convert; the axis messages name it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- pyproject.toml | 5 + src/morphobricks_preprocessing/__init__.py | 17 + src/morphobricks_preprocessing/axis_check.py | 14 +- src/morphobricks_preprocessing/cli.py | 17 +- src/morphobricks_preprocessing/cli_convert.py | 141 +++++ src/morphobricks_preprocessing/cli_run.py | 10 +- src/morphobricks_preprocessing/conversion.py | 566 ++++++++++++++++++ .../conversion_record.py | 381 ++++++++++++ src/morphobricks_preprocessing/copy_axes.py | 224 +++++++ .../discovery_report.py | 21 +- .../notebook_axes.py | 3 +- .../ome_tiff_copy.py | 130 ++++ src/morphobricks_preprocessing/report.py | 2 +- .../run/pipeline.py | 1 + .../run/preflight.py | 7 + tests/test_axis_check.py | 6 +- 16 files changed, 1522 insertions(+), 23 deletions(-) create mode 100644 src/morphobricks_preprocessing/cli_convert.py create mode 100644 src/morphobricks_preprocessing/conversion.py create mode 100644 src/morphobricks_preprocessing/conversion_record.py create mode 100644 src/morphobricks_preprocessing/copy_axes.py create mode 100644 src/morphobricks_preprocessing/ome_tiff_copy.py diff --git a/pyproject.toml b/pyproject.toml index fd355943..88bf6c76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -156,6 +156,11 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.cli_calibrate" = "ui" "morphobricks_preprocessing.cli_masks" = "ui" "morphobricks_preprocessing.cli_design" = "ui" +"morphobricks_preprocessing.cli_convert" = "ui" +"morphobricks_preprocessing.conversion" = "orchestration" +"morphobricks_preprocessing.conversion_record" = "io" +"morphobricks_preprocessing.copy_axes" = "pure" +"morphobricks_preprocessing.ome_tiff_copy" = "io" "morphobricks_preprocessing.discovery_report" = "pure" "morphobricks_preprocessing.dataset" = "pure" "morphobricks_preprocessing.design" = "io" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index ad87cd36..66f0bed7 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -38,6 +38,17 @@ review_calibration, ) from .calibration_io import CalibrationFormatError, read_calibration +from .conversion import ConversionResult, convert_experiment +from .conversion_record import ( + RECORD_FILENAME as CONVERSION_RECORD_FILENAME, +) +from .conversion_record import ( + ConversionRecord, + ConversionRecordError, +) +from .conversion_record import ( + read_record as read_conversion_record, +) from .dataset import ( CONSISTENCY_CODES, BalanceCell, @@ -219,6 +230,7 @@ "AXIS_ERROR_CODES", "CALIBRATION_FILENAME", "CONSISTENCY_CODES", + "CONVERSION_RECORD_FILENAME", "DEFAULT_MEMORY_LIMIT", "DEFAULT_THRESHOLDS", "DESIGN_COLUMNS", @@ -251,6 +263,9 @@ "CalibrationFormatError", "CalibrationReview", "ConfirmedDesign", + "ConversionRecord", + "ConversionRecordError", + "ConversionResult", "DatasetReport", "DesignChanges", "DesignCheckError", @@ -320,6 +335,7 @@ "check_planes", "check_sources", "check_table", + "convert_experiment", "declared_mapping", "default_design", "default_output_folder", @@ -365,6 +381,7 @@ "prefill_mapping", "preflight", "read_calibration", + "read_conversion_record", "read_confirmed_design", "read_experiment_design", "read_manifest", diff --git a/src/morphobricks_preprocessing/axis_check.py b/src/morphobricks_preprocessing/axis_check.py index c3eb9c38..ec00a4ad 100644 --- a/src/morphobricks_preprocessing/axis_check.py +++ b/src/morphobricks_preprocessing/axis_check.py @@ -207,8 +207,8 @@ def axis_check_issues( "with the roles you confirmed, and recorded as confirmed by you.", paths, "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.", + "other tools, consider saving named-axis copies (OME-TIFF) with the convert " + "command, so every tool reads the same axes.", ) ) continue @@ -280,15 +280,17 @@ def _x_before_y_text(n: str, details: str) -> str: _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, " + "Save these images again with Y before X (rows, then columns), or convert them: the " + "convert command (morphobricks-preprocessing convert) saves named-axis copies (OME-TIFF) " + "with Y before X in a new folder; 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, " - "then run preprocessing on the copies." + "your roles are right, the convert command (morphobricks-preprocessing convert) saves " + "these images as named-axis copies (OME-TIFF) with them in a new folder; then run " + "preprocessing on the copies." ) diff --git a/src/morphobricks_preprocessing/cli.py b/src/morphobricks_preprocessing/cli.py index 5f5e5f9c..3e00fc38 100644 --- a/src/morphobricks_preprocessing/cli.py +++ b/src/morphobricks_preprocessing/cli.py @@ -1,4 +1,4 @@ -"""Command-line interface: ``check``, ``run``, ``masks``, ``calibrate`` and ``design``. +"""Command-line interface: ``check``, ``run``, ``masks``, ``calibrate``, ``design``, ``convert``. ``check`` runs the preflight (folders and image headers, no pixels) and prints what will be analysed, the problems found and the suggested axis roles; it writes the @@ -18,8 +18,9 @@ label images of OME-Zarr images), reads them, and writes ``mask_relationships.json`` with each mask type and the relationships between them (contract §15). ``calibrate`` (:mod:`.cli_calibrate`) shows or states the pixel sizes in the output folder's -``calibration.csv`` (design v0.4 §3). All four exit with status 1 when an error blocks -processing. +``calibration.csv`` (design v0.4 §3). ``convert`` (:mod:`.cli_convert`) saves the images whose +axes are wrong or not named as named-axis OME-TIFF copies in a new folder (design v0.4 §5). +All exit with status 1 when an error blocks processing. """ from __future__ import annotations @@ -31,6 +32,7 @@ from pathlib import Path from .cli_calibrate import add_calibrate_parser, calibrate_command, calibration_summary +from .cli_convert import add_convert_parser, convert_command from .cli_design import add_design_parser, design_command, design_instructions from .cli_masks import add_masks_arguments, masks_command from .cli_run import add_run_arguments, command_preflight, parse_size, run_command @@ -89,6 +91,7 @@ def build_parser() -> argparse.ArgumentParser: add_masks_arguments(masks) for command in (add_calibrate_parser(commands), add_design_parser(commands)): _remove_stale_lock_option(command) + add_convert_parser(commands) return parser @@ -105,6 +108,8 @@ def main(argv: Sequence[str] | None = None) -> int: parser.error("--exclude and --no-exclude cannot be given together") handler = _attach_log(args.log) if getattr(args, "log", None) else None try: + if args.command == "convert": + return convert_command(args) if _remove_stale_lock(args) else 1 try: args.mask_axes = mask_axes_option(args.mask_axes) except ValueError as exc: @@ -180,7 +185,11 @@ def _mask_axes_option(parser: argparse.ArgumentParser) -> None: def _remove_stale_lock(args: argparse.Namespace) -> bool: """Remove the output folder's lock the user said is stale; False when it is not.""" - if args.output: + if not args.remove_stale_lock: + return True + if getattr(args, "to", None): # 'convert' locks the folder of the copies + output = Path(args.to).resolve() + elif args.output: output = Path(args.output).resolve() elif getattr(args, "experiment", None): # 'design' takes only --output output = default_output_folder(Path(args.experiment).resolve()) diff --git a/src/morphobricks_preprocessing/cli_convert.py b/src/morphobricks_preprocessing/cli_convert.py new file mode 100644 index 00000000..f722909e --- /dev/null +++ b/src/morphobricks_preprocessing/cli_convert.py @@ -0,0 +1,141 @@ +"""The command line's ``convert``: named-axis copies of an experiment (design v0.4 §5). + +``convert EXPERIMENT --to FOLDER --axes ROLES`` writes the copy tree with +:func:`.conversion.convert_experiment`: every image with an axis error or unnamed axes as +a named-axis OME-TIFF with the roles given in ``--axes`` (never taken from the headers), +every other image copied as it is, and the conversion record last. It prints what it +wrote and the next commands: ``check`` and ``run`` on the copy tree, with the output +folder of the runs on the originals. The command is offered for any experiment. +""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +from .cli_run import mapping_option, parse_size, print_progress +from .conversion import ConversionResult, convert_experiment +from .conversion_record import RECORD_FILENAME +from .discovery import default_output_folder +from .planes import DEFAULT_MEMORY_LIMIT +from .report import format_issues +from .run import preflight +from .validation import plural + + +def add_convert_parser(commands: argparse._SubParsersAction) -> argparse.ArgumentParser: + """Add the ``convert`` command and its options; return its parser. + + Parameters + ---------- + commands + The command line's subcommands. + + Returns + ------- + argparse.ArgumentParser + The ``convert`` command's parser. + """ + convert = commands.add_parser( + "convert", + help="save the images whose axes are wrong or not named as named-axis OME-TIFF copies " + "with your roles, in a new folder (the originals are only read)", + ) + convert.add_argument("experiment", help="the experiment folder") + convert.add_argument( + "--to", + metavar="FOLDER", + required=True, + help="the folder for the copies: new or empty, outside the experiment (or the folder " + "of an earlier conversion of this experiment, whose unchanged copies are kept)", + ) + convert.add_argument( + "--axes", + metavar="ROLES", + required=True, + help="the role of each axis of the images, in the order of their axes (X, Y, Z, C, T, " + "or - for none): the roles the copies are written with", + ) + convert.add_argument( + "--exclude", + action="append", + default=[], + metavar="PATH", + help="image to leave unread, relative to the experiment folder; it is copied as it " + "is (repeatable)", + ) + convert.add_argument( + "--missing-as-singleton", + action="store_true", + help="confirm that images lacking some axes have one plane along them", + ) + convert.add_argument( + "--memory-limit", + type=parse_size, + default=DEFAULT_MEMORY_LIMIT, + metavar="SIZE", + help="working-memory limit per plane, e.g. 2GiB (default: 1GiB)", + ) + convert.add_argument( + "--remove-stale-lock", + action="store_true", + help="remove the lock of an earlier conversion into --to that was stopped; give it " + "only when you are sure no other conversion writes there", + ) + convert.add_argument("--log", metavar="FILE", help="write technical details to this file") + return convert + + +def convert_command(args: argparse.Namespace) -> int: + """Write the copy tree with the command's options, print what was done; return the status.""" + pre = preflight(args.experiment, args.to, exclude=args.exclude) + mapping = mapping_option(args, pre, None) + if mapping is None or mapping is False: + return 1 + result = convert_experiment( + args.experiment, + mapping, + args.to, + exclude=args.exclude, + missing_as_singleton=args.missing_as_singleton, + memory_limit=args.memory_limit, + progress=print_progress, + ) + if result.issues: + print(format_issues(result.issues)) + print() + print(conversion_text(result, default_output_folder(Path(args.experiment).resolve()))) + return 0 if result.written else 1 + + +def conversion_text(result: ConversionResult, output: Path) -> str: + """Say what a conversion wrote and what to run next, in plain ASCII-safe words. + + Parameters + ---------- + result + What :func:`.conversion.convert_experiment` returned. + output + The output folder of the runs on the originals, named in the next commands. + + Returns + ------- + str + """ + dest = result.destination + if not result.written: + return f"No copies were written in {dest}: read the messages above." + lines = [ + f"Named-axis copies in {dest}:", + f" written as OME-TIFF with your roles: {plural(len(result.converted), 'image')}", + f" copied as they are: {plural(len(result.copied), 'image')}", + f" already there and unchanged: {plural(len(result.reused), 'image')}", + f"Conversion record: {dest / RECORD_FILENAME} (SHA-256 {result.record_sha256})", + "", + "Next, run preprocessing on the copies with the output folder of your runs on the " + f"originals (by default {output}):", + f" morphobricks-preprocessing check {dest} --output {output}", + ] + axes = f" --axes {result.suggested_axes}" if result.suggested_axes else " --axes ROLES" + lines.append(f" morphobricks-preprocessing run {dest} --output {output}{axes}") + return "\n".join(lines) diff --git a/src/morphobricks_preprocessing/cli_run.py b/src/morphobricks_preprocessing/cli_run.py index b9eb4c49..7e6eee9d 100644 --- a/src/morphobricks_preprocessing/cli_run.py +++ b/src/morphobricks_preprocessing/cli_run.py @@ -237,7 +237,7 @@ def run_command(args: argparse.Namespace) -> int: if recorded is not None and not args.exclude and not args.no_exclude: args.exclude = list(recorded.settings.exclude) # the preflight leaves them out too pre = command_preflight(args) - mapping = _mapping_option(args, pre, recorded) + mapping = mapping_option(args, pre, recorded) settings = _settings_options(args, recorded) if mapping is False or settings is None: return 1 @@ -259,7 +259,7 @@ def run_command(args: argparse.Namespace) -> int: missing_as_singleton=args.missing_as_singleton, exclude=[] if args.no_exclude else args.exclude or None, mode=args.mode, - progress=_progress, + progress=print_progress, design=design, masks_folder=False if args.no_masks else args.masks, mask_axes=args.mask_axes or None, @@ -283,13 +283,13 @@ def run_command(args: argparse.Namespace) -> int: return 0 if result.written else 1 -def _progress(done: int, total: int, path: str) -> None: - """Print which file is read.""" +def print_progress(done: int, total: int, path: str) -> None: + """Print which file is read or written, as ``[2/10] path``.""" if path: print(f"[{done + 1}/{total}] {path}", flush=True) -def _mapping_option( +def mapping_option( args: argparse.Namespace, pre: Preflight, recorded: RunManifest | None ) -> AxisMapping | None | Literal[False]: """Return the mapping of ``--axes``, None for the recorded one, or False when refused.""" diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py new file mode 100644 index 00000000..a200c3b3 --- /dev/null +++ b/src/morphobricks_preprocessing/conversion.py @@ -0,0 +1,566 @@ +"""Convert: save an experiment as named-axis copies, the exception path (design v0.4 §5). + +For images whose axes are wrong or unnamed: :func:`convert_experiment` writes a copy tree, +a folder with the experiment's folders and file names, in which + +- every image with an axis error (``axes_yx_swapped``, ``axes_contradict_header``, + ``axes_x_before_y``) or a plain stack (``axes_not_named``) is written as a named-axis + OME-TIFF with the roles the user confirmed (:mod:`.copy_axes`, :mod:`.ome_tiff_copy`): + each plane is the plane read under those roles, bit for bit; pixel sizes follow the roles + and a relabelled axis gets an unknown scale; +- every other image is copied byte for byte: its header names its axes as confirmed, it is + an OME-Zarr image, or it is excluded (never read); +- masks are never converted (they stay where they are, checked as stored by the run). + +Nothing is guessed: the roles are the user's, and an error other than the axis errors +stops the conversion before anything is written. The originals are only read. The copy +tree must be outside the experiment (and not hold it); the user names it. The copies keep +each image's relative path, so ``source_relative_path``, ``plane_id`` and the FOV key stay +the same, and the conversion record (:mod:`.conversion_record`) is written last. + +Safe to repeat: the copy tree's lock is held throughout; every copy is first written as +``.partial`` and moved into place only once every copy is written, so a failed +conversion leaves the folder as it was; a copy already there is reused when its source, +roles and copy are unchanged, and anything else already there is refused, never +overwritten. Then the user runs preprocessing on the copy tree, with the same output +folder: the recorded experiment root becomes the copy tree (the report says so) and every +row is recomputed. +""" + +from __future__ import annotations + +import logging +import os +import shutil +from collections.abc import Callable, Iterable +from dataclasses import dataclass, field, replace +from datetime import UTC, datetime +from pathlib import Path + +from ._io import free_space +from .axes import AxisMapping, prefill_mapping +from .axis_check import AXIS_ERROR_CODES, ConfirmedAxes, header_axis_issue +from .axis_review import review_axes +from .conversion_record import ( + CONVERTED, + COPIED, + RECORD_FILENAME, + ConversionRecord, + ConversionRecordError, + CopyEntry, + read_record, + write_record, +) +from .copy_axes import CopyLayout, copy_layout +from .fingerprint import Fingerprint, current_fingerprint +from .manifest import package_versions +from .metadata import TiffMetadata +from .ome_tiff_copy import CopyWriteError, copy_nbytes, write_named_copy +from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError +from .run.lock import LOCK_FILENAME, OutputLockedError, output_lock +from .run.preflight import preflight +from .validation import ValidationIssue, error, has_errors, plural + +logger = logging.getLogger(__name__) + +PARTIAL_SUFFIX = ".partial" +"""A copy is written as ``.partial`` and renamed once every copy is written.""" + + +@dataclass(frozen=True) +class PlannedCopy: + """One image of the copy tree, before it is written. + + Attributes + ---------- + entry + Its conversion-record entry, without the copy's fingerprint. + layout + The named-axis copy of a converted image, else None. + mapping + The roles a converted image is read with, else None. + nbytes + The bytes the copy takes (pixels only for a converted image). + """ + + entry: CopyEntry + layout: CopyLayout | None = None + mapping: AxisMapping | None = None + nbytes: int = 0 + + @property + def relative_path(self) -> str: + """The image, relative to the experiment and to the copy tree.""" + return self.entry.relative_path + + +@dataclass(frozen=True) +class ConversionResult: + """What :func:`convert_experiment` did. + + Attributes + ---------- + destination + The copy tree. + written + True when the copy tree holds every image and its conversion record; False when + the conversion was refused (nothing was written then). + issues + The errors that stopped it, and the warnings found. + converted + Images written as named-axis OME-TIFF copies in this conversion, sorted. + copied + Images copied byte for byte in this conversion, sorted. + reused + Images whose copy was already there, unchanged, sorted. + record_sha256 + The SHA-256 of the conversion record written. + suggested_axes + The roles to give ``run`` on the copy tree (``--axes``), when every copy has the + same axes; else None. + """ + + destination: Path + written: bool + issues: tuple[ValidationIssue, ...] = () + converted: tuple[str, ...] = () + copied: tuple[str, ...] = () + reused: tuple[str, ...] = () + record_sha256: str | None = None + suggested_axes: str | None = field(default=None) + + +def convert_experiment( + experiment_root: str | os.PathLike, + mapping: AxisMapping | ConfirmedAxes, + destination: str | os.PathLike, + *, + exclude: Iterable[str] = (), + missing_as_singleton: bool = False, + memory_limit: int = DEFAULT_MEMORY_LIMIT, + progress: Callable[[int, int, str], None] | None = None, +) -> ConversionResult: + """Write named-axis copies of an experiment's images with the roles the user confirmed. + + Parameters + ---------- + experiment_root + The experiment folder. It is only read. + mapping + The experiment axis mapping the user confirmed (as for :func:`.run.run_experiment`): + the roles the copies are written with. Never taken from the headers. + destination + The copy tree: a new or empty folder outside the experiment, or the copy tree of + an earlier conversion of this experiment (its unchanged copies are reused). + exclude + Images to leave unread; they are copied byte for byte, so the copy tree mirrors + the experiment, and stay excluded by the run. + missing_as_singleton + The user confirmed that images lacking some axes have one plane along them. + memory_limit + Working-memory limit per plane while reading and writing, as the run's. + progress + Called as ``progress(done, total, relative_path)`` before each image is written. + + Returns + ------- + ConversionResult + """ + root, dest = Path(experiment_root).resolve(), Path(destination).resolve() + refused = _folder_issues(root, dest) + if refused: + return ConversionResult(dest, False, tuple(refused)) + planned, issues = plan_conversion(root, mapping, dest, exclude, missing_as_singleton) + if has_errors(issues): + return ConversionResult(dest, False, _sorted(issues)) + dest.mkdir(parents=True, exist_ok=True) + try: + with output_lock(dest): + return _locked_conversion(root, dest, planned, issues, (memory_limit, progress)) + except OutputLockedError as exc: + return ConversionResult(dest, False, (_lock_issue(exc),)) + + +def plan_conversion( + root: Path, + mapping: AxisMapping | ConfirmedAxes, + destination: Path, + exclude: Iterable[str] = (), + missing_as_singleton: bool = False, +) -> tuple[tuple[PlannedCopy, ...], list[ValidationIssue]]: + """Check the experiment with the confirmed roles and plan each image's copy. + + Only the headers are read. The axis errors do not stop the conversion: they are what + it corrects. Any other error does, and nothing is planned. + + Parameters + ---------- + root + The experiment folder (absolute). + mapping + The confirmed mapping. + destination + The copy tree (absolute). + exclude + Images to leave unread. + missing_as_singleton + As :func:`convert_experiment`. + + Returns + ------- + tuple + The planned copies, sorted by path (empty when an error stops the conversion), + and the issues: the errors that stop it, and the warnings. + """ + pre = preflight(root, destination, exclude=exclude) + review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) + others = [issue for issue in pre.issues if issue not in pre.metadata.issues] + issues = [*others, *review.combined_issues(pre.metadata)] + if any(i.is_error and i.code not in AXIS_ERROR_CODES for i in issues): + return (), [i for i in issues if not (i.is_error and i.code in AXIS_ERROR_CODES)] + issues = [i for i in issues if not i.is_error] # the axis errors are what is corrected + to_convert = set(review.axis_error_files) | set(review.confirmed_files) + headers = {meta.relative_path: meta for meta in pre.metadata.files} + planned = [] + for rel in pre.discovery.images: + source = Fingerprint.of(root / rel) + if rel in to_convert: + planned.append(_converted(headers[rel], review.file_mappings[rel], source)) + else: + reason = "excluded" if rel in pre.excluded else None + meta = headers.get(rel) + entry = CopyEntry(rel, COPIED, reason, source, source_axes=meta.axes if meta else "") + planned.append(PlannedCopy(entry, nbytes=source.size)) + return tuple(planned), issues + + +def _converted(meta: TiffMetadata, mapping: AxisMapping, source: Fingerprint) -> PlannedCopy: + """Plan the named-axis copy of one image read under its confirmed roles.""" + layout = copy_layout(meta, mapping) + entry = CopyEntry( + relative_path=meta.relative_path, + action=CONVERTED, + reason=header_axis_issue(meta, mapping), + source=source, + source_axes=meta.axes, + roles=mapping.roles, + copy_axes=layout.axes, + pixel_sizes=layout.sizes, + unknown_sizes=layout.unknown, + ) + return PlannedCopy(entry, layout, mapping, copy_nbytes(layout, meta.dtype)) + + +def _locked_conversion( + root: Path, + dest: Path, + planned: tuple[PlannedCopy, ...], + issues: list[ValidationIssue], + options: tuple[int, Callable[[int, int, str], None] | None], +) -> ConversionResult: + """Compare with what the copy tree holds, write the new copies and the record.""" + try: + earlier = read_record(dest) + except ConversionRecordError as exc: + return ConversionResult(dest, False, (_record_issue(exc, dest),)) + reused, to_write, refusal = _compare(root, dest, planned, earlier) + refusal = refusal or _space_issue(dest, to_write) + if refusal is not None: + return ConversionResult(dest, False, (refusal, *issues)) + try: + written = _write_all(root, dest, to_write, *options) + except _ConversionFailed as failed: + return ConversionResult(dest, False, (failed.issue, *issues)) + files = sorted([*reused, *written], key=lambda entry: entry.relative_path) + record = ConversionRecord( + source_root=root.as_posix(), + mapping=_experiment_mapping(planned, earlier), + missing_as_singleton=_singleton(planned), + files=tuple(files), + versions=package_versions(), + written_at=datetime.now(UTC).isoformat(timespec="seconds"), + ) + sha256 = write_record(record, dest) + logger.info("Converted %d images into %s", len(written), dest) + return ConversionResult( + destination=dest, + written=True, + issues=_sorted(issues), + converted=tuple(e.relative_path for e in written if e.action == CONVERTED), + copied=tuple(e.relative_path for e in written if e.action == COPIED), + reused=tuple(e.relative_path for e in reused), + record_sha256=sha256, + suggested_axes=_suggested_axes(files), + ) + + +def _compare( + root: Path, + dest: Path, + planned: tuple[PlannedCopy, ...], + earlier: tuple[ConversionRecord, str] | None, +) -> tuple[list[CopyEntry], list[PlannedCopy], ValidationIssue | None]: + """Split the plan into copies to reuse and to write; refuse anything else there.""" + if earlier is None: + unknown = _unknown_content(dest, planned) + return [], list(planned), _not_empty_issue(dest, unknown) if unknown else None + record = earlier[0] + if record.source_root != root.as_posix(): + return [], [], _other_experiment_issue(dest, record.source_root) + reused, to_write, differing = [], [], [] + for item in planned: + old = record.entry(item.relative_path) + target = dest / item.relative_path + if old is None: + (differing if os.path.lexists(target) else to_write).append(item) + elif old.same_plan(item.entry) and current_fingerprint(target) == old.copy: + reused.append(old) + else: + differing.append(item) + paths = [item.relative_path for item in differing] + names = {item.relative_path for item in planned} + paths += [e.relative_path for e in record.files if e.relative_path not in names] + return reused, to_write, _differs_issue(dest, sorted(paths)) if paths else None + + +def _unknown_content(dest: Path, planned: tuple[PlannedCopy, ...]) -> list[str]: + """Return the files in a folder without a record that this conversion did not leave. + + Only the lock and the ``.partial`` files of the planned copies (left by an interrupted + conversion, removed before they are written again) are allowed. + """ + partials = {item.relative_path + PARTIAL_SUFFIX for item in planned} + found = [] + for folder, subfolders, names in os.walk(dest): + relative = Path(folder).relative_to(dest) + subfolders[:] = [d for d in subfolders if (relative / d).as_posix() not in partials] + for name in names: + path = (relative / name).as_posix() + if path != LOCK_FILENAME and path not in partials: + found.append(path) + return sorted(found) + + +class _ConversionFailed(Exception): + """A copy could not be written; carries the error for the user.""" + + def __init__(self, issue: ValidationIssue) -> None: + super().__init__(issue.message) + self.issue = issue + + +def _write_all( + root: Path, + dest: Path, + to_write: list[PlannedCopy], + memory_limit: int, + progress: Callable[[int, int, str], None] | None, +) -> list[CopyEntry]: + """Write every copy as ``.partial``, then move them all into place. + + On a failure the ``.partial`` files written so far are removed, so the copy tree is + left as it was, and :class:`_ConversionFailed` is raised. + """ + partials: list[Path] = [] + try: + for done, item in enumerate(to_write): + if progress is not None: + progress(done, len(to_write), item.relative_path) + partials.append(_write_partial(root, dest, item, memory_limit)) + except (PlaneReadError, CopyWriteError, OSError, _SourceChanged) as exc: + logger.debug("Conversion of %s failed", item.relative_path, exc_info=True) + for partial in [*partials, _partial(dest, item)]: + _remove(partial) + raise _ConversionFailed(_failed_issue(item.relative_path, exc)) from exc + if progress is not None: + progress(len(to_write), len(to_write), "") + entries = [] + for item, partial in zip(to_write, partials, strict=True): + target = dest / item.relative_path + os.replace(partial, target) + entries.append(replace(item.entry, copy=Fingerprint.of(target))) + return entries + + +class _SourceChanged(Exception): + """A source image changed since it was checked, or while it was copied.""" + + +def _write_partial(root: Path, dest: Path, item: PlannedCopy, memory_limit: int) -> Path: + """Write one copy as ``.partial``, refusing a source that changes meanwhile.""" + source, partial = root / item.relative_path, _partial(dest, item) + partial.parent.mkdir(parents=True, exist_ok=True) + _remove(partial) # left by an interrupted conversion: it is never reused + if current_fingerprint(source) != item.entry.source: + raise _SourceChanged("it changed since it was checked") + if item.layout is not None: + write_named_copy(source, partial, item.layout, item.mapping, memory_limit=memory_limit) + elif source.is_dir(): + shutil.copytree(source, partial, copy_function=shutil.copyfile) + else: + shutil.copyfile(source, partial) + if current_fingerprint(source) != item.entry.source: + raise _SourceChanged("it changed while it was copied") + return partial + + +def _partial(dest: Path, item: PlannedCopy) -> Path: + """Return the ``.partial`` path of a copy.""" + return dest / (item.relative_path + PARTIAL_SUFFIX) + + +def _remove(path: Path) -> None: + """Remove a file or folder this conversion wrote (or left earlier) if it is there.""" + if path.is_dir() and not path.is_symlink(): + shutil.rmtree(path) + else: + path.unlink(missing_ok=True) + + +def _experiment_mapping( + planned: tuple[PlannedCopy, ...], earlier: tuple[ConversionRecord, str] | None +) -> AxisMapping: + """Return the experiment mapping of the conversion, as the record lists it.""" + for item in planned: + if item.mapping is not None: + return item.mapping + if earlier is not None: + return earlier[0].mapping + return AxisMapping("YX", ("Y", "X")) + + +def _singleton(planned: tuple[PlannedCopy, ...]) -> bool: + """Return True if some image was converted with roles confirmed as single planes.""" + return any(item.mapping is not None and item.mapping.singleton_roles for item in planned) + + +def _suggested_axes(files: list[CopyEntry]) -> str | None: + """Return the roles for ``run --axes`` on the copies when they all have the same axes.""" + axes = { + e.copy_axes if e.action == CONVERTED else e.source_axes + for e in files + if e.reason != "excluded" + } + if len(axes) != 1 or not next(iter(axes)): + return None + try: + return prefill_mapping(next(iter(axes))).roles_string + except ValueError: + return None + + +def _folder_issues(root: Path, dest: Path) -> list[ValidationIssue]: + """Refuse a copy tree inside the experiment, holding it, or that is a file.""" + if dest == root or root in dest.parents or dest in root.parents: + return [ + error( + "copy_folder_overlaps_experiment", + "The folder for the named-axis copies is the experiment folder, inside it, or " + "holds it. The copies must be kept apart from the originals, which are never " + "changed.", + [dest.as_posix()], + "Choose a new folder outside the experiment, for example " + f"{root.with_name(root.name + '_named_axes')}.", + ) + ] + if dest.exists() and not dest.is_dir(): + return [ + error( + "copy_folder_not_a_folder", + "The path given for the named-axis copies is a file, not a folder.", + [dest.as_posix()], + "Choose a new folder for the copies.", + ) + ] + return [] + + +def _space_issue(dest: Path, to_write: list[PlannedCopy]) -> ValidationIssue | None: + """Refuse a conversion that would not fit on the drive of the copy tree.""" + needed, free = sum(item.nbytes for item in to_write), free_space(dest) + if free is None or needed <= free: + return None + return error( + "not_enough_space", + f"The copies need about {needed / 2**30:.1f} GiB, but the drive holding {dest} has " + f"{free / 2**30:.1f} GiB free. Nothing was written.", + [dest.as_posix()], + "Free some space, or choose a folder on a drive with more space.", + ) + + +def _not_empty_issue(dest: Path, paths: list[str]) -> ValidationIssue: + """Refuse a folder that holds files and no conversion record.""" + return error( + "copy_folder_not_empty", + f"The folder for the named-axis copies already holds {plural(len(paths), 'file')} " + "and no conversion record, so it is not a copy tree this conversion can complete. " + "Nothing was overwritten.", + paths, + f"Choose a new or empty folder for the copies, or empty {dest} first.", + ) + + +def _differs_issue(dest: Path, paths: list[str]) -> ValidationIssue: + """Refuse to overwrite copies that differ from the ones this conversion would write.""" + return error( + "copy_differs", + f"{plural(len(paths), 'image')} in {dest} differ from the copies this conversion " + "would write: their source or roles changed, the copy was changed, or the image is no " + "longer in the experiment. Nothing was overwritten.", + paths, + "Convert into a new, empty folder, or remove these copies first (with the conversion " + f"record {RECORD_FILENAME} if you remove the whole folder's contents).", + ) + + +def _other_experiment_issue(dest: Path, source_root: str) -> ValidationIssue: + """Refuse a copy tree of another experiment.""" + return error( + "copy_differs", + f"{dest} holds named-axis copies of another experiment ({source_root}). Nothing was " + "overwritten.", + [RECORD_FILENAME], + "Choose a new, empty folder for the copies of this experiment.", + ) + + +def _record_issue(exc: ConversionRecordError, dest: Path) -> ValidationIssue: + """Refuse a copy tree whose conversion record cannot be read.""" + return error( + "conversion_record_unreadable", + f"The conversion record in {dest} cannot be read ({exc}), so its copies cannot be " + "checked. Nothing was overwritten.", + [RECORD_FILENAME], + "Choose a new, empty folder for the copies.", + ) + + +def _failed_issue(path: str, exc: Exception) -> ValidationIssue: + """Say that a copy could not be written, and that nothing was kept.""" + return error( + "conversion_failed", + f"{path} could not be copied ({exc}). The copies written so far in this conversion " + "were removed; the folder is as it was.", + [path], + "Check that the image can be read and is not being changed, that the folder can be " + "written, then convert again (or exclude the image).", + ) + + +def _lock_issue(exc: OutputLockedError) -> ValidationIssue: + """Refuse a conversion while another one writes into the same folder.""" + return error( + "output_locked", + f"Another conversion is writing into this folder: {exc.holder.describe()}. Nothing " + "was done.", + [exc.path.as_posix()], + "Wait for it to finish. If none is going on (it was stopped or the computer " + "restarted), give --remove-stale-lock to the command, or call " + "remove_stale_lock(folder), then convert again.", + ) + + +def _sorted(issues: Iterable[ValidationIssue]) -> tuple[ValidationIssue, ...]: + """Return the issues with the errors first.""" + return tuple(sorted(issues, key=lambda issue: not issue.is_error)) diff --git a/src/morphobricks_preprocessing/conversion_record.py b/src/morphobricks_preprocessing/conversion_record.py new file mode 100644 index 00000000..4da8624c --- /dev/null +++ b/src/morphobricks_preprocessing/conversion_record.py @@ -0,0 +1,381 @@ +"""The conversion record of a tree of named-axis copies (design v0.4 §5, contract §17). + +:func:`.conversion.convert_experiment` writes it last, into the root of the copy tree, as +:data:`RECORD_FILENAME` (a dot-file: discovery skips it, so it is never taken for an +image). It lists every image of the tree with its source's and its copy's fingerprints, +relative paths, roles, axes and pixel sizes, and the package versions. A run on the copy +tree records the record's SHA-256 in its manifest (``conversion_record_sha256``) and +:func:`review_conversion` warns when a source changed after the conversion, or a copy +since it was written. +""" + +from __future__ import annotations + +import json +import os +from collections.abc import Iterable, Mapping +from dataclasses import dataclass, field +from pathlib import Path + +from ._io import write_text_atomic +from .axes import AxisMapping +from .fingerprint import Fingerprint, current_fingerprint +from .manifest import file_sha256 +from .recorded_sizes import RecordedSize, sizes_from_data, sizes_to_data +from .validation import ValidationIssue, error, plural, warning + +RECORD_FILENAME = ".morphobricks-conversion.json" +"""The conversion record, in the root of the copy tree.""" + +RECORD_VERSION = "1" + +CONVERTED = "converted" +"""An image written as a named-axis OME-TIFF with the confirmed roles.""" + +COPIED = "copied" +"""An image copied byte for byte: its header already names its axes as confirmed, it is +an OME-Zarr image, or it is excluded (never read).""" + + +class ConversionRecordError(ValueError): + """Raised when a conversion record cannot be read.""" + + +@dataclass(frozen=True) +class CopyEntry: + """One image of a copy tree, as the conversion record lists it. + + Attributes + ---------- + relative_path + The image, relative to the experiment root and to the copy tree (the same path). + action + :data:`CONVERTED` or :data:`COPIED`. + reason + Why: the axis-check code of a converted image (``"axes_yx_swapped"``, + ``"axes_contradict_header"``, ``"axes_x_before_y"``, ``"axes_not_named"``), + ``"excluded"`` for an excluded image, or None for an image copied as it was. + source + The source's fingerprint when it was read. + copy + The copy's fingerprint when it was written (None before it is written). + source_axes + The axes the source header gives (tifffile letters). + roles + The roles the source was read with (converted images), else None. + copy_axes + The copy's axes (converted images), else None. + pixel_sizes + The copy's pixel sizes by role (converted images). + unknown_sizes + ``{role: reason}`` for each size the source header gives that the copy does not. + """ + + relative_path: str + action: str + reason: str | None + source: Fingerprint + copy: Fingerprint | None = None + source_axes: str = "" + roles: tuple[str | None, ...] | None = None + copy_axes: str | None = None + pixel_sizes: tuple[RecordedSize, ...] = () + unknown_sizes: Mapping[str, str] = field(default_factory=dict) + + def same_plan(self, other: CopyEntry) -> bool: + """Return True if ``other`` copies the same source the same way (copies aside).""" + return ( + self.relative_path, + self.action, + self.reason, + self.source, + self.roles, + self.copy_axes, + ) == ( + other.relative_path, + other.action, + other.reason, + other.source, + other.roles, + other.copy_axes, + ) + + def to_dict(self) -> dict: + """Return the entry as JSON-compatible data.""" + return { + "relative_path": self.relative_path, + "action": self.action, + "reason": self.reason, + "source": _fingerprint_data(self.source), + "copy": None if self.copy is None else _fingerprint_data(self.copy), + "source_axes": self.source_axes, + "roles": None if self.roles is None else list(self.roles), + "copy_axes": self.copy_axes, + "pixel_sizes": sizes_to_data(self.pixel_sizes), + "unknown_sizes": dict(sorted(self.unknown_sizes.items())), + } + + @classmethod + def from_dict(cls, data: Mapping) -> CopyEntry: + """Read an entry written by :meth:`to_dict`.""" + roles = data.get("roles") + copy = data.get("copy") + return cls( + relative_path=str(data["relative_path"]), + action=_choice(data["action"], (CONVERTED, COPIED), "action"), + reason=None if data.get("reason") is None else str(data["reason"]), + source=Fingerprint.recorded(data["source"]), + copy=None if copy is None else Fingerprint.recorded(copy), + source_axes=str(data.get("source_axes", "")), + roles=None if roles is None else tuple(None if r is None else str(r) for r in roles), + copy_axes=None if data.get("copy_axes") is None else str(data["copy_axes"]), + pixel_sizes=sizes_from_data(data.get("pixel_sizes")), + unknown_sizes={str(k): str(v) for k, v in (data.get("unknown_sizes") or {}).items()}, + ) + + +@dataclass(frozen=True) +class ConversionRecord: + """What a conversion wrote into a copy tree. + + Attributes + ---------- + source_root + The experiment folder the copies were made from (absolute, ``/`` separators). + mapping + The experiment mapping the user confirmed for the conversion. + missing_as_singleton + Whether the user confirmed missing axes as single planes. + files + One entry per image of the copy tree, sorted by path. + versions + The package versions that wrote the copies. + written_at + When the record was written (UTC, ISO 8601). + """ + + source_root: str + mapping: AxisMapping + missing_as_singleton: bool + files: tuple[CopyEntry, ...] + versions: Mapping[str, str] = field(default_factory=dict) + written_at: str = "" + + def to_dict(self) -> dict: + """Return the record as JSON-compatible data.""" + return { + "record_version": RECORD_VERSION, + "source_root": self.source_root, + "mapping": {"axes": self.mapping.axes, "roles": list(self.mapping.roles)}, + "missing_as_singleton": self.missing_as_singleton, + "written_at": self.written_at, + "versions": dict(self.versions), + "files": [entry.to_dict() for entry in self.files], + } + + @classmethod + def from_dict(cls, data: Mapping) -> ConversionRecord: + """Read a record written by :meth:`to_dict`. + + Raises + ------ + ConversionRecordError + For another record version or data that is not a record. + """ + if not isinstance(data, Mapping) or data.get("record_version") != RECORD_VERSION: + raise ConversionRecordError(f"not a conversion record of version {RECORD_VERSION}") + try: + mapping = data["mapping"] + return cls( + source_root=str(data["source_root"]), + mapping=AxisMapping(str(mapping["axes"]), tuple(mapping["roles"])), + missing_as_singleton=_flag(data["missing_as_singleton"]), + files=tuple(CopyEntry.from_dict(item) for item in data["files"]), + versions={str(k): str(v) for k, v in (data.get("versions") or {}).items()}, + written_at=str(data.get("written_at", "")), + ) + except (KeyError, TypeError, ValueError) as exc: + raise ConversionRecordError(f"not a valid conversion record: {exc}") from exc + + def entry(self, relative_path: str) -> CopyEntry | None: + """Return the entry of an image, or None when the record does not list it.""" + return next((e for e in self.files if e.relative_path == relative_path), None) + + +def write_record(record: ConversionRecord, folder: str | os.PathLike) -> str: + """Write the record into the copy tree atomically; return its SHA-256. + + Parameters + ---------- + record + The record. + folder + The root of the copy tree. + + Returns + ------- + str + The SHA-256 of the file written. + """ + text = json.dumps(record.to_dict(), indent=2, ensure_ascii=False) + "\n" + return file_sha256(write_text_atomic(Path(folder) / RECORD_FILENAME, text)) + + +def read_record(folder: str | os.PathLike) -> tuple[ConversionRecord, str] | None: + """Read the conversion record of a copy tree, with its SHA-256. + + Parameters + ---------- + folder + The root of the copy tree (or any experiment folder). + + Returns + ------- + tuple or None + ``(record, sha256)``, or None when the folder holds no record. + + Raises + ------ + ConversionRecordError + If the record cannot be read. + """ + path = Path(folder) / RECORD_FILENAME + try: + text = path.read_text(encoding="utf-8") + except FileNotFoundError: + return None + except (OSError, UnicodeDecodeError) as exc: + raise ConversionRecordError(f"{path} cannot be read: {exc}") from exc + try: + data = json.loads(text) + except json.JSONDecodeError as exc: + raise ConversionRecordError(f"{path} is not valid JSON: {exc}") from exc + return ConversionRecord.from_dict(data), file_sha256(path) + + +@dataclass(frozen=True) +class ConversionReview: + """What a run finds about the conversion record of its experiment folder. + + Attributes + ---------- + sha256 + The record's SHA-256 (recorded in the manifest), or None when it cannot be read. + source_root + The experiment folder the copies were made from, or None. + issues + ``conversion_source_changed`` and ``copy_changed_after_conversion`` (warnings), or + ``conversion_record_unreadable`` (an error). + record + The record's file name, in the experiment folder. + """ + + sha256: str | None + source_root: str | None + issues: tuple[ValidationIssue, ...] = () + record: str = RECORD_FILENAME + + +def review_conversion(root: str | os.PathLike, images: Iterable[str]) -> ConversionReview | None: + """Check a copy tree against its conversion record; None for an ordinary experiment. + + Parameters + ---------- + root + The experiment folder of the run. + images + The images discovery found in it, relative to it. + + Returns + ------- + ConversionReview or None + None when the folder holds no conversion record. + """ + try: + found = read_record(root) + except ConversionRecordError as exc: + return ConversionReview(None, None, (_unreadable(exc),)) + if found is None: + return None + record, sha256 = found + sources = Path(record.source_root) + changed = [ + e.relative_path + for e in record.files + if current_fingerprint(sources / e.relative_path) != e.source + ] + listed = {e.relative_path for e in record.files} + copies = sorted( + { + e.relative_path + for e in record.files + if current_fingerprint(Path(root) / e.relative_path) != e.copy + } + | (set(images) - listed) + ) + issues = [*_source_issue(changed, record.source_root), *_copy_issue(copies)] + return ConversionReview(sha256, record.source_root, tuple(issues)) + + +def _source_issue(paths: list[str], source_root: str) -> list[ValidationIssue]: + """Warn about sources changed or gone since the conversion.""" + if not paths: + return [] + return [ + warning( + "conversion_source_changed", + f"{plural(len(paths), 'source image')} changed or went missing in {source_root} " + "after these named-axis copies were made. The copies hold the images as they were " + "converted.", + paths, + "If the originals were corrected or replaced, convert them again into a new folder " + "and run preprocessing on that one.", + ) + ] + + +def _copy_issue(paths: list[str]) -> list[ValidationIssue]: + """Warn about copies changed, gone or added since the conversion.""" + if not paths: + return [] + return [ + warning( + "copy_changed_after_conversion", + f"{plural(len(paths), 'image')} of this folder of named-axis copies changed, went " + "missing or were added after the conversion, so the conversion record does not " + "describe them.", + paths, + "Convert the originals again into a new folder, or check that these images are " + "the ones you want analysed.", + ) + ] + + +def _unreadable(exc: ConversionRecordError) -> ValidationIssue: + """Refuse a copy tree whose conversion record cannot be read.""" + return error( + "conversion_record_unreadable", + f"The conversion record of this folder of named-axis copies cannot be read ({exc}), " + "so it is not known what was converted and how.", + [RECORD_FILENAME], + "Convert the originals again into a new folder, and run preprocessing on that one.", + ) + + +def _fingerprint_data(fingerprint: Fingerprint) -> dict: + """Return a fingerprint as JSON-compatible data.""" + return {"size": fingerprint.size, "mtime_ns": fingerprint.mtime_ns} + + +def _choice(value: object, allowed: tuple[str, ...], name: str) -> str: + """Return ``value`` when it is one of ``allowed``; raise otherwise.""" + if value not in allowed: + raise ValueError(f"{name} {value!r} is not one of {', '.join(allowed)}") + return str(value) + + +def _flag(value: object) -> bool: + """Return a JSON boolean; raise for anything else.""" + if not isinstance(value, bool): + raise ValueError(f"{value!r} is not true or false") + return value diff --git a/src/morphobricks_preprocessing/copy_axes.py b/src/morphobricks_preprocessing/copy_axes.py new file mode 100644 index 00000000..564d578c --- /dev/null +++ b/src/morphobricks_preprocessing/copy_axes.py @@ -0,0 +1,224 @@ +"""The axes, shape and metadata of an image's named-axis copy (design v0.4 §5). + +A named-axis copy is an OME-TIFF holding the planes of its source read under the roles the +user confirmed (:func:`.planes.iter_planes`), with those roles written as its axes, so the +copy's own header names every axis. :func:`copy_layout` works it out from the header alone: + +- **Axes.** Every axis with a role, in the source's order, then Y and X (rows, then + columns): ``ZCYX`` for a stack confirmed ``Z C Y X``, ``ZYX`` for a ``ZYX`` header whose + Y and X are confirmed the other way round. An axis without a role (length 1) is left out, + and so is an axis the image lacks (``missing_as_singleton``): nothing is added. +- **Pixel sizes follow the roles; a relabelled axis gets an unknown scale, never a guessed + one.** A size the header gives is carried only to the role the header names it for, and + only when the user confirmed that role for that same axis. An axis the user gives + another role than the header's letter is relabelled: its size is not written, and the + reason is kept for the conversion record. So a Y/X swap never exchanges the sizes of Y + and X (both become unknown), and a z-step is never written as a time step. A plain + stack's positional Y and X keep the header's Y and X sizes when confirmed Y before X. +- **Channel names** are carried when the channel axis keeps its role and every channel is + named; the significant bits are carried as they are. + +Nothing here reads pixels or files. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from dataclasses import dataclass, field +from types import MappingProxyType +from typing import TYPE_CHECKING + +from .recorded_sizes import SIZE_ROLES, RecordedSize, header_sizes + +if TYPE_CHECKING: + from .axes import AxisMapping + from .metadata import TiffMetadata + +OME_UNITS = MappingProxyType( + { + "angstrom": "Å", + "picometer": "pm", + "nanometer": "nm", + "micrometer": "µm", + "millimeter": "mm", + "centimeter": "cm", + "meter": "m", + "nanosecond": "ns", + "microsecond": "µs", + "millisecond": "ms", + "second": "s", + "minute": "min", + "hour": "h", + } +) +"""OME-XML unit symbols of the OME-NGFF unit names a size is recorded with.""" + +_OME_SIZE_KEYS = { + "X": ("PhysicalSizeX", "PhysicalSizeXUnit"), + "Y": ("PhysicalSizeY", "PhysicalSizeYUnit"), + "Z": ("PhysicalSizeZ", "PhysicalSizeZUnit"), + "T": ("TimeIncrement", "TimeIncrementUnit"), +} + + +@dataclass(frozen=True) +class CopyLayout: + """How one image is written as a named-axis copy. + + Attributes + ---------- + axes + The copy's axes, one role letter each, Y and X last (``"ZCYX"``). + shape + The copy's shape, one length per axis. + sizes + The copy's pixel sizes by role (:data:`.recorded_sizes.SIZE_ROLES`), source + ``"header"``; unknown ones have no value. + unknown + ``{role: reason}`` for each size the source header gives that is not written. + channel_names + The channel names written, or ``()`` for none. + significant_bits + The significant bits written, or None. + """ + + axes: str + shape: tuple[int, ...] + sizes: tuple[RecordedSize, ...] = () + unknown: Mapping[str, str] = field(default_factory=dict) + channel_names: tuple[str, ...] = () + significant_bits: int | None = None + + def __post_init__(self) -> None: + object.__setattr__(self, "unknown", MappingProxyType(dict(self.unknown))) + + @property + def plane_shape(self) -> tuple[int, int]: + """The (Y, X) shape of one plane.""" + return self.shape[-2], self.shape[-1] + + @property + def plane_count(self) -> int: + """The number of 2D planes (TIFF pages) of the copy.""" + count = 1 + for length in self.shape[:-2]: + count *= length + return count + + def plane_index(self, coordinates: Mapping[str, int | None]) -> int: + """Return the page number of the plane at these coordinates in the copy. + + Parameters + ---------- + coordinates + ``{"z": ..., "c": ..., "t": ...}``, as :attr:`.planes.Plane.coordinates`. + + Returns + ------- + int + The zero-based page of the copy, in its axes' order. + """ + index = 0 + for role, length in zip(self.axes[:-2], self.shape[:-2], strict=True): + index = index * length + int(coordinates[role.lower()]) + return index + + def ome_metadata(self) -> dict: + """Return the metadata ``tifffile`` writes into the copy's OME-XML.""" + metadata: dict = {"axes": self.axes} + for size in self.sizes: + if size.value is not None: + value_key, unit_key = _OME_SIZE_KEYS[size.role] + metadata[value_key] = size.value + metadata[unit_key] = OME_UNITS[size.unit] + if self.channel_names: + metadata["Channel"] = {"Name": list(self.channel_names)} + if self.significant_bits is not None: + metadata["SignificantBits"] = self.significant_bits + return metadata + + +def copy_layout(meta: TiffMetadata, mapping: AxisMapping) -> CopyLayout: + """Work out the named-axis copy of an image read under its confirmed roles. + + Parameters + ---------- + meta + The source image's header. + mapping + The image's own confirmed mapping (:attr:`.axes.AxisReview.file_mappings`). + + Returns + ------- + CopyLayout + + Raises + ------ + ValueError + If the mapping does not fit the header or has no Y or X. + """ + if len(mapping.roles) != len(meta.shape) or mapping.axes != meta.axes: + raise ValueError(f"the axis mapping {mapping.axes} does not fit axes {meta.axes}") + y, x = mapping.index_of("Y"), mapping.index_of("X") + if y is None or x is None: + raise ValueError("the axis mapping needs one Y and one X axis") + leading = [d for d, role in enumerate(mapping.roles) if role is not None and role not in "YX"] + dims = [*leading, y, x] + axes = "".join(mapping.roles[d] for d in dims) + sizes, unknown = _carried_sizes(meta, mapping, axes) + return CopyLayout( + axes=axes, + shape=tuple(meta.shape[d] for d in dims), + sizes=sizes, + unknown=unknown, + channel_names=_channel_names(meta, mapping), + significant_bits=meta.significant_bits, + ) + + +def _carried_sizes( + meta: TiffMetadata, mapping: AxisMapping, axes: str +) -> tuple[tuple[RecordedSize, ...], dict[str, str]]: + """Return the sizes written for each role and why the header's others are not.""" + header = {size.role: size for size in header_sizes(meta.physical_sizes)} + sizes, unknown = [], {} + for role in SIZE_ROLES: + found = header[role] + reason = _not_carried(meta, mapping, axes, role, found) + if found.value is None or reason is not None: + sizes.append(RecordedSize(role)) + if found.value is not None: + unknown[role] = reason + continue + sizes.append(found) + return tuple(sizes), unknown + + +def _not_carried( + meta: TiffMetadata, mapping: AxisMapping, axes: str, role: str, found: RecordedSize +) -> str | None: + """Return why the header's size for ``role`` is not written, or None to write it.""" + if role not in axes: + return "the copy has no such axis" + letter = meta.axes[mapping.index_of(role)] + if letter != role: + return ( + f"relabelled: the header names this axis {letter}, so its size is not taken for " + f"{role} (unknown scale)" + ) + if found.unit not in OME_UNITS: + return f"its unit {found.unit!r} cannot be written in OME-XML" + return None + + +def _channel_names(meta: TiffMetadata, mapping: AxisMapping) -> tuple[str, ...]: + """Return the channel names when the C axis keeps its role and every channel is named.""" + c = mapping.index_of("C") + names = meta.channel_names + if c is None or len(names) != meta.shape[c]: + return () + if meta.axes[c] != "C" and not (meta.axes[c] == "S" and "C" not in meta.axes): + return () # relabelled: the names are of another axis + if any(name is None for name in names): + return () + return tuple(names) diff --git a/src/morphobricks_preprocessing/discovery_report.py b/src/morphobricks_preprocessing/discovery_report.py index 2045fb9c..efaecf9c 100644 --- a/src/morphobricks_preprocessing/discovery_report.py +++ b/src/morphobricks_preprocessing/discovery_report.py @@ -8,9 +8,10 @@ It contains the experiment root (and whether it changed), the conditions, repeats and positions with image counts; the channels and their names; each image's axes with ``axes_from`` and pixel sizes with their source (``header``, ``stated`` in -``calibration.csv``, or ``pixels``), or ``"uncalibrated"``; the calibration file; the design and -image type; the mask types with paired, unpaired and ambiguous masks and the images -without masks; every warning and error with its action; and what the mode reused, +``calibration.csv``, or ``pixels``), or ``"uncalibrated"``; the calibration file; for a +tree of named-axis copies, its conversion record and the folder the copies were made from; +the design and image type; the mask types with paired, unpaired and ambiguous masks and +the images without masks; every warning and error with its action; and what the mode reused, recomputed and replaced, and why. Nothing here checks anything: the issues are those the checks gave, with their own severities. """ @@ -134,6 +135,8 @@ def text(self) -> str: lines.append(f"Experiment: {data['experiment_root']}") if data.get("experiment_root_changed"): lines.append(f" changed from {self.outcome.previous_root}") + if data.get("conversion"): + lines.append(f" named-axis copies of {data['conversion']['source_root']}") lines.extend(_folder_lines(data)) lines.extend(_image_lines(data)) lines.extend(_mask_lines(data)) @@ -181,6 +184,7 @@ def build_report( data = { "experiment_root": root, "experiment_root_changed": outcome.previous_root not in (None, root), + "conversion": _conversion(pre), "output_folder": output.as_posix(), "masks_folder": _masks_folder(pre), "excluded": list(pre.excluded), @@ -196,6 +200,17 @@ def build_report( return DiscoveryReport(data, ordered, outcome) +def _conversion(pre: Preflight) -> dict | None: + """Return the conversion record of a tree of named-axis copies, or None (design §5).""" + if pre.conversion is None: + return None + return { + "record": pre.conversion.record, + "sha256": pre.conversion.sha256, + "source_root": pre.conversion.source_root, + } + + def _issue_dict(issue: ValidationIssue) -> dict: """Return one issue as data, with its action.""" return { diff --git a/src/morphobricks_preprocessing/notebook_axes.py b/src/morphobricks_preprocessing/notebook_axes.py index a76b4724..5c30888e 100644 --- a/src/morphobricks_preprocessing/notebook_axes.py +++ b/src/morphobricks_preprocessing/notebook_axes.py @@ -101,7 +101,8 @@ def header_marks(mapping: AxisMapping, files: Iterable) -> dict[int, str]: what = "swaps the Y and X it names" if codes == {"axes_yx_swapped"} else "contradicts it" text = ( f"The image header names this axis {html.escape(named)}; this role {what}. " - "Choose the header's role, or save named-axis copies with these axes." + "Choose the header's role, or save named-axis copies with these axes (the " + "convert command)." ) marks[index] = f"
⚠ {text}
" return marks diff --git a/src/morphobricks_preprocessing/ome_tiff_copy.py b/src/morphobricks_preprocessing/ome_tiff_copy.py new file mode 100644 index 00000000..28ea532a --- /dev/null +++ b/src/morphobricks_preprocessing/ome_tiff_copy.py @@ -0,0 +1,130 @@ +"""Write one image as a named-axis OME-TIFF copy, plane by plane (design v0.4 §5). + +The planes are read with the run's reader under the confirmed roles +(:func:`.planes.iter_planes`), so each copy plane is the plane the run would read, value for +value: same pixel type, no rescaling, no rounding. Planes over the memory limit are read in +blocks, as the run reads them, and each block is written to its place in the file: no plane +is ever held whole when the run would not hold it. The copy is uncompressed, so every plane +has a fixed place in the file, and BigTIFF when it would not fit a classic TIFF. +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import BinaryIO + +import numpy as np +import tifffile + +from .axes import AxisMapping +from .copy_axes import CopyLayout +from .planes import DEFAULT_MEMORY_LIMIT, Block, iter_planes, plane_layout + +BIGTIFF_FROM = 2**32 - 2**25 +"""Copies whose pixels take more bytes than this are written as BigTIFF (a classic TIFF +addresses at most 4 GiB; the margin leaves room for the headers and the OME-XML).""" + + +class CopyWriteError(OSError): + """Raised when a copy cannot be written as planned.""" + + +def copy_nbytes(layout: CopyLayout, dtype: np.dtype) -> int: + """Return the bytes the pixels of a copy take. + + Parameters + ---------- + layout + The copy's layout. + dtype + Its pixel type. + + Returns + ------- + int + """ + count = 1 + for length in layout.shape: + count *= length + return count * np.dtype(dtype).itemsize + + +def write_named_copy( + source: str | os.PathLike, + target: str | os.PathLike, + layout: CopyLayout, + mapping: AxisMapping, + *, + memory_limit: int = DEFAULT_MEMORY_LIMIT, +) -> None: + """Write ``source``, read under ``mapping``, to ``target`` as a named-axis OME-TIFF. + + Parameters + ---------- + source + The TIFF image. + target + The file to write (the caller's ``.partial`` file); replaced if it exists. + layout + The copy's axes, shape and metadata (:func:`.copy_axes.copy_layout`). + mapping + The image's own confirmed mapping. + memory_limit + Working-memory limit per plane, as the run's. + + Raises + ------ + PlaneReadError + If the source cannot be read under ``mapping`` within the memory limit. + CopyWriteError + If the planes do not lie where the file was laid out. + OSError + If the file cannot be written. + """ + dtype = plane_layout(source, mapping, memory_limit=memory_limit).dtype + offset, _ = tifffile.imwrite( + target, + shape=layout.shape, + dtype=dtype, + ome=True, + photometric="minisblack", + bigtiff=copy_nbytes(layout, dtype) > BIGTIFF_FROM, + metadata=layout.ome_metadata(), + returnoffset=True, + ) + rows, cols = layout.plane_shape + plane_nbytes = rows * cols * dtype.itemsize + _check_contiguous(Path(target), offset, plane_nbytes, layout.plane_count) + with open(target, "r+b") as stream: + for plane in iter_planes(source, mapping, memory_limit=memory_limit): + start = offset + layout.plane_index(plane.coordinates) * plane_nbytes + for block in plane.blocks(): + _write_block(stream, start, block, cols, dtype) + + +def _check_contiguous(path: Path, offset: int, plane_nbytes: int, count: int) -> None: + """Refuse a laid-out file whose pages do not follow each other from ``offset``.""" + with tifffile.TiffFile(path) as tif: + pages = tif.pages + if len(pages) != count: + raise CopyWriteError(f"{path} was laid out with {len(pages)} pages, not {count}") + for index in range(count): + page = pages[index] + start = offset + index * plane_nbytes + if page.dataoffsets[0] != start or sum(page.databytecounts) != plane_nbytes: + raise CopyWriteError(f"page {index} of {path} does not lie where it was planned") + + +def _write_block(stream: BinaryIO, start: int, block: Block, cols: int, dtype: np.dtype) -> None: + """Write one block of a plane at its place; a block as wide as the plane in one write.""" + pixels = np.ascontiguousarray(block.pixels, dtype=dtype) + row_nbytes = cols * dtype.itemsize + first = start + block.y * row_nbytes + block.x * dtype.itemsize + if pixels.shape[1] == cols: + stream.seek(first) + stream.write(memoryview(pixels).cast("B")) + return + for r in range(pixels.shape[0]): + stream.seek(first + r * row_nbytes) + stream.write(memoryview(pixels[r]).cast("B")) diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index bdfe2930..df198141 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -204,7 +204,7 @@ def axis_check_summary(review: AxisReview) -> str: n = plural(len(review.axis_error_files), "image") return ( f"Axes check: {n} cannot be read with these roles (see the errors below): correct " - "the axes, or save named-axis copies with your roles." + "the axes, or save named-axis copies with your roles (the convert command)." ) if review.confirmed_files: n = plural(len(review.confirmed_files), "image") diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 4b2a6719..352aba8d 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -330,6 +330,7 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> intensity_checks=intensity.settings, narrow_ranges=tuple(rows.narrow), calibration_sha256=pre.calibration.sha256, + conversion_record_sha256=None if pre.conversion is None else pre.conversion.sha256, ) manifest_sha256 = file_sha256(write_manifest(manifest, staged / MANIFEST_FILENAME)) report = _report(run, outcome, intensity) diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py index 4a0e36b8..a12d7021 100644 --- a/src/morphobricks_preprocessing/run/preflight.py +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -21,6 +21,7 @@ ) from ..calibration import CalibrationReview, fit_to_roles, review_calibration from ..calibration_io import load_calibration +from ..conversion_record import ConversionReview, review_conversion from ..dataset import DatasetReport, check_dataset from ..discovery import DiscoveryResult, default_output_folder, discover_experiment from ..mask_names import MASK_NAME_CODES, mask_name_issues @@ -66,6 +67,9 @@ class Preflight: Each analysed image's pixel sizes from its header and ``calibration.csv`` (:mod:`.calibration`). Its issues, and the pixel-size checks on these sizes, are among the metadata's issues. + conversion + The conversion record of a tree of named-axis copies (design v0.4 §5), checked + against the sources and copies; None for an ordinary experiment. """ discovery: DiscoveryResult @@ -76,6 +80,7 @@ class Preflight: dataset: DatasetReport = field(default_factory=DatasetReport) masks: MaskInventory | None = None calibration: CalibrationReview = field(default_factory=CalibrationReview) + conversion: ConversionReview | None = None @property def issues(self) -> tuple[ValidationIssue, ...]: @@ -86,6 +91,7 @@ def issues(self) -> tuple[ValidationIssue, ...]: *self.metadata.issues, *self.dataset.issues, *(self.masks.issues if self.masks is not None else ()), + *(self.conversion.issues if self.conversion is not None else ()), ] return tuple(sorted(merged, key=lambda issue: not issue.is_error)) @@ -199,6 +205,7 @@ def preflight( dataset=check_dataset(discovery, excluded=excluded), masks=_masks(discovery, metadata, masks_folder, mask_axes, output), calibration=calibration, + conversion=review_conversion(root, discovery.images), ) diff --git a/tests/test_axis_check.py b/tests/test_axis_check.py index b59820ae..d461a9d3 100644 --- a/tests/test_axis_check.py +++ b/tests/test_axis_check.py @@ -124,7 +124,7 @@ def test_plain_stacks_never_name_their_axes(tmp_path, writer, roles): assert "read with the roles you confirmed" in found.message # The run manifest (version 1) has no axes_from field: the message must not claim it. assert "manifest" not in found.message - assert "named-axis copies (OME-TIFF)" in found.action + assert "named-axis copies (OME-TIFF) with the convert command" in found.action def test_shaped_axes_with_a_placeholder_are_not_named(tmp_path): @@ -213,8 +213,8 @@ def test_every_file_is_named_with_its_header_and_confirmed_axes(tmp_path): found.message ) assert "Correct the axes" in found.action and "Or convert" in found.action - # There is no convert command yet (design v0.4 §5, PP-11): the fix must not name one. - assert "morphobricks-preprocessing convert" not in found.action + # The fix names the convert command (design v0.4 §5, PP-11). + assert "morphobricks-preprocessing convert" in found.action def test_the_likely_cause_of_tifffiles_cyx_default_is_named(tmp_path): From 053f5c71cb5e38da0d2d00efef9efb9bd2eaf9f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 13:36:11 +0000 Subject: [PATCH 02/13] PP-11: tests, docs and the recorded mapping of a conversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests (test_convert.py, test_convert_run.py): bitwise-equal planes for plain stacks, ImageJ stacks and hyperstacks, Y/X-swapped headers, channels, Z/T, X before Y, RGB, every pixel type and planes read in blocks; sizes by role; reuse, refusals, failure clean-up, partial files, the lock; the run on the copies (named axes, every row recomputed and equal, record SHA, warnings); OME-Zarr and masks; the CLI. The record keeps the confirmed experiment mapping. Contract §17, plan §10 and question 59 (open choices for the owner), user guide, README, AGENTS.md and the notebook's common problems. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 18 +- README.md | 13 +- docs/contract.md | 30 +- docs/plan.md | 68 ++++ docs/user_guide.md | 59 ++- notebooks/MorphoBricks_Preprocessing.ipynb | 5 +- src/morphobricks_preprocessing/conversion.py | 59 +-- .../notebook_axes.py | 2 +- src/morphobricks_preprocessing/run/steps.py | 1 + tests/test_convert.py | 353 ++++++++++++++++++ tests/test_convert_run.py | 308 +++++++++++++++ 11 files changed, 878 insertions(+), 38 deletions(-) create mode 100644 tests/test_convert.py create mode 100644 tests/test_convert_run.py diff --git a/AGENTS.md b/AGENTS.md index 8c003819..d8dc076a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,8 +132,9 @@ src/morphobricks_preprocessing/ mask_hierarchy_lines); moved from report.py in PP-10d discovery_report.py the run report (discovery_report.json and .txt, design v0.4 §4): status, manifest SHA-256, what was found, reused, recomputed and replaced - cli.py, __main__ `morphobricks-preprocessing check|run|masks|calibrate` (also `python -m morphobricks_preprocessing`); - cli_run.py holds `run`'s options and output; cli_calibrate.py `calibrate` + cli.py, __main__ `morphobricks-preprocessing check|run|masks|calibrate|design|convert` (also `python -m morphobricks_preprocessing`); + cli_run.py holds `run`'s options and output; cli_calibrate.py `calibrate`; + cli_convert.py `convert` masks/ masks (Q32; a package since PP-5: model, files, header, layout, read, checks, labels): find_masks pairs every image with its masks (masks folder with one folder per type mirroring the experiment, or _.tif in the @@ -149,6 +150,15 @@ src/morphobricks_preprocessing/ scan_masks -> MaskReport (kinds, relations, direct-parent hierarchy, evidence), write_mask_report (mask_relationships.json); DEFAULT_THRESHOLDS, mask_thresholds (checked changes) + copy_axes.py convert (PP-11, contract §17): the axes, shape, pixel sizes (by role; a + relabelled axis gets an unknown scale) and OME metadata of a named-axis copy + ome_tiff_copy.py write_named_copy: one image as uncompressed OME-TIFF, planes from iter_planes + under the confirmed roles, blocks written in place (BigTIFF when large) + conversion_record.py the copy tree's .morphobricks-conversion.json (fingerprints, roles, sizes), + review_conversion (the run's conversion_source_changed and + copy_changed_after_conversion warnings) + conversion.py convert_experiment/plan_conversion: the copy tree under its lock, .partial + files moved into place together, unchanged copies reused, record last zarr_input.py OME-Zarr input: open_ngff (OME-NGFF 0.4/0.5 metadata -> level-0 array, axis letters and physical sizes from the scale and axis units), ngff_label_names/open_ngff_labels (label images, for later bricks), @@ -825,6 +835,7 @@ pytest morphobricks-preprocessing check /data/exp1 morphobricks-preprocessing run /data/exp1 --axes ZCYX morphobricks-preprocessing masks /data/exp1 --masks /data/exp1_masks +morphobricks-preprocessing convert /data/exp1 --to /data/exp1_named_axes --axes ZXY jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb ``` @@ -893,6 +904,9 @@ jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb - **Notebook masks step (PP-10d):** step 3's **Describe the masks** runs the `masks` step (`mask_relationships.json`) with the run's recorded inputs, through `run.mask_step`, which the CLI calls too; see plan §10. +- **Named-axis copies (PP-11):** `convert` / `convert_experiment` writes OME-TIFF copies of + the images with axis errors or unnamed axes with the confirmed roles into a new folder + (contract §17, plan §10 and question 59); the notebook card is PP-11b. - **Next (not started):** the notebook step for the masks (name each mask, confirm or correct the relationships) and a file saving those answers for the other bricks (question 32); 3D and time-lapse masks later. diff --git a/README.md b/README.md index 9c9f68ed..b349689c 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,9 @@ Pre-alpha. Implemented and tested: recorded per axis with their source; never guessed (`calibration`, command `calibrate`) - Axis mapping: prefill from metadata, validation, applying one mapping to every file (`axes`) +- Named-axis copies: images whose metadata name their axes wrongly or not at all saved as + OME-TIFF with the confirmed roles, pixel for pixel, in a new folder with a conversion + record (`conversion`, command `convert`) - Reading TIFFs one 2D plane at a time, with a memory limit and a preview plane, and the channels of a table row one after another; planes too large for the limit are read in blocks with identical results (`planes`) @@ -52,7 +55,8 @@ Pre-alpha. Implemented and tested: refused, projections included (`mask_planes_mismatch`), and the manifest records each mask's `axes` and `shape` - A step-by-step notebook ([notebooks/MorphoBricks_Preprocessing.ipynb](notebooks/MorphoBricks_Preprocessing.ipynb)) - and a command line (`morphobricks-preprocessing check` / `run` / `masks` / `calibrate`) + and a command line (`morphobricks-preprocessing check` / `run` / `masks` / `calibrate` / + `design` / `convert`) From the normalization brick (`morphobricks_normalization`, a required dependency, pinned to its release v0.3.2): per-plane histograms with the robust range and the range check @@ -155,6 +159,7 @@ morphobricks-preprocessing run /data/exp1 --axes ZCYX morphobricks-preprocessing run --output /data/exp1_MorphoBricks_Preprocessing # later runs morphobricks-preprocessing masks --output /data/exp1_MorphoBricks_Preprocessing # mask step morphobricks-preprocessing calibrate --output /data/exp1_MorphoBricks_Preprocessing +morphobricks-preprocessing convert /data/exp1 --to /data/exp1_named_axes --axes ZXY # axis errors ``` `check` writes the report and, when the output folder holds no design, an @@ -163,7 +168,11 @@ groups (`--confirm GROUP`, `--confirm-all`), marks controls (`--control`, `--no- and pairs repeats (`--repeat-group CONDITION/REPEAT=GROUP`) without editing the file; nothing is confirmed unless asked. `run` and `masks` reuse every input the run in the output folder recorded. A run that stops writes its report and says so. Every output file -gets the permissions a file you save yourself would (your umask). Details in the +gets the permissions a file you save yourself would (your umask). `convert` saves the +images whose metadata name their axes wrongly or not at all as named-axis OME-TIFF copies +with your roles in a new folder (each plane bit for bit the plane read with those roles; +every other image copied as it is; a conversion record written last), then you run +preprocessing on that folder with the same output folder. Details in the [user guide](docs/user_guide.md) §5. ## Example diff --git a/docs/contract.md b/docs/contract.md index 235add92..ebbc007a 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -121,8 +121,8 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | -| Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | +| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles; every image excluded; more than one image series in a file; unsupported sample format or axis; missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | +| Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); in a tree of named-axis copies, sources changed or gone since the conversion (`conversion_source_changed`) and copies changed, gone or added since (`copy_changed_after_conversion`, §17); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and suggests an action. Technical details go to the diagnostic log. @@ -175,7 +175,7 @@ does not change either file's schema or statistical interpretation. | `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. | +| `conversion_record_sha256` | SHA-256 of the conversion record (`.morphobricks-conversion.json`) in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5, §17); null otherwise. Information only: the copies are checked by their fingerprints like any image, and a changed record changes no row. | | `calibration_sha256` | SHA-256 of the output folder's `calibration.csv` as the run read it (§9.3); null when there was none (or absent in manifests written before PP-8). `RunInputs` checks it. | | 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). | @@ -254,7 +254,7 @@ the bricks' tests. | `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report and the file written, or the `output_locked` refusal with no mask read and nothing written: the lock is taken before the first mask is read and held to the write; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run and for plain stacks that are new or changed. | | Lock | `.morphobricks-preprocessing.lock` in the output folder, created with `O_CREAT \| O_EXCL`, JSON `host`, `pid`, `started_at`; held by `run`, by `check` while it writes its report (and the design template), by `masks` from before its first mask read to the write of `mask_relationships.json` (a locked folder is refused before any mask is read), and by `design` and `calibrate` while they change `experiment_design.csv` and `calibration.csv` (CLI `run` also holds it to write the design template after a run stopped on the design), removed on every exit, and removed when the lock file cannot be written. A second run is refused with `output_locked`, naming the holder, and writes nothing. `remove_stale_lock(output)` / `--remove-stale-lock` (every command) removes a lock only on that request, never one of a process still running on the same computer (checked on POSIX). Not written inside the experiment. The lock is not part of the manifest. | -| Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | +| Report | `discovery_report.json` (report version 1) and `discovery_report.txt`, written on every run that holds the lock, also when it stops: `step` (`run` or `check`), `status` (`stopped` or `written`; `check` is always `stopped`), `provisional` (true for the report written after the header checks, before any pixel is read), `manifest_sha256` (the manifest published with it, `written` only), `experiment_root` and `experiment_root_changed`, `conversion` (for a tree of named-axis copies, §17: `record`, `sha256`, `source_root`; null otherwise), `output_folder`, `masks_folder`, `excluded`, `groups` (labels, position identifiers, image counts), `channels`, `channel_names`, `images` (per image `axes`, `roles`, `axes_from`, `pixel_sizes` by role with unit and source, `header`, `stated` or `pixels`, or `"uncalibrated"` without an X and a Y entry), `calibration` (`file`, `sha256` and the `statements`), `design`, `image_type`, `masks` (per type paired masks and images without one; unpaired and ambiguous masks), `mode`, `reused`, `recomputed` (file: reason), `removed`, `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is replaced atomically with the table and manifest, last. | | `check` | The same step stopped after the report; it writes the report, and the design template when the output folder holds no design (§12; owner decision of 2 November 2026, plan §12 question 57, which amends design v0.4 §1 "it writes only the report"). The notebook's **Check folder** does the same with the same functions (`run.check_step.suggested_review`, `write_check_outputs`; v0.4.0, PP-10): the report, and the template with the design card; nothing when an embedding brick hides the output folder (`folder_step(show_output=False)`). | | Notebook (v0.4.0, PP-10) | Step 3 offers the mode (`restart`, `full`) with its effect, and lists before the run the inputs that differ from the recorded ones with their effect (`run.planned.planned_changes(given, output_folder, design=, calibration=)`, from `resolve_inputs`, `design_for_run` and `calibration_change`: the changes the run then reports). After a run it names the mode, the files replaced and the input changes, and shows the report; a stopped run says what it wrote (question 55). A refusal `output_locked` (check, run, a design answer or a stated size) shows the holder (computer, process ID, start time) and a **Remove the lock of a stopped run** button: `remove_stale_lock` only on that click, never the lock of a process still running on the same computer; on Google Drive one session per output folder (question 58). | | Notebook masks (v0.4.0, PP-10c) | Step 1 has an optional masks folder; **Check folder** pairs it with `preflight(masks_folder=, mask_axes=)`, the masks folder and statements being those the run would use (`run.planned.planned_masks`, from `resolve_inputs`: given, else recorded). The **Masks** card shows each mask type (`mask_review.mask_type_rows`), the images without one and the masks' issues (`masks.issues`, and `mask_checks.mask_issues` once the roles are confirmed, as `run`). Mask axes are asked per type only where needed (`mask_review.mask_axes_questions`: `mask_not_2d`, `invalid_mask_axes`, `mask_axes_contradict_header`, and `mask_planes_mismatch` of 2-D masks, answered `YX`), with only the statements `masks.stated.mask_roles` accepts for every mask of the type; nothing is preselected or applied until **Check the masks** (`run.preflight.remasked`, which keeps the other answers). The run is given the same `masks_folder` and `mask_axes`; a masks folder not given reuses the recorded one, and **Drop the recorded masks** gives `masks_folder=False`. Step 3 is gated on masks without errors and on a pairing that matches the masks folder and statements chosen. `folder_step(show_output=False)` shows neither and pairs no masks folder. The mask step (`mask_relationships.json`) is step 3's **Describe the masks** (next row). | @@ -279,7 +279,7 @@ compute its rows from the copies (`convert_tiffs`, `zarr_export`, CLI `--convert notebook's "(Optional) Image conversion" card). The bricks never read those copies, and v0.4 reads every image through the run's own readers, so the option was removed (spec revision 4, §8 PP-2). Its rows were the TIFF's own, so a later run keeps them. Saving images with axis -errors as named-axis copies is a separate step, planned for v0.4.0 as OME-TIFF. +errors as named-axis copies is a separate step, `convert` (OME-TIFF, §17). ## 11. Dataset consistency (folder check) @@ -496,3 +496,23 @@ instead of keeping copies. | Pixel sizes (D-a) | `same_pixel_size(a, b, roles=("X", "Y"))`, `pixel_size_groups(sizes, roles=)`, `PIXEL_SIZE_TOLERANCE` (`physical_sizes`) | Two pixel sizes are the same when `|a - b| <= 1e-3 * max(a, b)` on every compared axis, in micrometres; one rule for a declared size against a header and for two headers. `same_pixel_size` takes `PhysicalSizes` (converted with `micrometers`) or sizes already in micrometres, one per role, and returns None when either is unknown (no size, or a unit that cannot be converted), never True. The rule is not transitive (1.0 ≈ 1.0009 ≈ 1.0018, but 1.0 ≠ 1.0018), so there is no per-image grouping key: `pixel_size_groups` links images by single linkage, so images of two groups always differ while the extremes of one group may drift; unknown sizes are left out; the largest group first. The folder check's `different_pixel_sizes` uses both (§11). | | Experiment design | `read_confirmed_design(path, expected_sha256=)`, `ConfirmedDesign`, `DesignCheckError` (`design`) | The one reader of `experiment_design.csv` (§12) for a brick that pools biological repeats (formerly CellPatcher `load_experiment_design` and MorphoFeature `ExperimentDesign.read`). It takes the file as this brick writes it and fills nothing in: UTF-8 without a byte-order mark; the header exactly the §12 columns in order; one value per column in every row (an unclosed quote, a missing or extra value, or a blank line is refused); no value with outer whitespace; `control` and `confirmed` exactly `true` or `false`; counts written as whole numbers of at least 0; `schema_version` 1; `confirmed_on` empty or `YYYY-MM-DD`; `condition`, `biological_repeat` and `repeat_group` pass `check_identity_text` and do not differ only in case (`check_identity_case`); no folder twice, one `control` per condition, at least one row; every group confirmed with one folder per condition (`design_issues`); with `expected_sha256` (the manifest's `experiment_design_sha256`) the file must be that one. The SHA-256 is of the bytes parsed. `ConfirmedDesign` gives `path`, `sha256`, the `design`, read-only `repeat_groups` (`{(condition, biological_repeat): repeat_group}`, the normalization brick's input), `group_members()` (`{repeat_group: {condition: biological_repeat}}`), `conditions`, `controls` and `groups`. Refusals raise `DesignCheckError` (a `ValueError`) with its `issues`: `missing_experiment_design`, `unreadable_experiment_design`, `invalid_experiment_design`, `repeat_group_conflict`, `design_not_confirmed` or `changed_experiment_design`, each an error naming the file (or the group's folders). This brick's step 1 and `check` read the file being edited with the lenient `load_design(output_folder, repeats)` (§12), which returns `(design, changes)`: the saved design brought up to date with the folders, or the default design when there is no file. A saved file that cannot be read raises `DesignCheckError` with the one error `unreadable_experiment_design`, whose message is the same "The experiment design cannot be read: …" as here followed by the lenient reader's refusal, naming the line and column where it can (the `DesignFormatError` is its `__cause__`); no default design is returned and the file is not touched (owner decision of 31 October 2026, plan §12 question 43). **Breaking change for the next release:** up to v0.3.4 `load_design` returned a third value, `problem`, the reason as text, together with the default design. | +## 17. Named-axis copies (`convert`, v0.4.0, design v0.4 §5) + +The exception path for images whose axes are wrong or not named: they are saved again as +named-axis OME-TIFF copies with the roles the user confirmed, and the user then runs +preprocessing on the copies. `convert_experiment(experiment_root, mapping, destination, +exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the CLI +`convert EXPERIMENT --to FOLDER --axes ROLES`. Masks are never converted (§15). + +| Rule | Detail | +|---|---| +| What is converted | Every image with an axis error (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2) or unnamed axes (`axes_not_named`) under the mapping given; it is never taken from the headers. Every other image is copied byte for byte: its header names its axes as confirmed, it is an OME-Zarr image (with its label images), or it is excluded (`exclude`: never read). So the copy tree is the whole experiment. The command and the API are offered for any experiment; an experiment without these images is copied as it is. | +| Checks first | The experiment is checked with the mapping as a run would check it (folders, headers, `review_axes`), from the headers only. The axis errors are what the conversion corrects; any other error stops it before anything is written, and so do images of a type OME-TIFF cannot hold (`cannot_convert_pixel_type`: `int64`, `uint64`; pixel values are never changed to fit). | +| Copy tree | A folder the user names (`--to`), outside the experiment and not holding it (`copy_folder_overlaps_experiment`; a file is `copy_folder_not_a_folder`): new or empty (else `copy_folder_not_empty`, nothing overwritten), or the copy tree of an earlier conversion of the same experiment. The originals are only read. Each image keeps its relative path and file name (an OME-TIFF may end in `.tif`), so `source_relative_path`, `plane_id` and the FOV key are the originals'. | +| Copies | Uncompressed OME-TIFF, BigTIFF when the pixels take more than 4 GiB less 32 MiB. Axes: every axis with a role in the source's order, then Y and X (`ZCYX`); an axis without a role (length 1) or missing (`missing_as_singleton`) is not added. Each plane is the plane the run reads under the confirmed roles (`iter_planes`), bit for bit: same pixel type (in native byte order), no rescaling, NaN and infinities kept; a Y/X swap and X before Y are written rows (Y) then columns (X). Planes over the memory limit are read in blocks, as the run reads them, and each block is written to its place: no plane is held whole when the run would not hold it. The copy's metadata names every axis, so `check` and `run` read it with `axes_from: "header"`. | +| Pixel sizes | They follow the roles; a relabelled axis gets an unknown scale, never a guessed one (`copy_axes`). A size the source header gives is written only for the role the header names it for, and only when the user confirmed that role for that same axis; otherwise it is left out and the record says why. A Y/X swap therefore never exchanges the sizes of Y and X: both are unknown in the copy (state them in `calibration.csv`, §9.3, if known). A z-step is never written as a time step. A plain stack's Y and X confirmed Y before X keep the header's sizes. Units are written as OME symbols (`µm`, `s`, ...). Channel names are written when the C axis keeps its role and every channel is named; significant bits as they are. | +| Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). A failure (`conversion_failed`, `not_enough_space` before writing) removes the `.partial` files of the conversion, so the folder is left as it was; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | +| Repeat | A copy already in the tree is reused when its source, roles and copy are unchanged since the record; a new image is added. Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | +| Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 1: `source_root`, `mapping`, `missing_as_singleton`, `written_at`, `versions`, and per image `relative_path`, `action` (`converted` or `copied`), `reason` (the axis code, `excluded`, or null), `source` and `copy` fingerprints (`size`, `mtime_ns`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9) and `unknown_sizes` (role: why). `read_conversion_record(folder)` reads it with its SHA-256. | +| The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed, went missing or was added since (`copy_changed_after_conversion`); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images. | +| Notebook | Not yet: the axis-check card names the `convert` command; a notebook card is a follow-up (PP-11b). | diff --git a/docs/plan.md b/docs/plan.md index 90cf2dcc..6dbc3582 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -489,6 +489,26 @@ API, CLI). The notebook and CLI only call the API. is unchanged. - The masks' texts and the new table rows moved from `report` to `mask_text` (pure), so `report.py` stays within §9's 600 lines. +- **PP-11 (named-axis OME-TIFF copies, the exception path)** *(implemented, this PR; design + v0.4 §5; contract §17; question 59)*: + - `convert_experiment` (`conversion`, orchestration) and the CLI `convert EXPERIMENT --to + FOLDER --axes ROLES`: images with an axis error or unnamed axes are written as + named-axis OME-TIFF with the confirmed roles (`copy_axes`, pure; `ome_tiff_copy`, io: + planes from `iter_planes`, in blocks over the memory limit, each block written in + place; BigTIFF when large); every other image copied byte for byte; masks never + converted. Any error other than the axis errors stops it before anything is written. + - Pixel sizes follow the roles; a relabelled axis gets an unknown scale (a Y/X swap + leaves X and Y unknown, never exchanged); channel names kept when C keeps its role. + - Safe: the copy tree's lock; `.partial` files moved into place together once all are + written (a failure leaves the folder as it was); unchanged copies reused, anything else + refused (`copy_differs`, `copy_folder_not_empty`); the conversion record + (`conversion_record`, `.morphobricks-conversion.json`) written last. + - The run on the copy tree: `conversion_record_sha256` in the manifest, `conversion` in + the report, the warnings `conversion_source_changed` and `copy_changed_after_conversion` + (`review_conversion`, from `preflight`), every row recomputed (the read path changes). + - The axis messages (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, + `axes_not_named`) and the notebook's axis marks name the `convert` command. + - Not in this PR: the notebook card for the conversion (PP-11b, proposed). - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; @@ -1381,3 +1401,51 @@ Raised during review; to be decided before the milestones that need them: Colab, Step 0 installs the `notebook` and `zarr` extras (locally `notebook`, unchanged), so OME-Zarr images can be read there; (5) `RELEASE_TAG` stays `v0.4.0rc2`; the owner tags it after PP-8c and PP-8d are merged.* +59. How are images with axis errors saved as named-axis copies (design v0.4 §5, PP-11)? + *Implemented (2 November 2026, PP-11) as design v0.4 §5 says: images only, as + named-axis OME-TIFF (BigTIFF when large); copies keep the file name and folder + structure, so `source_relative_path`, `plane_id` and the FOV key stay the same; pixel + sizes follow the roles and a relabelled axis gets an unknown scale; `.partial` files, the + conversion record written last with source and copy fingerprints and relative paths, a + warning when a source changes after conversion; the run on the copies keeps the output + folder, reports the new experiment root and recomputes every row, keeping the design, + image type and calibration; the CLI and API offer it always. Contract §17.* + *Open for the owner: where §5 is silent, the conservative choice was taken.* + (1) **Pixel sizes of a Y/X swap**: both X and Y are left unknown in the copy (the swap + relabels both axes), so no size is ever exchanged; the user states them in + `calibration.csv`. The axes redesign of 1 November (B2) had proposed that sizes follow + the stored dimension (the header's X size becoming the copy's Y size). Keep unknown, or + adopt B2? + (2) **Where the copies go**: a folder the user names (`--to`, required; no default), + outside the experiment and not holding it. A default such as `_named_axes` + beside the experiment could be added. + (3) **What the copy tree holds**: the whole experiment, since it becomes the experiment + root: converted images, the other images copied byte for byte (OME-Zarr stores with + their label images), and excluded images copied unread (they stay excluded by the run). + Non-image files are not copied. + (4) **Which lock**: the copy tree's own (`.morphobricks-preprocessing.lock` there); + `convert` takes no output folder and writes nothing there. + (5) **The record's name**: `.morphobricks-conversion.json`, a dot-file so discovery + skips it without a change to the discovery rules; a visible name would need one. + (6) **A source changed since an earlier conversion into the same folder**: refused + (`copy_differs`), never updated in place: convert into a new folder. New images are + added and unchanged copies reused. + (7) **A conversion killed while moving the copies into place** (copies there, no record): + the folder is refused (`copy_folder_not_empty`) until emptied. + (8) **64-bit integer images**: refused (`cannot_convert_pixel_type`); OME-TIFF has no + such pixel type and values are never changed. + (9) **Byte order**: copies are written in the computer's byte order; values are + identical (a big-endian source gives a little-endian copy on usual computers). + (10) **Channel names**: written only when the C axis keeps its role and every channel is + named; otherwise none. + (11) **Compression**: none (each plane then has a fixed place, so blocks are written + in place within the memory limit). + (12) **Axes confirmed as missing single planes** (`missing_as_singleton`) are not added + to the copies: the run on the copies needs `--missing-as-singleton` again. + (13) **RGB sources** with an axis error are written with their channels as separate + pages (`CYX`), so the suggested image type may change; the recorded type is kept. + (14) **`RunInputs`** does not check the conversion record (the manifest records its + SHA-256; the copies are checked by their fingerprints like any image). + (15) **Notebook card**: left to a follow-up, PP-11b; the axis-check card names the + command. + diff --git a/docs/user_guide.md b/docs/user_guide.md index d9c203f7..a29716e3 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -697,6 +697,49 @@ statement for an axis some images lack is a warning naming them (`calibration_axis_absent`). In Python, `write_calibration(calibration, output_folder)` writes the file (see §9). +### Named-axis copies (`convert`) + +When the metadata of some images name their axes wrongly (the errors `axes_yx_swapped`, +`axes_contradict_header`, `axes_x_before_y`) or do not name them at all (the warning +`axes_not_named`), and you are sure of the roles, save the images again as named-axis +copies with your roles, then run preprocessing on the copies: + +```bash +morphobricks-preprocessing convert /data/exp1 --to /data/exp1_named_axes --axes ZXY +morphobricks-preprocessing check /data/exp1_named_axes --output /data/exp1_MorphoBricks_Preprocessing +morphobricks-preprocessing run /data/exp1_named_axes --output /data/exp1_MorphoBricks_Preprocessing --axes ZYX +``` + +`--axes` gives your roles for the original images, exactly as for `run`; they are never +taken from the metadata. `convert` writes a new folder (`--to`, outside the experiment) +with the experiment's folders and file names: the images with axis problems as OME-TIFF +whose metadata name your roles (Y and X are written rows then columns), every other image +copied as it is. The originals are only read. Each plane of a copy holds exactly the pixel +values preprocessing reads from the original with your roles: same pixel type, nothing +rescaled. Large planes are read and written in blocks within `--memory-limit`. Masks are +never converted: keep using your masks folder. + +Pixel sizes follow the roles. An axis you give another role than its metadata gets no size +in the copy: after a Y/X swap the copy has no X or Y pixel size (the metadata's X size +belongs to the axis you call Y), and a depth step is never written as a time step. State +the sizes in `calibration.csv` if you know them (`calibrate`). The command prints what it +wrote and the next commands, with the suggested `--axes` for the copies. + +Give `run` the output folder of your runs on the originals: the design, image type and +stated pixel sizes are kept (the copies have the same names), the report says the +experiment is now the folder of copies, and every row is computed again. The conversion +record, **.morphobricks-conversion.json** in the folder of copies, lists every image with +its original's and its copy's size and modification time, the roles and the pixel sizes; +the run records its SHA-256 and warns when an original changes after the conversion +(`conversion_source_changed`) or a copy is changed (`copy_changed_after_conversion`). + +Running `convert` again with the same images and roles reuses the copies and only adds new +images. It never overwrites anything else: a folder holding other files, copies of other +images or made with other roles is refused (`copy_folder_not_empty`, `copy_differs`); +convert into a new folder then. If it fails (`conversion_failed`), the folder is left as it +was. Images of 64-bit integers cannot be saved as OME-TIFF (`cannot_convert_pixel_type`). +The notebook does not offer the conversion yet: run the command in a terminal. + ## 6. Output files The default output folder is next to the experiment folder: @@ -821,10 +864,10 @@ continues. | `inconsistent_dimensions`, `inconsistent_mapping` | Error | Images have their dimensions in a different order. Export them the same way or process separately. | | `missing_axes` | Error | Some images lack a dimension. Confirm they have one plane along it (step 2), or export them again. | | `ambiguous_missing_axes` | Error | It is unclear which dimension an image lacks. Export it again. | -| `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_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) with the `convert` command (§5), 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 with the `convert` command, §5, 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. | +| `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 (the `convert` command, §5); 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. | @@ -836,6 +879,16 @@ continues. | `experiment_not_recorded` | Error | `run` was given no experiment folder, and the output folder holds no v0.4.0 run that records one (or no output folder was given). Give the experiment folder. | | `axes_not_confirmed` | Error | No axis roles were given and the output folder holds no earlier run that records them. Confirm the roles (`--axes`). | | `output_locked` | Error | Another run is using the output folder; it is named with its computer, process and start time. Wait for it to finish; if it was stopped or the computer restarted, remove its lock (`--remove-stale-lock`). Nothing was written. | +| `copy_folder_overlaps_experiment` | Error | `convert`: the folder for the copies is the experiment folder, inside it, or holds it. Choose a new folder outside the experiment. | +| `copy_folder_not_a_folder` | Error | `convert`: the path given with `--to` is a file. Choose a folder. | +| `copy_folder_not_empty` | Error | `convert`: the folder for the copies holds files and no conversion record. Nothing is overwritten; choose a new or empty folder. | +| `copy_differs` | Error | `convert`: images in the folder of copies differ from the copies this conversion would write (an original changed, other roles, a copy changed, an image no longer in the experiment), or the folder holds copies of another experiment. Nothing is overwritten; convert into a new folder, or remove those copies first. | +| `cannot_convert_pixel_type` | Error | `convert`: images of 64-bit integers cannot be saved as OME-TIFF, and pixel values are never changed to fit. Save them again with named axes in a 32-bit type if their values fit, or correct the roles. | +| `conversion_failed` | Error | `convert`: an image could not be copied (unreadable, changed while copied, or the folder cannot be written). The copies written by this conversion were removed; fix the cause and convert again, or exclude the image. | +| `not_enough_space` | Error | `convert`: the copies would not fit on the drive. Free space or choose another drive. | +| `conversion_record_unreadable` | Error | The conversion record of a folder of named-axis copies (**.morphobricks-conversion.json**) cannot be read, so `run` stops and `convert` refuses the folder. Convert again into a new folder. | +| `conversion_source_changed` | Warning | An original changed or went missing after its named-axis copy was made; the copies hold the images as converted. If the originals were corrected, convert again into a new folder. | +| `copy_changed_after_conversion` | Warning | An image of the folder of named-axis copies changed, went missing or was added after the conversion, so the conversion record does not describe it. Convert again, or check it is the image you want. | | `design_unreadable` | Error | The `experiment_design.csv` an earlier run wrote cannot be read. Correct it, or delete it to confirm the design again. | | `no_usable_planes` | Error | No plane has finite pixel values. Check the images. | | `tiff_outside_leaf` | Error | Images sit in a folder that also holds subfolders, so they have no condition or repeat of their own. A folder holds either images or subfolders, never both. Move the images into a folder of their own, or out of the experiment folder. | diff --git a/notebooks/MorphoBricks_Preprocessing.ipynb b/notebooks/MorphoBricks_Preprocessing.ipynb index c687990d..27ff317f 100644 --- a/notebooks/MorphoBricks_Preprocessing.ipynb +++ b/notebooks/MorphoBricks_Preprocessing.ipynb @@ -636,8 +636,9 @@ "\n", "**\"… swap the Y and X their header names\"** or **\"… contradict their header\"**: the images\n", "are not read; the message names the likely cause. Correct the axes in step 2 (choose the\n", - "header's roles if it is right), or save named-axis copies with your roles and run preprocessing on\n", - "the copies. **\"… do not name their axes in their metadata\"** is a warning: check the roles with\n", + "header's roles if it is right), or save named-axis copies with your roles (in a terminal:\n", + "**morphobricks-preprocessing convert** with **--to** a new folder and **--axes** your roles) and run\n", + "preprocessing on the copies. **\"… do not name their axes in their metadata\"** is a warning: check the roles with\n", "the preview; they are recorded as confirmed by you.\n", "\n", "**\"The images have different numbers of channels\"**: each row of the table holds every channel of\n", diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index a200c3b3..bfdf0675 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -39,7 +39,7 @@ from ._io import free_space from .axes import AxisMapping, prefill_mapping -from .axis_check import AXIS_ERROR_CODES, ConfirmedAxes, header_axis_issue +from .axis_check import AXIS_ERROR_CODES, ConfirmedAxes, confirmation_of, header_axis_issue from .axis_review import review_axes from .conversion_record import ( CONVERTED, @@ -63,6 +63,9 @@ logger = logging.getLogger(__name__) +_NOT_IN_OME = frozenset({"int64", "uint64"}) +"""Pixel types the brick reads that OME-TIFF cannot hold: they are never changed to fit.""" + PARTIAL_SUFFIX = ".partial" """A copy is written as ``.partial`` and renamed once every copy is written.""" @@ -176,11 +179,21 @@ def convert_experiment( dest.mkdir(parents=True, exist_ok=True) try: with output_lock(dest): - return _locked_conversion(root, dest, planned, issues, (memory_limit, progress)) + options = _Options(confirmation_of(mapping)[0], missing_as_singleton, memory_limit) + return _locked_conversion(root, dest, planned, issues, options, progress) except OutputLockedError as exc: return ConversionResult(dest, False, (_lock_issue(exc),)) +@dataclass(frozen=True) +class _Options: + """What the conversion records and how it reads, besides the plan.""" + + mapping: AxisMapping + missing_as_singleton: bool + memory_limit: int + + def plan_conversion( root: Path, mapping: AxisMapping | ConfirmedAxes, @@ -221,6 +234,9 @@ def plan_conversion( issues = [i for i in issues if not i.is_error] # the axis errors are what is corrected to_convert = set(review.axis_error_files) | set(review.confirmed_files) headers = {meta.relative_path: meta for meta in pre.metadata.files} + wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) + if wide: + return (), [_pixel_type_issue(wide, headers[wide[0]].dtype.name), *issues] planned = [] for rel in pre.discovery.images: source = Fingerprint.of(root / rel) @@ -256,7 +272,8 @@ def _locked_conversion( dest: Path, planned: tuple[PlannedCopy, ...], issues: list[ValidationIssue], - options: tuple[int, Callable[[int, int, str], None] | None], + options: _Options, + progress: Callable[[int, int, str], None] | None, ) -> ConversionResult: """Compare with what the copy tree holds, write the new copies and the record.""" try: @@ -268,14 +285,14 @@ def _locked_conversion( if refusal is not None: return ConversionResult(dest, False, (refusal, *issues)) try: - written = _write_all(root, dest, to_write, *options) + written = _write_all(root, dest, to_write, options.memory_limit, progress) except _ConversionFailed as failed: return ConversionResult(dest, False, (failed.issue, *issues)) files = sorted([*reused, *written], key=lambda entry: entry.relative_path) record = ConversionRecord( source_root=root.as_posix(), - mapping=_experiment_mapping(planned, earlier), - missing_as_singleton=_singleton(planned), + mapping=options.mapping, + missing_as_singleton=options.missing_as_singleton, files=tuple(files), versions=package_versions(), written_at=datetime.now(UTC).isoformat(timespec="seconds"), @@ -417,23 +434,6 @@ def _remove(path: Path) -> None: path.unlink(missing_ok=True) -def _experiment_mapping( - planned: tuple[PlannedCopy, ...], earlier: tuple[ConversionRecord, str] | None -) -> AxisMapping: - """Return the experiment mapping of the conversion, as the record lists it.""" - for item in planned: - if item.mapping is not None: - return item.mapping - if earlier is not None: - return earlier[0].mapping - return AxisMapping("YX", ("Y", "X")) - - -def _singleton(planned: tuple[PlannedCopy, ...]) -> bool: - """Return True if some image was converted with roles confirmed as single planes.""" - return any(item.mapping is not None and item.mapping.singleton_roles for item in planned) - - def _suggested_axes(files: list[CopyEntry]) -> str | None: """Return the roles for ``run --axes`` on the copies when they all have the same axes.""" axes = { @@ -489,6 +489,19 @@ def _space_issue(dest: Path, to_write: list[PlannedCopy]) -> ValidationIssue | N ) +def _pixel_type_issue(paths: list[str], dtype: str) -> ValidationIssue: + """Refuse to convert images of a pixel type that OME-TIFF cannot hold.""" + return error( + "cannot_convert_pixel_type", + f"{plural(len(paths), 'image')} with axis problems have the pixel type {dtype}, which " + "OME-TIFF cannot hold, and pixel values are never changed to fit. Nothing was " + "written.", + paths, + "Save these images again with their axes named in their metadata (for example as " + "OME-TIFF in a 32-bit type, if their values fit), or correct the roles.", + ) + + def _not_empty_issue(dest: Path, paths: list[str]) -> ValidationIssue: """Refuse a folder that holds files and no conversion record.""" return error( diff --git a/src/morphobricks_preprocessing/notebook_axes.py b/src/morphobricks_preprocessing/notebook_axes.py index 5c30888e..5d356f63 100644 --- a/src/morphobricks_preprocessing/notebook_axes.py +++ b/src/morphobricks_preprocessing/notebook_axes.py @@ -94,7 +94,7 @@ def header_marks(mapping: AxisMapping, files: Iterable) -> dict[int, str]: 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.
" + "Y before X, or convert them (the convert command)." ) continue named = ", ".join(axis_name(letter) for letter in sorted(letters[index]) if letter) diff --git a/src/morphobricks_preprocessing/run/steps.py b/src/morphobricks_preprocessing/run/steps.py index 8202e0e6..f739a658 100644 --- a/src/morphobricks_preprocessing/run/steps.py +++ b/src/morphobricks_preprocessing/run/steps.py @@ -103,6 +103,7 @@ def header_checks( *design_problems, *dataset.issues, *(mask_issues(pre.masks, pre.metadata, review) if pre.masks is not None else ()), + *(pre.conversion.issues if pre.conversion is not None else ()), ] bins = _bins(pre, inputs, issues) if not has_errors(issues): diff --git a/tests/test_convert.py b/tests/test_convert.py new file mode 100644 index 00000000..ef091d3a --- /dev/null +++ b/tests/test_convert.py @@ -0,0 +1,353 @@ +"""Named-axis OME-TIFF copies (design v0.4 §5, PP-11): the copies themselves. + +Every converted plane equals the original read under the confirmed roles, bit for bit, +with its pixel type: plain stacks, ImageJ hyperstacks, Y/X-swapped headers, several +channels, Z and T, X before Y, planes read in blocks. Pixel sizes follow the roles and a +relabelled axis gets an unknown scale (a Y/X swap never exchanges sizes). The originals are +only read; the copy tree is outside the experiment. +""" + +import hashlib +import os +import stat + +import numpy as np +import pytest +import tifffile + +import morphobricks_preprocessing.ome_tiff_copy as copy_module +import morphobricks_preprocessing.planes as planes_module +from morphobricks_preprocessing import ( + AxisMapping, + convert_experiment, + inspect_image, + iter_planes, + named_axes, + plane_layout, + read_conversion_record, +) +from morphobricks_preprocessing.copy_axes import copy_layout + +ZYX_SWAPPED = AxisMapping("ZYX", ("Z", "X", "Y")) + + +def ome(path, data, axes, **metadata): + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite( + path, data, ome=True, photometric="minisblack", metadata={"axes": axes, **metadata} + ) + return path + + +def imagej(path, data, axes, **kwargs): + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite(path, data, imagej=True, metadata={"axes": axes}, **kwargs) + return path + + +def plain(path, data, **kwargs): + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite(path, data, photometric="minisblack", metadata=None, **kwargs) + return path + + +def pixels(shape, dtype=np.uint16, seed=0): + rng = np.random.default_rng(seed) + if np.dtype(dtype).kind == "f": + values = rng.normal(100, 50, shape).astype(dtype) + values.flat[::7] = np.nan + values.flat[3] = np.inf + return values + info = np.iinfo(dtype) + return rng.integers(info.min, info.max, shape, dtype=dtype, endpoint=True) + + +def originals(path, mapping, **kwargs): + """Every plane of ``path`` read under ``mapping``, by its (z, c, t) coordinates.""" + planes = {} + for plane in iter_planes(path, mapping, **kwargs): + whole = np.zeros(plane.shape, plane.dtype) + for block in plane.blocks(): + h, w = block.pixels.shape + whole[block.y : block.y + h, block.x : block.x + w] = block.pixels + planes[(plane.z, plane.c, plane.t)] = whole + return planes + + +def assert_bitwise_equal(source, mapping, copy, **kwargs): + """Each plane of ``copy`` (named axes) has the bytes of the source plane read under roles.""" + meta = inspect_image(copy) + assert "?" not in named_axes(meta), "the copy names every axis" + expected = originals(source, mapping, **kwargs) + found = originals(copy, AxisMapping(meta.axes, tuple(meta.axes))) + assert found.keys() == expected.keys() + for key, plane in expected.items(): + assert found[key].dtype == plane.dtype + assert found[key].shape == plane.shape + assert found[key].tobytes() == plane.tobytes(), key + + +def fingerprints(folder): + """Every file under ``folder`` with its bytes' SHA-256 and modification time.""" + return { + path.relative_to(folder).as_posix(): ( + hashlib.sha256(path.read_bytes()).hexdigest(), + path.stat().st_mtime_ns, + ) + for path in sorted(folder.rglob("*")) + if path.is_file() + } + + +def convert(tmp_path, mapping, **kwargs): + return convert_experiment(tmp_path / "exp", mapping, tmp_path / "copies", **kwargs) + + +# Pixel exactness ---------------------------------------------------------------------- + + +def test_a_plain_stack_is_written_with_the_confirmed_roles(tmp_path): + source = plain(tmp_path / "exp" / "a" / "img.tif", pixels((4, 9, 13))) + mapping = AxisMapping("IYX", ("Z", "Y", "X")) + result = convert(tmp_path, mapping) + assert result.written and result.converted == ("a/img.tif",) + copy = tmp_path / "copies" / "a" / "img.tif" + with tifffile.TiffFile(copy) as tif: + assert tif.is_ome and tif.series[0].axes == "ZYX" and tif.series[0].shape == (4, 9, 13) + assert_bitwise_equal(source, mapping, copy) + assert result.suggested_axes == "ZYX" + + +def test_a_y_x_swap_is_written_rows_then_columns_and_never_exchanges_sizes(tmp_path): + data = pixels((3, 10, 14)) + source = ome( + tmp_path / "exp" / "img.tif", data, "ZYX", PhysicalSizeX=0.3, PhysicalSizeY=0.4, + PhysicalSizeZ=1.5, + ) # fmt: skip + result = convert(tmp_path, ZYX_SWAPPED) + copy = tmp_path / "copies" / "img.tif" + assert result.written and result.converted == ("img.tif",) + with tifffile.TiffFile(copy) as tif: + assert tif.series[0].axes == "ZYX" and tif.series[0].shape == (3, 14, 10) + np.testing.assert_array_equal(tif.series[0].asarray(), data.transpose(0, 2, 1)) + assert_bitwise_equal(source, ZYX_SWAPPED, copy) + sizes = inspect_image(copy).physical_sizes + # The header's X size describes what the user calls Y: it is not taken for either. + assert (sizes.x, sizes.y, sizes.z) == (None, None, 1.5) + entry = read_conversion_record(tmp_path / "copies")[0].entry("img.tif") + assert entry.reason == "axes_yx_swapped" and entry.copy_axes == "ZYX" + assert entry.roles == ("Z", "X", "Y") and entry.source_axes == "ZYX" + assert set(entry.unknown_sizes) == {"X", "Y"} + assert "relabelled" in entry.unknown_sizes["X"] + + +def test_an_imagej_slice_stack_confirmed_as_time_drops_its_z_step(tmp_path): + data = pixels((5, 8, 6), np.uint8) + source = tmp_path / "exp" / "img.tif" + source.parent.mkdir(parents=True) + tifffile.imwrite( + source, data, imagej=True, resolution=(2.0, 4.0), + metadata={"axes": "ZYX", "spacing": 2.5, "unit": "um"}, + ) # fmt: skip + mapping = AxisMapping("ZYX", ("T", "Y", "X")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written and result.converted == ("img.tif",) + assert inspect_image(copy).axes == "TYX" + assert_bitwise_equal(source, mapping, copy) + sizes = inspect_image(copy).physical_sizes + assert (sizes.x, sizes.y, sizes.z, sizes.t) == (0.5, 0.25, None, None) + assert sizes.unit("X") == "micrometer" + entry = read_conversion_record(tmp_path / "copies")[0].entry("img.tif") + assert entry.reason == "axes_contradict_header" and set(entry.unknown_sizes) == {"Z"} + + +def test_an_imagej_hyperstack_with_channels_depth_and_time(tmp_path): + data = pixels((2, 3, 2, 7, 5)) # TZCYX + source = imagej(tmp_path / "exp" / "img.tif", data, "TZCYX") + # Confirmed with the header's Y and X the other way round. + mapping = AxisMapping("TZCYX", ("T", "Z", "C", "X", "Y")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written and inspect_image(copy).axes == "TZCYX" + assert inspect_image(copy).shape == (2, 3, 2, 5, 7) + assert_bitwise_equal(source, mapping, copy) + + +def test_channel_names_are_kept_when_the_channel_axis_keeps_its_role(tmp_path): + source = ome( + tmp_path / "exp" / "img.tif", pixels((2, 3, 6, 9)), "CZYX", + Channel={"Name": ["DAPI", "actin"]}, + ) # fmt: skip + mapping = AxisMapping("CZYX", ("C", "Z", "X", "Y")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written + assert inspect_image(copy).channel_names == ("DAPI", "actin") + assert_bitwise_equal(source, mapping, copy) + + +def test_tifffiles_cyx_default_confirmed_as_depth_loses_the_channel_names(tmp_path): + source = ome( + tmp_path / "exp" / "img.tif", pixels((3, 6, 8)), "CYX", Channel={"Name": list("abc")} + ) + mapping = AxisMapping("CYX", ("Z", "Y", "X")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written and inspect_image(copy).axes == "ZYX" + assert inspect_image(copy).channel_names == () + assert_bitwise_equal(source, mapping, copy) + + +def test_a_plain_stack_confirmed_x_before_y_is_written_y_first(tmp_path): + data = pixels((3, 6, 11)) + plain(tmp_path / "exp" / "img.tif", data) + mapping = AxisMapping("IYX", ("T", "X", "Y")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written + assert inspect_image(copy).axes == "TYX" and inspect_image(copy).shape == (3, 11, 6) + np.testing.assert_array_equal(tifffile.imread(copy), data.transpose(0, 2, 1)) + entry = read_conversion_record(tmp_path / "copies")[0].entry("img.tif") + assert entry.reason == "axes_x_before_y" + + +@pytest.mark.parametrize("dtype", [np.int8, np.uint16, np.int32, np.float32, ">f8"]) +def test_every_pixel_type_is_kept_value_for_value(tmp_path, dtype): + data = pixels((2, 5, 7), np.dtype(dtype).newbyteorder("=")).astype(dtype) + source = plain(tmp_path / "exp" / "img.tif", data) + mapping = AxisMapping("IYX", ("Z", "Y", "X")) + assert convert(tmp_path, mapping).written + copy = tmp_path / "copies" / "img.tif" + found = tifffile.imread(copy) + assert found.dtype == np.dtype(dtype).newbyteorder("=") + # Bit for bit: NaN and infinities included, no rescaling, no rounding. + assert found.tobytes() == np.ascontiguousarray(data, found.dtype).tobytes() + assert_bitwise_equal(source, mapping, copy) + + +def test_planes_over_the_memory_limit_are_read_and_written_in_blocks(tmp_path, monkeypatch): + data = pixels((2, 64, 48)) + source = ome(tmp_path / "exp" / "img.tif", data, "ZYX") + tifffile.imwrite( + source, data, ome=True, photometric="minisblack", rowsperstrip=4, + metadata={"axes": "ZYX"}, + ) # fmt: skip + limit = plane_layout(source, ZYX_SWAPPED, memory_limit=1).block_memory_estimate + assert limit < plane_layout(source, ZYX_SWAPPED).memory_estimate + reads = [] + original = planes_module._read_segment + monkeypatch.setattr( + planes_module, "_read_segment", lambda *a: reads.append(a[1]) or original(*a) + ) + for cls in (tifffile.TiffFile, tifffile.TiffPage, tifffile.TiffFrame, tifffile.TiffPageSeries): + monkeypatch.setattr(cls, "asarray", lambda *a, **k: pytest.fail("decoded whole")) + result = convert(tmp_path, ZYX_SWAPPED, memory_limit=limit) + assert result.written and len(reads) >= 2 * 64 // 4 + monkeypatch.undo() + np.testing.assert_array_equal( + tifffile.imread(tmp_path / "copies" / "img.tif"), data.transpose(0, 2, 1) + ) + + +@pytest.mark.parametrize("dtype", [np.int64, np.uint64]) +def test_64_bit_integers_which_ome_tiff_cannot_hold_are_refused(tmp_path, dtype): + plain(tmp_path / "exp" / "img.tif", pixels((2, 4, 5), dtype)) + result = convert(tmp_path, AxisMapping("IYX", ("Z", "Y", "X"))) + assert not result.written + assert [i.code for i in result.issues if i.is_error] == ["cannot_convert_pixel_type"] + assert not (tmp_path / "copies").exists() + + +def test_a_large_copy_is_written_as_bigtiff(tmp_path, monkeypatch): + monkeypatch.setattr(copy_module, "BIGTIFF_FROM", 0) + source = plain(tmp_path / "exp" / "img.tif", pixels((2, 5, 6))) + mapping = AxisMapping("IYX", ("Z", "Y", "X")) + assert convert(tmp_path, mapping).written + copy = tmp_path / "copies" / "img.tif" + with tifffile.TiffFile(copy) as tif: + assert tif.is_bigtiff and tif.is_ome + assert_bitwise_equal(source, mapping, copy) + + +# What is copied, and the originals ------------------------------------------------------ + + +def test_images_with_named_agreeing_axes_and_excluded_images_are_copied_byte_for_byte(tmp_path): + # A 2-D OME image beside plain stacks: its Z is a confirmed single plane. + good = ome(tmp_path / "exp" / "a" / "good.tif", pixels((5, 6)), "YX") + plain(tmp_path / "exp" / "a" / "stack.tif", pixels((3, 5, 6))) + damaged = tmp_path / "exp" / "a" / "damaged.tif" + damaged.write_bytes(b"not a tiff at all") + mapping = AxisMapping("IYX", ("Z", "Y", "X")) + before = fingerprints(tmp_path / "exp") + result = convert(tmp_path, mapping, exclude=["a/damaged.tif"], missing_as_singleton=True) + assert result.written, result.issues + assert result.converted == ("a/stack.tif",) + assert result.copied == ("a/damaged.tif", "a/good.tif") + copies = tmp_path / "copies" / "a" + assert (copies / "good.tif").read_bytes() == good.read_bytes() + assert (copies / "damaged.tif").read_bytes() == damaged.read_bytes() + record = read_conversion_record(tmp_path / "copies")[0] + assert record.mapping == mapping and record.missing_as_singleton + assert record.source_root == (tmp_path / "exp").resolve().as_posix() + assert record.entry("a/damaged.tif").reason == "excluded" + assert record.entry("a/good.tif").reason is None + assert fingerprints(tmp_path / "exp") == before, "the originals are never changed" + + +def test_new_copies_get_the_mode_a_plain_open_gives(tmp_path): + if os.name != "posix": + pytest.skip("POSIX file modes") + plain(tmp_path / "exp" / "img.tif", pixels((2, 4, 4))) + old = os.umask(0o027) + try: + assert convert(tmp_path, AxisMapping("IYX", ("Z", "Y", "X"))).written + finally: + os.umask(old) + for name in ("img.tif", ".morphobricks-conversion.json"): + mode = stat.S_IMODE((tmp_path / "copies" / name).stat().st_mode) + assert mode == 0o640, (name, oct(mode)) + + +def test_the_copy_tree_must_be_outside_the_experiment_and_not_hold_it(tmp_path): + plain(tmp_path / "exp" / "img.tif", pixels((2, 4, 4))) + mapping = AxisMapping("IYX", ("Z", "Y", "X")) + for target in (tmp_path / "exp", tmp_path / "exp" / "copies", tmp_path): + result = convert_experiment(tmp_path / "exp", mapping, target) + assert not result.written + assert [i.code for i in result.issues] == ["copy_folder_overlaps_experiment"] + assert not (tmp_path / "exp" / "copies").exists() + + +def test_another_error_stops_the_conversion_before_anything_is_written(tmp_path): + plain(tmp_path / "exp" / "img.tif", pixels((2, 4, 4))) + result = convert(tmp_path, AxisMapping("IYX", (None, "Y", "X"))) + assert not result.written and "unassigned_axis" in [i.code for i in result.issues] + assert not (tmp_path / "copies").exists() + + +def test_copy_layout_says_why_each_size_is_not_written(tmp_path): + source = ome( + tmp_path / "img.tif", pixels((2, 4, 5)), "CYX", PhysicalSizeX=0.2, PhysicalSizeY=0.2, + PhysicalSizeZ=3.0, + ) # fmt: skip + layout = copy_layout(inspect_image(source), AxisMapping("CYX", ("Z", "Y", "X"))) + assert layout.axes == "ZYX" and layout.plane_count == 2 + assert [s.value for s in layout.sizes] == [0.2, 0.2, None, None] + assert "relabelled" in layout.unknown["Z"] and "header names this axis C" in layout.unknown["Z"] + assert layout.ome_metadata()["PhysicalSizeXUnit"] == "µm" + assert layout.plane_index({"z": 1, "c": None, "t": None}) == 1 + + +def test_an_rgb_image_with_swapped_y_and_x_is_written_as_channel_pages(tmp_path): + data = pixels((6, 9, 3), np.uint8) + source = tmp_path / "exp" / "img.tif" + source.parent.mkdir(parents=True) + tifffile.imwrite(source, data, photometric="rgb", metadata={"axes": "YXS"}) + mapping = AxisMapping("YXS", ("X", "Y", "C")) + result = convert(tmp_path, mapping) + copy = tmp_path / "copies" / "img.tif" + assert result.written and inspect_image(copy).axes == "CYX" + np.testing.assert_array_equal(tifffile.imread(copy), data.transpose(2, 1, 0)) + assert_bitwise_equal(source, mapping, copy) diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py new file mode 100644 index 00000000..1b68f2cb --- /dev/null +++ b/tests/test_convert_run.py @@ -0,0 +1,308 @@ +"""Converting safely, and the run on the copies (design v0.4 §5, §6, PP-11). + +A conversion holds the copy tree's lock, writes ``.partial`` files and moves them into place +together, writes the record last, reuses unchanged copies and refuses to overwrite anything +else. The second run, on the copy tree with the same output folder, reads the copies like +any other images (``axes_from: "header"``), recomputes every row, records the record's +SHA-256 and warns when a source or a copy changed after the conversion. +""" + +import json +import os + +import numpy as np +import pytest +import tifffile + +import morphobricks_preprocessing.conversion as conversion_module +from morphobricks_preprocessing import ( + CONVERSION_RECORD_FILENAME, + AxisMapping, + RunInputs, + convert_experiment, + file_sha256, + read_conversion_record, + run_experiment, +) +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.run.lock import LOCK_FILENAME, output_lock +from morphobricks_preprocessing.testing import _confirmed_design + +SWAPPED = AxisMapping("ZYX", ("Z", "X", "Y")) +PLAIN = AxisMapping("IYX", ("Z", "Y", "X")) + + +def swapped_experiment(root, names=("ctrl/r1/a.tif", "ctrl/r1/b.tif", "drug/r1/a.tif")): + """Images whose OME header names Y and X the other way round from the truth.""" + rng = np.random.default_rng(1) + for name in names: + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + data = rng.integers(0, 255, (2, 6, 9), dtype=np.uint8) + tifffile.imwrite( + path, data, ome=True, photometric="minisblack", + metadata={"axes": "ZYX", "PhysicalSizeZ": 2.0}, + ) # fmt: skip + return root + + +def plain_experiment(root): + rng = np.random.default_rng(2) + for name in ("ctrl/r1/a.tif", "drug/r1/a.tif"): + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + data = rng.integers(0, 255, (3, 7, 5), dtype=np.uint8) + tifffile.imwrite(path, data, photometric="minisblack", metadata=None) + return root + + +def codes(issues): + return [issue.code for issue in issues] + + +def snapshot(folder): + return { + p.relative_to(folder).as_posix(): (p.stat().st_size, p.stat().st_mtime_ns) + for p in sorted(folder.rglob("*")) + if p.is_file() + } + + +# Safe to repeat ------------------------------------------------------------------------- + + +def test_a_second_conversion_reuses_the_unchanged_copies_and_writes_nothing(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + first = convert_experiment(exp, SWAPPED, copies) + assert first.written and len(first.converted) == 3 + before = snapshot(copies) + again = convert_experiment(exp, SWAPPED, copies) + assert again.written and again.converted == () and again.copied == () + assert again.reused == first.converted + files = {k: v for k, v in snapshot(copies).items() if k != CONVERSION_RECORD_FILENAME} + assert files == {k: v for k, v in before.items() if k != CONVERSION_RECORD_FILENAME} + + +def test_a_new_image_is_added_to_an_earlier_copy_tree(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + assert convert_experiment(exp, SWAPPED, copies).written + swapped_experiment(exp, names=("drug/r1/new.tif",)) + again = convert_experiment(exp, SWAPPED, copies) + assert again.written and again.converted == ("drug/r1/new.tif",) + assert len(again.reused) == 3 + record = read_conversion_record(copies)[0] + assert [e.relative_path for e in record.files] == sorted([*again.reused, "drug/r1/new.tif"]) + + +def test_a_different_existing_copy_is_never_overwritten(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + assert convert_experiment(exp, SWAPPED, copies).written + before = snapshot(copies) + # Other roles for the same images: the copies would differ. + other = convert_experiment(exp, AxisMapping("ZYX", ("T", "X", "Y")), copies) + assert not other.written and codes(other.issues)[0] == "copy_differs" + assert set(other.issues[0].paths) == {"ctrl/r1/a.tif", "ctrl/r1/b.tif", "drug/r1/a.tif"} + # A source changed since: refused too. + source = exp / "ctrl" / "r1" / "a.tif" + tifffile.imwrite(source, np.ones((2, 6, 9), np.uint8), ome=True, metadata={"axes": "ZYX"}) + changed = convert_experiment(exp, SWAPPED, copies) + assert not changed.written and changed.issues[0].paths == ("ctrl/r1/a.tif",) + assert snapshot(copies) == before + + +def test_a_folder_holding_other_files_or_another_experiments_copies_is_refused(tmp_path): + exp = swapped_experiment(tmp_path / "exp") + busy = tmp_path / "busy" + busy.mkdir() + (busy / "notes.txt").write_text("mine") + result = convert_experiment(exp, SWAPPED, busy) + assert not result.written and codes(result.issues) == ["copy_folder_not_empty"] + assert sorted(p.name for p in busy.iterdir()) == ["notes.txt"] + other = swapped_experiment(tmp_path / "other") + assert convert_experiment(other, SWAPPED, tmp_path / "copies").written + refused = convert_experiment(exp, SWAPPED, tmp_path / "copies") + assert not refused.written and "another experiment" in refused.issues[0].message + + +def test_a_failed_conversion_leaves_the_folder_as_it_was(tmp_path, monkeypatch): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + original, calls = conversion_module.write_named_copy, [] + + def failing(source, target, *args, **kwargs): + calls.append(target) + if len(calls) == 2: + target.write_bytes(b"half") + raise OSError("disk full") + return original(source, target, *args, **kwargs) + + monkeypatch.setattr(conversion_module, "write_named_copy", failing) + result = convert_experiment(exp, SWAPPED, copies) + assert not result.written and codes(result.issues) == ["conversion_failed"] + assert result.issues[0].paths == ("ctrl/r1/b.tif",) and "disk full" in result.issues[0].message + assert [p for p in copies.rglob("*") if p.is_file()] == [], "no copy, no record, no lock" + monkeypatch.undo() + assert convert_experiment(exp, SWAPPED, copies).written + + +def test_partial_files_left_by_an_interrupted_conversion_are_written_again(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + left = copies / "ctrl" / "r1" / "a.tif.partial" + left.parent.mkdir(parents=True) + left.write_bytes(b"interrupted") + result = convert_experiment(exp, SWAPPED, copies) + assert result.written and not left.exists() + assert tifffile.imread(copies / "ctrl" / "r1" / "a.tif").shape == (2, 9, 6) + + +def test_the_copy_trees_lock_refuses_a_second_conversion(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + copies.mkdir() + with output_lock(copies): + result = convert_experiment(exp, SWAPPED, copies) + assert not result.written and codes(result.issues) == ["output_locked"] + assert not (copies / CONVERSION_RECORD_FILENAME).exists() + assert not (copies / LOCK_FILENAME).exists() + + +# The run on the copies ------------------------------------------------------------------- + + +def test_the_run_on_the_copies_reads_them_with_their_named_axes(tmp_path): + exp, out = swapped_experiment(tmp_path / "exp"), tmp_path / "out" + first = run_experiment(exp, SWAPPED, output_folder=out, design=_confirmed_design(exp)) + assert not first.written and "axes_yx_swapped" in codes(first.issues) + result = convert_experiment(exp, SWAPPED, tmp_path / "copies") + copies = tmp_path / "copies" + run = run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=out) + assert run.written, run.issues + manifest = json.loads((out / "run_manifest.json").read_text()) + assert manifest["experiment_root"] == copies.as_posix() + assert manifest["conversion_record_sha256"] == result.record_sha256 + assert result.record_sha256 == file_sha256(copies / CONVERSION_RECORD_FILENAME) + assert {f["axes_from"] for f in manifest["files"]} == {"header"} + report = json.loads((out / "discovery_report.json").read_text()) + assert report["conversion"]["source_root"] == exp.as_posix() + assert "named-axis copies of" in (out / "discovery_report.txt").read_text() + # The FOV keys and plane identities are the originals' (same relative paths). + inputs = RunInputs.open(out) + assert [f.relative_path for f in inputs.files] == sorted( + p.relative_to(exp).as_posix() for p in exp.rglob("*.tif") + ) + np.testing.assert_array_equal( + inputs.read_plane(inputs.fovs()[0], z=1)[0], + tifffile.imread(exp / "ctrl" / "r1" / "a.tif")[1].T, + ) + + +def test_every_row_is_recomputed_and_equals_the_originals_rows(tmp_path): + exp, out = plain_experiment(tmp_path / "exp"), tmp_path / "out" + design = _confirmed_design(exp) + first = run_experiment(exp, PLAIN, output_folder=out, design=design) + assert first.written and "axes_not_named" in codes(first.issues) + rows = (out / "plane_histograms.csv").read_text() + assert convert_experiment(exp, PLAIN, tmp_path / "copies").written + run = run_experiment(tmp_path / "copies", AxisMapping("ZYX", tuple("ZYX")), output_folder=out) + assert run.written and "axes_not_named" not in codes(run.issues) + assert len(run.read) == 2 and run.unchanged == () + assert "experiment_root" in [c.name for c in run.input_changes] + # Same plane identities and the same histograms: the copies hold the same pixels. + assert (out / "plane_histograms.csv").read_text() == rows + report = json.loads((out / "discovery_report.json").read_text()) + assert report["experiment_root_changed"] and report["design"] is not None + + +def test_the_run_warns_when_a_source_or_a_copy_changed_after_the_conversion(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + assert convert_experiment(exp, SWAPPED, copies).written + (exp / "ctrl" / "r1" / "a.tif").unlink() + copy = copies / "drug" / "r1" / "a.tif" + os.utime(copy, ns=(copy.stat().st_atime_ns, copy.stat().st_mtime_ns + 10**9)) + run = run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=tmp_path / "o") + assert run.written + found = {issue.code: issue for issue in run.issues} + assert found["conversion_source_changed"].paths == ("ctrl/r1/a.tif",) + assert found["copy_changed_after_conversion"].paths == ("drug/r1/a.tif",) + + +def test_an_unreadable_conversion_record_stops_the_run(tmp_path): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + assert convert_experiment(exp, SWAPPED, copies).written + (copies / CONVERSION_RECORD_FILENAME).write_text("{not json") + run = run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=tmp_path / "o") + assert not run.written and "conversion_record_unreadable" in codes(run.issues) + + +def test_masks_are_never_converted_and_are_checked_against_the_copies(tmp_path): + exp = tmp_path / "exp" + path = exp / "c" / "img.tif" + path.parent.mkdir(parents=True) + tifffile.imwrite(path, np.zeros((6, 9), np.uint8), ome=True, metadata={"axes": "YX"}) + mask = tmp_path / "masks" / "Cells" / "c" / "img.tif" + mask.parent.mkdir(parents=True) + tifffile.imwrite(mask, np.ones((9, 6), np.uint16), metadata={"axes": "YX"}) + before = mask.read_bytes() + roles = AxisMapping("YX", ("X", "Y")) + assert convert_experiment(exp, roles, tmp_path / "copies").written + run = run_experiment( + tmp_path / "copies", AxisMapping("YX", ("Y", "X")), output_folder=tmp_path / "out", + masks_folder=tmp_path / "masks", + ) # fmt: skip + assert run.written, run.issues + assert mask.read_bytes() == before + assert not list((tmp_path / "copies").rglob("*Cells*")) + + +# Command line ---------------------------------------------------------------------------- + + +def test_the_convert_command_writes_the_copies_and_names_the_next_commands(tmp_path, capsys): + exp = swapped_experiment(tmp_path / "exp") + copies = tmp_path / "copies" + assert main(["convert", str(exp), "--to", str(copies), "--axes", "ZXY"]) == 0 + out = capsys.readouterr().out + assert "[1/3] ctrl/r1/a.tif" in out + assert "written as OME-TIFF with your roles: 3 images" in out + assert f"morphobricks-preprocessing run {copies} --output" in out and "--axes ZYX" in out + assert main(["convert", str(exp), "--to", str(copies), "--axes", "ZXY"]) == 0 + assert "already there and unchanged: 3 images" in capsys.readouterr().out + + +def test_the_convert_command_refuses_and_exits_1(tmp_path, capsys): + exp = swapped_experiment(tmp_path / "exp") + assert main(["convert", str(exp), "--to", str(exp / "inside"), "--axes", "ZXY"]) == 1 + assert "The copies must be kept apart from the originals" in capsys.readouterr().out + assert main(["convert", str(exp), "--to", str(tmp_path / "c"), "--axes", "ZX"]) == 1 + assert "does not fit the images" in capsys.readouterr().out + + +@pytest.mark.skipif(os.name != "posix", reason="a stale lock of a finished process") +def test_the_convert_command_removes_a_stale_lock_only_on_request(tmp_path, capsys): + from run_files import finished_pid + + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + copies.mkdir() + lock = copies / LOCK_FILENAME + lock.write_text(json.dumps({"host": os.uname().nodename, "pid": finished_pid()})) + assert main(["convert", str(exp), "--to", str(copies), "--axes", "ZXY"]) == 1 + assert "Another conversion is writing into this folder" in capsys.readouterr().out + args = ["convert", str(exp), "--to", str(copies), "--axes", "ZXY", "--remove-stale-lock"] + assert main(args) == 0 + assert "Removed the lock" in capsys.readouterr().out + + +def test_an_ome_zarr_image_is_copied_as_it_is(tmp_path): + from zarr_tree import write_ngff + + exp = plain_experiment(tmp_path / "exp") + store = write_ngff(exp / "ctrl" / "r1" / "z.zarr", np.ones((3, 7, 5), np.uint8), "zyx") + result = convert_experiment(exp, PLAIN, tmp_path / "copies") + assert result.written and result.copied == ("ctrl/r1/z.zarr",) + copy = tmp_path / "copies" / "ctrl" / "r1" / "z.zarr" + found = {p.relative_to(copy).as_posix(): p.read_bytes() for p in copy.rglob("*") if p.is_file()} + assert found == { + p.relative_to(store).as_posix(): p.read_bytes() for p in store.rglob("*") if p.is_file() + } + run = run_experiment( + tmp_path / "copies", AxisMapping("ZYX", tuple("ZYX")), output_folder=tmp_path / "o" + ) + assert run.written, run.issues From 4c28040ead5437a83bd79360c7537e36f33ae426 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 13:58:55 +0000 Subject: [PATCH 03/13] Review PP-11: move the conversion's messages to conversion_issues (pure) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conversion.py was at 579 of the 600 lines §9 allows; the review's fixes need room. The error builders move unchanged (one per refusal), and the layer is declared. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- pyproject.toml | 1 + src/morphobricks_preprocessing/conversion.py | 143 +++--------------- .../conversion_issues.py | 136 +++++++++++++++++ 3 files changed, 160 insertions(+), 120 deletions(-) create mode 100644 src/morphobricks_preprocessing/conversion_issues.py diff --git a/pyproject.toml b/pyproject.toml index 88bf6c76..d0c42afc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -158,6 +158,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.cli_design" = "ui" "morphobricks_preprocessing.cli_convert" = "ui" "morphobricks_preprocessing.conversion" = "orchestration" +"morphobricks_preprocessing.conversion_issues" = "pure" "morphobricks_preprocessing.conversion_record" = "io" "morphobricks_preprocessing.copy_axes" = "pure" "morphobricks_preprocessing.ome_tiff_copy" = "io" diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index bfdf0675..5f89d55d 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -41,10 +41,21 @@ from .axes import AxisMapping, prefill_mapping from .axis_check import AXIS_ERROR_CODES, ConfirmedAxes, confirmation_of, header_axis_issue from .axis_review import review_axes +from .conversion_issues import ( + differs_issue, + failed_issue, + lock_issue, + not_a_folder_issue, + not_empty_issue, + other_experiment_issue, + overlap_issue, + pixel_type_issue, + record_issue, + space_issue, +) from .conversion_record import ( CONVERTED, COPIED, - RECORD_FILENAME, ConversionRecord, ConversionRecordError, CopyEntry, @@ -59,7 +70,7 @@ from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError from .run.lock import LOCK_FILENAME, OutputLockedError, output_lock from .run.preflight import preflight -from .validation import ValidationIssue, error, has_errors, plural +from .validation import ValidationIssue, has_errors logger = logging.getLogger(__name__) @@ -182,7 +193,7 @@ def convert_experiment( options = _Options(confirmation_of(mapping)[0], missing_as_singleton, memory_limit) return _locked_conversion(root, dest, planned, issues, options, progress) except OutputLockedError as exc: - return ConversionResult(dest, False, (_lock_issue(exc),)) + return ConversionResult(dest, False, (lock_issue(exc),)) @dataclass(frozen=True) @@ -236,7 +247,7 @@ def plan_conversion( headers = {meta.relative_path: meta for meta in pre.metadata.files} wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) if wide: - return (), [_pixel_type_issue(wide, headers[wide[0]].dtype.name), *issues] + return (), [pixel_type_issue(wide, headers[wide[0]].dtype.name), *issues] planned = [] for rel in pre.discovery.images: source = Fingerprint.of(root / rel) @@ -279,7 +290,7 @@ def _locked_conversion( try: earlier = read_record(dest) except ConversionRecordError as exc: - return ConversionResult(dest, False, (_record_issue(exc, dest),)) + return ConversionResult(dest, False, (record_issue(exc, dest),)) reused, to_write, refusal = _compare(root, dest, planned, earlier) refusal = refusal or _space_issue(dest, to_write) if refusal is not None: @@ -320,10 +331,10 @@ def _compare( """Split the plan into copies to reuse and to write; refuse anything else there.""" if earlier is None: unknown = _unknown_content(dest, planned) - return [], list(planned), _not_empty_issue(dest, unknown) if unknown else None + return [], list(planned), not_empty_issue(dest, unknown) if unknown else None record = earlier[0] if record.source_root != root.as_posix(): - return [], [], _other_experiment_issue(dest, record.source_root) + return [], [], other_experiment_issue(dest, record.source_root) reused, to_write, differing = [], [], [] for item in planned: old = record.entry(item.relative_path) @@ -337,7 +348,7 @@ def _compare( paths = [item.relative_path for item in differing] names = {item.relative_path for item in planned} paths += [e.relative_path for e in record.files if e.relative_path not in names] - return reused, to_write, _differs_issue(dest, sorted(paths)) if paths else None + return reused, to_write, differs_issue(dest, sorted(paths)) if paths else None def _unknown_content(dest: Path, planned: tuple[PlannedCopy, ...]) -> list[str]: @@ -388,7 +399,7 @@ def _write_all( logger.debug("Conversion of %s failed", item.relative_path, exc_info=True) for partial in [*partials, _partial(dest, item)]: _remove(partial) - raise _ConversionFailed(_failed_issue(item.relative_path, exc)) from exc + raise _ConversionFailed(failed_issue(item.relative_path, exc)) from exc if progress is not None: progress(len(to_write), len(to_write), "") entries = [] @@ -452,26 +463,9 @@ def _suggested_axes(files: list[CopyEntry]) -> str | None: def _folder_issues(root: Path, dest: Path) -> list[ValidationIssue]: """Refuse a copy tree inside the experiment, holding it, or that is a file.""" if dest == root or root in dest.parents or dest in root.parents: - return [ - error( - "copy_folder_overlaps_experiment", - "The folder for the named-axis copies is the experiment folder, inside it, or " - "holds it. The copies must be kept apart from the originals, which are never " - "changed.", - [dest.as_posix()], - "Choose a new folder outside the experiment, for example " - f"{root.with_name(root.name + '_named_axes')}.", - ) - ] + return [overlap_issue(root, dest)] if dest.exists() and not dest.is_dir(): - return [ - error( - "copy_folder_not_a_folder", - "The path given for the named-axis copies is a file, not a folder.", - [dest.as_posix()], - "Choose a new folder for the copies.", - ) - ] + return [not_a_folder_issue(dest)] return [] @@ -480,98 +474,7 @@ def _space_issue(dest: Path, to_write: list[PlannedCopy]) -> ValidationIssue | N needed, free = sum(item.nbytes for item in to_write), free_space(dest) if free is None or needed <= free: return None - return error( - "not_enough_space", - f"The copies need about {needed / 2**30:.1f} GiB, but the drive holding {dest} has " - f"{free / 2**30:.1f} GiB free. Nothing was written.", - [dest.as_posix()], - "Free some space, or choose a folder on a drive with more space.", - ) - - -def _pixel_type_issue(paths: list[str], dtype: str) -> ValidationIssue: - """Refuse to convert images of a pixel type that OME-TIFF cannot hold.""" - return error( - "cannot_convert_pixel_type", - f"{plural(len(paths), 'image')} with axis problems have the pixel type {dtype}, which " - "OME-TIFF cannot hold, and pixel values are never changed to fit. Nothing was " - "written.", - paths, - "Save these images again with their axes named in their metadata (for example as " - "OME-TIFF in a 32-bit type, if their values fit), or correct the roles.", - ) - - -def _not_empty_issue(dest: Path, paths: list[str]) -> ValidationIssue: - """Refuse a folder that holds files and no conversion record.""" - return error( - "copy_folder_not_empty", - f"The folder for the named-axis copies already holds {plural(len(paths), 'file')} " - "and no conversion record, so it is not a copy tree this conversion can complete. " - "Nothing was overwritten.", - paths, - f"Choose a new or empty folder for the copies, or empty {dest} first.", - ) - - -def _differs_issue(dest: Path, paths: list[str]) -> ValidationIssue: - """Refuse to overwrite copies that differ from the ones this conversion would write.""" - return error( - "copy_differs", - f"{plural(len(paths), 'image')} in {dest} differ from the copies this conversion " - "would write: their source or roles changed, the copy was changed, or the image is no " - "longer in the experiment. Nothing was overwritten.", - paths, - "Convert into a new, empty folder, or remove these copies first (with the conversion " - f"record {RECORD_FILENAME} if you remove the whole folder's contents).", - ) - - -def _other_experiment_issue(dest: Path, source_root: str) -> ValidationIssue: - """Refuse a copy tree of another experiment.""" - return error( - "copy_differs", - f"{dest} holds named-axis copies of another experiment ({source_root}). Nothing was " - "overwritten.", - [RECORD_FILENAME], - "Choose a new, empty folder for the copies of this experiment.", - ) - - -def _record_issue(exc: ConversionRecordError, dest: Path) -> ValidationIssue: - """Refuse a copy tree whose conversion record cannot be read.""" - return error( - "conversion_record_unreadable", - f"The conversion record in {dest} cannot be read ({exc}), so its copies cannot be " - "checked. Nothing was overwritten.", - [RECORD_FILENAME], - "Choose a new, empty folder for the copies.", - ) - - -def _failed_issue(path: str, exc: Exception) -> ValidationIssue: - """Say that a copy could not be written, and that nothing was kept.""" - return error( - "conversion_failed", - f"{path} could not be copied ({exc}). The copies written so far in this conversion " - "were removed; the folder is as it was.", - [path], - "Check that the image can be read and is not being changed, that the folder can be " - "written, then convert again (or exclude the image).", - ) - - -def _lock_issue(exc: OutputLockedError) -> ValidationIssue: - """Refuse a conversion while another one writes into the same folder.""" - return error( - "output_locked", - f"Another conversion is writing into this folder: {exc.holder.describe()}. Nothing " - "was done.", - [exc.path.as_posix()], - "Wait for it to finish. If none is going on (it was stopped or the computer " - "restarted), give --remove-stale-lock to the command, or call " - "remove_stale_lock(folder), then convert again.", - ) + return space_issue(dest, needed, free) def _sorted(issues: Iterable[ValidationIssue]) -> tuple[ValidationIssue, ...]: diff --git a/src/morphobricks_preprocessing/conversion_issues.py b/src/morphobricks_preprocessing/conversion_issues.py new file mode 100644 index 00000000..d6d4ff8a --- /dev/null +++ b/src/morphobricks_preprocessing/conversion_issues.py @@ -0,0 +1,136 @@ +"""The messages of :mod:`.conversion`: why a conversion was refused or failed (contract §17). + +Each function builds one error, in plain language, naming the paths and an action. Nothing +here reads or writes anything. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +from .conversion_record import RECORD_FILENAME +from .validation import ValidationIssue, error, plural + +if TYPE_CHECKING: + from .conversion_record import ConversionRecordError + from .run.lock import OutputLockedError + + +def overlap_issue(root: Path, dest: Path) -> ValidationIssue: + """Refuse a copy tree that is the experiment folder, inside it, or holds it.""" + return error( + "copy_folder_overlaps_experiment", + "The folder for the named-axis copies is the experiment folder, inside it, or " + "holds it. The copies must be kept apart from the originals, which are never " + "changed.", + [dest.as_posix()], + "Choose a new folder outside the experiment, for example " + f"{root.with_name(root.name + '_named_axes')}.", + ) + + +def not_a_folder_issue(dest: Path) -> ValidationIssue: + """Refuse a path for the copies that is a file.""" + return error( + "copy_folder_not_a_folder", + "The path given for the named-axis copies is a file, not a folder.", + [dest.as_posix()], + "Choose a new folder for the copies.", + ) + + +def space_issue(dest: Path, needed: int, free: int) -> ValidationIssue: + """Refuse a conversion that would not fit on the drive of the copy tree.""" + return error( + "not_enough_space", + f"The copies need about {needed / 2**30:.1f} GiB, but the drive holding {dest} has " + f"{free / 2**30:.1f} GiB free. Nothing was written.", + [dest.as_posix()], + "Free some space, or choose a folder on a drive with more space.", + ) + + +def pixel_type_issue(paths: list[str], dtype: str) -> ValidationIssue: + """Refuse to convert images of a pixel type that OME-TIFF cannot hold.""" + return error( + "cannot_convert_pixel_type", + f"{plural(len(paths), 'image')} with axis problems have the pixel type {dtype}, which " + "OME-TIFF cannot hold, and pixel values are never changed to fit. Nothing was " + "written.", + paths, + "Save these images again with their axes named in their metadata (for example as " + "OME-TIFF in a 32-bit type, if their values fit), or correct the roles.", + ) + + +def not_empty_issue(dest: Path, paths: list[str]) -> ValidationIssue: + """Refuse a folder that holds files and no conversion record.""" + return error( + "copy_folder_not_empty", + f"The folder for the named-axis copies already holds {plural(len(paths), 'file')} " + "and no conversion record, so it is not a copy tree this conversion can complete. " + "Nothing was overwritten.", + paths, + f"Choose a new or empty folder for the copies, or empty {dest} first.", + ) + + +def differs_issue(dest: Path, paths: list[str]) -> ValidationIssue: + """Refuse to overwrite copies that differ from the ones this conversion would write.""" + return error( + "copy_differs", + f"{plural(len(paths), 'image')} in {dest} differ from the copies this conversion " + "would write: their source or roles changed, the copy was changed, or the image is no " + "longer in the experiment. Nothing was overwritten.", + paths, + "Convert into a new, empty folder, or remove these copies first (with the conversion " + f"record {RECORD_FILENAME} if you remove the whole folder's contents).", + ) + + +def other_experiment_issue(dest: Path, source_root: str) -> ValidationIssue: + """Refuse a copy tree of another experiment.""" + return error( + "copy_differs", + f"{dest} holds named-axis copies of another experiment ({source_root}). Nothing was " + "overwritten.", + [RECORD_FILENAME], + "Choose a new, empty folder for the copies of this experiment.", + ) + + +def record_issue(exc: ConversionRecordError, dest: Path) -> ValidationIssue: + """Refuse a copy tree whose conversion record cannot be read.""" + return error( + "conversion_record_unreadable", + f"The conversion record in {dest} cannot be read ({exc}), so its copies cannot be " + "checked. Nothing was overwritten.", + [RECORD_FILENAME], + "Choose a new, empty folder for the copies.", + ) + + +def failed_issue(path: str, exc: Exception) -> ValidationIssue: + """Say that a copy could not be written, and that nothing was kept.""" + return error( + "conversion_failed", + f"{path} could not be copied ({exc}). The copies written so far in this conversion " + "were removed; the folder is as it was.", + [path], + "Check that the image can be read and is not being changed, that the folder can be " + "written, then convert again (or exclude the image).", + ) + + +def lock_issue(exc: OutputLockedError) -> ValidationIssue: + """Refuse a conversion while another one writes into the same folder.""" + return error( + "output_locked", + f"Another conversion is writing into this folder: {exc.holder.describe()}. Nothing " + "was done.", + [exc.path.as_posix()], + "Wait for it to finish. If none is going on (it was stopped or the computer " + "restarted), give --remove-stale-lock to the command, or call " + "remove_stale_lock(folder), then convert again.", + ) From 805cee2e9af9b4145fca2aa18f32004b3193d7a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 13:59:56 +0000 Subject: [PATCH 04/13] Review PP-11: any failure or interruption leaves the copy tree as it was A ValueError from tifffile (for example a file too large for a classic TIFF) or an interruption left a .partial file and the folders made behind, and a failure while moving the copies into place left the copies moved so far without a record. Now every .partial file, every copy moved (each where nothing was before) and every folder made is removed on any exception; read and write failures (ValueError included) are conversion_failed, anything else is re-raised. Regression tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/conversion.py | 72 ++++++++++++++------ tests/test_convert_run.py | 49 +++++++++++++ 2 files changed, 102 insertions(+), 19 deletions(-) diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index 5f89d55d..d247d21b 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -377,6 +377,12 @@ def __init__(self, issue: ValidationIssue) -> None: self.issue = issue +_FAILURES = (PlaneReadError, CopyWriteError, OSError, ValueError) +"""What reading a source or writing a copy raises when it fails (``tifffile`` raises +:class:`ValueError` for a file it cannot write); anything else is re-raised, after the same +clean-up.""" + + def _write_all( root: Path, dest: Path, @@ -386,35 +392,64 @@ def _write_all( ) -> list[CopyEntry]: """Write every copy as ``.partial``, then move them all into place. - On a failure the ``.partial`` files written so far are removed, so the copy tree is - left as it was, and :class:`_ConversionFailed` is raised. + On any failure, an interruption included, every ``.partial`` file of the conversion, + every copy already moved into place (each where nothing was before) and every folder + the conversion made are removed, so the copy tree is left as it was; a failure to read + or write raises :class:`_ConversionFailed`, anything else is re-raised. """ - partials: list[Path] = [] + made = _missing_folders(dest, to_write) + written: list[Path] = [] + current = to_write[0].relative_path if to_write else "" try: for done, item in enumerate(to_write): + current = item.relative_path if progress is not None: - progress(done, len(to_write), item.relative_path) - partials.append(_write_partial(root, dest, item, memory_limit)) - except (PlaneReadError, CopyWriteError, OSError, _SourceChanged) as exc: - logger.debug("Conversion of %s failed", item.relative_path, exc_info=True) - for partial in [*partials, _partial(dest, item)]: - _remove(partial) - raise _ConversionFailed(failed_issue(item.relative_path, exc)) from exc - if progress is not None: - progress(len(to_write), len(to_write), "") - entries = [] - for item, partial in zip(to_write, partials, strict=True): - target = dest / item.relative_path - os.replace(partial, target) - entries.append(replace(item.entry, copy=Fingerprint.of(target))) + progress(done, len(to_write), current) + _write_partial(root, dest, item, memory_limit) + if progress is not None: + progress(len(to_write), len(to_write), "") + entries = [] + for item in to_write: + current, target = item.relative_path, dest / item.relative_path + os.replace(_partial(dest, item), target) + written.append(target) + entries.append(replace(item.entry, copy=Fingerprint.of(target))) + except BaseException as exc: + logger.debug("Conversion of %s failed", current, exc_info=True) + for path in [*written, *(_partial(dest, item) for item in to_write)]: + _remove(path) + _remove_empty(made) + if isinstance(exc, (*_FAILURES, _SourceChanged)): + raise _ConversionFailed(failed_issue(current, exc)) from exc + raise return entries +def _missing_folders(dest: Path, to_write: list[PlannedCopy]) -> list[Path]: + """Return the folders of the copy tree that writing these copies will make.""" + missing = set() + for item in to_write: + folder = (dest / item.relative_path).parent + while folder != dest and not folder.exists(): + missing.add(folder) + folder = folder.parent + return sorted(missing, key=lambda folder: len(folder.parts), reverse=True) + + +def _remove_empty(folders: list[Path]) -> None: + """Remove the folders this conversion made, deepest first, when they are empty.""" + for folder in folders: + try: + folder.rmdir() + except OSError: + logger.debug("Folder %s kept: not empty or gone", folder, exc_info=True) + + class _SourceChanged(Exception): """A source image changed since it was checked, or while it was copied.""" -def _write_partial(root: Path, dest: Path, item: PlannedCopy, memory_limit: int) -> Path: +def _write_partial(root: Path, dest: Path, item: PlannedCopy, memory_limit: int) -> None: """Write one copy as ``.partial``, refusing a source that changes meanwhile.""" source, partial = root / item.relative_path, _partial(dest, item) partial.parent.mkdir(parents=True, exist_ok=True) @@ -429,7 +464,6 @@ def _write_partial(root: Path, dest: Path, item: PlannedCopy, memory_limit: int) shutil.copyfile(source, partial) if current_fingerprint(source) != item.entry.source: raise _SourceChanged("it changed while it was copied") - return partial def _partial(dest: Path, item: PlannedCopy) -> Path: diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index 1b68f2cb..c70c0078 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -144,6 +144,55 @@ def failing(source, target, *args, **kwargs): assert convert_experiment(exp, SWAPPED, copies).written +@pytest.mark.parametrize( + ("error", "reported"), + [(ValueError("data too large for non-BigTIFF file"), True), (KeyboardInterrupt(), False)], +) +def test_any_failure_or_interruption_removes_the_partial_files_and_new_folders( + tmp_path, monkeypatch, error, reported +): + # Review of PP-11: an error other than OSError (tifffile raises ValueError) or an + # interruption left a .partial file and the folders behind. + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + original, calls = conversion_module.write_named_copy, [] + + def failing(source, target, *args, **kwargs): + calls.append(target) + if len(calls) == 2: + target.write_bytes(b"half") + raise error + return original(source, target, *args, **kwargs) + + monkeypatch.setattr(conversion_module, "write_named_copy", failing) + if reported: + result = convert_experiment(exp, SWAPPED, copies) + assert not result.written and codes(result.issues) == ["conversion_failed"] + assert "data too large" in result.issues[0].message + else: + with pytest.raises(KeyboardInterrupt): + convert_experiment(exp, SWAPPED, copies) + assert list(copies.iterdir()) == [], "no copy, no folder, no record, no lock" + + +def test_a_failure_while_moving_the_copies_into_place_removes_the_moved_ones(tmp_path, monkeypatch): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + original, moved = os.replace, [] + + def failing(source, target): + moved.append(target) + if len(moved) == 2: + raise OSError("device removed") + return original(source, target) + + monkeypatch.setattr(conversion_module.os, "replace", failing) + result = convert_experiment(exp, SWAPPED, copies) + monkeypatch.undo() + assert not result.written and codes(result.issues) == ["conversion_failed"] + assert result.issues[0].paths == ("ctrl/r1/b.tif",) + assert list(copies.iterdir()) == [] + assert convert_experiment(exp, SWAPPED, copies).written + + def test_partial_files_left_by_an_interrupted_conversion_are_written_again(tmp_path): exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" left = copies / "ctrl" / "r1" / "a.tif.partial" From ed644ed01edeab9db9e983cb53f574a9f47bbd4d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:00:57 +0000 Subject: [PATCH 05/13] Review PP-11: the memory budget fails fast, and settings are strict A plane that cannot be read within the memory limit, not even in blocks, was found only while writing, after the copies before it were written (and then removed). The planes of the images to convert are now checked from their headers as the run checks them (check_planes) before anything is written: plane_over_memory_limit and unsupported_layout stop the conversion, planes_read_in_blocks is reported. A memory_limit that is not a whole number of bytes above 0, or a missing_as_singleton that is not True or False, is refused (ValueError) before anything is read. Criteria of the obsolete issues #76 and #80. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/conversion.py | 54 +++++++++++++++++--- tests/test_convert.py | 32 ++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index d247d21b..adc24b42 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -32,7 +32,7 @@ import logging import os import shutil -from collections.abc import Callable, Iterable +from collections.abc import Callable, Iterable, Mapping from dataclasses import dataclass, field, replace from datetime import UTC, datetime from pathlib import Path @@ -67,7 +67,7 @@ from .manifest import package_versions from .metadata import TiffMetadata from .ome_tiff_copy import CopyWriteError, copy_nbytes, write_named_copy -from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError +from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError, check_planes from .run.lock import LOCK_FILENAME, OutputLockedError, output_lock from .run.preflight import preflight from .validation import ValidationIssue, has_errors @@ -179,12 +179,21 @@ def convert_experiment( Returns ------- ConversionResult + + Raises + ------ + ValueError + If ``missing_as_singleton`` is not True or False, or ``memory_limit`` is not a + whole number of bytes above 0 (nothing is read or written then). """ + _check_arguments(missing_as_singleton, memory_limit) root, dest = Path(experiment_root).resolve(), Path(destination).resolve() refused = _folder_issues(root, dest) if refused: return ConversionResult(dest, False, tuple(refused)) - planned, issues = plan_conversion(root, mapping, dest, exclude, missing_as_singleton) + planned, issues = plan_conversion( + root, mapping, dest, exclude, missing_as_singleton, memory_limit + ) if has_errors(issues): return ConversionResult(dest, False, _sorted(issues)) dest.mkdir(parents=True, exist_ok=True) @@ -196,6 +205,18 @@ def convert_experiment( return ConversionResult(dest, False, (lock_issue(exc),)) +def _check_arguments(missing_as_singleton: object, memory_limit: object) -> None: + """Refuse arguments that are not allowed, before anything is read or written.""" + if not isinstance(missing_as_singleton, bool): + raise ValueError( + f"missing_as_singleton must be True or False, found {missing_as_singleton!r}" + ) + if isinstance(memory_limit, bool) or not isinstance(memory_limit, int) or memory_limit < 1: + raise ValueError( + f"memory_limit must be a whole number of bytes above 0, found {memory_limit!r}" + ) + + @dataclass(frozen=True) class _Options: """What the conversion records and how it reads, besides the plan.""" @@ -211,11 +232,14 @@ def plan_conversion( destination: Path, exclude: Iterable[str] = (), missing_as_singleton: bool = False, + memory_limit: int = DEFAULT_MEMORY_LIMIT, ) -> tuple[tuple[PlannedCopy, ...], list[ValidationIssue]]: """Check the experiment with the confirmed roles and plan each image's copy. Only the headers are read. The axis errors do not stop the conversion: they are what - it corrects. Any other error does, and nothing is planned. + it corrects. Any other error does, and nothing is planned: so does an image to convert + of a pixel type OME-TIFF cannot hold, or whose planes cannot be read within the memory + limit, not even in blocks (checked as the run checks them, before anything is written). Parameters ---------- @@ -229,6 +253,8 @@ def plan_conversion( Images to leave unread. missing_as_singleton As :func:`convert_experiment`. + memory_limit + As :func:`convert_experiment`. Returns ------- @@ -245,9 +271,9 @@ def plan_conversion( issues = [i for i in issues if not i.is_error] # the axis errors are what is corrected to_convert = set(review.axis_error_files) | set(review.confirmed_files) headers = {meta.relative_path: meta for meta in pre.metadata.files} - wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) - if wide: - return (), [pixel_type_issue(wide, headers[wide[0]].dtype.name), *issues] + issues += _cannot_convert(root, to_convert, headers, review.file_mappings, memory_limit) + if has_errors(issues): + return (), issues planned = [] for rel in pre.discovery.images: source = Fingerprint.of(root / rel) @@ -261,6 +287,20 @@ def plan_conversion( return tuple(planned), issues +def _cannot_convert( + root: Path, + to_convert: set[str], + headers: dict[str, TiffMetadata], + mappings: Mapping[str, AxisMapping], + memory_limit: int, +) -> list[ValidationIssue]: + """Return why images cannot be converted (errors) and the planes read in blocks.""" + wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) + if wide: + return [pixel_type_issue(wide, headers[wide[0]].dtype.name)] + return check_planes(root, {rel: mappings[rel] for rel in to_convert}, memory_limit=memory_limit) + + def _converted(meta: TiffMetadata, mapping: AxisMapping, source: Fingerprint) -> PlannedCopy: """Plan the named-axis copy of one image read under its confirmed roles.""" layout = copy_layout(meta, mapping) diff --git a/tests/test_convert.py b/tests/test_convert.py index ef091d3a..c2375f9c 100644 --- a/tests/test_convert.py +++ b/tests/test_convert.py @@ -351,3 +351,35 @@ def test_an_rgb_image_with_swapped_y_and_x_is_written_as_channel_pages(tmp_path) assert result.written and inspect_image(copy).axes == "CYX" np.testing.assert_array_equal(tifffile.imread(copy), data.transpose(2, 1, 0)) assert_bitwise_equal(source, mapping, copy) + + +def test_planes_that_cannot_be_read_within_the_memory_limit_stop_it_before_writing(tmp_path): + # Review of PP-11 (issue #76's criterion): the memory budget fails fast. It used to be + # found while writing, after the copies before it were written. + ome(tmp_path / "exp" / "a.tif", pixels((2, 8, 9)), "ZYX") + compressed = pixels((2, 64, 48)) + tifffile.imwrite( + tmp_path / "exp" / "b.tif", compressed, ome=True, photometric="minisblack", + compression="zlib", rowsperstrip=64, metadata={"axes": "ZYX"}, + ) # fmt: skip + limit = plane_layout(tmp_path / "exp" / "a.tif", ZYX_SWAPPED).memory_estimate + result = convert(tmp_path, ZYX_SWAPPED, memory_limit=limit) + assert not result.written + assert [i.code for i in result.issues if i.is_error] == ["plane_over_memory_limit"] + assert result.issues[0].paths == ("b.tif",) + assert not (tmp_path / "copies").exists(), "nothing written, not even the folder" + + +@pytest.mark.parametrize( + ("keyword", "value"), + [("memory_limit", 0), ("memory_limit", 2.5), ("memory_limit", True), ("memory_limit", "1GiB"), + ("missing_as_singleton", 1), ("missing_as_singleton", "yes")], +) # fmt: skip +def test_settings_that_are_not_allowed_are_refused_before_anything_is_read( + tmp_path, keyword, value +): + # Review of PP-11 (issue #80's criterion): strict settings, never coerced. + plain(tmp_path / "exp" / "img.tif", pixels((2, 4, 5))) + with pytest.raises(ValueError, match=keyword): + convert(tmp_path, AxisMapping("IYX", ("Z", "Y", "X")), **{keyword: value}) + assert not (tmp_path / "copies").exists() From 38e702058709ce8fc95a77d85964fca300a15371 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:01:51 +0000 Subject: [PATCH 06/13] Review PP-11: count the pages' directories when choosing BigTIFF The copy was written as a classic TIFF whenever its pixels took at most 4 GiB less 32 MiB. With many small pages (about 170 bytes of directory each), the file can pass 4 GiB while the pixels do not, and tifffile then refuses to write it. needs_bigtiff adds 1 KiB per page. Regression test at the old threshold with 2**18 pages. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 2 +- .../ome_tiff_copy.py | 28 +++++++++++++++++-- tests/test_convert.py | 14 +++++++++- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index ebbc007a..bb5779f3 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -509,7 +509,7 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | What is converted | Every image with an axis error (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2) or unnamed axes (`axes_not_named`) under the mapping given; it is never taken from the headers. Every other image is copied byte for byte: its header names its axes as confirmed, it is an OME-Zarr image (with its label images), or it is excluded (`exclude`: never read). So the copy tree is the whole experiment. The command and the API are offered for any experiment; an experiment without these images is copied as it is. | | Checks first | The experiment is checked with the mapping as a run would check it (folders, headers, `review_axes`), from the headers only. The axis errors are what the conversion corrects; any other error stops it before anything is written, and so do images of a type OME-TIFF cannot hold (`cannot_convert_pixel_type`: `int64`, `uint64`; pixel values are never changed to fit). | | Copy tree | A folder the user names (`--to`), outside the experiment and not holding it (`copy_folder_overlaps_experiment`; a file is `copy_folder_not_a_folder`): new or empty (else `copy_folder_not_empty`, nothing overwritten), or the copy tree of an earlier conversion of the same experiment. The originals are only read. Each image keeps its relative path and file name (an OME-TIFF may end in `.tif`), so `source_relative_path`, `plane_id` and the FOV key are the originals'. | -| Copies | Uncompressed OME-TIFF, BigTIFF when the pixels take more than 4 GiB less 32 MiB. Axes: every axis with a role in the source's order, then Y and X (`ZCYX`); an axis without a role (length 1) or missing (`missing_as_singleton`) is not added. Each plane is the plane the run reads under the confirmed roles (`iter_planes`), bit for bit: same pixel type (in native byte order), no rescaling, NaN and infinities kept; a Y/X swap and X before Y are written rows (Y) then columns (X). Planes over the memory limit are read in blocks, as the run reads them, and each block is written to its place: no plane is held whole when the run would not hold it. The copy's metadata names every axis, so `check` and `run` read it with `axes_from: "header"`. | +| Copies | Uncompressed OME-TIFF, BigTIFF when the pixels and 1 KiB per page (for its directory) would take more than 4 GiB less 32 MiB. Axes: every axis with a role in the source's order, then Y and X (`ZCYX`); an axis without a role (length 1) or missing (`missing_as_singleton`) is not added. Each plane is the plane the run reads under the confirmed roles (`iter_planes`), bit for bit: same pixel type (in native byte order), no rescaling, NaN and infinities kept; a Y/X swap and X before Y are written rows (Y) then columns (X). Planes over the memory limit are read in blocks, as the run reads them, and each block is written to its place: no plane is held whole when the run would not hold it. The copy's metadata names every axis, so `check` and `run` read it with `axes_from: "header"`. | | Pixel sizes | They follow the roles; a relabelled axis gets an unknown scale, never a guessed one (`copy_axes`). A size the source header gives is written only for the role the header names it for, and only when the user confirmed that role for that same axis; otherwise it is left out and the record says why. A Y/X swap therefore never exchanges the sizes of Y and X: both are unknown in the copy (state them in `calibration.csv`, §9.3, if known). A z-step is never written as a time step. A plain stack's Y and X confirmed Y before X keep the header's sizes. Units are written as OME symbols (`µm`, `s`, ...). Channel names are written when the C axis keeps its role and every channel is named; significant bits as they are. | | Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). A failure (`conversion_failed`, `not_enough_space` before writing) removes the `.partial` files of the conversion, so the folder is left as it was; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | | Repeat | A copy already in the tree is reused when its source, roles and copy are unchanged since the record; a new image is added. Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | diff --git a/src/morphobricks_preprocessing/ome_tiff_copy.py b/src/morphobricks_preprocessing/ome_tiff_copy.py index 28ea532a..a0d4eb47 100644 --- a/src/morphobricks_preprocessing/ome_tiff_copy.py +++ b/src/morphobricks_preprocessing/ome_tiff_copy.py @@ -22,8 +22,13 @@ from .planes import DEFAULT_MEMORY_LIMIT, Block, iter_planes, plane_layout BIGTIFF_FROM = 2**32 - 2**25 -"""Copies whose pixels take more bytes than this are written as BigTIFF (a classic TIFF -addresses at most 4 GiB; the margin leaves room for the headers and the OME-XML).""" +"""Copies whose file would take more bytes than this are written as BigTIFF (a classic +TIFF addresses at most 4 GiB; the margin leaves room for the header and the OME-XML).""" + +PAGE_OVERHEAD = 1024 +"""Bytes allowed per page for its directory and tags besides its pixels (``tifffile`` +writes about 170 in an uncompressed OME-TIFF), so that many small pages cannot overflow a +classic TIFF whose pixels alone would fit.""" class CopyWriteError(OSError): @@ -50,6 +55,23 @@ def copy_nbytes(layout: CopyLayout, dtype: np.dtype) -> int: return count * np.dtype(dtype).itemsize +def needs_bigtiff(layout: CopyLayout, dtype: np.dtype) -> bool: + """Return True when a copy would not fit a classic TIFF, its pages' directories counted. + + Parameters + ---------- + layout + The copy's layout. + dtype + Its pixel type. + + Returns + ------- + bool + """ + return copy_nbytes(layout, dtype) + layout.plane_count * PAGE_OVERHEAD > BIGTIFF_FROM + + def write_named_copy( source: str | os.PathLike, target: str | os.PathLike, @@ -89,7 +111,7 @@ def write_named_copy( dtype=dtype, ome=True, photometric="minisblack", - bigtiff=copy_nbytes(layout, dtype) > BIGTIFF_FROM, + bigtiff=needs_bigtiff(layout, dtype), metadata=layout.ome_metadata(), returnoffset=True, ) diff --git a/tests/test_convert.py b/tests/test_convert.py index c2375f9c..165b7d4b 100644 --- a/tests/test_convert.py +++ b/tests/test_convert.py @@ -26,7 +26,7 @@ plane_layout, read_conversion_record, ) -from morphobricks_preprocessing.copy_axes import copy_layout +from morphobricks_preprocessing.copy_axes import CopyLayout, copy_layout ZYX_SWAPPED = AxisMapping("ZYX", ("Z", "X", "Y")) @@ -270,6 +270,18 @@ def test_a_large_copy_is_written_as_bigtiff(tmp_path, monkeypatch): assert_bitwise_equal(source, mapping, copy) +def test_many_small_pages_that_would_overflow_a_classic_tiff_make_a_bigtiff(): + # Review of PP-11: pixels at the old threshold in 2**18 pages; their directories + # (about 170 bytes each) would take 44 MB, more than the 32 MiB margin, and tifffile + # would refuse to write a classic TIFF. + many = CopyLayout("TYX", (2**18, 127, 128)) + assert copy_module.copy_nbytes(many, np.dtype(np.uint8)) == copy_module.BIGTIFF_FROM + assert copy_module.needs_bigtiff(many, np.dtype(np.uint8)) + few = CopyLayout("ZYX", (16, 4096, 8192)) + assert not copy_module.needs_bigtiff(few, np.dtype(np.uint16)) + assert copy_module.needs_bigtiff(few, np.dtype(np.float64)) + + # What is copied, and the originals ------------------------------------------------------ From a95eed0d0b6d762378730d179102fca3f333cbe3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:03:13 +0000 Subject: [PATCH 07/13] Review PP-11: equal X and Y sizes are kept through a Y/X swap (owner decision) Owner decision of 2 November 2026 (plan question 59, (1)): when a Y or X is relabelled as the header's other one of Y and X, and the header gives X and Y exactly equal (same value and unit), the size is carried to the copy, since nothing can be exchanged; otherwise both stay unknown, with the reason in the record. Never exchanged, never guessed; other relabelled axes (a z-step on an axis confirmed as T) stay unknown. Tests: OME equal, unequal, other unit, one size only; ImageJ hyperstack equal/unequal. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/copy_axes.py | 39 ++++++++++++++---- tests/test_convert.py | 45 +++++++++++++++++++++ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/morphobricks_preprocessing/copy_axes.py b/src/morphobricks_preprocessing/copy_axes.py index 564d578c..8350558b 100644 --- a/src/morphobricks_preprocessing/copy_axes.py +++ b/src/morphobricks_preprocessing/copy_axes.py @@ -12,9 +12,13 @@ one.** A size the header gives is carried only to the role the header names it for, and only when the user confirmed that role for that same axis. An axis the user gives another role than the header's letter is relabelled: its size is not written, and the - reason is kept for the conversion record. So a Y/X swap never exchanges the sizes of Y - and X (both become unknown), and a z-step is never written as a time step. A plain - stack's positional Y and X keep the header's Y and X sizes when confirmed Y before X. + reason is kept for the conversion record. One exception (owner, 2 November 2026): a Y + or X relabelled from the header's other one of Y and X (a Y/X swap, or a plain stack + confirmed X before Y) keeps the header's size when the header's X and Y sizes are + exactly equal, value and unit, since nothing can be exchanged; otherwise both are + unknown (the user states them in ``calibration.csv``). So a Y/X swap never exchanges + the sizes of Y and X, and a z-step is never written as a time step. A plain stack's + positional Y and X keep the header's Y and X sizes when confirmed Y before X. - **Channel names** are carried when the channel axis keeps its role and every channel is named; the significant bits are carried as they are. @@ -184,7 +188,7 @@ def _carried_sizes( sizes, unknown = [], {} for role in SIZE_ROLES: found = header[role] - reason = _not_carried(meta, mapping, axes, role, found) + reason = _not_carried(meta, mapping, axes, role, header) if found.value is None or reason is not None: sizes.append(RecordedSize(role)) if found.value is not None: @@ -195,22 +199,41 @@ def _carried_sizes( def _not_carried( - meta: TiffMetadata, mapping: AxisMapping, axes: str, role: str, found: RecordedSize + meta: TiffMetadata, + mapping: AxisMapping, + axes: str, + role: str, + header: Mapping[str, RecordedSize], ) -> str | None: """Return why the header's size for ``role`` is not written, or None to write it.""" if role not in axes: return "the copy has no such axis" letter = meta.axes[mapping.index_of(role)] - if letter != role: + if letter != role and not _y_x_equal(header, letter, role): + if {letter, role} == {"X", "Y"}: + return ( + f"relabelled: the header names this axis {letter}, and its X and Y sizes are " + "not the same, so neither is taken (unknown scale)" + ) return ( f"relabelled: the header names this axis {letter}, so its size is not taken for " f"{role} (unknown scale)" ) - if found.unit not in OME_UNITS: - return f"its unit {found.unit!r} cannot be written in OME-XML" + if header[role].unit not in OME_UNITS: + return f"its unit {header[role].unit!r} cannot be written in OME-XML" return None +def _y_x_equal(header: Mapping[str, RecordedSize], letter: str, role: str) -> bool: + """Return True for Y and X relabelled as each other when the header gives them equal.""" + x, y = header["X"], header["Y"] + return ( + {letter, role} == {"X", "Y"} + and x.value is not None + and (x.value, x.unit) == (y.value, y.unit) + ) + + def _channel_names(meta: TiffMetadata, mapping: AxisMapping) -> tuple[str, ...]: """Return the channel names when the C axis keeps its role and every channel is named.""" c = mapping.index_of("C") diff --git a/tests/test_convert.py b/tests/test_convert.py index 165b7d4b..76fb5a02 100644 --- a/tests/test_convert.py +++ b/tests/test_convert.py @@ -141,6 +141,51 @@ def test_a_y_x_swap_is_written_rows_then_columns_and_never_exchanges_sizes(tmp_p assert "relabelled" in entry.unknown_sizes["X"] +@pytest.mark.parametrize( + ("sizes", "expected"), + [ + ({"PhysicalSizeX": 0.25, "PhysicalSizeY": 0.25}, (0.25, 0.25)), + ({"PhysicalSizeX": 0.3, "PhysicalSizeY": 0.4}, (None, None)), + ({"PhysicalSizeX": 0.25, "PhysicalSizeY": 0.25, "PhysicalSizeYUnit": "nm"}, (None, None)), + ({"PhysicalSizeX": 0.25}, (None, None)), + ], +) +def test_a_y_x_swap_keeps_the_sizes_only_when_the_header_gives_x_and_y_equal( + tmp_path, sizes, expected +): + # Owner decision of 2 November 2026 (plan question 59, (1)): equal X and Y sizes, same + # value and unit, are carried (nothing can be exchanged); otherwise both are unknown. + source = ome(tmp_path / "exp" / "img.tif", pixels((2, 6, 8)), "ZYX", PhysicalSizeZ=1.5, **sizes) + assert convert(tmp_path, ZYX_SWAPPED).written + copy = tmp_path / "copies" / "img.tif" + found = inspect_image(copy).physical_sizes + assert (found.x, found.y, found.z) == (*expected, 1.5) + entry = read_conversion_record(tmp_path / "copies")[0].entry("img.tif") + carried = {s.role: (s.value, s.source) for s in entry.pixel_sizes if s.value is not None} + if expected[0] is None: + assert "X" not in carried and "Y" not in carried + assert "Y" not in entry.unknown_sizes or "not the same" in entry.unknown_sizes["Y"] + else: + assert carried["X"] == carried["Y"] == (0.25, "header") and not entry.unknown_sizes + assert_bitwise_equal(source, ZYX_SWAPPED, copy) + + +@pytest.mark.parametrize(("resolution", "expected"), [((2.0, 2.0), 0.5), ((2.0, 4.0), None)]) +def test_an_imagej_hyperstack_swap_keeps_equal_y_and_x_sizes_only(tmp_path, resolution, expected): + source = tmp_path / "exp" / "img.tif" + source.parent.mkdir(parents=True) + tifffile.imwrite( + source, pixels((2, 3, 6, 8)), imagej=True, resolution=resolution, + metadata={"axes": "ZCYX", "unit": "um"}, + ) # fmt: skip + roles = AxisMapping("ZCYX", ("Z", "C", "X", "Y")) + assert convert(tmp_path, roles).written + copy = tmp_path / "copies" / "img.tif" + found = inspect_image(copy).physical_sizes + assert (found.x, found.y) == (expected, expected) + assert_bitwise_equal(source, roles, copy) + + def test_an_imagej_slice_stack_confirmed_as_time_drops_its_z_step(tmp_path): data = pixels((5, 8, 6), np.uint8) source = tmp_path / "exp" / "img.tif" From 1c4629d1b78be24ff383ba93808d563812c3b84f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:03:45 +0000 Subject: [PATCH 08/13] Review PP-11: check a copy's recorded scales before reusing it A rerun reused a copy when its source, roles and copy axes were unchanged, whatever pixel sizes the record gave it, so copies written under another rule (for example before the owner's decision on equal X and Y sizes) could be mixed with new ones. same_plan now also compares the source axes, the copy's pixel sizes and which sizes are unknown; a copy that differs is copy_differs, never reused nor overwritten. Criteria of the obsolete issues #92 and #96. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- .../conversion_record.py | 21 +++++++++------- tests/test_convert_run.py | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/morphobricks_preprocessing/conversion_record.py b/src/morphobricks_preprocessing/conversion_record.py index 4da8624c..cbbfc317 100644 --- a/src/morphobricks_preprocessing/conversion_record.py +++ b/src/morphobricks_preprocessing/conversion_record.py @@ -83,21 +83,26 @@ class CopyEntry: unknown_sizes: Mapping[str, str] = field(default_factory=dict) def same_plan(self, other: CopyEntry) -> bool: - """Return True if ``other`` copies the same source the same way (copies aside).""" + """Return True if ``other`` copies the same source the same way (copies aside). + + The source's fingerprint and axes, the action and reason, the roles, the copy's + axes and its pixel sizes (and which sizes are unknown) must all be the same, so a + copy written under another rule is never reused beside new ones. + """ + return self._plan() == other._plan() + + def _plan(self) -> tuple: + """Return what decides how the copy is written.""" return ( self.relative_path, self.action, self.reason, self.source, + self.source_axes, self.roles, self.copy_axes, - ) == ( - other.relative_path, - other.action, - other.reason, - other.source, - other.roles, - other.copy_axes, + self.pixel_sizes, + frozenset(self.unknown_sizes), ) def to_dict(self) -> dict: diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index c70c0078..f8c5ba19 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -110,6 +110,30 @@ def test_a_different_existing_copy_is_never_overwritten(tmp_path): assert snapshot(copies) == before +def test_a_copy_recorded_with_other_pixel_sizes_is_not_reused(tmp_path): + # Review of PP-11 (issues #92 and #96's criteria): the copy's scales are checked before + # it is reused, so copies written under another rule are never mixed with new ones. + exp, copies = tmp_path / "exp", tmp_path / "copies" + path = exp / "c" / "img.tif" + path.parent.mkdir(parents=True) + tifffile.imwrite( + path, np.zeros((2, 6, 9), np.uint8), ome=True, photometric="minisblack", + metadata={"axes": "ZYX", "PhysicalSizeX": 0.5, "PhysicalSizeY": 0.5}, + ) # fmt: skip + assert convert_experiment(exp, SWAPPED, copies).written + record = json.loads((copies / CONVERSION_RECORD_FILENAME).read_text()) + sizes = record["files"][0]["pixel_sizes"] + assert [s["value"] for s in sizes if s["role"] in "XY"] == [0.5, 0.5] + for size in sizes: + if size["role"] in "XY": + size.update(value=None, unit=None, source=None) + record["files"][0]["unknown_sizes"] = {"X": "relabelled", "Y": "relabelled"} + (copies / CONVERSION_RECORD_FILENAME).write_text(json.dumps(record)) + again = convert_experiment(exp, SWAPPED, copies) + assert not again.written and codes(again.issues) == ["copy_differs"] + assert again.issues[0].paths == ("c/img.tif",) + + def test_a_folder_holding_other_files_or_another_experiments_copies_is_refused(tmp_path): exp = swapped_experiment(tmp_path / "exp") busy = tmp_path / "busy" From 8357a5a2e6af2bc203769a99916e3b11c9e5e99d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:04:34 +0000 Subject: [PATCH 09/13] Review PP-11: RunInputs refuses a conversion record changed or gone (owner decision) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner decision of 2 November 2026 (plan question 59, (14)): a run whose manifest records conversion_record_sha256 is refused by RunInputs.open when the record in the experiment root is missing or changed since the run, as for calibration_sha256 and the design's SHA-256, with a message naming the record and what to do. Copies moved with their record are read with experiment_root=. Contract §9, §9.1 and §17; test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- docs/contract.md | 9 +++---- src/morphobricks_preprocessing/run_inputs.py | 22 ++++++++++++++--- tests/test_convert_run.py | 25 ++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/docs/contract.md b/docs/contract.md index bb5779f3..a7dd80b6 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -175,7 +175,7 @@ does not change either file's schema or statistical interpretation. | `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 (`.morphobricks-conversion.json`) in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5, §17); null otherwise. Information only: the copies are checked by their fingerprints like any image, and a changed record changes no row. | +| `conversion_record_sha256` | SHA-256 of the conversion record (`.morphobricks-conversion.json`) in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5, §17); null otherwise. `RunInputs` checks it (owner decision of 2 November 2026, plan question 59): a record changed or gone since the run is refused, as for `calibration_sha256`. A changed record changes no row (the copies are checked by their fingerprints like any image). | | `calibration_sha256` | SHA-256 of the output folder's `calibration.csv` as the run read it (§9.3); null when there was none (or absent in manifests written before PP-8). `RunInputs` checks it. | | 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). | @@ -193,8 +193,9 @@ is not empty. Bricks read a run's files only through `RunInputs.open(output_folder, experiment_root=None, masks_root=None)` (design v0.4 §6). It refuses (`RunInputsError`) a version 1 manifest, a `plane_histograms.csv`, `experiment_design.csv` or `calibration.csv` whose SHA-256 is not the -manifest's (also a `calibration.csv` added since the run), an image or mask changed or -gone since the run, and a mask recorded without axes (a v0.4.0rc1 run wrote `axes: ""` for a +manifest's (also a `calibration.csv` added since the run), the conversion record of a run +on named-axis copies whose SHA-256 is not the manifest's `conversion_record_sha256` (changed +or gone since the run; §17), an image or mask changed or gone since the run, and a mask recorded without axes (a v0.4.0rc1 run wrote `axes: ""` for a mask whose header puts X before Y; the message says to save the mask again with `numpy.swapaxes` and run again, never "changed or missing"); every read checks the file's fingerprint again. @@ -514,5 +515,5 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). A failure (`conversion_failed`, `not_enough_space` before writing) removes the `.partial` files of the conversion, so the folder is left as it was; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | | Repeat | A copy already in the tree is reused when its source, roles and copy are unchanged since the record; a new image is added. Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | | Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 1: `source_root`, `mapping`, `missing_as_singleton`, `written_at`, `versions`, and per image `relative_path`, `action` (`converted` or `copied`), `reason` (the axis code, `excluded`, or null), `source` and `copy` fingerprints (`size`, `mtime_ns`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9) and `unknown_sizes` (role: why). `read_conversion_record(folder)` reads it with its SHA-256. | -| The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed, went missing or was added since (`copy_changed_after_conversion`); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images. | +| The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed, went missing or was added since (`copy_changed_after_conversion`); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images, and refuses a run whose conversion record changed or went missing since (`conversion_record_sha256`). | | Notebook | Not yet: the axis-check card names the `convert` command; a notebook card is a follow-up (PP-11b). | diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index 733910d6..d7ff1bc7 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -6,8 +6,9 @@ - :meth:`RunInputs.open` refuses a version 1 manifest (a run before the axis check), a table, design or ``calibration.csv`` whose SHA-256 is not the manifest's (also a - ``calibration.csv`` added since the run), an image or mask changed or gone since the - run, a mask recorded without axes (a v0.4.0rc1 run of a mask whose header puts X before + ``calibration.csv`` added since the run), a conversion record changed or gone since a + run on named-axis copies (``conversion_record_sha256``), an image or mask changed or gone + since the run, a mask recorded without axes (a v0.4.0rc1 run of a mask whose header puts X before Y), and an image or mask in the experiment the run neither recorded nor excluded (:class:`UnrecordedFilesError`, from :func:`.unrecorded.unrecorded_files`: folders are listed, no pixel is read). OME-Zarr label images are never skipped: a run holding @@ -34,6 +35,7 @@ from .axes import AxisMapping from .calibration import CALIBRATION_FILENAME +from .conversion_record import RECORD_FILENAME as CONVERSION_RECORD from .design import DESIGN_FILENAME, ExperimentDesign, read_experiment_design from .fingerprint import Fingerprint from .manifest import ( @@ -166,7 +168,8 @@ def open( Raises ------ RunInputsError - For a run before v0.4.0, a table, design or calibration changed since the run, + For a run before v0.4.0, a table, design, calibration or conversion record (of a + run on named-axis copies) changed or gone since the run, an image or mask changed or gone, a mask recorded without axes, OME-Zarr images without the ``zarr`` extra (``morphobricks-preprocessing[zarr]``), or an OME-Zarr image whose ``labels`` group cannot be read. @@ -181,6 +184,7 @@ def open( _check_sha(output / DESIGN_FILENAME, manifest.experiment_design_sha256, "design") _check_calibration(output / CALIBRATION_FILENAME, manifest.calibration_sha256) root = Path(experiment_root or manifest.experiment_root).resolve() + _check_conversion(root, manifest.conversion_record_sha256) recorded_masks = (manifest.masks or {}).get("folder") masks = masks_root or recorded_masks inputs = cls( @@ -470,6 +474,18 @@ def _check_calibration(path: Path, expected: str | None) -> None: ) +def _check_conversion(root: Path, expected: str | None) -> None: + """Refuse a conversion record changed or gone since a run on named-axis copies.""" + path = root / CONVERSION_RECORD + if expected is not None and (not path.exists() or file_sha256(path) != expected): + raise RunInputsError( + f"The conversion record {path.name} in {root} is not the one the run read " + "(missing, or changed since), so it is not known how these named-axis copies " + "were made. Run preprocessing again on the copies (or, if the copies were only " + "moved, give experiment_root= where they are now)." + ) + + def _changed_text(paths: list[str], what: str = "image") -> str: """Say which files changed or went missing since the run.""" where = "experiment_root=" if what == "image" else "masks_root=" diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index f8c5ba19..cc0c9b6a 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -19,6 +19,7 @@ CONVERSION_RECORD_FILENAME, AxisMapping, RunInputs, + RunInputsError, convert_experiment, file_sha256, read_conversion_record, @@ -267,6 +268,30 @@ def test_the_run_on_the_copies_reads_them_with_their_named_axes(tmp_path): ) +def test_run_inputs_refuses_a_conversion_record_changed_or_gone_since_the_run(tmp_path): + # Owner decision of 2 November 2026 (plan question 59, (14)): RunInputs.open checks the + # manifest's conversion_record_sha256, like calibration_sha256 and the design's SHA-256. + exp, out = swapped_experiment(tmp_path / "exp"), tmp_path / "out" + copies = tmp_path / "copies" + assert convert_experiment(exp, SWAPPED, copies).written + assert run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=out).written + record = copies / CONVERSION_RECORD_FILENAME + RunInputs.open(out) # the record as the run read it + moved = tmp_path / "moved" + copies.rename(moved) + RunInputs.open(out, experiment_root=moved) # the copies moved, with their record + text = (moved / CONVERSION_RECORD_FILENAME).read_text() + (moved / CONVERSION_RECORD_FILENAME).write_text(text.replace('"converted"', '"copied"', 1)) + with pytest.raises(RunInputsError, match="conversion record .* changed since"): + RunInputs.open(out, experiment_root=moved) + (moved / CONVERSION_RECORD_FILENAME).unlink() + with pytest.raises(RunInputsError, match="missing"): + RunInputs.open(out, experiment_root=moved) + moved.rename(copies) + record.write_text(text) + RunInputs.open(out) # the same bytes again: accepted + + def test_every_row_is_recomputed_and_equals_the_originals_rows(tmp_path): exp, out = plain_experiment(tmp_path / "exp"), tmp_path / "out" design = _confirmed_design(exp) From b089b64fb89bce4ee50c3d37bddefaad20a46876 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:04:52 +0000 Subject: [PATCH 10/13] Review PP-11: test the free-space check at its boundary not_enough_space had no test: one byte short is refused with nothing written, the exact size is accepted. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_convert_run.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index cc0c9b6a..d66c80d6 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -218,6 +218,18 @@ def failing(source, target): assert convert_experiment(exp, SWAPPED, copies).written +def test_a_conversion_that_would_not_fit_is_refused_before_writing(tmp_path, monkeypatch): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + needed = 3 * 2 * 6 * 9 # three converted images of 2 x 6 x 9 uint8 pixels + monkeypatch.setattr(conversion_module, "free_space", lambda path: needed - 1) + result = convert_experiment(exp, SWAPPED, copies) + assert not result.written and codes(result.issues) == ["not_enough_space"] + assert "Nothing was written" in result.issues[0].message + assert [p for p in copies.rglob("*")] == [] + monkeypatch.setattr(conversion_module, "free_space", lambda path: needed) + assert convert_experiment(exp, SWAPPED, copies).written + + def test_partial_files_left_by_an_interrupted_conversion_are_written_again(tmp_path): exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" left = copies / "ctrl" / "r1" / "a.tif.partial" From 942780e82134fdbd74b7235188e59b529236524d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:06:14 +0000 Subject: [PATCH 11/13] Review PP-11: a conversion reusing every copy keeps its record; no 'use convert' warning in convert A rerun that reused every copy rewrote the record with a new written_at, so its SHA-256 changed: the run on the copies, which recorded it, was then refused by RunInputs. The record is now kept when it would say the same (files, mapping, missing_as_singleton). convert also printed the axes_not_named warning advising to save named-axis copies with the convert command; like the axis errors, it is what the conversion corrects (each entry records it as its reason), so it is no longer reported there. Regression test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- src/morphobricks_preprocessing/conversion.py | 53 ++++++++++++++++---- tests/test_convert_run.py | 16 ++++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index adc24b42..ac808fee 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -39,7 +39,13 @@ from ._io import free_space from .axes import AxisMapping, prefill_mapping -from .axis_check import AXIS_ERROR_CODES, ConfirmedAxes, confirmation_of, header_axis_issue +from .axis_check import ( + AXES_NOT_NAMED, + AXIS_ERROR_CODES, + ConfirmedAxes, + confirmation_of, + header_axis_issue, +) from .axis_review import review_axes from .conversion_issues import ( differs_issue, @@ -268,7 +274,8 @@ def plan_conversion( issues = [*others, *review.combined_issues(pre.metadata)] if any(i.is_error and i.code not in AXIS_ERROR_CODES for i in issues): return (), [i for i in issues if not (i.is_error and i.code in AXIS_ERROR_CODES)] - issues = [i for i in issues if not i.is_error] # the axis errors are what is corrected + # The axis errors and unnamed axes are what is corrected (each entry records its code). + issues = [i for i in issues if not i.is_error and i.code != AXES_NOT_NAMED] to_convert = set(review.axis_error_files) | set(review.confirmed_files) headers = {meta.relative_path: meta for meta in pre.metadata.files} issues += _cannot_convert(root, to_convert, headers, review.file_mappings, memory_limit) @@ -340,15 +347,7 @@ def _locked_conversion( except _ConversionFailed as failed: return ConversionResult(dest, False, (failed.issue, *issues)) files = sorted([*reused, *written], key=lambda entry: entry.relative_path) - record = ConversionRecord( - source_root=root.as_posix(), - mapping=options.mapping, - missing_as_singleton=options.missing_as_singleton, - files=tuple(files), - versions=package_versions(), - written_at=datetime.now(UTC).isoformat(timespec="seconds"), - ) - sha256 = write_record(record, dest) + sha256 = _publish_record(root, dest, files, options, earlier) logger.info("Converted %d images into %s", len(written), dest) return ConversionResult( destination=dest, @@ -362,6 +361,38 @@ def _locked_conversion( ) +def _publish_record( + root: Path, + dest: Path, + files: list[CopyEntry], + options: _Options, + earlier: tuple[ConversionRecord, str] | None, +) -> str: + """Write the conversion record; keep the earlier one when it would say the same. + + A conversion that reuses every copy leaves the record as it was, so a run on the + copies, which recorded its SHA-256, stays valid (``RunInputs`` checks it). + """ + if earlier is not None: + old, sha256 = earlier + same = (old.files, old.mapping, old.missing_as_singleton) == ( + tuple(files), + options.mapping, + options.missing_as_singleton, + ) + if same: + return sha256 + record = ConversionRecord( + source_root=root.as_posix(), + mapping=options.mapping, + missing_as_singleton=options.missing_as_singleton, + files=tuple(files), + versions=package_versions(), + written_at=datetime.now(UTC).isoformat(timespec="seconds"), + ) + return write_record(record, dest) + + def _compare( root: Path, dest: Path, diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index d66c80d6..1b3cac79 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -304,6 +304,22 @@ def test_run_inputs_refuses_a_conversion_record_changed_or_gone_since_the_run(tm RunInputs.open(out) # the same bytes again: accepted +def test_a_conversion_reusing_every_copy_keeps_the_record_so_the_run_stays_valid(tmp_path): + # Review of PP-11: a rerun that reused every copy rewrote the record (a new written_at), + # so its SHA-256 changed and RunInputs then refused the run on the copies. + exp, out = plain_experiment(tmp_path / "exp"), tmp_path / "out" + copies = tmp_path / "copies" + first = convert_experiment(exp, PLAIN, copies) + assert first.written and "axes_not_named" not in codes(first.issues) + assert run_experiment(copies, AxisMapping("ZYX", tuple("ZYX")), output_folder=out).written + record = (copies / CONVERSION_RECORD_FILENAME).read_bytes() + again = convert_experiment(exp, PLAIN, copies) + assert again.written and again.reused == first.converted + assert again.record_sha256 == first.record_sha256 + assert (copies / CONVERSION_RECORD_FILENAME).read_bytes() == record + RunInputs.open(out) + + def test_every_row_is_recomputed_and_equals_the_originals_rows(tmp_path): exp, out = plain_experiment(tmp_path / "exp"), tmp_path / "out" design = _confirmed_design(exp) From fa9692a53104b3e60e9a12823ffa6360a4ad36ca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:08:05 +0000 Subject: [PATCH 12/13] Review PP-11: record the owner's decisions (question 59 decided, PP-11b next) in the docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan question 59 is decided: (1) equal X and Y sizes kept through a Y/X swap, otherwise unknown; (14) RunInputs checks the conversion record; (15) the notebook card is PP-11b, now an entry of plan §10; the other interpretations accepted as built. The PP-11 entry lists the review's changes and the criteria of the obsolete convert_tiffs/zarr_export issues it meets. Contract §17, the user guide and AGENTS.md follow. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 7 +++-- docs/contract.md | 10 +++---- docs/plan.md | 67 +++++++++++++++++++++++++++++----------------- docs/user_guide.md | 11 +++++--- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d8dc076a..2fae5804 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -151,14 +151,17 @@ src/morphobricks_preprocessing/ write_mask_report (mask_relationships.json); DEFAULT_THRESHOLDS, mask_thresholds (checked changes) copy_axes.py convert (PP-11, contract §17): the axes, shape, pixel sizes (by role; a - relabelled axis gets an unknown scale) and OME metadata of a named-axis copy + relabelled axis gets an unknown scale; equal X and Y kept through a Y/X + swap, question 59) and OME metadata of a named-axis copy ome_tiff_copy.py write_named_copy: one image as uncompressed OME-TIFF, planes from iter_planes under the confirmed roles, blocks written in place (BigTIFF when large) conversion_record.py the copy tree's .morphobricks-conversion.json (fingerprints, roles, sizes), review_conversion (the run's conversion_source_changed and copy_changed_after_conversion warnings) conversion.py convert_experiment/plan_conversion: the copy tree under its lock, .partial - files moved into place together, unchanged copies reused, record last + files moved into place together (any failure removes what it wrote), + planes checked against the memory limit first, unchanged copies reused, + record last; conversion_issues.py its messages (pure) zarr_input.py OME-Zarr input: open_ngff (OME-NGFF 0.4/0.5 metadata -> level-0 array, axis letters and physical sizes from the scale and axis units), ngff_label_names/open_ngff_labels (label images, for later bricks), diff --git a/docs/contract.md b/docs/contract.md index a7dd80b6..6284a5c4 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -508,12 +508,12 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | Rule | Detail | |---|---| | What is converted | Every image with an axis error (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2) or unnamed axes (`axes_not_named`) under the mapping given; it is never taken from the headers. Every other image is copied byte for byte: its header names its axes as confirmed, it is an OME-Zarr image (with its label images), or it is excluded (`exclude`: never read). So the copy tree is the whole experiment. The command and the API are offered for any experiment; an experiment without these images is copied as it is. | -| Checks first | The experiment is checked with the mapping as a run would check it (folders, headers, `review_axes`), from the headers only. The axis errors are what the conversion corrects; any other error stops it before anything is written, and so do images of a type OME-TIFF cannot hold (`cannot_convert_pixel_type`: `int64`, `uint64`; pixel values are never changed to fit). | +| Checks first | The experiment is checked with the mapping as a run would check it (folders, headers, `review_axes`), from the headers only. The axis errors are what the conversion corrects; any other error stops it before anything is written, and so do images of a type OME-TIFF cannot hold (`cannot_convert_pixel_type`: `int64`, `uint64`; pixel values are never changed to fit) and images to convert whose planes cannot be read within the memory limit, not even in blocks (`check_planes`: `plane_over_memory_limit`, `unsupported_layout`; `planes_read_in_blocks` is a warning). `memory_limit` must be a whole number of bytes above 0 and `missing_as_singleton` True or False (`ValueError` before anything is read). The axis errors and `axes_not_named` are not reported: each converted image's record entry gives its code. | | Copy tree | A folder the user names (`--to`), outside the experiment and not holding it (`copy_folder_overlaps_experiment`; a file is `copy_folder_not_a_folder`): new or empty (else `copy_folder_not_empty`, nothing overwritten), or the copy tree of an earlier conversion of the same experiment. The originals are only read. Each image keeps its relative path and file name (an OME-TIFF may end in `.tif`), so `source_relative_path`, `plane_id` and the FOV key are the originals'. | | Copies | Uncompressed OME-TIFF, BigTIFF when the pixels and 1 KiB per page (for its directory) would take more than 4 GiB less 32 MiB. Axes: every axis with a role in the source's order, then Y and X (`ZCYX`); an axis without a role (length 1) or missing (`missing_as_singleton`) is not added. Each plane is the plane the run reads under the confirmed roles (`iter_planes`), bit for bit: same pixel type (in native byte order), no rescaling, NaN and infinities kept; a Y/X swap and X before Y are written rows (Y) then columns (X). Planes over the memory limit are read in blocks, as the run reads them, and each block is written to its place: no plane is held whole when the run would not hold it. The copy's metadata names every axis, so `check` and `run` read it with `axes_from: "header"`. | -| Pixel sizes | They follow the roles; a relabelled axis gets an unknown scale, never a guessed one (`copy_axes`). A size the source header gives is written only for the role the header names it for, and only when the user confirmed that role for that same axis; otherwise it is left out and the record says why. A Y/X swap therefore never exchanges the sizes of Y and X: both are unknown in the copy (state them in `calibration.csv`, §9.3, if known). A z-step is never written as a time step. A plain stack's Y and X confirmed Y before X keep the header's sizes. Units are written as OME symbols (`µm`, `s`, ...). Channel names are written when the C axis keeps its role and every channel is named; significant bits as they are. | -| Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). A failure (`conversion_failed`, `not_enough_space` before writing) removes the `.partial` files of the conversion, so the folder is left as it was; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | -| Repeat | A copy already in the tree is reused when its source, roles and copy are unchanged since the record; a new image is added. Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | +| Pixel sizes | They follow the roles; a relabelled axis gets an unknown scale, never a guessed one (`copy_axes`). A size the source header gives is written only for the role the header names it for, and only when the user confirmed that role for that same axis; otherwise it is left out and the record says why. One exception (owner decision of 2 November 2026, plan §12 question 59): a Y or X relabelled as the header's other one of Y and X (a Y/X swap, X before Y) keeps the header's size when the header gives X and Y exactly equal, same value and unit, since nothing can be exchanged; otherwise both are unknown in the copy (state them in `calibration.csv`, §9.3, if known). A Y/X swap therefore never exchanges the sizes of Y and X. A z-step is never written as a time step. A plain stack's Y and X confirmed Y before X keep the header's sizes. Units are written as OME symbols (`µm`, `s`, ...). Channel names are written when the C axis keeps its role and every channel is named; significant bits as they are. | +| Safe | The copy tree's lock (`.morphobricks-preprocessing.lock`, §9.2's rule; `--remove-stale-lock`) is held throughout; a second conversion is `output_locked`. Each copy is written as `.partial`, the source's fingerprint checked before and after; once every copy is written they are moved into place, then the record is written (atomic, with the file-mode rule of §9). `not_enough_space` is refused before writing. Any failure or interruption removes the `.partial` files of the conversion, the copies already moved into place (each where nothing was) and the folders it made, so the folder is left as it was; a read or write failure is `conversion_failed`, anything else is raised again; `.partial` files of an interrupted conversion are written again. New files get the mode a plain `open()` gives. | +| Repeat | A copy already in the tree is reused when its source (fingerprint and axes), roles, copy axes, pixel sizes (and unknown ones) and the copy's fingerprint are as recorded; a new image is added; when every copy is reused the record is kept as it was (same SHA-256). Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | | Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 1: `source_root`, `mapping`, `missing_as_singleton`, `written_at`, `versions`, and per image `relative_path`, `action` (`converted` or `copied`), `reason` (the axis code, `excluded`, or null), `source` and `copy` fingerprints (`size`, `mtime_ns`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9) and `unknown_sizes` (role: why). `read_conversion_record(folder)` reads it with its SHA-256. | | The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed, went missing or was added since (`copy_changed_after_conversion`); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images, and refuses a run whose conversion record changed or went missing since (`conversion_record_sha256`). | -| Notebook | Not yet: the axis-check card names the `convert` command; a notebook card is a follow-up (PP-11b). | +| Notebook | Not yet: the axis-check card names the `convert` command; the notebook card is the next PR (PP-11b, plan §10). | diff --git a/docs/plan.md b/docs/plan.md index 6dbc3582..7e63cc48 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -497,18 +497,33 @@ API, CLI). The notebook and CLI only call the API. planes from `iter_planes`, in blocks over the memory limit, each block written in place; BigTIFF when large); every other image copied byte for byte; masks never converted. Any error other than the axis errors stops it before anything is written. - - Pixel sizes follow the roles; a relabelled axis gets an unknown scale (a Y/X swap - leaves X and Y unknown, never exchanged); channel names kept when C keeps its role. - - Safe: the copy tree's lock; `.partial` files moved into place together once all are - written (a failure leaves the folder as it was); unchanged copies reused, anything else - refused (`copy_differs`, `copy_folder_not_empty`); the conversion record - (`conversion_record`, `.morphobricks-conversion.json`) written last. + - Pixel sizes follow the roles; a relabelled axis gets an unknown scale, except that a Y + or X relabelled as the other keeps the header's size when X and Y are exactly equal + (owner, question 59 (1)); never exchanged; channel names kept when C keeps its role. + - Safe: the copy tree's lock; the planes checked against the memory limit and the free + space before anything is written; `.partial` files moved into place together once all + are written (any failure or interruption leaves the folder as it was); unchanged copies + (source, roles, axes and pixel sizes as recorded) reused, anything else refused + (`copy_differs`, `copy_folder_not_empty`); the conversion record (`conversion_record`, + `.morphobricks-conversion.json`) written last, and kept as it was when every copy is + reused. The messages are in `conversion_issues` (pure). - The run on the copy tree: `conversion_record_sha256` in the manifest, `conversion` in the report, the warnings `conversion_source_changed` and `copy_changed_after_conversion` - (`review_conversion`, from `preflight`), every row recomputed (the read path changes). + (`review_conversion`, from `preflight`), every row recomputed (the read path changes); + `RunInputs.open` refuses a record changed or gone since the run (question 59 (14)). - The axis messages (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, `axes_not_named`) and the notebook's axis marks name the `convert` command. - - Not in this PR: the notebook card for the conversion (PP-11b, proposed). + - Not in this PR: the notebook card for the conversion (PP-11b). + - Review (2 November 2026): the obsolete issues of `convert_tiffs`/`zarr_export` had + acceptance criteria kept here: a memory budget that fails fast (#76), strict settings + (#80), no 64-bit precision loss (refused, #75), the copy's fingerprint and scales + checked before reuse (#87, #88, #92), no mixing of metadata epochs (#96); the run reads + the copies themselves, so the table describes the persisted pixels (#79). +- **PP-11b (notebook card for the conversion)** *(next PR; owner decision of 2 November + 2026, plan §12 question 59 (15))*: design v0.4 §5 "the notebook offers conversion only + for files with errors": a card offered after the axis check when images have an axis + error or unnamed axes, calling `convert_experiment` with the confirmed roles and a + folder the user names, showing the result and the next run on the copies. - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; @@ -1402,36 +1417,37 @@ Raised during review; to be decided before the milestones that need them: so OME-Zarr images can be read there; (5) `RELEASE_TAG` stays `v0.4.0rc2`; the owner tags it after PP-8c and PP-8d are merged.* 59. How are images with axis errors saved as named-axis copies (design v0.4 §5, PP-11)? - *Implemented (2 November 2026, PP-11) as design v0.4 §5 says: images only, as - named-axis OME-TIFF (BigTIFF when large); copies keep the file name and folder + *Decided (owner, 2 November 2026; implemented in PP-11) as design v0.4 §5 says: images + only, as named-axis OME-TIFF (BigTIFF when large); copies keep the file name and folder structure, so `source_relative_path`, `plane_id` and the FOV key stay the same; pixel sizes follow the roles and a relabelled axis gets an unknown scale; `.partial` files, the conversion record written last with source and copy fingerprints and relative paths, a warning when a source changes after conversion; the run on the copies keeps the output folder, reports the new experiment root and recomputes every row, keeping the design, image type and calibration; the CLI and API offer it always. Contract §17.* - *Open for the owner: where §5 is silent, the conservative choice was taken.* - (1) **Pixel sizes of a Y/X swap**: both X and Y are left unknown in the copy (the swap - relabels both axes), so no size is ever exchanged; the user states them in - `calibration.csv`. The axes redesign of 1 November (B2) had proposed that sizes follow - the stored dimension (the header's X size becoming the copy's Y size). Keep unknown, or - adopt B2? + (1) **Pixel sizes after a Y/X swap (or any Y or X relabelled as the other)**: when the + header's X and Y sizes are exactly equal (same value and unit) they are carried to the + copy, since nothing can be exchanged; otherwise both are unknown and the user states + them in `calibration.csv`. Never exchanged, never guessed (the B2 proposal of 1 November, + sizes following the stored dimension, is not adopted). Other relabelled axes (a z-step + on an axis confirmed as T) stay unknown. (2) **Where the copies go**: a folder the user names (`--to`, required; no default), - outside the experiment and not holding it. A default such as `_named_axes` - beside the experiment could be added. + outside the experiment and not holding it. (3) **What the copy tree holds**: the whole experiment, since it becomes the experiment root: converted images, the other images copied byte for byte (OME-Zarr stores with their label images), and excluded images copied unread (they stay excluded by the run). Non-image files are not copied. (4) **Which lock**: the copy tree's own (`.morphobricks-preprocessing.lock` there); `convert` takes no output folder and writes nothing there. - (5) **The record's name**: `.morphobricks-conversion.json`, a dot-file so discovery - skips it without a change to the discovery rules; a visible name would need one. + (5) **The record's name**: `.morphobricks-conversion.json`, a dot-file (hidden), so + discovery skips it without a change to the discovery rules. (6) **A source changed since an earlier conversion into the same folder**: refused (`copy_differs`), never updated in place: convert into a new folder. New images are added and unchanged copies reused. (7) **A conversion killed while moving the copies into place** (copies there, no record): - the folder is refused (`copy_folder_not_empty`) until emptied. + the folder is refused (`copy_folder_not_empty`) until emptied. (A failure or an + interruption that Python sees removes what the conversion wrote; only a killed process + leaves this state.) (8) **64-bit integer images**: refused (`cannot_convert_pixel_type`); OME-TIFF has no such pixel type and values are never changed. (9) **Byte order**: copies are written in the computer's byte order; values are @@ -1444,8 +1460,9 @@ Raised during review; to be decided before the milestones that need them: to the copies: the run on the copies needs `--missing-as-singleton` again. (13) **RGB sources** with an axis error are written with their channels as separate pages (`CYX`), so the suggested image type may change; the recorded type is kept. - (14) **`RunInputs`** does not check the conversion record (the manifest records its - SHA-256; the copies are checked by their fingerprints like any image). - (15) **Notebook card**: left to a follow-up, PP-11b; the axis-check card names the - command. + (14) **`RunInputs` checks the conversion record**: a run whose manifest records + `conversion_record_sha256` is refused by `RunInputs.open` when the record changed or + disappeared, as for `calibration_sha256` and the design's SHA-256. + (15) **Notebook card**: the next PR, PP-11b (§10); the axis-check card names the + command meanwhile. diff --git a/docs/user_guide.md b/docs/user_guide.md index 10bda85b..65d6f434 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -721,8 +721,11 @@ never converted: keep using your masks folder. Pixel sizes follow the roles. An axis you give another role than its metadata gets no size in the copy: after a Y/X swap the copy has no X or Y pixel size (the metadata's X size -belongs to the axis you call Y), and a depth step is never written as a time step. State -the sizes in `calibration.csv` if you know them (`calibrate`). The command prints what it +belongs to the axis you call Y), unless the metadata give X and Y exactly the same size, +which is then kept; a depth step is never written as a time step. State the missing sizes +in `calibration.csv` if you know them (`calibrate`). Images whose planes cannot be read +within the memory limit, and a folder without the space for the copies, are refused +before anything is written. The command prints what it wrote and the next commands, with the suggested `--axes` for the copies. Give `run` the output folder of your runs on the originals: the design, image type and @@ -734,7 +737,9 @@ the run records its SHA-256 and warns when an original changes after the convers (`conversion_source_changed`) or a copy is changed (`copy_changed_after_conversion`). Running `convert` again with the same images and roles reuses the copies and only adds new -images. It never overwrites anything else: a folder holding other files, copies of other +images; when nothing is added the record is left as it was. Bricks reading the run on the +copies (`RunInputs`) refuse it if the conversion record is changed or removed afterwards. +`convert` never overwrites anything else: a folder holding other files, copies of other images or made with other roles is refused (`copy_folder_not_empty`, `copy_differs`); convert into a new folder then. If it fails (`conversion_failed`), the folder is left as it was. Images of 64-bit integers cannot be saved as OME-TIFF (`cannot_convert_pixel_type`). From de920312c153ef39e91cf9417eac2311014e5adc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:03:35 +0000 Subject: [PATCH 13/13] Review PP-11: restore the conversion record as bytes in its RunInputs test write_text added carriage returns on Windows, so the restored record's SHA-256 differed and the test failed there. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_convert_run.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index 1b3cac79..6f65586a 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -292,15 +292,15 @@ def test_run_inputs_refuses_a_conversion_record_changed_or_gone_since_the_run(tm moved = tmp_path / "moved" copies.rename(moved) RunInputs.open(out, experiment_root=moved) # the copies moved, with their record - text = (moved / CONVERSION_RECORD_FILENAME).read_text() - (moved / CONVERSION_RECORD_FILENAME).write_text(text.replace('"converted"', '"copied"', 1)) + data = (moved / CONVERSION_RECORD_FILENAME).read_bytes() + (moved / CONVERSION_RECORD_FILENAME).write_bytes(data.replace(b'"converted"', b'"copied"', 1)) with pytest.raises(RunInputsError, match="conversion record .* changed since"): RunInputs.open(out, experiment_root=moved) (moved / CONVERSION_RECORD_FILENAME).unlink() with pytest.raises(RunInputsError, match="missing"): RunInputs.open(out, experiment_root=moved) moved.rename(copies) - record.write_text(text) + record.write_bytes(data) # bytes: write_text would add CR on Windows RunInputs.open(out) # the same bytes again: accepted