From ef070f7cac83c5428c49dd7ed1cabf9dcf69e9dc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 12:03:32 +0000 Subject: [PATCH] PP-3: split run.py into the run package, moving code only Spec revision 4, section 8 PP-3 (plan-structure audit, section 5). No behaviour change: every function and class is moved as it was, reviewable with git diff --color-moved. - run/preflight.py: Preflight, SamplePlane, preflight, sample_planes, _exclusion_issues, _normalise. - run/pipeline.py: run_experiment. - run/result.py: RunResult. - run/reuse.py: _load_previous, _describe_histogram. - run/rows.py: _read_file, _bin_count, _range_issues, _pixel_issues, _row_order. - run/publish.py: _staged_outputs, _publish_outputs, _OutputRollbackError. - run/__init__.py keeps the module docstring and re-exports the public names, so `from morphobricks_preprocessing.run import ...` still works. Tests now patch the submodule that uses a name: run.rows.iter_xyc_planes, run.pipeline.file_sha256 and the like. tools/check.py --rename OLD=NEW re-keys the baseline entries of moved code; it never adds or raises one. Each new module has a declared layer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a --- AGENTS.md | 11 +- pyproject.toml | 6 + src/morphobricks_preprocessing/run.py | 895 ------------------ .../run/__init__.py | 29 + .../run/pipeline.py | 364 +++++++ .../run/preflight.py | 236 +++++ src/morphobricks_preprocessing/run/publish.py | 91 ++ src/morphobricks_preprocessing/run/result.py | 91 ++ src/morphobricks_preprocessing/run/reuse.py | 89 ++ src/morphobricks_preprocessing/run/rows.py | 131 +++ tests/test_axis_check.py | 6 +- tests/test_histogram_range.py | 6 +- tests/test_image_type.py | 6 +- tests/test_run.py | 30 +- tools/check.py | 24 + tools/code_rules_baseline.json | 11 +- 16 files changed, 1098 insertions(+), 928 deletions(-) delete mode 100644 src/morphobricks_preprocessing/run.py create mode 100644 src/morphobricks_preprocessing/run/__init__.py create mode 100644 src/morphobricks_preprocessing/run/pipeline.py create mode 100644 src/morphobricks_preprocessing/run/preflight.py create mode 100644 src/morphobricks_preprocessing/run/publish.py create mode 100644 src/morphobricks_preprocessing/run/result.py create mode 100644 src/morphobricks_preprocessing/run/reuse.py create mode 100644 src/morphobricks_preprocessing/run/rows.py diff --git a/AGENTS.md b/AGENTS.md index 6b11e5fc..4cbad051 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,7 +97,11 @@ src/morphobricks_preprocessing/ _io.py write_text_atomic (every file the brick writes) and free_space manifest.py RunManifest / RunSettings / FileEntry, JSON read/write, SHA-256 of the CSV, check_sources (images against a run, for other bricks, contract §16) - run.py preflight(), run_experiment() (incremental), sample_planes() for previews + run/ the run (a package since PP-3; `morphobricks_preprocessing.run` re-exports + its public names): preflight.py (Preflight, preflight, sample_planes), + pipeline.py (run_experiment, incremental), result.py (RunResult), + reuse.py (which earlier rows are reused), rows.py (a file's rows and their + pixel checks), publish.py (staged outputs, published together) report.py plain-language summaries shared by CLI and notebook, group statistics tables cli.py, __main__ `morphobricks-preprocessing check|run|masks` (also `python -m morphobricks_preprocessing`) masks.py masks (Q32): find_masks pairs every image with its masks (masks folder with one @@ -547,8 +551,9 @@ Technical details go to `logging` (shown in the notebook's "Technical details" p `write_plate`) writes stores with zarr directly; 0.5 tests skip with zarr 2.18. - **Proving "one page at a time":** spy on `planes._read_page`. To prove "only changed - files are read", spy on `run.iter_xyc_planes` (`run.iter_planes` only converts TIFF - images to OME-Zarr). For planes in blocks, spy on + files are read", spy on `run.rows.iter_xyc_planes` (patch the submodule that uses a + name: `run.pipeline.file_sha256`, `run.publish` for `os`/`shutil`). For planes in blocks, + spy on `planes._read_segment` and make `asarray` of every tifffile class fail. - **Planes in blocks with small test images:** the memory limit `plane_layout(path, mapping, memory_limit=1).block_memory_estimate` gives the smallest diff --git a/pyproject.toml b/pyproject.toml index cd8d1cd8..a0bc77ed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -139,5 +139,11 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.planes" = "io" "morphobricks_preprocessing.report" = "pure" "morphobricks_preprocessing.run" = "orchestration" +"morphobricks_preprocessing.run.pipeline" = "orchestration" +"morphobricks_preprocessing.run.preflight" = "io" +"morphobricks_preprocessing.run.publish" = "io" +"morphobricks_preprocessing.run.result" = "pure" +"morphobricks_preprocessing.run.reuse" = "io" +"morphobricks_preprocessing.run.rows" = "io" "morphobricks_preprocessing.validation" = "pure" "morphobricks_preprocessing.zarr_input" = "io" diff --git a/src/morphobricks_preprocessing/run.py b/src/morphobricks_preprocessing/run.py deleted file mode 100644 index ebaa2e5f..00000000 --- a/src/morphobricks_preprocessing/run.py +++ /dev/null @@ -1,895 +0,0 @@ -"""Run the brick: check an experiment, then write or update its histogram table. - -:func:`preflight` is pass 1: it walks the folder and inspects image headers -without reading pixels, and suggests an axis mapping for the user to confirm. -:func:`run_experiment` is pass 2: with the confirmed mapping it checks -everything again, reads only the images that are new or changed since the last -run, and writes ``plane_histograms.csv`` and ``run_manifest.json`` to the output -folder (``docs/plan.md`` §5–§7). Given the confirmed experiment design, it also -writes ``experiment_design.csv`` and records its hash in the manifest; given the -confirmed image type, it records it there too. After writing, it checks the -images' intensities from the table's rows (:mod:`.intensity`) and records the -limits used. Every plane is binned with the chosen histogram range, and its -range check reports channels whose central intensity range covers too few bins -(``narrow_histogram_range``, follow-up P6). - -Each row depends only on its own plane, so rows of unchanged files are kept as -they are. Everything is recomputed when there is no usable earlier output, when -a rebuild is requested, or when a setting that affects every row (pixel type, -bin count, histogram range, axis mapping) changed. Nothing is written unless the whole run -succeeds; the earlier output is then replaced. -""" - -from __future__ import annotations - -import logging -import os -import shutil -import tempfile -from collections.abc import Callable, Iterable, Iterator -from contextlib import contextmanager -from dataclasses import dataclass, field, replace -from datetime import UTC, datetime -from pathlib import Path - -from morphobricks_normalization.checks import IntensityCheckSettings -from morphobricks_normalization.csv_io import ( - OUTPUT_FILENAME, - SCHEMA_VERSION, - CsvFormatError, - read_plane_histograms, - write_plane_histograms, -) -from morphobricks_normalization.dtypes import default_bin_count, validate_bin_count -from morphobricks_normalization.ranges import HistogramSettings, range_check_issues -from morphobricks_normalization.records import PlaneRecord - -from .axes import ( - AxisMapping, - file_mapping_for, - prefill_experiment_mapping, - review_axes, -) -from .dataset import DatasetReport, check_dataset -from .design import ( - DESIGN_FILENAME, - ExperimentDesign, - design_issues, - reconcile_design, - write_experiment_design, -) -from .discovery import DiscoveryResult, default_output_folder, discover_experiment -from .image_type import check_image_type, image_type_issues, review_image_type -from .intensity import IntensityReport, check_intensities -from .manifest import ( - MANIFEST_FILENAME, - FileEntry, - ManifestFormatError, - NarrowRange, - RunManifest, - RunSettings, - file_entry_for, - file_sha256, - package_versions, - read_manifest, - write_manifest, -) -from .mask_checks import mask_issues -from .masks import MaskInventory, find_masks -from .metadata import MetadataReport, inspect_images -from .planes import ( - DEFAULT_MEMORY_LIMIT, - Plane, - PlaneReadError, - check_planes, - iter_xyc_planes, - read_sample_plane, -) -from .validation import ValidationIssue, error, has_errors, plural, warning - -logger = logging.getLogger(__name__) - - -@dataclass(frozen=True) -class Preflight: - """Pass 1: what was found in the experiment, before any pixel is read. - - Attributes - ---------- - discovery - Folder structure and labels. - metadata - Image headers of the images to analyse (excluded images left out). - excluded - Images the user excluded, sorted. - exclusion_issues - Warnings about excluded images, and an error if nothing is left. - suggested_mapping - Axis mapping prefilled from the headers, or None if the images' - dimensions conflict. - dataset - Images per condition and biological repeat, and the warnings about - their balance. - masks - The masks of the images (contract §15), found from their headers, when - masks were asked for; None otherwise. - """ - - discovery: DiscoveryResult - metadata: MetadataReport - excluded: tuple[str, ...] = () - exclusion_issues: tuple[ValidationIssue, ...] = () - suggested_mapping: AxisMapping | None = None - dataset: DatasetReport = field(default_factory=DatasetReport) - masks: MaskInventory | None = None - - @property - def issues(self) -> tuple[ValidationIssue, ...]: - """All issues found so far, errors first.""" - merged = [ - *self.discovery.issues, - *self.exclusion_issues, - *self.metadata.issues, - *self.dataset.issues, - *(self.masks.issues if self.masks is not None else ()), - ] - return tuple(sorted(merged, key=lambda issue: not issue.is_error)) - - @property - def ok(self) -> bool: - """True if no issue blocks processing.""" - return not has_errors(self.issues) - - -@dataclass(frozen=True) -class RunResult: - """What a run did. - - Attributes - ---------- - output_folder - Where the table and manifest are (or would have been) written. - written - True if the run succeeded and the output was written. - issues - Errors that stopped the run, and warnings, errors first. - new, changed, unchanged, removed - Files compared with the earlier run: not in it, modified since, as they - were, and in it but no longer analysed (deleted, renamed or excluded). - Without a usable earlier manifest, every file is new. - read - Files whose pixels were read in this run. - excluded - Images the user excluded. - rebuild_reason - Why every row was recomputed, or None when the earlier table was updated. - plane_count - Number of rows in the written table. - design - The experiment design written with the table, or None when the run was - given none. - image_type - The image type recorded in the manifest, or None when the run was given - none. - intensity - The intensity checks of the written table (warnings, never errors), or - None when nothing was written. - """ - - output_folder: Path - written: bool - issues: tuple[ValidationIssue, ...] = () - new: tuple[str, ...] = () - changed: tuple[str, ...] = () - unchanged: tuple[str, ...] = () - removed: tuple[str, ...] = () - read: tuple[str, ...] = () - excluded: tuple[str, ...] = () - rebuild_reason: str | None = None - plane_count: int = 0 - design: ExperimentDesign | None = None - image_type: str | None = None - intensity: IntensityReport | None = None - - @property - def ok(self) -> bool: - """True if no issue blocks processing.""" - return not has_errors(self.issues) - - @property - def design_path(self) -> Path | None: - """Location of ``experiment_design.csv``, or None when the run had no design.""" - return self.output_folder / DESIGN_FILENAME if self.design is not None else None - - @property - def csv_path(self) -> Path: - """Location of ``plane_histograms.csv``.""" - return self.output_folder / OUTPUT_FILENAME - - @property - def manifest_path(self) -> Path: - """Location of ``run_manifest.json``.""" - return self.output_folder / MANIFEST_FILENAME - - -@dataclass(frozen=True) -class SamplePlane: - """A plane shown in the axis preview. - - Attributes - ---------- - relative_path - The file the plane comes from. - axes - The file's axis labels; one sample is taken per arrangement of axes. - plane - The plane, or None if it could not be read. - error - Why the plane could not be read, in plain language. - """ - - relative_path: str - axes: str - plane: Plane | None = None - error: str | None = None - - -def sample_planes( - pre: Preflight, - mapping: AxisMapping, - *, - memory_limit: int = DEFAULT_MEMORY_LIMIT, -) -> list[SamplePlane]: - """Read one preview plane per format and arrangement of axes, from the first image. - - Images lacking some of the mapping's axes are previewed with those axes as - length 1, so the user can see them before confirming that this is right. - """ - samples = [] - seen: set[tuple[str, str]] = set() - for f in pre.metadata.files: - if not f.readable or (f.kind, f.axes) in seen: - continue - seen.add((f.kind, f.axes)) - file_mapping = file_mapping_for(f, mapping) - if file_mapping is None: - samples.append( - SamplePlane(f.relative_path, f.axes, error="The axis mapping does not fit it.") - ) - continue - try: - plane = read_sample_plane( - pre.discovery.root / f.relative_path, file_mapping, memory_limit=memory_limit - ) - except PlaneReadError as exc: - logger.debug("Cannot preview %s", f.relative_path, exc_info=True) - samples.append(SamplePlane(f.relative_path, f.axes, error=str(exc))) - continue - samples.append(SamplePlane(f.relative_path, f.axes, plane)) - return samples - - -def preflight( - experiment_root: str | os.PathLike, - output_folder: str | os.PathLike | None = None, - *, - exclude: Iterable[str] = (), - masks_folder: str | os.PathLike | None = None, - ome_zarr_labels: bool = False, -) -> Preflight: - """Pass 1: check the folder and the image headers without reading pixels. - - Parameters - ---------- - experiment_root - The experiment folder. - output_folder - The output folder; defaults to :func:`default_output_folder`. - exclude - Relative paths of images to leave out (``/`` or backslash separators), - for example damaged files. - masks_folder - The masks folder (contract §15): one subfolder per mask type, each - mirroring the experiment's folders. The masks are paired with the images - and checked from their headers; the label images of OME-Zarr images are - included. - ome_zarr_labels - Use the label images of OME-Zarr images as masks, also without a masks - folder. - - Returns - ------- - Preflight - """ - root = Path(experiment_root).resolve() - output = default_output_folder(root) if output_folder is None else Path(output_folder) - discovery = discover_experiment(root, output) - excluded = tuple(sorted({_normalise(path) for path in exclude})) - images = [path for path in discovery.images if path not in excluded] - metadata = inspect_images(root, images) - masks = None - if masks_folder is not None or ome_zarr_labels: - masks = find_masks(masks_folder, discovery, metadata.files) - return Preflight( - discovery=discovery, - metadata=metadata, - excluded=excluded, - exclusion_issues=tuple(_exclusion_issues(discovery, excluded)), - suggested_mapping=prefill_experiment_mapping(metadata), - dataset=check_dataset(discovery, excluded=excluded), - masks=masks, - ) - - -def run_experiment( # noqa: C901, PLR0913, PLR0915 - experiment_root: str | os.PathLike, - mapping: AxisMapping, - *, - output_folder: str | os.PathLike | None = None, - bin_count: int | None = None, - memory_limit: int = DEFAULT_MEMORY_LIMIT, - missing_as_singleton: bool = False, - exclude: Iterable[str] = (), - force_rebuild: bool = False, - progress: Callable[[int, int, str], None] | None = None, - design: ExperimentDesign | None = None, - masks_folder: str | os.PathLike | None = None, - ome_zarr_labels: bool = False, - image_type: str | None = None, - intensity_settings: IntensityCheckSettings | None = None, - histogram_settings: HistogramSettings | None = None, -) -> RunResult: - """Pass 2: write or update the histogram table and the run manifest. - - Parameters - ---------- - experiment_root - The experiment folder. It is never written to. - mapping - The experiment axis mapping the user confirmed. - output_folder - Where to write; defaults to :func:`default_output_folder`. Created if - needed; must not be inside the experiment. - bin_count - Bins per plane; the pixel type's default if None. - memory_limit - Working-memory limit per plane, in bytes. - missing_as_singleton - The user confirmed that files lacking some axes have one plane along them. - exclude - Relative paths of images to leave out. Their rows from earlier runs are - removed. - force_rebuild - Recompute every row even if the earlier output is up to date. - progress - Called as ``progress(done, total, relative_path)`` before each file is - read, and as ``progress(total, total, "")`` when reading is finished. - design - The experiment design the user confirmed: how biological repeats pair - across conditions, and the controls. It is brought up to date with the - folders (:func:`.design.reconcile_design`); the run stops with - ``design_not_confirmed`` while a repeat group is not confirmed, or with - ``repeat_group_conflict``. It is written to ``experiment_design.csv`` - with the table, and its SHA-256 recorded in the manifest. Without a - design nothing is asked, the manifest records none, and an existing - ``experiment_design.csv`` is left as it is. - masks_folder, ome_zarr_labels - As :func:`preflight`. The masks are recorded in the manifest (``masks``, - contract §9); an error about them stops the run, including a mask whose - depth slices and time points are not its image's under the confirmed - roles (``mask_planes_mismatch``). Their pixels are not read: the mask - step (:func:`.mask_scan.scan_masks`) does that. - image_type - The image type the user confirmed, ``"fluorescence"`` or ``"rgb"`` - (:mod:`.image_type`), recorded in the manifest. ``"rgb"`` for images that - are not three channels of an integer type is the error - ``rgb_not_possible``. It changes no row, so changing it recomputes - nothing. Without it the manifest records none. - intensity_settings - Limits of the intensity checks run on the written table - (:func:`.intensity.check_intensities`); None uses the defaults. They are - recorded in the manifest and change no row. - histogram_settings - How histograms that are not exact are binned: the minimum to maximum - (default) or the robust range, and the limits of the range check - (normalization contract §1.4). Exact 8- and 16-bit histograms are the - same either way. Recorded in the manifest; changing it recomputes every - row. After the histograms, a ``narrow_histogram_range`` warning names - each channel whose central range (1st to 99.8th percentile) covers too - few bins in some planes. - - Returns - ------- - RunResult - If any error is found, nothing is written and ``written`` is False. - - Raises - ------ - ValueError - For an image type other than ``"fluorescence"`` and ``"rgb"``, or for - ``missing_as_singleton`` that is not True or False - (the manifest records them as JSON booleans, contract §9). - """ - if image_type is not None: - check_image_type(image_type) - for name, value in (("missing_as_singleton", missing_as_singleton),): - # Refused before anything is read rather than when the manifest is written. - if not isinstance(value, bool): - raise ValueError(f"{name} must be True or False, found {value!r}") - root = Path(experiment_root).resolve() - output = default_output_folder(root) if output_folder is None else Path(output_folder).resolve() - pre = preflight( - root, output, exclude=exclude, masks_folder=masks_folder, ome_zarr_labels=ome_zarr_labels - ) - review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) - type_review = review_image_type(pre.metadata, review) - dataset = pre.dataset - design_problems: list[ValidationIssue] = [] - if design is not None: - design, _ = reconcile_design(design, pre.dataset.repeats) - design = design.dated(datetime.now().date().isoformat()) - design_problems = design_issues(design) - dataset = check_dataset(pre.discovery, design.repeat_groups(), excluded=pre.excluded) - issues = [ - *pre.discovery.issues, - *pre.exclusion_issues, - *review.combined_issues(pre.metadata), - *image_type_issues(image_type, type_review), - *design_problems, - *dataset.issues, - *(mask_issues(pre.masks, pre.metadata, review) if pre.masks is not None else ()), - ] - dtype = pre.metadata.dtype - bins = None - if dtype is not None and not has_errors(issues): - bins, bin_issue = _bin_count(dtype, bin_count) - issues.extend(bin_issue) - if not has_errors(issues): - issues.extend( - check_planes( - root, - review, - memory_limit=memory_limit, - bin_count=bins, - histogram_settings=histogram_settings, - ) - ) - - def stop(**counts) -> RunResult: # noqa: ANN003 - return RunResult(output, False, _sorted(issues), excluded=pre.excluded, **counts) - - if has_errors(issues): - return stop() - - settings = RunSettings( - bins, - memory_limit, - missing_as_singleton, - pre.excluded, - histogram=histogram_settings or HistogramSettings(), - ) - previous, previous_records, reason = _load_previous( - output, settings, mapping, dtype.name, force_rebuild - ) - labels = {image: leaf.labels for leaf in pre.discovery.leaves for image in leaf.images} - in_blocks = {path for i in issues if i.code == "planes_read_in_blocks" for path in i.paths} - headers = {f.relative_path: f for f in pre.metadata.files} - entries: dict[str, FileEntry] = {} - new, changed, unchanged = [], [], [] - for rel, file_mapping in sorted(review.file_mappings.items()): - header = headers[rel] - entries[rel] = file_entry_for( - root, - rel, - labels[rel], - file_mapping, - channel_names=header.channel_names, - significant_bits=header.significant_bits, - read_in_blocks=rel in in_blocks, - channel_names_trimmed=header.channel_names_trimmed, - ) - if header.channel_names_trimmed: - logger.info( - "Leading or trailing whitespace was removed from the channel names of %s", rel - ) - before = previous.file(rel) if previous is not None else None - if before is None: - new.append(rel) - elif ( - before.same_source(entries[rel]) - # During a rebuild there are no rows to check; otherwise the rows must be complete. - and (reason is not None or len(previous_records.get(rel, ())) == before.plane_count) - ): - unchanged.append(rel) - else: - changed.append(rel) - removed = sorted( - f.relative_path - for f in (previous.files if previous else ()) - if f.relative_path not in entries - ) - to_read = sorted([*new, *changed, *(unchanged if reason else ())]) - counts = { - "new": tuple(new), - "changed": tuple(changed), - "unchanged": tuple(unchanged), - "removed": tuple(removed), - "rebuild_reason": reason, - } - - records: list[PlaneRecord] = [] - narrow: list[NarrowRange] = [] - for rel in unchanged if not reason else (): - # Tables written before position identifiers were recorded have them blank; - # the identifier depends only on the path, so it is filled without reading. - records.extend( - replace(record, position_id=entries[rel].position_id) - for record in previous_records[rel] - ) - # Kept rows keep the range checks they were written with. - narrow.extend(entry for entry in previous.narrow_ranges if entry.relative_path == rel) - failed: list[str] = [] - for done, rel in enumerate(to_read): - if progress is not None: - progress(done, len(to_read), rel) - try: - rows, ranges = _read_file( - root / rel, - rel, - labels[rel], - review.file_mappings[rel], - settings, - entries[rel].position_id, - ) - except PlaneReadError: - logger.warning("Cannot read the planes of %s", rel, exc_info=True) - failed.append(rel) - continue - records.extend(rows) - narrow.extend(ranges) - if progress is not None: - progress(len(to_read), len(to_read), "") - if failed: - issues.append( - error( - "unreadable_planes", - f"The pixel data of {plural(len(failed), 'image')} could not be read, although " - "the file headers could. The files may be damaged or incomplete.", - failed, - "Export the images again, or exclude them from the run to continue without them.", - ) - ) - return stop(read=tuple(to_read), **counts) - - issues.extend(_pixel_issues(records)) - narrow.sort(key=lambda entry: (entry.plane_id, entry.channel)) - issues.extend(_range_issues(narrow)) - if has_errors(issues): - return stop(read=tuple(to_read), **counts) - - records.sort(key=_row_order) - # From the rows in memory: no image is read again. - intensity = check_intensities(records, intensity_settings) - per_file: dict[str, int] = {} - for record in records: - per_file[record.source_relative_path] = per_file.get(record.source_relative_path, 0) + 1 - output.mkdir(parents=True, exist_ok=True) - with _staged_outputs(output, with_design=design is not None) as staged: - csv_path = write_plane_histograms(records, staged / OUTPUT_FILENAME) - design_sha256 = None - if design is not None: - design_sha256 = file_sha256(write_experiment_design(design, staged / DESIGN_FILENAME)) - manifest = RunManifest( - experiment_root=root.as_posix(), - dtype=dtype.name, - settings=settings, - mapping=mapping, - files=tuple( - replace(entry, plane_count=per_file[rel]) for rel, entry in sorted(entries.items()) - ), - csv_sha256=file_sha256(csv_path), - versions=package_versions(), - written_at=datetime.now(UTC).isoformat(timespec="seconds"), - experiment_design_sha256=design_sha256, - masks=None if pre.masks is None else pre.masks.to_manifest(), - image_type=image_type, - channels_from_rgb_samples=type_review.channels_from_rgb_samples, - intensity_checks=intensity.settings, - narrow_ranges=tuple(narrow), - ) - write_manifest(manifest, staged / MANIFEST_FILENAME) - logger.info( - "Wrote %d planes (%d files read, %d kept) to %s", - len(records), - len(to_read), - len(records) - sum(per_file[rel] for rel in to_read), - output, - ) - return RunResult( - output, - True, - _sorted(issues), - read=tuple(to_read), - excluded=pre.excluded, - plane_count=len(records), - design=design, - image_type=image_type, - intensity=intensity, - **counts, - ) - - -class _OutputRollbackError(RuntimeError): - """Raised when an output update fails and its earlier files cannot be restored.""" - - -@contextmanager -def _staged_outputs(output: Path, *, with_design: bool = False) -> Iterator[Path]: - """Prepare all outputs before publishing, and retain backups if recovery fails. - - The outputs are written to a temporary folder inside ``output``; ``with_design`` - publishes ``experiment_design.csv`` too. - """ - staged = Path(tempfile.mkdtemp(prefix=".run-", dir=output)) - keep_backups = False - try: - yield staged - _publish_outputs(output, staged, with_design) - except _OutputRollbackError: - keep_backups = True - raise - finally: - if not keep_backups: - try: - shutil.rmtree(staged) - except OSError: - # A leftover temporary directory does not invalidate completed output. - logger.warning("Cannot remove temporary output folder %s", staged, exc_info=True) - - -def _publish_outputs(output: Path, staged: Path, with_design: bool = False) -> None: - """Replace the outputs, restoring their previous state on a publication failure. - - The manifest is published last, so a manifest never names a design that is not in - place. - """ - filenames = (OUTPUT_FILENAME, *((DESIGN_FILENAME,) if with_design else ()), MANIFEST_FILENAME) - backup = staged / "previous" - backup.mkdir() - # Copy before replacing either file, so even a backup failure leaves the - # earlier output intact. Keep backups on the same filesystem for atomic restores. - for name in filenames: - previous = output / name - if previous.exists() or previous.is_symlink(): - shutil.copy2(previous, backup / name, follow_symlinks=False) - published: list[str] = [] - try: - for name in filenames: - os.replace(staged / name, output / name) - published.append(name) - except BaseException: - try: - for name in reversed(published): - if (backup / name).exists() or (backup / name).is_symlink(): - os.replace(backup / name, output / name) - else: - (output / name).unlink() - except BaseException as exc: - raise _OutputRollbackError( - "The output update failed and the earlier files could not be fully restored. " - f"Backup files have been kept in {backup}. Close programs using the output " - "files and restore the backups before running again." - ) from exc - raise - - -def _read_file( - path: Path, - rel: str, - labels: tuple[str, str, str], - mapping: AxisMapping, - settings: RunSettings, - position: str | None, -) -> tuple[list[PlaneRecord], list[NarrowRange]]: - """Histogram every XYC plane of ``path``, recording the rows under the source ``rel``. - - One row per plane holds the histograms of all its channels (schema version 4), - binned with the run's histogram range. Also returns the channel planes whose - range check found the central range too narrow. - """ - records, narrow = [], [] - for plane in iter_xyc_planes( - path, - mapping, - memory_limit=settings.memory_limit, - bin_count=settings.bin_count, - histogram_settings=settings.histogram, - ): - measured = plane.histograms_with_checks(settings.bin_count, settings.histogram) - record = PlaneRecord( - rel, - *labels, - histograms=tuple(histogram for histogram, _ in measured), - z=plane.z, - t=plane.t, - position_id=position, - ) - records.append(record) - narrow.extend( - NarrowRange(rel, record.plane_id, channel, check) - for channel, (_, check) in enumerate(measured) - if check is not None and check.narrow - ) - return records, narrow - - -def _load_previous( - output: Path, settings: RunSettings, mapping: AxisMapping, dtype: str, force: bool -) -> tuple[RunManifest | None, dict[str, list[PlaneRecord]], str | None]: - """Load the earlier manifest and its rows by file, and say why they cannot be reused. - - The manifest is returned whenever it can be read, so files can still be - reported as new, changed or unchanged during a rebuild. - """ - manifest_path, csv_path = output / MANIFEST_FILENAME, output / OUTPUT_FILENAME - if not manifest_path.exists(): - return None, {}, "there is no earlier run in the output folder" - try: - manifest = read_manifest(manifest_path) - except ManifestFormatError: - logger.warning("Cannot read %s", manifest_path, exc_info=True) - return None, {}, "the earlier run manifest could not be read" - if force: - return manifest, {}, "a full rebuild was requested" - if manifest.schema_version != SCHEMA_VERSION: - return manifest, {}, "the earlier table uses a different format version" - if manifest.dtype != dtype: - return manifest, {}, f"the pixel type changed from {manifest.dtype} to {dtype}" - if manifest.settings.bin_count != settings.bin_count: - return ( - manifest, - {}, - f"the bin count changed from {manifest.settings.bin_count:,} to {settings.bin_count:,}", - ) - if manifest.settings.histogram != settings.histogram: - before = _describe_histogram(manifest.settings.histogram) - now = _describe_histogram(settings.histogram) - if before == now: - return manifest, {}, "the limit of the histogram range check changed" - return manifest, {}, f"the histogram range changed from {before} to {now}" - if (manifest.mapping.axes, manifest.mapping.roles) != (mapping.axes, mapping.roles): - return manifest, {}, "the axis mapping changed" - if not csv_path.exists() or file_sha256(csv_path) != manifest.csv_sha256: - return manifest, {}, "the histogram table was changed or replaced after the earlier run" - try: - rows = read_plane_histograms(csv_path) - except CsvFormatError: - logger.warning("Cannot read %s", csv_path, exc_info=True) - return manifest, {}, "the earlier histogram table could not be read" - by_file: dict[str, list[PlaneRecord]] = {} - for record in rows: - by_file.setdefault(record.source_relative_path, []).append(record) - return manifest, by_file, None - - -def _bin_count(dtype, bin_count: int | None) -> tuple[int | None, list[ValidationIssue]]: # noqa: ANN001 - if bin_count is None: - return default_bin_count(dtype), [] - try: - return validate_bin_count(dtype, bin_count), [] - except ValueError as exc: - return None, [ - error( - "invalid_bin_count", - str(exc), - action="Choose a bin count in the allowed range, or use the default.", - ) - ] - - -def _describe_histogram(settings: HistogramSettings) -> str: - """Describe the histogram range in words, e.g. ``"robust (0.01-99.99 %)"``.""" - if settings.robust: - low, high = settings.robust_quantiles - return f"robust ({low:g}-{high:g} %)" - return "minimum to maximum" - - -def _range_issues(narrow: list[NarrowRange]) -> list[ValidationIssue]: - """Return the brick's ``narrow_histogram_range`` warnings, one per channel (N60).""" - found = range_check_issues((entry.plane_id, entry.channel, entry.check) for entry in narrow) - return [warning(issue.code, issue.message, issue.items, issue.action) for issue in found] - - -def _pixel_issues(records: list[PlaneRecord]) -> list[ValidationIssue]: - # Every channel of every row is checked. - channels = [(r.source_relative_path, h) for r in records for h in r.histograms] - issues = [] - with_non_finite = sorted({rel for rel, h in channels if h.non_finite_pixel_count}) - if with_non_finite: - total = sum(h.non_finite_pixel_count for _, h in channels) - issues.append( - warning( - "non_finite_pixels", - f"{plural(len(with_non_finite), 'image')} contain {plural(total, 'pixel')} that " - "are not numbers or infinite (NaN, ±infinity). They are left out of the " - "histograms and counted separately.", - with_non_finite, - "No action needed if these values mark missing data.", - ) - ) - empty = [rel for rel, h in channels if h.is_empty] - if empty and len(empty) == len(channels): - issues.append( - error( - "no_usable_planes", - "No plane has any finite pixel value, so no statistics can be calculated.", - sorted(set(empty)), - "Check that these are the right images and that they were exported correctly.", - ) - ) - elif empty: - planes = {r.plane_id for r in records if any(h.is_empty for h in r.histograms)} - issues.append( - warning( - "no_finite_pixels", - f"{plural(len(planes), 'plane')} have no finite pixel values in at least one " - "channel. These channels are kept in the table with zero counts and left out " - "of group statistics.", - sorted(set(empty)), - "No action needed if these planes are expected to be empty.", - ) - ) - return issues - - -def _exclusion_issues(discovery: DiscoveryResult, excluded: tuple[str, ...]) -> list: - issues = [] - found = [path for path in excluded if path in discovery.images] - unknown = [path for path in excluded if path not in discovery.images] - if found: - issues.append( - warning( - "excluded_images", - f"{plural(len(found), 'image')} {'is' if len(found) == 1 else 'are'} excluded " - "as requested and will not be analysed. Their rows from earlier runs are " - "removed.", - found, - "Remove them from the exclusion list to analyse them again.", - ) - ) - if unknown: - issues.append( - warning( - "excluded_path_not_found", - f"{plural(len(unknown), 'excluded path')} {'does' if len(unknown) == 1 else 'do'} " - "not match any analysed image of the experiment.", - unknown, - "Check the spelling of the paths; they are relative to the experiment folder.", - ) - ) - if discovery.images and not set(discovery.images) - set(excluded): - issues.append( - error( - "no_images", - "Every image of the experiment is excluded, so there is nothing to analyse.", - ["."], - "Remove images from the exclusion list.", - ) - ) - return issues - - -def _normalise(path: str) -> str: - path = str(path).replace("\\", "/") - while path.startswith("./"): - path = path[2:] - return path - - -def _row_order(record: PlaneRecord) -> tuple: - return (record.source_relative_path, record.z, record.t) - - -def _sorted(issues: list[ValidationIssue]) -> tuple[ValidationIssue, ...]: - return tuple(sorted(issues, key=lambda issue: not issue.is_error)) diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py new file mode 100644 index 00000000..f3328421 --- /dev/null +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -0,0 +1,29 @@ +"""Run the brick: check an experiment, then write or update its histogram table. + +:func:`preflight` is pass 1: it walks the folder and inspects image headers +without reading pixels, and suggests an axis mapping for the user to confirm. +:func:`run_experiment` is pass 2: with the confirmed mapping it checks +everything again, reads only the images that are new or changed since the last +run, and writes ``plane_histograms.csv`` and ``run_manifest.json`` to the output +folder (``docs/plan.md`` §5–§7). Given the confirmed experiment design, it also +writes ``experiment_design.csv`` and records its hash in the manifest; given the +confirmed image type, it records it there too. After writing, it checks the +images' intensities from the table's rows (:mod:`.intensity`) and records the +limits used. Every plane is binned with the chosen histogram range, and its +range check reports channels whose central intensity range covers too few bins +(``narrow_histogram_range``, follow-up P6). + +Each row depends only on its own plane, so rows of unchanged files are kept as +they are. Everything is recomputed when there is no usable earlier output, when +a rebuild is requested, or when a setting that affects every row (pixel type, +bin count, histogram range, axis mapping) changed. Nothing is written unless the whole run +succeeds; the earlier output is then replaced. +""" + +from __future__ import annotations + +from .pipeline import run_experiment +from .preflight import Preflight, SamplePlane, preflight, sample_planes +from .result import RunResult + +__all__ = ["Preflight", "RunResult", "SamplePlane", "preflight", "run_experiment", "sample_planes"] diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py new file mode 100644 index 00000000..51fb3b20 --- /dev/null +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -0,0 +1,364 @@ +"""Pass 2: :func:`run_experiment`, which checks everything and writes the table. + +Moved from ``run.py`` (PP-3). +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Callable, Iterable +from dataclasses import replace +from datetime import UTC, datetime +from pathlib import Path + +from morphobricks_normalization.checks import IntensityCheckSettings +from morphobricks_normalization.csv_io import ( + OUTPUT_FILENAME, + write_plane_histograms, +) +from morphobricks_normalization.ranges import HistogramSettings +from morphobricks_normalization.records import PlaneRecord + +from ..axes import ( + AxisMapping, + review_axes, +) +from ..dataset import check_dataset +from ..design import ( + DESIGN_FILENAME, + ExperimentDesign, + design_issues, + reconcile_design, + write_experiment_design, +) +from ..discovery import default_output_folder +from ..image_type import check_image_type, image_type_issues, review_image_type +from ..intensity import check_intensities +from ..manifest import ( + MANIFEST_FILENAME, + FileEntry, + NarrowRange, + RunManifest, + RunSettings, + file_entry_for, + file_sha256, + package_versions, + write_manifest, +) +from ..mask_checks import mask_issues +from ..planes import ( + DEFAULT_MEMORY_LIMIT, + PlaneReadError, + check_planes, +) +from ..validation import ValidationIssue, error, has_errors, plural +from .preflight import preflight +from .publish import _staged_outputs +from .result import RunResult +from .reuse import _load_previous +from .rows import _bin_count, _pixel_issues, _range_issues, _read_file, _row_order + +logger = logging.getLogger(__name__) + + +def run_experiment( # noqa: C901, PLR0913, PLR0915 + experiment_root: str | os.PathLike, + mapping: AxisMapping, + *, + output_folder: str | os.PathLike | None = None, + bin_count: int | None = None, + memory_limit: int = DEFAULT_MEMORY_LIMIT, + missing_as_singleton: bool = False, + exclude: Iterable[str] = (), + force_rebuild: bool = False, + progress: Callable[[int, int, str], None] | None = None, + design: ExperimentDesign | None = None, + masks_folder: str | os.PathLike | None = None, + ome_zarr_labels: bool = False, + image_type: str | None = None, + intensity_settings: IntensityCheckSettings | None = None, + histogram_settings: HistogramSettings | None = None, +) -> RunResult: + """Pass 2: write or update the histogram table and the run manifest. + + Parameters + ---------- + experiment_root + The experiment folder. It is never written to. + mapping + The experiment axis mapping the user confirmed. + output_folder + Where to write; defaults to :func:`default_output_folder`. Created if + needed; must not be inside the experiment. + bin_count + Bins per plane; the pixel type's default if None. + memory_limit + Working-memory limit per plane, in bytes. + missing_as_singleton + The user confirmed that files lacking some axes have one plane along them. + exclude + Relative paths of images to leave out. Their rows from earlier runs are + removed. + force_rebuild + Recompute every row even if the earlier output is up to date. + progress + Called as ``progress(done, total, relative_path)`` before each file is + read, and as ``progress(total, total, "")`` when reading is finished. + design + The experiment design the user confirmed: how biological repeats pair + across conditions, and the controls. It is brought up to date with the + folders (:func:`.design.reconcile_design`); the run stops with + ``design_not_confirmed`` while a repeat group is not confirmed, or with + ``repeat_group_conflict``. It is written to ``experiment_design.csv`` + with the table, and its SHA-256 recorded in the manifest. Without a + design nothing is asked, the manifest records none, and an existing + ``experiment_design.csv`` is left as it is. + masks_folder, ome_zarr_labels + As :func:`preflight`. The masks are recorded in the manifest (``masks``, + contract §9); an error about them stops the run, including a mask whose + depth slices and time points are not its image's under the confirmed + roles (``mask_planes_mismatch``). Their pixels are not read: the mask + step (:func:`.mask_scan.scan_masks`) does that. + image_type + The image type the user confirmed, ``"fluorescence"`` or ``"rgb"`` + (:mod:`.image_type`), recorded in the manifest. ``"rgb"`` for images that + are not three channels of an integer type is the error + ``rgb_not_possible``. It changes no row, so changing it recomputes + nothing. Without it the manifest records none. + intensity_settings + Limits of the intensity checks run on the written table + (:func:`.intensity.check_intensities`); None uses the defaults. They are + recorded in the manifest and change no row. + histogram_settings + How histograms that are not exact are binned: the minimum to maximum + (default) or the robust range, and the limits of the range check + (normalization contract §1.4). Exact 8- and 16-bit histograms are the + same either way. Recorded in the manifest; changing it recomputes every + row. After the histograms, a ``narrow_histogram_range`` warning names + each channel whose central range (1st to 99.8th percentile) covers too + few bins in some planes. + + Returns + ------- + RunResult + If any error is found, nothing is written and ``written`` is False. + + Raises + ------ + ValueError + For an image type other than ``"fluorescence"`` and ``"rgb"``, or for + ``missing_as_singleton`` that is not True or False + (the manifest records them as JSON booleans, contract §9). + """ + if image_type is not None: + check_image_type(image_type) + for name, value in (("missing_as_singleton", missing_as_singleton),): + # Refused before anything is read rather than when the manifest is written. + if not isinstance(value, bool): + raise ValueError(f"{name} must be True or False, found {value!r}") + root = Path(experiment_root).resolve() + output = default_output_folder(root) if output_folder is None else Path(output_folder).resolve() + pre = preflight( + root, output, exclude=exclude, masks_folder=masks_folder, ome_zarr_labels=ome_zarr_labels + ) + review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) + type_review = review_image_type(pre.metadata, review) + dataset = pre.dataset + design_problems: list[ValidationIssue] = [] + if design is not None: + design, _ = reconcile_design(design, pre.dataset.repeats) + design = design.dated(datetime.now().date().isoformat()) + design_problems = design_issues(design) + dataset = check_dataset(pre.discovery, design.repeat_groups(), excluded=pre.excluded) + issues = [ + *pre.discovery.issues, + *pre.exclusion_issues, + *review.combined_issues(pre.metadata), + *image_type_issues(image_type, type_review), + *design_problems, + *dataset.issues, + *(mask_issues(pre.masks, pre.metadata, review) if pre.masks is not None else ()), + ] + dtype = pre.metadata.dtype + bins = None + if dtype is not None and not has_errors(issues): + bins, bin_issue = _bin_count(dtype, bin_count) + issues.extend(bin_issue) + if not has_errors(issues): + issues.extend( + check_planes( + root, + review, + memory_limit=memory_limit, + bin_count=bins, + histogram_settings=histogram_settings, + ) + ) + + def stop(**counts) -> RunResult: # noqa: ANN003 + return RunResult(output, False, _sorted(issues), excluded=pre.excluded, **counts) + + if has_errors(issues): + return stop() + + settings = RunSettings( + bins, + memory_limit, + missing_as_singleton, + pre.excluded, + histogram=histogram_settings or HistogramSettings(), + ) + previous, previous_records, reason = _load_previous( + output, settings, mapping, dtype.name, force_rebuild + ) + labels = {image: leaf.labels for leaf in pre.discovery.leaves for image in leaf.images} + in_blocks = {path for i in issues if i.code == "planes_read_in_blocks" for path in i.paths} + headers = {f.relative_path: f for f in pre.metadata.files} + entries: dict[str, FileEntry] = {} + new, changed, unchanged = [], [], [] + for rel, file_mapping in sorted(review.file_mappings.items()): + header = headers[rel] + entries[rel] = file_entry_for( + root, + rel, + labels[rel], + file_mapping, + channel_names=header.channel_names, + significant_bits=header.significant_bits, + read_in_blocks=rel in in_blocks, + channel_names_trimmed=header.channel_names_trimmed, + ) + if header.channel_names_trimmed: + logger.info( + "Leading or trailing whitespace was removed from the channel names of %s", rel + ) + before = previous.file(rel) if previous is not None else None + if before is None: + new.append(rel) + elif ( + before.same_source(entries[rel]) + # During a rebuild there are no rows to check; otherwise the rows must be complete. + and (reason is not None or len(previous_records.get(rel, ())) == before.plane_count) + ): + unchanged.append(rel) + else: + changed.append(rel) + removed = sorted( + f.relative_path + for f in (previous.files if previous else ()) + if f.relative_path not in entries + ) + to_read = sorted([*new, *changed, *(unchanged if reason else ())]) + counts = { + "new": tuple(new), + "changed": tuple(changed), + "unchanged": tuple(unchanged), + "removed": tuple(removed), + "rebuild_reason": reason, + } + + records: list[PlaneRecord] = [] + narrow: list[NarrowRange] = [] + for rel in unchanged if not reason else (): + # Tables written before position identifiers were recorded have them blank; + # the identifier depends only on the path, so it is filled without reading. + records.extend( + replace(record, position_id=entries[rel].position_id) + for record in previous_records[rel] + ) + # Kept rows keep the range checks they were written with. + narrow.extend(entry for entry in previous.narrow_ranges if entry.relative_path == rel) + failed: list[str] = [] + for done, rel in enumerate(to_read): + if progress is not None: + progress(done, len(to_read), rel) + try: + rows, ranges = _read_file( + root / rel, + rel, + labels[rel], + review.file_mappings[rel], + settings, + entries[rel].position_id, + ) + except PlaneReadError: + logger.warning("Cannot read the planes of %s", rel, exc_info=True) + failed.append(rel) + continue + records.extend(rows) + narrow.extend(ranges) + if progress is not None: + progress(len(to_read), len(to_read), "") + if failed: + issues.append( + error( + "unreadable_planes", + f"The pixel data of {plural(len(failed), 'image')} could not be read, although " + "the file headers could. The files may be damaged or incomplete.", + failed, + "Export the images again, or exclude them from the run to continue without them.", + ) + ) + return stop(read=tuple(to_read), **counts) + + issues.extend(_pixel_issues(records)) + narrow.sort(key=lambda entry: (entry.plane_id, entry.channel)) + issues.extend(_range_issues(narrow)) + if has_errors(issues): + return stop(read=tuple(to_read), **counts) + + records.sort(key=_row_order) + # From the rows in memory: no image is read again. + intensity = check_intensities(records, intensity_settings) + per_file: dict[str, int] = {} + for record in records: + per_file[record.source_relative_path] = per_file.get(record.source_relative_path, 0) + 1 + output.mkdir(parents=True, exist_ok=True) + with _staged_outputs(output, with_design=design is not None) as staged: + csv_path = write_plane_histograms(records, staged / OUTPUT_FILENAME) + design_sha256 = None + if design is not None: + design_sha256 = file_sha256(write_experiment_design(design, staged / DESIGN_FILENAME)) + manifest = RunManifest( + experiment_root=root.as_posix(), + dtype=dtype.name, + settings=settings, + mapping=mapping, + files=tuple( + replace(entry, plane_count=per_file[rel]) for rel, entry in sorted(entries.items()) + ), + csv_sha256=file_sha256(csv_path), + versions=package_versions(), + written_at=datetime.now(UTC).isoformat(timespec="seconds"), + experiment_design_sha256=design_sha256, + masks=None if pre.masks is None else pre.masks.to_manifest(), + image_type=image_type, + channels_from_rgb_samples=type_review.channels_from_rgb_samples, + intensity_checks=intensity.settings, + narrow_ranges=tuple(narrow), + ) + write_manifest(manifest, staged / MANIFEST_FILENAME) + logger.info( + "Wrote %d planes (%d files read, %d kept) to %s", + len(records), + len(to_read), + len(records) - sum(per_file[rel] for rel in to_read), + output, + ) + return RunResult( + output, + True, + _sorted(issues), + read=tuple(to_read), + excluded=pre.excluded, + plane_count=len(records), + design=design, + image_type=image_type, + intensity=intensity, + **counts, + ) + + +def _sorted(issues: list[ValidationIssue]) -> tuple[ValidationIssue, ...]: + return tuple(sorted(issues, key=lambda issue: not issue.is_error)) diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py new file mode 100644 index 00000000..9bab31ce --- /dev/null +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -0,0 +1,236 @@ +"""Pass 1: discover the experiment and read its headers, without pixels (contract §1, §2). + +:func:`preflight` walks the folder, inspects every header and suggests an axis mapping; +:func:`sample_planes` reads the preview planes. Moved from ``run.py`` (PP-3). +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Iterable +from dataclasses import dataclass, field +from pathlib import Path + +from ..axes import ( + AxisMapping, + file_mapping_for, + prefill_experiment_mapping, +) +from ..dataset import DatasetReport, check_dataset +from ..discovery import DiscoveryResult, default_output_folder, discover_experiment +from ..masks import MaskInventory, find_masks +from ..metadata import MetadataReport, inspect_images +from ..planes import ( + DEFAULT_MEMORY_LIMIT, + Plane, + PlaneReadError, + read_sample_plane, +) +from ..validation import ValidationIssue, error, has_errors, plural, warning + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class Preflight: + """Pass 1: what was found in the experiment, before any pixel is read. + + Attributes + ---------- + discovery + Folder structure and labels. + metadata + Image headers of the images to analyse (excluded images left out). + excluded + Images the user excluded, sorted. + exclusion_issues + Warnings about excluded images, and an error if nothing is left. + suggested_mapping + Axis mapping prefilled from the headers, or None if the images' + dimensions conflict. + dataset + Images per condition and biological repeat, and the warnings about + their balance. + masks + The masks of the images (contract §15), found from their headers, when + masks were asked for; None otherwise. + """ + + discovery: DiscoveryResult + metadata: MetadataReport + excluded: tuple[str, ...] = () + exclusion_issues: tuple[ValidationIssue, ...] = () + suggested_mapping: AxisMapping | None = None + dataset: DatasetReport = field(default_factory=DatasetReport) + masks: MaskInventory | None = None + + @property + def issues(self) -> tuple[ValidationIssue, ...]: + """All issues found so far, errors first.""" + merged = [ + *self.discovery.issues, + *self.exclusion_issues, + *self.metadata.issues, + *self.dataset.issues, + *(self.masks.issues if self.masks is not None else ()), + ] + return tuple(sorted(merged, key=lambda issue: not issue.is_error)) + + @property + def ok(self) -> bool: + """True if no issue blocks processing.""" + return not has_errors(self.issues) + + +@dataclass(frozen=True) +class SamplePlane: + """A plane shown in the axis preview. + + Attributes + ---------- + relative_path + The file the plane comes from. + axes + The file's axis labels; one sample is taken per arrangement of axes. + plane + The plane, or None if it could not be read. + error + Why the plane could not be read, in plain language. + """ + + relative_path: str + axes: str + plane: Plane | None = None + error: str | None = None + + +def sample_planes( + pre: Preflight, + mapping: AxisMapping, + *, + memory_limit: int = DEFAULT_MEMORY_LIMIT, +) -> list[SamplePlane]: + """Read one preview plane per format and arrangement of axes, from the first image. + + Images lacking some of the mapping's axes are previewed with those axes as + length 1, so the user can see them before confirming that this is right. + """ + samples = [] + seen: set[tuple[str, str]] = set() + for f in pre.metadata.files: + if not f.readable or (f.kind, f.axes) in seen: + continue + seen.add((f.kind, f.axes)) + file_mapping = file_mapping_for(f, mapping) + if file_mapping is None: + samples.append( + SamplePlane(f.relative_path, f.axes, error="The axis mapping does not fit it.") + ) + continue + try: + plane = read_sample_plane( + pre.discovery.root / f.relative_path, file_mapping, memory_limit=memory_limit + ) + except PlaneReadError as exc: + logger.debug("Cannot preview %s", f.relative_path, exc_info=True) + samples.append(SamplePlane(f.relative_path, f.axes, error=str(exc))) + continue + samples.append(SamplePlane(f.relative_path, f.axes, plane)) + return samples + + +def preflight( + experiment_root: str | os.PathLike, + output_folder: str | os.PathLike | None = None, + *, + exclude: Iterable[str] = (), + masks_folder: str | os.PathLike | None = None, + ome_zarr_labels: bool = False, +) -> Preflight: + """Pass 1: check the folder and the image headers without reading pixels. + + Parameters + ---------- + experiment_root + The experiment folder. + output_folder + The output folder; defaults to :func:`default_output_folder`. + exclude + Relative paths of images to leave out (``/`` or backslash separators), + for example damaged files. + masks_folder + The masks folder (contract §15): one subfolder per mask type, each + mirroring the experiment's folders. The masks are paired with the images + and checked from their headers; the label images of OME-Zarr images are + included. + ome_zarr_labels + Use the label images of OME-Zarr images as masks, also without a masks + folder. + + Returns + ------- + Preflight + """ + root = Path(experiment_root).resolve() + output = default_output_folder(root) if output_folder is None else Path(output_folder) + discovery = discover_experiment(root, output) + excluded = tuple(sorted({_normalise(path) for path in exclude})) + images = [path for path in discovery.images if path not in excluded] + metadata = inspect_images(root, images) + masks = None + if masks_folder is not None or ome_zarr_labels: + masks = find_masks(masks_folder, discovery, metadata.files) + return Preflight( + discovery=discovery, + metadata=metadata, + excluded=excluded, + exclusion_issues=tuple(_exclusion_issues(discovery, excluded)), + suggested_mapping=prefill_experiment_mapping(metadata), + dataset=check_dataset(discovery, excluded=excluded), + masks=masks, + ) + + +def _exclusion_issues(discovery: DiscoveryResult, excluded: tuple[str, ...]) -> list: + issues = [] + found = [path for path in excluded if path in discovery.images] + unknown = [path for path in excluded if path not in discovery.images] + if found: + issues.append( + warning( + "excluded_images", + f"{plural(len(found), 'image')} {'is' if len(found) == 1 else 'are'} excluded " + "as requested and will not be analysed. Their rows from earlier runs are " + "removed.", + found, + "Remove them from the exclusion list to analyse them again.", + ) + ) + if unknown: + issues.append( + warning( + "excluded_path_not_found", + f"{plural(len(unknown), 'excluded path')} {'does' if len(unknown) == 1 else 'do'} " + "not match any analysed image of the experiment.", + unknown, + "Check the spelling of the paths; they are relative to the experiment folder.", + ) + ) + if discovery.images and not set(discovery.images) - set(excluded): + issues.append( + error( + "no_images", + "Every image of the experiment is excluded, so there is nothing to analyse.", + ["."], + "Remove images from the exclusion list.", + ) + ) + return issues + + +def _normalise(path: str) -> str: + path = str(path).replace("\\", "/") + while path.startswith("./"): + path = path[2:] + return path diff --git a/src/morphobricks_preprocessing/run/publish.py b/src/morphobricks_preprocessing/run/publish.py new file mode 100644 index 00000000..84e2abdd --- /dev/null +++ b/src/morphobricks_preprocessing/run/publish.py @@ -0,0 +1,91 @@ +"""Publish a run's outputs together, restoring the earlier ones on failure (contract §9). + +Moved from ``run.py`` (PP-3). +""" + +from __future__ import annotations + +import logging +import os +import shutil +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path + +from morphobricks_normalization.csv_io import ( + OUTPUT_FILENAME, +) + +from ..design import ( + DESIGN_FILENAME, +) +from ..manifest import ( + MANIFEST_FILENAME, +) + +logger = logging.getLogger(__name__) + + +class _OutputRollbackError(RuntimeError): + """Raised when an output update fails and its earlier files cannot be restored.""" + + +@contextmanager +def _staged_outputs(output: Path, *, with_design: bool = False) -> Iterator[Path]: + """Prepare all outputs before publishing, and retain backups if recovery fails. + + The outputs are written to a temporary folder inside ``output``; ``with_design`` + publishes ``experiment_design.csv`` too. + """ + staged = Path(tempfile.mkdtemp(prefix=".run-", dir=output)) + keep_backups = False + try: + yield staged + _publish_outputs(output, staged, with_design) + except _OutputRollbackError: + keep_backups = True + raise + finally: + if not keep_backups: + try: + shutil.rmtree(staged) + except OSError: + # A leftover temporary directory does not invalidate completed output. + logger.warning("Cannot remove temporary output folder %s", staged, exc_info=True) + + +def _publish_outputs(output: Path, staged: Path, with_design: bool = False) -> None: + """Replace the outputs, restoring their previous state on a publication failure. + + The manifest is published last, so a manifest never names a design that is not in + place. + """ + filenames = (OUTPUT_FILENAME, *((DESIGN_FILENAME,) if with_design else ()), MANIFEST_FILENAME) + backup = staged / "previous" + backup.mkdir() + # Copy before replacing either file, so even a backup failure leaves the + # earlier output intact. Keep backups on the same filesystem for atomic restores. + for name in filenames: + previous = output / name + if previous.exists() or previous.is_symlink(): + shutil.copy2(previous, backup / name, follow_symlinks=False) + published: list[str] = [] + try: + for name in filenames: + os.replace(staged / name, output / name) + published.append(name) + except BaseException: + try: + for name in reversed(published): + if (backup / name).exists() or (backup / name).is_symlink(): + os.replace(backup / name, output / name) + else: + (output / name).unlink() + except BaseException as exc: + raise _OutputRollbackError( + "The output update failed and the earlier files could not be fully restored. " + f"Backup files have been kept in {backup}. Close programs using the output " + "files and restore the backups before running again." + ) from exc + raise diff --git a/src/morphobricks_preprocessing/run/result.py b/src/morphobricks_preprocessing/run/result.py new file mode 100644 index 00000000..38df3c55 --- /dev/null +++ b/src/morphobricks_preprocessing/run/result.py @@ -0,0 +1,91 @@ +"""What a run did: :class:`RunResult` (moved from ``run.py``, PP-3).""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path + +from morphobricks_normalization.csv_io import ( + OUTPUT_FILENAME, +) + +from ..design import ( + DESIGN_FILENAME, + ExperimentDesign, +) +from ..intensity import IntensityReport +from ..manifest import ( + MANIFEST_FILENAME, +) +from ..validation import ValidationIssue, has_errors + + +@dataclass(frozen=True) +class RunResult: + """What a run did. + + Attributes + ---------- + output_folder + Where the table and manifest are (or would have been) written. + written + True if the run succeeded and the output was written. + issues + Errors that stopped the run, and warnings, errors first. + new, changed, unchanged, removed + Files compared with the earlier run: not in it, modified since, as they + were, and in it but no longer analysed (deleted, renamed or excluded). + Without a usable earlier manifest, every file is new. + read + Files whose pixels were read in this run. + excluded + Images the user excluded. + rebuild_reason + Why every row was recomputed, or None when the earlier table was updated. + plane_count + Number of rows in the written table. + design + The experiment design written with the table, or None when the run was + given none. + image_type + The image type recorded in the manifest, or None when the run was given + none. + intensity + The intensity checks of the written table (warnings, never errors), or + None when nothing was written. + """ + + output_folder: Path + written: bool + issues: tuple[ValidationIssue, ...] = () + new: tuple[str, ...] = () + changed: tuple[str, ...] = () + unchanged: tuple[str, ...] = () + removed: tuple[str, ...] = () + read: tuple[str, ...] = () + excluded: tuple[str, ...] = () + rebuild_reason: str | None = None + plane_count: int = 0 + design: ExperimentDesign | None = None + image_type: str | None = None + intensity: IntensityReport | None = None + + @property + def ok(self) -> bool: + """True if no issue blocks processing.""" + return not has_errors(self.issues) + + @property + def design_path(self) -> Path | None: + """Location of ``experiment_design.csv``, or None when the run had no design.""" + return self.output_folder / DESIGN_FILENAME if self.design is not None else None + + @property + def csv_path(self) -> Path: + """Location of ``plane_histograms.csv``.""" + return self.output_folder / OUTPUT_FILENAME + + @property + def manifest_path(self) -> Path: + """Location of ``run_manifest.json``.""" + return self.output_folder / MANIFEST_FILENAME diff --git a/src/morphobricks_preprocessing/run/reuse.py b/src/morphobricks_preprocessing/run/reuse.py new file mode 100644 index 00000000..abf088ff --- /dev/null +++ b/src/morphobricks_preprocessing/run/reuse.py @@ -0,0 +1,89 @@ +"""Decide which rows of an earlier run can be reused (contract §9, plan §7). + +Moved from ``run.py`` (PP-3). +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +from morphobricks_normalization.csv_io import ( + OUTPUT_FILENAME, + SCHEMA_VERSION, + CsvFormatError, + read_plane_histograms, +) +from morphobricks_normalization.ranges import HistogramSettings +from morphobricks_normalization.records import PlaneRecord + +from ..axes import ( + AxisMapping, +) +from ..manifest import ( + MANIFEST_FILENAME, + ManifestFormatError, + RunManifest, + RunSettings, + file_sha256, + read_manifest, +) + +logger = logging.getLogger(__name__) + + +def _load_previous( + output: Path, settings: RunSettings, mapping: AxisMapping, dtype: str, force: bool +) -> tuple[RunManifest | None, dict[str, list[PlaneRecord]], str | None]: + """Load the earlier manifest and its rows by file, and say why they cannot be reused. + + The manifest is returned whenever it can be read, so files can still be + reported as new, changed or unchanged during a rebuild. + """ + manifest_path, csv_path = output / MANIFEST_FILENAME, output / OUTPUT_FILENAME + if not manifest_path.exists(): + return None, {}, "there is no earlier run in the output folder" + try: + manifest = read_manifest(manifest_path) + except ManifestFormatError: + logger.warning("Cannot read %s", manifest_path, exc_info=True) + return None, {}, "the earlier run manifest could not be read" + if force: + return manifest, {}, "a full rebuild was requested" + if manifest.schema_version != SCHEMA_VERSION: + return manifest, {}, "the earlier table uses a different format version" + if manifest.dtype != dtype: + return manifest, {}, f"the pixel type changed from {manifest.dtype} to {dtype}" + if manifest.settings.bin_count != settings.bin_count: + return ( + manifest, + {}, + f"the bin count changed from {manifest.settings.bin_count:,} to {settings.bin_count:,}", + ) + if manifest.settings.histogram != settings.histogram: + before = _describe_histogram(manifest.settings.histogram) + now = _describe_histogram(settings.histogram) + if before == now: + return manifest, {}, "the limit of the histogram range check changed" + return manifest, {}, f"the histogram range changed from {before} to {now}" + if (manifest.mapping.axes, manifest.mapping.roles) != (mapping.axes, mapping.roles): + return manifest, {}, "the axis mapping changed" + if not csv_path.exists() or file_sha256(csv_path) != manifest.csv_sha256: + return manifest, {}, "the histogram table was changed or replaced after the earlier run" + try: + rows = read_plane_histograms(csv_path) + except CsvFormatError: + logger.warning("Cannot read %s", csv_path, exc_info=True) + return manifest, {}, "the earlier histogram table could not be read" + by_file: dict[str, list[PlaneRecord]] = {} + for record in rows: + by_file.setdefault(record.source_relative_path, []).append(record) + return manifest, by_file, None + + +def _describe_histogram(settings: HistogramSettings) -> str: + """Describe the histogram range in words, e.g. ``"robust (0.01-99.99 %)"``.""" + if settings.robust: + low, high = settings.robust_quantiles + return f"robust ({low:g}-{high:g} %)" + return "minimum to maximum" diff --git a/src/morphobricks_preprocessing/run/rows.py b/src/morphobricks_preprocessing/run/rows.py new file mode 100644 index 00000000..f905f442 --- /dev/null +++ b/src/morphobricks_preprocessing/run/rows.py @@ -0,0 +1,131 @@ +"""Compute the table's rows, one per XYC plane, and the checks on them (contract §4). + +Moved from ``run.py`` (PP-3). +""" + +from __future__ import annotations + +from pathlib import Path + +from morphobricks_normalization.dtypes import default_bin_count, validate_bin_count +from morphobricks_normalization.ranges import range_check_issues +from morphobricks_normalization.records import PlaneRecord + +from ..axes import ( + AxisMapping, +) +from ..manifest import ( + NarrowRange, + RunSettings, +) +from ..planes import ( + iter_xyc_planes, +) +from ..validation import ValidationIssue, error, plural, warning + + +def _read_file( + path: Path, + rel: str, + labels: tuple[str, str, str], + mapping: AxisMapping, + settings: RunSettings, + position: str | None, +) -> tuple[list[PlaneRecord], list[NarrowRange]]: + """Histogram every XYC plane of ``path``, recording the rows under the source ``rel``. + + One row per plane holds the histograms of all its channels (schema version 4), + binned with the run's histogram range. Also returns the channel planes whose + range check found the central range too narrow. + """ + records, narrow = [], [] + for plane in iter_xyc_planes( + path, + mapping, + memory_limit=settings.memory_limit, + bin_count=settings.bin_count, + histogram_settings=settings.histogram, + ): + measured = plane.histograms_with_checks(settings.bin_count, settings.histogram) + record = PlaneRecord( + rel, + *labels, + histograms=tuple(histogram for histogram, _ in measured), + z=plane.z, + t=plane.t, + position_id=position, + ) + records.append(record) + narrow.extend( + NarrowRange(rel, record.plane_id, channel, check) + for channel, (_, check) in enumerate(measured) + if check is not None and check.narrow + ) + return records, narrow + + +def _bin_count(dtype, bin_count: int | None) -> tuple[int | None, list[ValidationIssue]]: # noqa: ANN001 + if bin_count is None: + return default_bin_count(dtype), [] + try: + return validate_bin_count(dtype, bin_count), [] + except ValueError as exc: + return None, [ + error( + "invalid_bin_count", + str(exc), + action="Choose a bin count in the allowed range, or use the default.", + ) + ] + + +def _range_issues(narrow: list[NarrowRange]) -> list[ValidationIssue]: + """Return the brick's ``narrow_histogram_range`` warnings, one per channel (N60).""" + found = range_check_issues((entry.plane_id, entry.channel, entry.check) for entry in narrow) + return [warning(issue.code, issue.message, issue.items, issue.action) for issue in found] + + +def _pixel_issues(records: list[PlaneRecord]) -> list[ValidationIssue]: + # Every channel of every row is checked. + channels = [(r.source_relative_path, h) for r in records for h in r.histograms] + issues = [] + with_non_finite = sorted({rel for rel, h in channels if h.non_finite_pixel_count}) + if with_non_finite: + total = sum(h.non_finite_pixel_count for _, h in channels) + issues.append( + warning( + "non_finite_pixels", + f"{plural(len(with_non_finite), 'image')} contain {plural(total, 'pixel')} that " + "are not numbers or infinite (NaN, ±infinity). They are left out of the " + "histograms and counted separately.", + with_non_finite, + "No action needed if these values mark missing data.", + ) + ) + empty = [rel for rel, h in channels if h.is_empty] + if empty and len(empty) == len(channels): + issues.append( + error( + "no_usable_planes", + "No plane has any finite pixel value, so no statistics can be calculated.", + sorted(set(empty)), + "Check that these are the right images and that they were exported correctly.", + ) + ) + elif empty: + planes = {r.plane_id for r in records if any(h.is_empty for h in r.histograms)} + issues.append( + warning( + "no_finite_pixels", + f"{plural(len(planes), 'plane')} have no finite pixel values in at least one " + "channel. These channels are kept in the table with zero counts and left out " + "of group statistics.", + sorted(set(empty)), + "No action needed if these planes are expected to be empty.", + ) + ) + return issues + + +def _row_order(record: PlaneRecord) -> tuple: + return (record.source_relative_path, record.z, record.t) diff --git a/tests/test_axis_check.py b/tests/test_axis_check.py index e349efda..a24d8a25 100644 --- a/tests/test_axis_check.py +++ b/tests/test_axis_check.py @@ -242,16 +242,16 @@ def test_named_images_are_recorded_as_from_the_header(tmp_path): @pytest.fixture def reads(monkeypatch): - import morphobricks_preprocessing.run as run_module + import morphobricks_preprocessing.run.rows as rows_module seen = [] - original = run_module.iter_xyc_planes + original = rows_module.iter_xyc_planes def spy(path, *args, **kwargs): seen.append(str(path)) return original(path, *args, **kwargs) - monkeypatch.setattr(run_module, "iter_xyc_planes", spy) + monkeypatch.setattr(rows_module, "iter_xyc_planes", spy) return seen diff --git a/tests/test_histogram_range.py b/tests/test_histogram_range.py index 907c2eb9..0853b122 100644 --- a/tests/test_histogram_range.py +++ b/tests/test_histogram_range.py @@ -18,8 +18,8 @@ read_manifest, run_experiment, ) -from morphobricks_preprocessing import run as run_module from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.run import rows as rows_module from tiff_tree import write_data YX = prefill_mapping("YX") @@ -53,13 +53,13 @@ def hot_experiment(tmp_path): @pytest.fixture def reads(monkeypatch): paths = [] - original = run_module.iter_xyc_planes + original = rows_module.iter_xyc_planes def spy(path, *args, **kwargs): paths.append(path) return original(path, *args, **kwargs) - monkeypatch.setattr(run_module, "iter_xyc_planes", spy) + monkeypatch.setattr(rows_module, "iter_xyc_planes", spy) return paths diff --git a/tests/test_image_type.py b/tests/test_image_type.py index 21cad303..44b1bc2a 100644 --- a/tests/test_image_type.py +++ b/tests/test_image_type.py @@ -15,7 +15,7 @@ review_image_type, run_experiment, ) -from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.run import rows as rows_module from tiff_tree import write_data @@ -158,13 +158,13 @@ def test_check_image_type(): @pytest.fixture def reads(monkeypatch): paths = [] - original = run_module.iter_xyc_planes + original = rows_module.iter_xyc_planes def spy(path, *args, **kwargs): paths.append(Path(path).name) return original(path, *args, **kwargs) - monkeypatch.setattr(run_module, "iter_xyc_planes", spy) + monkeypatch.setattr(rows_module, "iter_xyc_planes", spy) return paths diff --git a/tests/test_run.py b/tests/test_run.py index b52f9ac3..3303724c 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,6 +1,7 @@ import hashlib import json import os +import shutil from dataclasses import replace from pathlib import Path @@ -28,6 +29,7 @@ ) from morphobricks_preprocessing import planes as planes_module from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.manifest import file_sha256 from simulation import simulate_plane from tiff_tree import write_data, write_file @@ -75,13 +77,13 @@ def relative(self, root): def reads(monkeypatch): """Record which files have their pixels read.""" paths = Reads() - original = run_module.iter_xyc_planes + original = run_module.rows.iter_xyc_planes def spy(path, *args, **kwargs): paths.append(path) return original(path, *args, **kwargs) - monkeypatch.setattr(run_module, "iter_xyc_planes", spy) + monkeypatch.setattr(run_module.rows, "iter_xyc_planes", spy) return paths @@ -577,7 +579,7 @@ def test_preparing_outputs_preserves_earlier_files_on_failure(experiment, monkey first = run_experiment(experiment, ZYX) before = {path.name: path.read_bytes() for path in (first.csv_path, first.manifest_path)} touch(experiment / IMAGES[0], stack(99)) - original_hash = run_module.file_sha256 + original_hash = run_module.pipeline.file_sha256 def broken_csv(*args, **kwargs): raise OSError("simulated CSV failure") @@ -595,13 +597,13 @@ def broken_backup(*args, **kwargs): with monkeypatch.context() as patch: if failure == "csv": - patch.setattr(run_module, "write_plane_histograms", broken_csv) + patch.setattr(run_module.pipeline, "write_plane_histograms", broken_csv) elif failure == "hash": - patch.setattr(run_module, "file_sha256", broken_hash) + patch.setattr(run_module.pipeline, "file_sha256", broken_hash) elif failure == "manifest": - patch.setattr(run_module, "write_manifest", broken_manifest) + patch.setattr(run_module.pipeline, "write_manifest", broken_manifest) else: - patch.setattr(run_module.shutil, "copy2", broken_backup) + patch.setattr(shutil, "copy2", broken_backup) with pytest.raises(OSError, match="simulated"): run_experiment(experiment, ZYX) assert {path.name: path.read_bytes() for path in first.output_folder.iterdir()} == before @@ -634,16 +636,14 @@ def broken_replace(source, destination): return original_replace(source, destination) with monkeypatch.context() as patch: - patch.setattr(run_module.os, "replace", broken_replace) + patch.setattr(os, "replace", broken_replace) with pytest.raises(PermissionError, match="simulated publication failure"): run_experiment(experiment, ZYX) assert failed assert {path.name: path.read_bytes() for path in first.output_folder.iterdir()} == before retried = run_experiment(experiment, ZYX) assert retried.written - assert read_manifest(retried.manifest_path).csv_sha256 == run_module.file_sha256( - retried.csv_path - ) + assert read_manifest(retried.manifest_path).csv_sha256 == file_sha256(retried.csv_path) assert unrelated.read_text(encoding="utf-8") == "keep this file" @@ -658,7 +658,7 @@ def interrupted(source, destination): raise KeyboardInterrupt return original_replace(source, destination) - monkeypatch.setattr(run_module.os, "replace", interrupted) + monkeypatch.setattr(os, "replace", interrupted) with pytest.raises(KeyboardInterrupt): run_experiment(experiment, ZYX) assert {path.name: path.read_bytes() for path in first.output_folder.iterdir()} == before @@ -675,7 +675,7 @@ def broken_replace(source, destination): raise PermissionError("simulated locked output") return original_replace(source, destination) - monkeypatch.setattr(run_module.os, "replace", broken_replace) + monkeypatch.setattr(os, "replace", broken_replace) with pytest.raises(RuntimeError, match="could not be fully restored") as excinfo: run_experiment(experiment, ZYX) (staged,) = first.output_folder.glob(".run-*") @@ -690,10 +690,10 @@ def test_cleanup_failure_does_not_report_completed_output_as_failed( def broken_cleanup(path): raise PermissionError("simulated temporary folder in use") - monkeypatch.setattr(run_module.shutil, "rmtree", broken_cleanup) + monkeypatch.setattr(shutil, "rmtree", broken_cleanup) result = run_experiment(experiment, ZYX) assert result.written - assert read_manifest(result.manifest_path).csv_sha256 == run_module.file_sha256(result.csv_path) + assert read_manifest(result.manifest_path).csv_sha256 == file_sha256(result.csv_path) assert "Cannot remove temporary output folder" in caplog.text diff --git a/tools/check.py b/tools/check.py index 598243ea..e51ea0f5 100644 --- a/tools/check.py +++ b/tools/check.py @@ -7,6 +7,10 @@ - ``python tools/check.py --rules``: only the code rules against their baseline (CI); - ``python tools/check.py --update-baseline``: lower the baseline to the current code. It never adds or raises an entry: new violations must be fixed. +- ``python tools/check.py --rename OLD=NEW``: re-key the baseline entries of code moved from + ``OLD`` to ``NEW`` (a file, or a function as ``path name``), for a pull request that only + moves code; then ``--update-baseline`` drops what the move removed. Nothing is added or + raised: an entry whose new key already exists keeps the smaller size. The code rules are the shared ``tools/code_rules.py``, configured in ``pyproject.toml`` under ``[tool.morphobricks.code-rules]`` (owner, package, layers, baseline file). @@ -94,8 +98,28 @@ def update_baseline() -> Path: return path +def rename_baseline(renames: list[str]) -> Path: + """Re-key baseline entries for moved code, as ``OLD=NEW`` key prefixes.""" + path = ROOT / config()["baseline"] + baseline = read_baseline(path) + problems = {} + for key, size in baseline.get("problems", {}).items(): + for rename in renames: + old, new = rename.split("=", 1) + if key.startswith(old): + key = new + key[len(old) :] + problems[key] = min(size, problems.get(key, size)) + baseline["problems"] = dict(sorted(problems.items())) + path.write_text(json.dumps(baseline, indent=2, sort_keys=True) + "\n", encoding="utf-8") + return path + + def main(argv: list[str]) -> int: """Run the checks and return the process exit code.""" + if "--rename" in argv: + renames = [argv[i + 1] for i, arg in enumerate(argv) if arg == "--rename"] + print(f"Baseline re-keyed: {rename_baseline(renames)}") + return 0 if "--update-baseline" in argv: print(f"Baseline lowered: {update_baseline()}") return 0 diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 3f2b5030..70b39c4a 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -91,12 +91,11 @@ "src/morphobricks_preprocessing/planes.py _zarr_layout: private function of # lines without a docstring": 40, "src/morphobricks_preprocessing/planes.py check_planes: # executable lines (max #)": 65, "src/morphobricks_preprocessing/planes.py: # lines (max #)": 1422, - "src/morphobricks_preprocessing/run.py _bin_count: private function of # lines without a docstring": 12, - "src/morphobricks_preprocessing/run.py _exclusion_issues: private function of # lines without a docstring": 34, - "src/morphobricks_preprocessing/run.py _pixel_issues: private function of # lines without a docstring": 38, - "src/morphobricks_preprocessing/run.py run_experiment: # executable lines (max #)": 195, - "src/morphobricks_preprocessing/run.py run_experiment: # parameters (max #)": 15, - "src/morphobricks_preprocessing/run.py: # lines (max #)": 895, + "src/morphobricks_preprocessing/run/pipeline.py run_experiment: # executable lines (max #)": 195, + "src/morphobricks_preprocessing/run/pipeline.py run_experiment: # parameters (max #)": 15, + "src/morphobricks_preprocessing/run/preflight.py _exclusion_issues: private function of # lines without a docstring": 34, + "src/morphobricks_preprocessing/run/rows.py _bin_count: private function of # lines without a docstring": 12, + "src/morphobricks_preprocessing/run/rows.py _pixel_issues: private function of # lines without a docstring": 38, "tests/test_discovery.py: # lines (max #)": 848, "tests/test_masks.py: # lines (max #)": 618, "tests/test_metadata.py: # lines (max #)": 629,