diff --git a/AGENTS.md b/AGENTS.md index 51b4a144..96d55c3a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -601,7 +601,7 @@ Technical details go to `logging` (shown in the notebook's "Technical details" p | Significant bits (Q4) | Manifest only, information only; `bit_depth` and binning unchanged | | Corrupt files (Q6) | Explicit exclusion list, recorded in manifest, rows removed | | Notebook target | Google Colab and JupyterLab from one file, following LabConstrictor's notebook portability guide (plan Q58; was local Jupyter only): `# @title` cells, Colab-only code (Drive mount, custom widget manager) inside `if "google.colab" in sys.modules:`, no magics, GitHub install pinned to `RELEASE_TAG` | -| CLI | `check ` (summary, label tree, issues, suggested `--axes`) and `run --axes ROLES` with `--output`, `--exclude`, `--bins`, `--histogram-range`, `--robust-quantiles`, `--memory-limit 2GiB`, `--missing-as-singleton`, `--mode restart|full`, `--no-masks`, `--no-exclude`, `--remove-stale-lock`, `--log`, and the conversion options above. Axis roles are always stated explicitly, never taken silently from metadata. Exit 1 on errors | +| CLI | `check ` (summary, label tree, issues, suggested `--axes`) and `run --axes ROLES` with `--output`, `--exclude`, `--bins`, `--histogram-range`, `--robust-quantiles`, `--memory-limit 2GiB`, `--missing-as-singleton`, `--mode restart|full`, `--no-masks`, `--no-exclude`, `--remove-stale-lock`, `--log`, and the conversion options above. Axis roles are always stated explicitly, never taken silently from metadata (amended 3 November 2026, plan §12 question 73: when every image names all its axes, `run` uses those roles without `--axes` and prints them as the images name them; plain stacks still need `--axes`). Exit 1 on errors | | Large planes (Q15) | Planes over the memory limit are read in blocks automatically, announced by the warning `planes_read_in_blocks` and `read_in_blocks` in the manifest. Block size derived from the limit (whole strips/tiles/chunks, uncompressed pieces and chunks also by rows). Compressed single-strip/tile planes keep the `plane_over_memory_limit` error. Two-pass histograms identical to whole planes. (Up to v0.3, OME-Zarr copies were written block by block; conversion was removed in v0.4.0.) Axis preview reduced (every n-th pixel) with a caption saying so | | Normalization brick (P1) | The modules `dtypes`, `histogram`, `records`, `csv_io`, `pooling` and `normalization` moved unchanged to `morphobricks_normalization` (normalization plan N1, N2, N6) and are imported from there. Up to v0.3 their public names stayed importable from `morphobricks_preprocessing` and the old submodules, with a `DeprecationWarning`; those stand-ins (`_moved.py`) were removed in v0.4.0 (plan Q46). The package's own code never uses the old paths. Outputs unchanged (CSV schema 2); the manifest's `versions` adds `morphobricks-normalization`; Step 0 checks for it; contract §3–§7 text moved to the normalization contract (§1.2, now §6.2 for the CSV) | | Dataset consistency (Q21, P2) | Step 1 "Dataset consistency" panel: balance table (images and technical-repeat folders per condition x biological repeat), `unbalanced_images` (< 0.5 or > 2 x median), `repeat_missing_conditions`, `different_pixel_sizes`, `missing_pixel_size` (OME-Zarr scale included), `different_plane_counts`; `different_sizes` narrowed to width/height; consistency issues shown only in that panel, not in Messages. Warnings only; outputs unchanged | diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d4bc0f..739f92af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,16 @@ and the bricks' migration). ### Fixed +- Cross-brick test of 3 November 2026 (plan §12 question 73): `masks` accepts per-plane masks + with a stated shared 2-D mask (`--mask-axes Nuclei=YX`) as `run` does, instead of + `mask_planes_differ` (F1); `run` needs no `--axes` when every image names its axes (F9); + a rerun that changes nothing leaves `run_manifest.json` byte-identical, keeping its + `written_at`, so pinned manifest SHA-256s stay valid (F10); `output_locked` says a lock + left by a process no longer running on this computer looks stale and how to remove it + (also `convert`'s, for the folder of the copies; it is never removed on its own), `run` + without `--axes` names the images' differing arrangements instead of `--axes None`, and a resumed run no longer says all rows were computed (F11); `convert --output` names + the runs' output folder in the printed next commands, and `inconsistent_dimensions` says + that named-axis copies keep each image's order (F8). - Preprocessing audit of 3 November 2026 (plan §12 question 72): `read_plane` and `RunInputs.read_plane` no longer count histogram memory, so a 5000² float32 plane is read within the 1 GiB limit (I2); a header size in an unsupported unit (OME `pixel`) or without diff --git a/docs/contract.md b/docs/contract.md index 42388625..0d56ff7e 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -168,7 +168,7 @@ interpretation. | Key | Meaning | |---|---| | `manifest_version`, `schema_version` | Manifest format (`"2"` since v0.4.0) and CSV format (`"4"`), written as JSON **strings**, not numbers. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | -| `written_at` | UTC time of writing, ISO 8601. | +| `written_at` | UTC time of writing, ISO 8601. A rerun whose manifest would differ from the earlier one only in `written_at` keeps the earlier time, so the manifest's bytes and SHA-256, which later bricks pin, stay the same (cross-brick test F10, 3 November 2026); the report still records the rerun. | | `experiment_root` | Absolute path of the experiment folder, for information. | | `dtype` | Pixel type of the experiment. | | `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6). | @@ -279,8 +279,8 @@ the bricks' tests. | Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | | `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. | +| 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 of an experiment with plain stacks and for plain stacks that are new or changed; when every readable image names all its axes (`header_axes`) and the suggested mapping gives each a role, the first run uses those roles (design v0.4 §3; cross-brick test F9). `convert` always needs `--axes` (§17). | +| 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; when the holder is a process on this computer that is no longer running (POSIX), the message says the lock looks stale and how to remove it, and the lock is kept (cross-brick test F11). `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`, `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`, `resumed` (files whose rows an interrupted run had staged, reused without reading their pixels, §9.4), `staged_discarded` (file: why its staged rows were not reused), `replaced` (file: what happened), `input_changes`, `issues` (code, severity, message, paths, action). For people only: bricks never read it. A published report is part of the run's generation (§9.4). | | `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)`). | | Exit status (v0.4.0) | Every command (`check`, `run`, `masks`, `calibrate`, `design`, `convert`) exits with status 1 when it prints an error and 0 otherwise; warnings never change it, and options argparse cannot read exit with 2. `check` exits with 1 on every error it reports: the preflight's, the axis check's, a refused write of its outputs (`output_locked`) and an experiment design it cannot read (`unreadable_experiment_design`; found in the review of #130, it exited with 0) or one with `repeat_group_conflict` or `invalid_experiment_design`, printed after its summary (found in the review of #133: `run` stopped on it, `check` exited with 0); `design_not_confirmed` does not count, since `check` writes the template to confirm. `design` exits with 1 when the design it prints has `repeat_group_conflict` or `invalid_experiment_design`, also after a change it wrote (it exited with 0); `design_not_confirmed` is not printed by `design` and does not count. | @@ -518,7 +518,9 @@ The report is UTF-8 JSON (two-space indent, LF), written atomically to the outpu | `versions` | As in the manifest (§9); information only. | The step is the command `masks` and the functions `find_masks` and `scan_masks`; it reads -each image's masks once and holds only one image's masks in memory. A mask that cannot be read +each image's masks once and holds only one image's masks in memory. A stated shared 2-D mask +(§15) is left out of the comparison of the image's masks' planes, as in `run`, and is compared +with each plane of the other masks (cross-brick test F1). A mask that cannot be read as the planes its header gave, or holds a label outside 0 to 2,147,483,647 (§16), when its pixels are read raises an error. @@ -548,7 +550,7 @@ The exception path for images whose axes are wrong or not named: they are saved 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). +`convert EXPERIMENT --to FOLDER --axes ROLES [--output FOLDER]`. Masks are never converted (§15). `--output` is the output folder of the runs on the originals (default: the experiment's default one), named in the printed next commands and where the recorded masks folder is looked up (cross-brick test F8). A copy keeps its source's stored order of dimensions, so `convert` does not solve `inconsistent_dimensions`, and the message says so (plan §12 question 73). | Rule | Detail | |---|---| @@ -557,7 +559,7 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | 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`), and neither inside the masks folder, where the copies would be read as masks, nor holding it (`copy_folder_overlaps_masks`; the masks folder given with `--masks`/`masks_folder=`, else the one recorded by the run in the default output folder, or in the notebook the one checked in step 1, else the one recorded in its output folder; without one known, the run refuses copies inside its masks folder, `copies_inside_masks`, §15): 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. 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. | +| 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` (a lock left by a conversion on this computer that is no longer running is called stale and kept, as in §9.2; cross-brick test F11). 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, unknown sizes (roles and reasons), channel names, significant bits, the copy's fingerprint and the copy's SHA-256 (read whole) 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, or of version 1, is `conversion_record_unreadable`. | | Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 2: `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`), `copy_sha256` (the copy's SHA-256, computed in one pass after it is written; an OME-Zarr copy's is the SHA-256 of its files' relative paths and SHA-256s in path order, its `labels` group left out as for fingerprints: `content_hash`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9), `unknown_sizes` (role: why), `channel_names` (as the copy holds them; `[]` for none) and `significant_bits` (or null). Sources keep the fingerprint rule (design v0.4 §1); the copies' SHA-256 is the owner's decision of 2 November 2026 (plan §12 question 59 (16)). Version 2 (PP-11c) added `copy_sha256`, `channel_names` and `significant_bits`; a version 1 record (v0.4.0rc3 or earlier) is refused everywhere it is read (`convert`, the run's `review_conversion`, `RunInputs`) with a message to convert the originals again into a new folder: its copies cannot be checked. `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 refuse a copy that changed (its fingerprint or its SHA-256: each copy is read whole, so `check` and `run` on a copy tree cost one more read of the copies), went missing or was added since (`copy_changed_after_conversion`: an error since PP-12, owner decision of 2 November 2026; the run stops after the header checks, before any pixel is read, writing only its report, since `RunInputs` would refuse the run; the message says to convert the originals again into a new folder); 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`), is of version 1, or whose recorded images' copies no longer have the recorded fingerprint and SHA-256 (§9.1). | diff --git a/docs/migration-0.3-to-0.4.md b/docs/migration-0.3-to-0.4.md index 31ad1b26..1d631f4a 100644 --- a/docs/migration-0.3-to-0.4.md +++ b/docs/migration-0.3-to-0.4.md @@ -256,6 +256,11 @@ later bricks on the new run. is refused). - `different_sizes` and `different_plane_counts` compare the images by their confirmed roles once the roles are confirmed. +- `run` needs `--axes` only when some images are plain stacks; images that all name their + axes are run with those roles. A rerun that changes nothing keeps `run_manifest.json` + byte-identical (its `written_at` too). `convert` takes `--output`, the output folder of + your runs on the originals, for the next commands it prints (cross-brick test, plan §12 + question 73). - The notebook runs on Google Colab and in JupyterLab. It installs the pinned release (`RELEASE_TAG`), never `main`. diff --git a/docs/plan.md b/docs/plan.md index bcf8022d..967940dd 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -1834,3 +1834,28 @@ Raised during review; to be decided before the milestones that need them: an ImageJ resolution tag of another type is `invalid_physical_calibration`, and an OME-Zarr scale without a unit is named by `unsupported_pixel_size_unit` ("without a unit"); `same_pixel_size` takes NumPy real scalars as m4 does. +73. How are the findings F1, F8, F9, F10 and F11 of the cross-brick test of 3 November 2026 + fixed? *Decided (3 November 2026, owner: fix before the round-2 audit; the choices in + F8 are the conservative ones, within design v0.4 §3 and §5; contract §9, §15, §17):* + (F1) `scan_masks` leaves a stated shared 2-D mask out of the comparison of an image's + masks' planes, as `run` does, and compares it with each plane of the other masks; never + across planes. (F9) `run` needs no `--axes` when every readable image names all its axes + and the suggested mapping gives each a role; a plain stack still needs it; `convert` + always needs it. (F10) a rerun whose manifest differs only in `written_at` keeps the + earlier time, so the manifest's bytes and SHA-256 stay the same; any other change writes + the new time. (F11) the `output_locked` message says that a lock left by a process on + this computer that is no longer running looks stale, and how to remove it (also + `convert`'s, for the folder of the copies); it is never removed automatically. A resumed run's summary says that the staged rows were resumed, + not that all rows were computed. (F8a) `convert` keeps each image's stored order of + dimensions (§17): writing copies in the experiment's common order would be a pixel-exact + transposition under named roles, but it changes §17's copy axes and is left as an open + question for the owner; for now `inconsistent_dimensions` and `inconsistent_mapping` say + that named-axis copies do not solve them. (F8b) `convert --output FOLDER` names the output + folder of the runs on the originals in the printed next commands (and is where the + recorded masks folder is looked up); without it, the default folder is named with what + to do if the runs used another one. (F8c) a plain stack whose pages interleave several + dimensions (`IYX`, no shape metadata) cannot be converted: a reshape is not a role, and + the spec has no way to state one, so the user guide says to save the image again with + its dimensions (for example as a Fiji hyperstack). Follow-up for the owner: whether + `convert` should accept a stated reshape (for example `--pages CZ=2x3`) or reorder copies + into the common order. diff --git a/docs/user_guide.md b/docs/user_guide.md index 1937fec4..73632125 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -546,8 +546,10 @@ morphobricks-preprocessing run /data/exp1 --axes ZCYX ``` `--axes` gives one role per dimension of the images, in their order: `X`, `Y`, `Z`, `C`, -`T`, or `-` for a length-1 dimension without a role. Roles are never taken from metadata -without being stated. A later `run` in the same output folder needs neither the roles nor +`T`, or `-` for a length-1 dimension without a role. It is needed only when some images are +plain stacks, whose metadata do not name their axes: when every image names all its axes, +`run` uses those roles without `--axes` (it prints **Axis roles (as every image names +them)**; design v0.4 §3), and they are still checked as below. A later `run` in the same output folder needs neither the roles nor the experiment folder again, unless plain stacks were added or changed: ```bash @@ -609,7 +611,9 @@ A later `run` in the same output folder reuses everything the earlier run record axis roles (`--axes` is needed again only for plain stacks that are new or changed), the masks folder and mask axes, the exclusions, the image type, the bins and the histogram and intensity settings. An option given that differs from the recorded value is used, and the -report names the change and its effect. Other options: +report names the change and its effect. A rerun that changes nothing leaves +`run_manifest.json` exactly as it was, its `written_at` included, so bricks that recorded its +SHA-256 still accept it; the report records the rerun. Other options: | Option | Meaning | |---|---| @@ -750,7 +754,7 @@ When the metadata of some images name their axes wrongly (the errors `axes_yx_sw 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 convert /data/exp1 --to /data/exp1_named_axes --axes ZXY --output /data/exp1_MorphoBricks_Preprocessing 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 ``` @@ -773,7 +777,20 @@ which is then kept; a depth step is never written as a time step. State the miss 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. +wrote and the next commands, with the suggested `--axes` for the copies and the output +folder of your runs on the originals: give it with `--output` (without it the default +output folder is named, which only works if your runs used it). `--output` is also where +the masks folder is looked up when `--masks` is not given. + +`convert` keeps each image's stored order of dimensions (`ZXY` stays three dimensions in +that order, with your roles): it does not reorder images, so it cannot solve +`inconsistent_dimensions` (images arranged differently, for example `CZYX` among `ZCYX`); +export those images again in the same order, or process them as a separate experiment +(plan §12 question 73). A plain stack that holds several dimensions in one sequence of pages +(for example channels and depth slices interleaved, read as `IYX`) cannot be converted +either: one role per stored dimension cannot say how the pages split, and the brick never +guesses it. Save it again with its dimensions (for example as a hyperstack in Fiji, which +writes ImageJ metadata naming them), then check it again. 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 @@ -919,7 +936,9 @@ with `chmod a+r /*`, or delete them before running again. Only one run at a time can use an output folder: a run holds a lock file (`.morphobricks-preprocessing.lock`) and a second run is refused (`output_locked`). If the process is forcibly stopped, its lock stays; remove it with `--remove-stale-lock` once you -are sure no run is going on (every command takes it). `check` holds the lock +are sure no run is going on (every command takes it). When the lock was left by a process on +this computer that is no longer running, the message says the lock looks stale; it is still +never removed without your request. `check` holds the lock while it writes its report, `masks` (and **Describe the masks**) from before the first mask is read until `mask_relationships.json` is written (a locked folder is refused before any mask is read), `design` and `calibrate` while they change their files, @@ -983,7 +1002,7 @@ continues. | `unsupported_dtype` | Error | Unsupported pixel type (e.g. float16). Convert, or exclude it. | | `mixed_dtypes` | Error | Images have different pixel types. Process them separately or convert. | | `different_channel_counts` | Error | Images have different numbers of channels (an image without a channel dimension has one). Every image of an experiment must have the same channels: split the others into a separate experiment, or exclude them. If a dimension got the channel role by mistake, correct the axis roles. | -| `inconsistent_dimensions`, `inconsistent_mapping` | Error | Images have their dimensions in a different order. Export them the same way or process separately. | +| `inconsistent_dimensions`, `inconsistent_mapping` | Error | Images have their dimensions in a different order. Export them the same way or process separately; named-axis copies (`convert`) keep each image's order, so they do not solve this. | | `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) with the `convert` command (§5), so every tool reads the same axes. | @@ -1000,7 +1019,7 @@ continues. | `invalid_bin_count` | Error | Bin count outside the allowed range. Use the default or a value in range. | | `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. | +| `output_locked` | Error | Another run is using the output folder; it is named with its computer, process and start time (a process on this computer that is no longer running is named as a lock that looks stale). 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_overlaps_masks` | Error | `convert`: the folder for the copies is the masks folder, inside it, or holds it; copies there would be read as masks. Choose a folder outside the masks folder. | | `copy_folder_not_a_folder` | Error | `convert`: the path given with `--to` is a file. Choose a folder. | @@ -1061,7 +1080,7 @@ continues. | `mask_not_2d` | Error | A mask has dimensions other than Y and X that its metadata does not name as depth (Z) or time (T), for example a plain stack. (A mask one pixel high or wide is a 2D mask, not this error.) State the axes of its mask type once (`--mask-axes Cells=TYX`), or save it as a 2D label image, or with its axes named (an ImageJ hyperstack or OME-TIFF with axes TYX or ZYX). | | `invalid_mask_axes`, `mask_axes_contradict_header` | Error | Axes stated for a mask type do not fit its masks (one role, Y, X, Z or T, per axis; Y and X once each), name a type that is not found, or contradict the axes a mask's metadata names. Correct the statement. | | `mask_layout` | Error | A mask is stored with its Y and X the other way round from its image as read (its header names X before Y, for example `XY`, named in the message with the mask; or the image's height and width swapped). Masks are never transposed: save it again with `numpy.swapaxes(mask, -1, -2)`. | -| `mask_planes_differ` | Error | The masks of one image have different numbers of depth slices or time points. Make every mask of an image with the image's planes. | +| `mask_planes_differ` | Error | The masks of one image have different numbers of depth slices or time points. Make every mask of an image with the image's planes. A 2-D mask stated as shared (`--mask-axes Nuclei=YX`) is one plane for every plane and is left out of this check, by `run` and by `masks` alike. | | `mask_planes_mismatch` | Error (`run`) | A mask does not have the depth slices and time points of its image under the confirmed axis roles: a 2D mask fits only an image with one plane (not a Z-stack or time-lapse) unless its type is stated `YX` (`--mask-axes Cells=YX`: one mask shared by every plane), and a mask with Z or T needs as many slices or time points as its image. Save one mask plane per image plane, with the axes named, or check the axis roles. | | `mask_size_mismatch` | Error | A mask does not have the width and height of its image. Check that it was made from that image at full resolution. | | `mask_pixel_type` | Error | A mask's pixels are not whole numbers (for example decimals). Save it as an integer label image. | diff --git a/src/morphobricks_preprocessing/axis_review.py b/src/morphobricks_preprocessing/axis_review.py index 652dad79..16bf439e 100644 --- a/src/morphobricks_preprocessing/axis_review.py +++ b/src/morphobricks_preprocessing/axis_review.py @@ -31,6 +31,7 @@ TiffMetadata, channel_count_issues, ) +from .size_checks import CONVERT_KEEPS_ORDER from .validation import ValidationIssue, error, plural @@ -222,8 +223,7 @@ def review_axes( f"{plural(len(misfits), 'image')}: their dimensions are arranged " "differently.", misfits, - "Export the images again with the same dimension order, or process the " - "differently arranged images as a separate experiment.", + CONVERT_KEEPS_ORDER, ) ) issues.extend( diff --git a/src/morphobricks_preprocessing/cli_convert.py b/src/morphobricks_preprocessing/cli_convert.py index 7531ed0d..9c37a6f5 100644 --- a/src/morphobricks_preprocessing/cli_convert.py +++ b/src/morphobricks_preprocessing/cli_convert.py @@ -5,7 +5,8 @@ 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. +folder of the runs on the originals (``--output``, by default the experiment's default +one). The command is offered for any experiment. """ from __future__ import annotations @@ -83,11 +84,17 @@ def add_convert_parser(commands: argparse._SubParsersAction) -> argparse.Argumen 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( + "--output", + metavar="FOLDER", + help="the output folder of your runs on the originals, named in the next commands " + "(default: the experiment folder's name with _MorphoBricks_Preprocessing)", + ) convert.add_argument( "--masks", metavar="FOLDER", help="the masks folder of your runs, which the copies may not be inside; by default " - "the one recorded by the run in the default output folder", + "the one recorded by the run in the output folder (--output)", ) convert.add_argument("--log", metavar="FILE", help="write technical details to this file") return convert @@ -99,7 +106,12 @@ def convert_command(args: argparse.Namespace) -> int: mapping = mapping_option(args, pre, None) if mapping is None or mapping is False: return 1 - output = default_output_folder(Path(args.experiment).resolve()) + given = args.output is not None + output = ( + Path(args.output).resolve() + if given + else default_output_folder(Path(args.experiment).resolve()) + ) result = convert_experiment( args.experiment, mapping, @@ -113,11 +125,11 @@ def convert_command(args: argparse.Namespace) -> int: if result.issues: print(format_issues(result.issues)) print() - print(conversion_text(result, output)) + print(conversion_text(result, output, given=given)) return 0 if result.written else 1 -def conversion_text(result: ConversionResult, output: Path) -> str: +def conversion_text(result: ConversionResult, output: Path, *, given: bool = True) -> str: """Say what a conversion wrote and what to run next, in plain ASCII-safe words. Parameters @@ -126,6 +138,9 @@ def conversion_text(result: ConversionResult, output: Path) -> str: What :func:`.conversion.convert_experiment` returned. output The output folder of the runs on the originals, named in the next commands. + given + False when ``output`` is the default one, not given with ``--output``: the text then + says to give the folder of the runs on the originals if it is another one. Returns ------- @@ -134,6 +149,9 @@ def conversion_text(result: ConversionResult, output: Path) -> str: dest = result.destination if not result.written: return f"No copies were written in {dest}: read the messages above." + # Cross-brick test F8: the next commands name the output folder of the user's runs, + # given with --output; a default one may hold no run (the user then gives theirs). + where = "" if given else f" (by default {output}; if your runs used another one, give it)" lines = [ f"Named-axis copies in {dest}:", f" written as OME-TIFF with your roles: {plural(len(result.converted), 'image')}", @@ -142,7 +160,7 @@ def conversion_text(result: ConversionResult, output: Path) -> str: 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"originals{where}:", f" morphobricks-preprocessing check {dest} --output {output}", ] axes = f" --axes {result.suggested_axes}" if result.suggested_axes else " --axes ROLES" diff --git a/src/morphobricks_preprocessing/cli_run.py b/src/morphobricks_preprocessing/cli_run.py index f8f8aa92..47dbb6eb 100644 --- a/src/morphobricks_preprocessing/cli_run.py +++ b/src/morphobricks_preprocessing/cli_run.py @@ -1,7 +1,8 @@ """The command line's ``run``: its options and what it prints (moved from :mod:`.cli`). Every input left out is the earlier run's in the same output folder (design v0.4 §1): -``--axes`` is needed for the first run and for plain stacks that are new or changed; +``--axes`` is needed for the first run of plain stacks (not when every image names its +axes, design v0.4 §3) and for plain stacks that are new or changed; ``--no-masks`` drops the recorded masks folder, ``--no-exclude`` the recorded exclusions; ``--mode full`` recomputes every row. Given only ``--output``, ``run`` uses the recorded experiment folder (``experiment_not_recorded`` without one). @@ -41,6 +42,7 @@ from .image_type import IMAGE_TYPES, RGB, ImageTypeReview, review_image_type from .intensity import intensity_text from .manifest import RunManifest +from .metadata import header_axes from .planes import DEFAULT_MEMORY_LIMIT from .report import ( axis_check_summary, @@ -150,7 +152,8 @@ def _run_inputs(run: argparse.ArgumentParser) -> None: metavar="ROLES", help="role of each axis of the images, in the order of their axes: X, Y, Z, C, T, or " "- for none (for example ZCYX). 'check' prints the suggested value. Needed for the " - "first run, and again for plain stacks that are new or changed", + "first run unless every image names its axes, and again for plain stacks that are " + "new or changed", ) run.add_argument( "--design", @@ -310,6 +313,12 @@ def mapping_option( if args.axes is None: if recorded is not None: return None + named = _named_mapping(pre) + if named is not None: # design v0.4 §3: --axes is only needed for plain stacks + return named + if axes is None: # no arrangement to give roles to: say why, not "--axes None" + print(format_issues(pre.issues) or "No readable images were found.") + return False suggested = pre.suggested_mapping.roles_string if pre.suggested_mapping else axes print( "Give the role of each axis with --axes (for example --axes " @@ -331,6 +340,20 @@ def mapping_option( return False +def _named_mapping(pre: Preflight) -> AxisMapping | None: + """Return the roles every image's own metadata names, or None when any image needs them. + + A plain stack names no axes, so its roles must be stated (``--axes``); when every + readable image names all its axes (:func:`.metadata.header_axes`) and the suggested + mapping gives each axis a role, the roles are the headers' and nothing is guessed. + """ + suggested = pre.suggested_mapping + readable = [f for f in pre.metadata.files if f.readable] + if suggested is None or None in suggested.roles or not readable: + return None + return None if any(header_axes(f) is None for f in readable) else suggested + + def _settings_options( args: argparse.Namespace, recorded: RunManifest | None ) -> tuple[IntensityCheckSettings | None, HistogramSettings | None] | None: @@ -375,7 +398,8 @@ def _print_inputs( if mapping is None: print(f"Axis roles: as confirmed for the earlier run ({recorded.mapping.roles_string})") else: - print("Axis roles:") + named = args.axes is None # mapping_option: every image names its axes + print("Axis roles (as every image names them):" if named else "Axis roles:") print(mapping_summary(mapping, pre.metadata.files)) print() print(axis_check_summary(review)) diff --git a/src/morphobricks_preprocessing/conversion_issues.py b/src/morphobricks_preprocessing/conversion_issues.py index eaa7ea68..9f101c9a 100644 --- a/src/morphobricks_preprocessing/conversion_issues.py +++ b/src/morphobricks_preprocessing/conversion_issues.py @@ -147,13 +147,31 @@ def failed_issue(path: str, exc: Exception) -> ValidationIssue: def lock_issue(exc: OutputLockedError) -> ValidationIssue: - """Refuse a conversion while another one writes into the same folder.""" + """Refuse a conversion while another one writes into the same folder. + + A lock left by a conversion on this computer that is no longer running is called stale + (cross-brick test F11), and it is kept: only the user removes it. + """ + from .run.lock import looks_stale + + removal = ( + "give --remove-stale-lock to the command, press 'Remove the lock of a stopped run' " + "in the notebook, or call remove_stale_lock(folder), then convert again." + ) + if looks_stale(exc.holder): + return error( + "output_locked", + f"The lock of this folder looks stale: it was left by {exc.holder.describe()}, " + "which is no longer running (the conversion was probably stopped). The lock was " + "not removed, so nothing was done.", + [exc.path.as_posix()], + f"If no conversion is going on in this folder, remove the lock: {removal}", + ) 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.", + f"restarted), remove the lock: {removal}", ) diff --git a/src/morphobricks_preprocessing/mask_scan.py b/src/morphobricks_preprocessing/mask_scan.py index 69ebc091..cc7dc1c7 100644 --- a/src/morphobricks_preprocessing/mask_scan.py +++ b/src/morphobricks_preprocessing/mask_scan.py @@ -419,8 +419,9 @@ def scan_masks( ------ ValueError For an invalid threshold, before any mask is read; for an image whose masks have - different depth slices or time points; for a mask that cannot be read as the planes - its header gave, or holds a label outside 0 to 2,147,483,647 (:func:`.masks.read_mask`). + different depth slices or time points (a stated shared 2-D mask aside); for a mask + that cannot be read as the planes its header gave, or holds a label outside 0 to + 2,147,483,647 (:func:`.masks.read_mask`). """ limits = mask_thresholds(thresholds) root = Path(experiment_root).resolve() @@ -433,7 +434,10 @@ def scan_masks( if progress is not None: progress(done, len(images), image) found_masks = inventory.for_image(image) - planes = {tuple(mask.planes.items()) for mask in found_masks.values()} + # A stated shared 2-D mask is one plane for every plane of its image, as in the + # run (masks.checks): it is left out of the comparison and read for every plane. + planes = {tuple(m.planes.items()) for m in found_masks.values() if not m.shared} + planes = planes or {tuple(m.planes.items()) for m in found_masks.values()} if len(planes) != 1: raise ValueError( f"the masks of {image} have different depth slices or time points " diff --git a/src/morphobricks_preprocessing/metadata.py b/src/morphobricks_preprocessing/metadata.py index f554e28c..907a1cbe 100644 --- a/src/morphobricks_preprocessing/metadata.py +++ b/src/morphobricks_preprocessing/metadata.py @@ -32,6 +32,7 @@ from .physical_sizes import PhysicalSizes, _tiff_sizes from .pixel_size_checks import pixel_size_issues from .size_checks import ( + CONVERT_KEEPS_ORDER, breakdown, image_size_issues, lettered_plane_counts, @@ -642,8 +643,7 @@ def _check_dimensions(files: list[TiffMetadata]) -> list[ValidationIssue]: f"The images have their dimensions arranged differently: {summary}. One " "axis mapping cannot describe all of them.", minority, - "Export the images again with the same dimension order, or process the " - "differently arranged images as a separate experiment.", + CONVERT_KEEPS_ORDER, ) ] return [] diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index aeb876d6..a7f72511 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -27,7 +27,7 @@ from .notebook_plots import IntensityPanel from .notebook_render import _bins_help, _label, change_lines, issues_html from .planes import DEFAULT_MEMORY_LIMIT -from .report import issue_counts +from .report import issue_counts, rebuild_line from .run import RESTART, Preflight, RunResult, run_experiment from .validation import has_errors, plural @@ -360,7 +360,7 @@ def _show_run_result(self, result: RunResult) -> None: lines.append(html.escape(f"Experiment design: {result.design_path}")) lines.append(f"Mode: {result.mode}") if result.rebuild_reason: - lines.append(html.escape(f"Every image was read because {result.rebuild_reason}.")) + lines.append(html.escape(rebuild_line(result))) if result.replaced: lines.append("Replaced: " + html.escape(", ".join(result.replaced))) changes = change_lines(result.input_changes) diff --git a/src/morphobricks_preprocessing/report.py b/src/morphobricks_preprocessing/report.py index ff15c2ce..108b67f2 100644 --- a/src/morphobricks_preprocessing/report.py +++ b/src/morphobricks_preprocessing/report.py @@ -281,7 +281,7 @@ def run_summary(result: RunResult, *, design_template: bool = False) -> str: if result.image_type is not None: lines.append(f"Image type: {result.image_type}") if result.rebuild_reason: - lines.append(f"All rows were computed because {result.rebuild_reason}.") + lines.append(rebuild_line(result)) for label, files in ( ("Read", result.read), ("Resumed (rows staged by an interrupted run)", result.resumed), @@ -437,3 +437,14 @@ def _number(value: float | None) -> str: if value is None: return "–" return f"{value:.6g}" + + +def rebuild_line(result: RunResult) -> str: + """Say why no earlier row was kept; a resumed run names the rows it resumed (F11).""" + if result.resumed: + return ( + f"No earlier run's rows were kept because {result.rebuild_reason}; the rows " + f"staged by the interrupted run were resumed ({plural(len(result.resumed), 'image')})" + " and the others computed." + ) + return f"All rows were computed because {result.rebuild_reason}." diff --git a/src/morphobricks_preprocessing/run/lock.py b/src/morphobricks_preprocessing/run/lock.py index a5cd4db8..596cd3ba 100644 --- a/src/morphobricks_preprocessing/run/lock.py +++ b/src/morphobricks_preprocessing/run/lock.py @@ -181,14 +181,44 @@ def lock_issue(exc: OutputLockedError) -> ValidationIssue: ValidationIssue The error ``output_locked``, naming the holder and the lock file. """ + holder = exc.holder + removal = ( + "give --remove-stale-lock to the command, press 'Remove the lock of a stopped run' " + "in the notebook, or call remove_stale_lock(output_folder), then run again." + ) + if looks_stale(holder): + # Cross-brick test F11: a run killed here left its lock. It looks stale, but the + # brick never removes a lock on its own (module docstring): the user decides. + return error( + "output_locked", + f"The output folder's lock looks stale: it was left by {holder.describe()}, " + "which is no longer running (the run was probably stopped). The lock was not " + "removed, so nothing was done.", + [exc.path.as_posix()], + f"If no run is going on in this output folder, remove the lock: {removal}", + ) return error( "output_locked", - f"Another run is using this output folder: {exc.holder.describe()}. Two runs must " + f"Another run is using this output folder: {holder.describe()}. Two runs must " "not write the same results at once, so nothing was done.", [exc.path.as_posix()], "Wait for the other run to finish. If no run is going on (it was stopped or the " - "computer restarted), remove the lock: give --remove-stale-lock to the command, or " - "call remove_stale_lock(output_folder), then run again.", + f"computer restarted), remove the lock: {removal}", + ) + + +def looks_stale(holder: LockHolder) -> bool: + """Return True only when the holder ran on this computer and is no longer running. + + Only POSIX can tell (:func:`_running`); elsewhere, and for a holder on another computer, + the answer is False. It only words the refusal: the lock is never removed on its own. + """ + pid = holder.pid + return ( + os.name == "posix" + and pid > 0 + and holder.host == socket.gethostname() + and (not _running(pid)) ) diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index b9fc423e..8be9e980 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -404,9 +404,23 @@ def _write_generation( calibration_sha256=pre.calibration.sha256, conversion_record_sha256=None if pre.conversion is None else pre.conversion.sha256, ) + manifest = _kept_time(manifest, run.previous[0]) return file_sha256(write_manifest(manifest, staged / MANIFEST_FILENAME)) +def _kept_time(manifest: RunManifest, earlier: RunManifest | None) -> RunManifest: + """Return the manifest with the earlier run's ``written_at`` when nothing else changed. + + Later bricks pin the manifest's SHA-256 (CellPatcher, MorphoFeature): a rerun that + changes nothing must leave its bytes as they were (cross-brick test F10). The report + still records the rerun. + """ + if earlier is None: + return manifest + same = replace(manifest, written_at=earlier.written_at) + return same if same.to_dict() == earlier.to_dict() else manifest + + def _replaced(run: _Run, masks: dict | None) -> dict[str, str]: """Return the earlier output files the run replaces or removes, and why. diff --git a/src/morphobricks_preprocessing/size_checks.py b/src/morphobricks_preprocessing/size_checks.py index 9ae3fa48..438d4032 100644 --- a/src/morphobricks_preprocessing/size_checks.py +++ b/src/morphobricks_preprocessing/size_checks.py @@ -21,6 +21,13 @@ """Dimensions that do not hold planes: the image itself, and channels (or RGB samples), which are compared by ``different_channel_counts``.""" +CONVERT_KEEPS_ORDER = ( + "Export the images again with the same dimension order, or process the differently " + "arranged images as a separate experiment. Named-axis copies ('convert') keep each " + "image's stored order, so they do not solve this." +) +"""What to do about images arranged differently (cross-brick test F8, plan question 73).""" + def lettered_plane_counts(axes: str, shape: Sequence[int]) -> dict[str, int]: """Return the planes along each header letter except X, Y, S and C; Z and T default to 1.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index f1532142..c196d1b8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -15,7 +15,7 @@ ) from morphobricks_preprocessing.cli import main, parse_size from run_files import check_outputs -from tiff_tree import write_data, write_file +from tiff_tree import write_data, write_file, write_plain @pytest.fixture @@ -239,13 +239,39 @@ def test_different_channel_counts_stop_check_and_run(tmp_path, capsys): assert "no table or manifest was written" in out -def test_axes_are_needed_for_the_first_run_only(experiment, capsys): +def test_axes_are_needed_for_the_first_run_of_plain_stacks_only(tmp_path, capsys): + root = tmp_path / "exp" + write_plain(root / "ctrl/rep1/a.tif", np.zeros((2, 8, 8), np.uint8)) + confirm_design(root) + assert main(["run", str(root)]) == 1 + assert "Give the role of each axis with --axes" in capsys.readouterr().out + assert main(["run", str(root), "--axes", "ZYX"]) == 0 + capsys.readouterr() + assert main(["run", str(root)]) == 0 + assert "as confirmed for the earlier run (ZYX)" in capsys.readouterr().out + + +def test_a_plain_stack_among_named_images_still_needs_axes(tmp_path, capsys): + # F9 review: one plain stack is enough for the roles not to be taken from the headers; + # images that no mapping describes are named instead of "--axes None". + root = tmp_path / "exp" + write_data(root / "ctrl/rep1/a.tif", np.zeros((2, 8, 8), np.uint8), axes="ZYX") + write_plain(root / "ctrl/rep1/b.tif", np.zeros((2, 8, 8), np.uint8)) + confirm_design(root) + assert main(["run", str(root)]) == 1 + out = capsys.readouterr().out + assert "--axes None" not in out and "Axis roles" not in out + assert "arranged differently: IYX (1 image), ZYX (1 image)" in out + assert not (root.parent / "exp_MorphoBricks_Preprocessing" / "run_manifest.json").exists() + + +def test_images_that_name_their_axes_need_no_axes_option(experiment, capsys): + # F9 (cross-brick test, 3 November 2026; design v0.4 §3): --axes is for plain stacks. confirm_design(experiment) - assert main(["run", str(experiment)]) == 1 - assert "Give the role of each axis with --axes (for example --axes ZCYX)" in ( - capsys.readouterr().out - ) - assert main(["run", str(experiment), "--axes", "ZCYX"]) == 0 + assert main(["run", str(experiment)]) == 0 + out = capsys.readouterr().out + assert "Axis roles (as every image names them):" in out + assert "Give the role of each axis" not in out capsys.readouterr() # A later run in the same output folder reuses the confirmed roles. assert main(["run", str(experiment)]) == 0 diff --git a/tests/test_convert_run.py b/tests/test_convert_run.py index 325ece80..e60d7b23 100644 --- a/tests/test_convert_run.py +++ b/tests/test_convert_run.py @@ -9,6 +9,7 @@ import json import os +import socket import numpy as np import pytest @@ -23,12 +24,14 @@ RunInputsError, convert_experiment, file_sha256, + preflight, 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 +from tiff_tree import write_data SWAPPED = AxisMapping("ZYX", ("Z", "X", "Y")) PLAIN = AxisMapping("IYX", ("Z", "Y", "X")) @@ -409,6 +412,35 @@ def test_the_convert_command_writes_the_copies_and_names_the_next_commands(tmp_p assert "already there and unchanged: 3 images" in capsys.readouterr().out +def test_the_printed_next_command_uses_the_output_folder_given_and_works(tmp_path, capsys): + # F8 (cross-brick test, 3 November 2026): the next command named the default output + # folder although the runs used another one, and was refused. + exp, copies, out = swapped_experiment(tmp_path / "exp"), tmp_path / "copies", tmp_path / "pre" + main(["check", str(exp), "--output", str(out)]) # writes the design template + assert main(["design", "--output", str(out), "--confirm-all"]) == 0 + capsys.readouterr() + args = ["convert", str(exp), "--to", str(copies), "--axes", "ZXY", "--output", str(out)] + assert main(args) == 0 + text = capsys.readouterr().out + (command,) = [line.split()[1:] for line in text.splitlines() if "preprocessing run " in line] + assert command == ["run", str(copies), "--output", str(out), "--axes", "ZYX"] + assert "by default" not in text + assert main(command) == 0, capsys.readouterr().out + # Without --output the default folder is named, with what to do if it is not theirs. + copies_b = tmp_path / "copies_b" + assert main(["convert", str(exp), "--to", str(copies_b), "--axes", "ZXY"]) == 0 + assert "if your runs used another one, give it" in capsys.readouterr().out + + +def test_images_arranged_differently_are_told_that_convert_keeps_the_order(tmp_path): + # F8: convert keeps each image's stored order, so it cannot solve this (question 73). + exp = tmp_path / "exp" + write_data(exp / "ctrl/r1/a.tif", np.zeros((2, 3, 6, 8), np.uint8), axes="ZCYX") + write_data(exp / "ctrl/r1/b.tif", np.zeros((3, 2, 6, 8), np.uint8), axes="CZYX") + (issue,) = [i for i in preflight(exp).issues if i.code == "inconsistent_dimensions"] + assert "('convert') keep each image's stored order" in issue.action + + 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 @@ -426,12 +458,29 @@ def test_the_convert_command_removes_a_stale_lock_only_on_request(tmp_path, caps 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 + # F11 (cross-brick test, 3 November 2026): a dead holder here is called stale, as in run. + text = capsys.readouterr().out + assert "Another conversion" not in text + assert "lock of this folder looks stale" in text and "no longer running" in text + assert "--remove-stale-lock" in text and lock.exists() # never removed on its own 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_a_conversion_whose_folder_a_running_process_locks_is_not_called_stale(tmp_path, capsys): + exp, copies = swapped_experiment(tmp_path / "exp"), tmp_path / "copies" + copies.mkdir() + lock = copies / LOCK_FILENAME + lock.write_text(json.dumps({"host": "another-computer", "pid": os.getpid()})) + assert main(["convert", str(exp), "--to", str(copies), "--axes", "ZXY"]) == 1 + text = capsys.readouterr().out + assert "Another conversion is writing into this folder" in text and "looks stale" not in text + lock.write_text(json.dumps({"host": socket.gethostname(), "pid": os.getpid()})) + assert main(["convert", str(exp), "--to", str(copies), "--axes", "ZXY"]) == 1 + assert "looks stale" not in capsys.readouterr().out + + def test_an_ome_zarr_image_is_copied_as_it_is(tmp_path): from zarr_tree import write_ngff diff --git a/tests/test_masks_v040.py b/tests/test_masks_v040.py index 96cc33bb..2835f2a8 100644 --- a/tests/test_masks_v040.py +++ b/tests/test_masks_v040.py @@ -317,3 +317,28 @@ def test_label_images_are_left_out_of_the_image_fingerprint(tmp_path): assert again.read == () # the image's rows are reused data = json.loads(again.manifest_path.read_text(encoding="utf-8")) assert data["masks"]["types"] == ["cells", "nuclei"] + + +def test_the_masks_command_scans_a_shared_2d_mask_with_per_plane_masks(tmp_path, capsys): + # F1 (cross-brick test, 3 November 2026): a set the run accepts (per-plane Cells and + # a stated shared 2-D Nuclei) was refused with mask_planes_differ. + root, masks = tmp_path / "exp", tmp_path / "masks" + write_data(root / "ctrl/r1/a.tif", np.zeros((3, 6, 8), np.uint8), "ZYX") + cells = np.zeros((3, 6, 8), np.uint16) + cells[:, 1:5, 1:7] = 1 + nuclei = np.zeros((6, 8), np.uint16) + nuclei[2:4, 3:5] = 1 + write_data(masks / "Cells/ctrl/r1/a.tif", cells, "ZYX") + write_plain(masks / "Nuclei/ctrl/r1/a.tif", nuclei) + stated = {"Nuclei": "YX"} + assert run_experiment( + root, prefill_mapping("ZYX"), masks_folder=masks, mask_axes=stated + ).written + out = tmp_path / "out" + args = [str(root), "--masks", str(masks), "--output", str(out), "--mask-axes", "Nuclei=YX"] + assert main(["masks", *args]) == 0, capsys.readouterr().out + report = json.loads((out / "mask_relationships.json").read_text(encoding="utf-8")) + (inside,) = [p for p in report["pairs"] if (p["child"], p["parent"]) == ("Nuclei", "Cells")] + # The shared plane is compared with each of the three planes: never across planes. + assert inside["relation"] == "inside" + assert inside["evidence"]["children"] == 3 diff --git a/tests/test_resume.py b/tests/test_resume.py index 05c09de4..2ec2a8ca 100644 --- a/tests/test_resume.py +++ b/tests/test_resume.py @@ -17,7 +17,13 @@ import pytest from morphobricks_normalization.csv_io import OUTPUT_FILENAME -from morphobricks_preprocessing import PlaneReadError, RunInputs, prefill_mapping, run_experiment +from morphobricks_preprocessing import ( + PlaneReadError, + RunInputs, + prefill_mapping, + read_manifest, + run_experiment, +) from morphobricks_preprocessing import run as run_module from morphobricks_preprocessing.discovery_report import REPORT_FILENAME from morphobricks_preprocessing.run.lock import remove_stale_lock @@ -299,7 +305,32 @@ def test_the_summary_names_the_resumed_images(tmp_path, experiment): root, out = experiment killed_run(tmp_path, root, out, kill_at=3) result = run_experiment(root, ZYX, output_folder=out) - assert "Resumed (rows staged by an interrupted run): 2 images" in run_summary(result) + summary = run_summary(result) + assert "Resumed (rows staged by an interrupted run): 2 images" in summary + # F11 (cross-brick test, 3 November 2026): not "All rows were computed". + assert "All rows were computed" not in summary + assert "the rows staged by the interrupted run were resumed (2 images)" in summary + + +@pytest.mark.skipif(os.name != "posix", reason="process IDs are checked on POSIX only") +def test_a_lock_left_by_a_killed_run_is_called_stale_and_kept(tmp_path, experiment, capsys): + import json + import socket + + from morphobricks_preprocessing.cli import main + from morphobricks_preprocessing.run.lock import LOCK_FILENAME + from run_files import finished_pid + + root, out = experiment + out.mkdir(parents=True, exist_ok=True) + lock = out / LOCK_FILENAME + lock.write_text(json.dumps({"host": socket.gethostname(), "pid": finished_pid()})) + assert main(["run", str(root), "--output", str(out), "--axes", "ZYX"]) == 1 + text = capsys.readouterr().out + assert "Another run is using" not in text + assert "lock looks stale" in text and "no longer running" in text + assert "--remove-stale-lock" in text and "Remove the lock of a stopped run" in text + assert lock.exists() # never removed without the user's request @pytest.mark.skipif(os.name != "posix", reason="SIGKILL is POSIX only") @@ -320,3 +351,28 @@ def test_a_discarded_record_is_reported_by_its_image_and_its_true_reason(tmp_pat discarded = report(out)["staged_discarded"] assert discarded[record["relative_path"]].startswith("the experiment's pixel type changed") assert discarded[named].startswith("its staged rows could not be read") + + +def test_a_rerun_that_changes_nothing_keeps_the_manifest_bytes(tmp_path, monkeypatch): + # F10 (cross-brick test, 3 November 2026): later bricks pin the manifest's SHA-256. + root = tmp_path / "exp" + write_data(root / "ctrl/r1/a.tif", stack(0), axes="ZYX") + times = iter(f"2026-11-0{day}T10:00:00+00:00" for day in range(1, 9)) + + class Clock: + @staticmethod + def now(tz=None): + return __import__("datetime").datetime.fromisoformat(next(times)) + + monkeypatch.setattr(run_module.pipeline, "datetime", Clock) + first = run_experiment(root, ZYX) + before = first.manifest_path.read_bytes() + again = run_experiment(root, ZYX) + assert again.written and again.manifest_path.read_bytes() == before + assert read_manifest(again.manifest_path).written_at == "2026-11-01T10:00:00+00:00" + # A real change still updates it, with the time of the run that made it. + write_data(root / "ctrl/r1/b.tif", stack(1), axes="ZYX") + changed = run_experiment(root, ZYX) + after = read_manifest(changed.manifest_path) + assert changed.manifest_path.read_bytes() != before + assert after.written_at == "2026-11-03T10:00:00+00:00" diff --git a/tools/code_rules_baseline.json b/tools/code_rules_baseline.json index 32f9ea0f..a2cad5fe 100644 --- a/tools/code_rules_baseline.json +++ b/tools/code_rules_baseline.json @@ -10,7 +10,7 @@ "identical bodies: tests/test_axis_check.py:# plain, tests/test_header_axes.py:# plain": 0, "identical bodies: tests/test_cli.py:# bright_tree, tests/test_notebook_ui.py:# bright_tree": 0, "src/morphobricks_preprocessing/axis_review.py check_mapping: # executable lines (max #)": 72, - "src/morphobricks_preprocessing/axis_review.py review_axes: # executable lines (max #)": 67, + "src/morphobricks_preprocessing/axis_review.py review_axes: # executable lines (max #)": 66, "src/morphobricks_preprocessing/dataset.py _missing_issues: private function of # lines without a docstring": 32, "src/morphobricks_preprocessing/dataset.py _unbalanced_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/design.py _check_strict_values: private function of # lines without a docstring": 19, @@ -24,7 +24,7 @@ "src/morphobricks_preprocessing/masks/files.py _duplicate_issues: private function of # lines without a docstring": 19, "src/morphobricks_preprocessing/masks/files.py _name_issues: private function of # lines without a docstring": 15, "src/morphobricks_preprocessing/metadata.py _check_channel_names: private function of # lines without a docstring": 32, - "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 30, + "src/morphobricks_preprocessing/metadata.py _check_dimensions: private function of # lines without a docstring": 29, "src/morphobricks_preprocessing/metadata.py check_metadata: # executable lines (max #)": 96, "src/morphobricks_preprocessing/metadata.py: # lines (max #)": 747, "src/morphobricks_preprocessing/planes.py _layout: private function of # lines without a docstring": 45,