From 84f9977cdb0fe91a3fb10b045fb8c69dbe6624d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:32:08 +0000 Subject: [PATCH 1/3] =?UTF-8?q?PP-11b:=20the=20notebook's=20Make=20named-a?= =?UTF-8?q?xis=20copies=20card=20(contract=20=C2=A717)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 2's card under the axis check, shown when it reports axes_yx_swapped, axes_contradict_header, axes_x_before_y or axes_not_named: a required folder for the copies (the command's rule), the user's confirmed roles, the plan from preview_conversion (the command's plan_conversion, moved to conversion_plan), the space, then convert_experiment with a progress bar under the copy folder's lock; refusals and the stale-lock button on the existing cards; afterwards the record and Check the copies in step 1. ConversionResult.unknown_sizes lists the copies whose relabelled sizes are unknown. Hidden with folder_step(show_output=False). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 14 +- README.md | 3 +- docs/contract.md | 2 +- docs/plan.md | 35 +- docs/user_guide.md | 36 +- notebooks/MorphoBricks_Preprocessing.ipynb | 52 ++- pyproject.toml | 2 + src/morphobricks_preprocessing/__init__.py | 3 + src/morphobricks_preprocessing/conversion.py | 166 +------ .../conversion_plan.py | 284 ++++++++++++ .../notebook_axis_step.py | 2 + .../notebook_convert.py | 433 ++++++++++++++++++ .../notebook_output.py | 36 +- src/morphobricks_preprocessing/notebook_ui.py | 3 + tests/test_notebook_convert.py | 397 ++++++++++++++++ tests/test_notebook_file.py | 3 + 16 files changed, 1291 insertions(+), 180 deletions(-) create mode 100644 src/morphobricks_preprocessing/conversion_plan.py create mode 100644 src/morphobricks_preprocessing/notebook_convert.py create mode 100644 tests/test_notebook_convert.py diff --git a/AGENTS.md b/AGENTS.md index 2fae5804..3de74986 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -158,7 +158,10 @@ src/morphobricks_preprocessing/ conversion_record.py the copy tree's .morphobricks-conversion.json (fingerprints, roles, sizes), review_conversion (the run's conversion_source_changed and copy_changed_after_conversion warnings) - conversion.py convert_experiment/plan_conversion: the copy tree under its lock, .partial + conversion_plan.py plan_conversion (headers only: what each image's copy will be), + copy_folder_issues (the folder rule) and preview_conversion (the plan, + space needed and free, for the notebook card; writes nothing, PP-11b) + conversion.py convert_experiment: the copy tree under its lock, .partial files moved into place together (any failure removes what it wrote), planes checked against the memory limit first, unchanged copies reused, record last; conversion_issues.py its messages (pure) @@ -171,6 +174,9 @@ src/morphobricks_preprocessing/ notebook_masks.py step 1's masks folder, Masks card and mask-axis statements (PP-10c) notebook_mask_step.py step 3's Describe the masks card: the masks step after a run that recorded masks (PP-10d) + notebook_convert.py step 2's Make named-axis copies card under the axis check (PP-11b): + preview_conversion, then convert_experiment under the copy folder's lock, + then Check the copies in step 1 (same output folder) notebook_ui.py PreprocessingNotebook: the four notebook steps (needs the `notebook` extra); folder_step/axis_step take title, purpose, intro and show_next (and show_output and show_design for step 1) so other bricks can embed them @@ -909,7 +915,11 @@ jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb the CLI calls too; see plan §10. - **Named-axis copies (PP-11):** `convert` / `convert_experiment` writes OME-TIFF copies of the images with axis errors or unnamed axes with the confirmed roles into a new folder - (contract §17, plan §10 and question 59); the notebook card is PP-11b. + (contract §17, plan §10 and question 59). +- **Notebook conversion card (PP-11b):** step 2's **Make named-axis copies (optional)** card, + under the axis check when it reports these images: the folder for the copies, the + confirmed roles, the plan (`preview_conversion`), the copies (`convert_experiment`) with a + progress bar, the lock card, and **Check the copies in step 1**; see plan §10. - **Next (not started):** the notebook step for the masks (name each mask, confirm or correct the relationships) and a file saving those answers for the other bricks (question 32); 3D and time-lapse masks later. diff --git a/README.md b/README.md index b0cc78ea..4130c46a 100644 --- a/README.md +++ b/README.md @@ -172,7 +172,8 @@ gets the permissions a file you save yourself would (your umask). `convert` save images whose metadata name their axes wrongly or not at all as named-axis OME-TIFF copies with your roles in a new folder (each plane bit for bit the plane read with those roles; every other image copied as it is; a conversion record written last), then you run -preprocessing on that folder with the same output folder. Details in the +preprocessing on that folder with the same output folder. The notebook offers the same +conversion in step 2, under the axis check, when it finds such images. Details in the [user guide](docs/user_guide.md) §5. ## Example diff --git a/docs/contract.md b/docs/contract.md index 6284a5c4..786e6b56 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -516,4 +516,4 @@ exclude=, missing_as_singleton=, memory_limit=, progress=)` (`conversion`), the | Repeat | A copy already in the tree is reused when its source (fingerprint and axes), roles, copy axes, pixel sizes (and unknown ones) and the copy's fingerprint are as recorded; a new image is added; when every copy is reused the record is kept as it was (same SHA-256). Anything else (a source changed, other roles, a copy changed, an image no longer in the experiment, another experiment's copy tree, a file at a new image's place that the record does not list) is `copy_differs`: nothing is overwritten. A conversion record that cannot be read is `conversion_record_unreadable`. | | Conversion record | `.morphobricks-conversion.json` in the copy tree's root (a dot-file, so discovery skips it), record version 1: `source_root`, `mapping`, `missing_as_singleton`, `written_at`, `versions`, and per image `relative_path`, `action` (`converted` or `copied`), `reason` (the axis code, `excluded`, or null), `source` and `copy` fingerprints (`size`, `mtime_ns`), `source_axes`, `roles`, `copy_axes`, `pixel_sizes` (by role, as §9) and `unknown_sizes` (role: why). `read_conversion_record(folder)` reads it with its SHA-256. | | The run on the copies | The user runs preprocessing on the copy tree with the same output folder (`--output`): the recorded experiment root becomes the copy tree and the report says so (`experiment_root_changed`, and `conversion` with the record, its SHA-256 and `source_root`; the text says "named-axis copies of ..."); every row is recomputed (the row key includes the read path, §9); the design, image type and calibration are kept (same relative paths). The manifest records `conversion_record_sha256`. The run and `check` warn when a source changed or went missing after the conversion (`conversion_source_changed`) and when a copy changed, went missing or was added since (`copy_changed_after_conversion`); an unreadable record stops the run (`conversion_record_unreadable`). `RunInputs` reads the copies like any images, and refuses a run whose conversion record changed or went missing since (`conversion_record_sha256`). | -| Notebook | Not yet: the axis-check card names the `convert` command; the notebook card is the next PR (PP-11b, plan §10). | +| Notebook (PP-11b) | Step 2's **Make named-axis copies (optional)** card, under the axis check, shown only when it reports images with `axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y` or `axes_not_named` for the roles being edited (design v0.4 §5: offered only for files with errors), and not with `folder_step(show_output=False)`. The roles are the user's: those confirmed in step 2, or the roles shown once the preview is checked and the card's own question is answered; any change of a role or of the missing-axes answer takes it back. A required folder for the copies (Browse…, or pasted) checked with the command's rule (`copy_folder_issues`); `preview_conversion` (`conversion_plan`, headers only, writes nothing) gives what will be converted and copied byte for byte (the command's `plan_conversion`), the space needed and the free space; then `convert_experiment` with a progress bar, under the copy tree's lock. Refusals use the existing cards (the lock card with **Remove the lock of a stopped run**, the rule of `--remove-stale-lock`). After the copies: the record's path, and **Check the copies in step 1**, which sets the experiment folder to the copy tree, keeps the output folder and checks again; the card says that the missing-axes answer must be given again and lists the copies whose relabelled sizes are unknown (`ConversionResult.unknown_sizes`), to state under **Pixel sizes**. | diff --git a/docs/plan.md b/docs/plan.md index 7e63cc48..516abb0c 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -513,17 +513,36 @@ API, CLI). The notebook and CLI only call the API. `RunInputs.open` refuses a record changed or gone since the run (question 59 (14)). - The axis messages (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, `axes_not_named`) and the notebook's axis marks name the `convert` command. - - Not in this PR: the notebook card for the conversion (PP-11b). + - The notebook card for the conversion followed in PP-11b. - Review (2 November 2026): the obsolete issues of `convert_tiffs`/`zarr_export` had acceptance criteria kept here: a memory budget that fails fast (#76), strict settings (#80), no 64-bit precision loss (refused, #75), the copy's fingerprint and scales checked before reuse (#87, #88, #92), no mixing of metadata epochs (#96); the run reads the copies themselves, so the table describes the persisted pixels (#79). -- **PP-11b (notebook card for the conversion)** *(next PR; owner decision of 2 November - 2026, plan §12 question 59 (15))*: design v0.4 §5 "the notebook offers conversion only - for files with errors": a card offered after the axis check when images have an axis - error or unnamed axes, calling `convert_experiment` with the confirmed roles and a - folder the user names, showing the result and the next run on the copies. +- **PP-11b (notebook card for the conversion)** *(implemented, this PR; owner decision of + 2 November 2026, plan §12 question 59 (15))*: design v0.4 §5 "the notebook offers + conversion only for files with errors" (contract §17 "Notebook"): + - Step 2's **Make named-axis copies (optional)** card (`notebook_convert`, ui), right + under the axis check, shown when it reports `axes_yx_swapped`, `axes_contradict_header`, + `axes_x_before_y` or `axes_not_named` for the roles being edited; hidden with + `folder_step(show_output=False)`; the pinned embedding API is unchanged. + - The roles: those confirmed with **Confirm axis roles**, or, while the axis errors keep + that button disabled, the roles shown once the preview is checked and the card's + **Yes, write the copies with these roles** is pressed; changing a role or the + missing-axes answer takes it back. Nothing is taken from the headers. + - A required folder for the copies (Browse…, the Colab **Copy path** card); the plan from + `preview_conversion` (`conversion_plan`, orchestration, new: the command's + `plan_conversion` and folder rule `copy_folder_issues`, moved there from `conversion`, + plus the space needed and free; writes nothing): images converted (with their code) and + copied byte for byte, the space; errors keep the button disabled. + - **Make named-axis copies** calls `convert_experiment` with a progress bar; refusals on + the existing cards (the lock card, now naming the folder for the copies, with **Remove + the lock of a stopped run**). + - After the copies: the record's path and **Check the copies in step 1** (the experiment + folder becomes the copy tree, the output folder is kept and filled in, and the folder is + checked again); the card says the missing-axes answer must be given again and lists the + copies whose relabelled Y/X sizes are unknown (`ConversionResult.unknown_sizes`, new), to + state under **Pixel sizes**. - **Release v0.4.0 (PP-13) checklist**, for every tag of v0.4.0 including release candidates, in the release PR before the tag is pushed: - set `version` in `pyproject.toml`; @@ -1463,6 +1482,6 @@ Raised during review; to be decided before the milestones that need them: (14) **`RunInputs` checks the conversion record**: a run whose manifest records `conversion_record_sha256` is refused by `RunInputs.open` when the record changed or disappeared, as for `calibration_sha256` and the design's SHA-256. - (15) **Notebook card**: the next PR, PP-11b (§10); the axis-check card names the - command meanwhile. + (15) **Notebook card**: PP-11b (§10), implemented: step 2's **Make named-axis copies + (optional)** card under the axis check. diff --git a/docs/user_guide.md b/docs/user_guide.md index 65d6f434..16b9bcb9 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -361,7 +361,8 @@ results or comparing another grouping. dropdown; the card gives the error (`axes_yx_swapped` or `axes_contradict_header`, §8) naming the images with their header axes, your roles and the likely cause. Such images are never read: correct the axes, or save named-axis copies with your roles and - run preprocessing on the copies. An axis the metadata does not name (a plain stack of + run preprocessing on the copies (the **Make named-axis copies (optional)** card under + the axis check, §5 "Named-axis copies"). An axis the metadata does not name (a plain stack of pages) takes the role you confirm: the card warns (`axes_not_named`) and the run records the roles as confirmed by you. - If the preview looks right, press **Yes, the preview looks right**, then click @@ -743,7 +744,38 @@ copies (`RunInputs`) refuse it if the conversion record is changed or removed af images or made with other roles is refused (`copy_folder_not_empty`, `copy_differs`); convert into a new folder then. If it fails (`conversion_failed`), the folder is left as it was. Images of 64-bit integers cannot be saved as OME-TIFF (`cannot_convert_pixel_type`). -The notebook does not offer the conversion yet: run the command in a terminal. + +**In the notebook.** When step 2's axis check reports these images, a **Make named-axis +copies (optional)** card appears under it, calling the same functions as the command: + +1. **The roles.** They are yours: the roles confirmed with **Confirm axis roles** (possible + with only `axes_not_named`), or, while the axis errors keep that button disabled, the + roles shown once you pressed **Yes, the preview looks right** and then **Yes, write the + copies with these roles** on the card. Changing a role, or the answer about missing + axes, takes that answer back. Nothing is taken from the header. +2. **The folder for the copies** (required, no default): paste it or use **Browse…** (on + Google Colab, **Copy path** in the **Files** panel). It is checked with the command's rule + (`copy_folder_overlaps_experiment`, `copy_folder_not_a_folder`). +3. **What will happen**, from the command's planning, reading headers only: the images + written as OME-TIFF (with their axis code) and those copied as they are (with `excluded`), + the space needed and the free space. Any error that would stop the conversion is shown + here and keeps **Make named-axis copies** disabled, as does a drive without the space. +4. **Make named-axis copies** writes the copies with a progress bar, holding the folder's + lock. Refusals are shown on the card with what to do; a folder locked by a stopped + conversion shows its holder and **Remove the lock of a stopped run** (the rule of + `--remove-stale-lock`). +5. **After the copies:** the card shows the folder, what was written, copied and reused, the + path of the conversion record and what to do next. **Check the copies in step 1** sets + the experiment folder to the copies, keeps the output folder (an empty box is filled with + the originals' default) and checks again. Confirm the labels, the axis roles (now named in + the copies' metadata) and the image type again; the design and stated pixel sizes are + kept. If you pressed **Yes, they have a single plane there**, press it again (the copies + do not add those axes; `--missing-as-singleton` on the command line). The card lists the + copies whose relabelled Y or X has no pixel size (unless the header gave X and Y the same + size): state them under **Pixel sizes** in step 1. + +The card is not shown when another brick embeds step 1 without the output folder +(`folder_step(show_output=False)`). ## 6. Output files diff --git a/notebooks/MorphoBricks_Preprocessing.ipynb b/notebooks/MorphoBricks_Preprocessing.ipynb index 26a2ebfc..580ab353 100644 --- a/notebooks/MorphoBricks_Preprocessing.ipynb +++ b/notebooks/MorphoBricks_Preprocessing.ipynb @@ -45,7 +45,9 @@ " biological repeats with **Yes, they are comparable**.\n", "3. **Step 2:** check the role of each axis, press **Show preview**, press\n", " **Yes, the preview looks right** and then **Confirm axis roles**. Check the suggested image\n", - " type and press **Yes, this is the right image type**.\n", + " type and press **Yes, this is the right image type**. If the axis check says the header names\n", + " the axes wrongly (or not at all), you may press **Make named-axis copies** and then\n", + " **Check the copies in step 1** (optional).\n", "4. **Step 3:** press **Compute histograms**. This writes the table the next bricks need. Read\n", " the **Intensity checks** that follow. With masks, you may then press **Describe the masks**\n", " (optional).\n", @@ -341,7 +343,9 @@ " size**; **Remove the statement** goes back to the metadata. The sizes are saved in\n", " **calibration.csv** in the output folder and recorded by the run; no histogram depends on them.\n", " Z and T sizes follow the axis roles: once you confirm them in step 2, an image without a Z\n", - " axis (a projection, for example) shows **no such axis**.\n", + " axis (a projection, for example) shows **no such axis**. In named-axis copies (step 2) the size\n", + " of a Y or X that was relabelled is unknown, unless the header gave X and Y the same size:\n", + " state it here if you know it.\n", "7. With a masks folder, look at **Masks**. It lists every mask type found, how many masks it\n", " has, their axes and the images without one, and the masks' messages. Each mask must have its\n", " image's height and width, whole-number pixels, and the image's depth slices and time points:\n", @@ -454,7 +458,33 @@ "> lists them. Press **Yes, they have a single plane there** if that is the case.\n", ">\n", "> ℹ️ RGB can only be chosen for images with exactly three channels of a whole-number pixel type\n", - "> (8- or 16-bit colour); otherwise the card says why." + "> (8- or 16-bit colour); otherwise the card says why.\n", + "\n", + "**Make named-axis copies (optional).** When the axis check says that the roles **swap the Y and X\n", + "their header names**, **contradict their header** or put **X before Y** (red: such images are not\n", + "read), or that the images **do not name their axes** (amber), and you are sure of your roles, a\n", + "**Make named-axis copies (optional)** card appears under the axis check. It saves the images again\n", + "as OME-TIFF copies whose metadata names your roles, in a new folder; the other images are copied as\n", + "they are, so the new folder holds the whole experiment with the same folders and file names. The\n", + "originals are only read, and masks are never converted.\n", + "\n", + "1. Check the roles with **Show preview** and press **Yes, the preview looks right**, then press\n", + " **Yes, write the copies with these roles** on the card (roles you confirmed with **Confirm axis\n", + " roles** need no second answer). Nothing is taken from the image header.\n", + "2. Give the folder for the copies: a new or empty folder outside the experiment folder (paste the\n", + " path or use **Browse…**; on Colab, **Copy path** in the **Files** panel). The card lists the\n", + " images written as OME-TIFF and those copied as they are, the space needed and the free space.\n", + "3. Press **Make named-axis copies**. The bar shows each image. If anything goes wrong, nothing is\n", + " left in the folder; the card says what to do. If the card says the folder is locked and no\n", + " other conversion is going on, press **Remove the lock of a stopped run**, then press the button\n", + " again.\n", + "4. Press **Check the copies in step 1**: the experiment folder becomes the folder of copies, the\n", + " output folder stays the same, and the folder is checked again. Confirm the labels, the axis\n", + " roles (now named in the copies' metadata) and the image type again; the experiment design and\n", + " stated pixel sizes are kept, and step 3 recomputes every row from the copies. If you pressed\n", + " **Yes, they have a single plane there**, press it again. The size of a relabelled Y or X is\n", + " unknown in the copies unless the header gave X and Y the same size: state it under **Pixel\n", + " sizes (calibration)** in step 1." ] }, { @@ -636,10 +666,18 @@ "\n", "**\"… swap the Y and X their header names\"** or **\"… contradict their header\"**: the images\n", "are not read; the message names the likely cause. Correct the axes in step 2 (choose the\n", - "header's roles if it is right), or save named-axis copies with your roles (in a terminal:\n", - "**morphobricks-preprocessing convert** with **--to** a new folder and **--axes** your roles) and run\n", - "preprocessing on the copies. **\"… do not name their axes in their metadata\"** is a warning: check the roles with\n", - "the preview; they are recorded as confirmed by you.\n", + "header's roles if it is right), or save named-axis copies with your roles with the **Make\n", + "named-axis copies (optional)** card under the axis check (in a terminal:\n", + "**morphobricks-preprocessing convert** with **--to** a new folder and **--axes** your roles), then\n", + "press **Check the copies in step 1** and run preprocessing on the copies. **\"… do not name their\n", + "axes in their metadata\"** is a warning: check the roles with the preview; they are recorded as\n", + "confirmed by you (the same card can make named-axis copies).\n", + "\n", + "**\"The folder for the named-axis copies is the experiment folder, inside it, or holds it\"**: choose a\n", + "new folder next to the experiment folder. **\"… already holds … and no conversion record\"**: choose a\n", + "new or empty folder; nothing is overwritten. **\"… differ from the copies this conversion would\n", + "write\"**: an original or a role changed since an earlier conversion into that folder; convert into\n", + "a new, empty folder.\n", "\n", "**\"The images have different numbers of channels\"**: each row of the table holds every channel of\n", "a plane, so all images need the same channels. Process the others as a separate experiment, or\n", diff --git a/pyproject.toml b/pyproject.toml index 3415b659..839fa6b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -149,6 +149,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_run" = "ui" "morphobricks_preprocessing.notebook_run_plan" = "ui" "morphobricks_preprocessing.notebook_calibration" = "ui" +"morphobricks_preprocessing.notebook_convert" = "ui" "morphobricks_preprocessing.notebook_output" = "ui" "morphobricks_preprocessing.notebook_values" = "pure" "morphobricks_preprocessing.cli" = "ui" @@ -159,6 +160,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.cli_convert" = "ui" "morphobricks_preprocessing.conversion" = "orchestration" "morphobricks_preprocessing.conversion_issues" = "pure" +"morphobricks_preprocessing.conversion_plan" = "orchestration" "morphobricks_preprocessing.conversion_record" = "io" "morphobricks_preprocessing.copy_axes" = "pure" "morphobricks_preprocessing.ome_tiff_copy" = "io" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index 66f0bed7..54fb4e84 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -39,6 +39,7 @@ ) from .calibration_io import CalibrationFormatError, read_calibration from .conversion import ConversionResult, convert_experiment +from .conversion_plan import ConversionPreview, preview_conversion from .conversion_record import ( RECORD_FILENAME as CONVERSION_RECORD_FILENAME, ) @@ -265,6 +266,7 @@ "ConfirmedDesign", "ConversionRecord", "ConversionRecordError", + "ConversionPreview", "ConversionResult", "DatasetReport", "DesignChanges", @@ -336,6 +338,7 @@ "check_sources", "check_table", "convert_experiment", + "preview_conversion", "declared_mapping", "default_design", "default_output_folder", diff --git a/src/morphobricks_preprocessing/conversion.py b/src/morphobricks_preprocessing/conversion.py index ac808fee..6e62713f 100644 --- a/src/morphobricks_preprocessing/conversion.py +++ b/src/morphobricks_preprocessing/conversion.py @@ -36,29 +36,21 @@ from dataclasses import dataclass, field, replace from datetime import UTC, datetime from pathlib import Path +from types import MappingProxyType from ._io import free_space from .axes import AxisMapping, prefill_mapping -from .axis_check import ( - AXES_NOT_NAMED, - AXIS_ERROR_CODES, - ConfirmedAxes, - confirmation_of, - header_axis_issue, -) -from .axis_review import review_axes +from .axis_check import ConfirmedAxes, confirmation_of from .conversion_issues import ( differs_issue, failed_issue, lock_issue, - not_a_folder_issue, not_empty_issue, other_experiment_issue, - overlap_issue, - pixel_type_issue, record_issue, space_issue, ) +from .conversion_plan import PlannedCopy, copy_folder_issues, plan_conversion from .conversion_record import ( CONVERTED, COPIED, @@ -68,52 +60,19 @@ read_record, write_record, ) -from .copy_axes import CopyLayout, copy_layout from .fingerprint import Fingerprint, current_fingerprint from .manifest import package_versions -from .metadata import TiffMetadata -from .ome_tiff_copy import CopyWriteError, copy_nbytes, write_named_copy -from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError, check_planes +from .ome_tiff_copy import CopyWriteError, write_named_copy +from .planes import DEFAULT_MEMORY_LIMIT, PlaneReadError from .run.lock import LOCK_FILENAME, OutputLockedError, output_lock -from .run.preflight import preflight from .validation import ValidationIssue, has_errors logger = logging.getLogger(__name__) -_NOT_IN_OME = frozenset({"int64", "uint64"}) -"""Pixel types the brick reads that OME-TIFF cannot hold: they are never changed to fit.""" - PARTIAL_SUFFIX = ".partial" """A copy is written as ``.partial`` and renamed once every copy is written.""" -@dataclass(frozen=True) -class PlannedCopy: - """One image of the copy tree, before it is written. - - Attributes - ---------- - entry - Its conversion-record entry, without the copy's fingerprint. - layout - The named-axis copy of a converted image, else None. - mapping - The roles a converted image is read with, else None. - nbytes - The bytes the copy takes (pixels only for a converted image). - """ - - entry: CopyEntry - layout: CopyLayout | None = None - mapping: AxisMapping | None = None - nbytes: int = 0 - - @property - def relative_path(self) -> str: - """The image, relative to the experiment and to the copy tree.""" - return self.entry.relative_path - - @dataclass(frozen=True) class ConversionResult: """What :func:`convert_experiment` did. @@ -138,6 +97,10 @@ class ConversionResult: suggested_axes The roles to give ``run`` on the copy tree (``--axes``), when every copy has the same axes; else None. + unknown_sizes + ``{image: roles}`` for each copy in the tree whose source header gives pixel sizes + that the copy does not carry (a relabelled axis gets an unknown scale), sorted; the + user states them (``calibration.csv``) if known. """ destination: Path @@ -148,6 +111,10 @@ class ConversionResult: reused: tuple[str, ...] = () record_sha256: str | None = None suggested_axes: str | None = field(default=None) + unknown_sizes: Mapping[str, tuple[str, ...]] = field(default_factory=dict) + + def __post_init__(self) -> None: + object.__setattr__(self, "unknown_sizes", MappingProxyType(dict(self.unknown_sizes))) def convert_experiment( @@ -194,7 +161,7 @@ def convert_experiment( """ _check_arguments(missing_as_singleton, memory_limit) root, dest = Path(experiment_root).resolve(), Path(destination).resolve() - refused = _folder_issues(root, dest) + refused = copy_folder_issues(root, dest) if refused: return ConversionResult(dest, False, tuple(refused)) planned, issues = plan_conversion( @@ -232,99 +199,6 @@ class _Options: memory_limit: int -def plan_conversion( - root: Path, - mapping: AxisMapping | ConfirmedAxes, - destination: Path, - exclude: Iterable[str] = (), - missing_as_singleton: bool = False, - memory_limit: int = DEFAULT_MEMORY_LIMIT, -) -> tuple[tuple[PlannedCopy, ...], list[ValidationIssue]]: - """Check the experiment with the confirmed roles and plan each image's copy. - - Only the headers are read. The axis errors do not stop the conversion: they are what - it corrects. Any other error does, and nothing is planned: so does an image to convert - of a pixel type OME-TIFF cannot hold, or whose planes cannot be read within the memory - limit, not even in blocks (checked as the run checks them, before anything is written). - - Parameters - ---------- - root - The experiment folder (absolute). - mapping - The confirmed mapping. - destination - The copy tree (absolute). - exclude - Images to leave unread. - missing_as_singleton - As :func:`convert_experiment`. - memory_limit - As :func:`convert_experiment`. - - Returns - ------- - tuple - The planned copies, sorted by path (empty when an error stops the conversion), - and the issues: the errors that stop it, and the warnings. - """ - pre = preflight(root, destination, exclude=exclude) - review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) - others = [issue for issue in pre.issues if issue not in pre.metadata.issues] - issues = [*others, *review.combined_issues(pre.metadata)] - if any(i.is_error and i.code not in AXIS_ERROR_CODES for i in issues): - return (), [i for i in issues if not (i.is_error and i.code in AXIS_ERROR_CODES)] - # The axis errors and unnamed axes are what is corrected (each entry records its code). - issues = [i for i in issues if not i.is_error and i.code != AXES_NOT_NAMED] - to_convert = set(review.axis_error_files) | set(review.confirmed_files) - headers = {meta.relative_path: meta for meta in pre.metadata.files} - issues += _cannot_convert(root, to_convert, headers, review.file_mappings, memory_limit) - if has_errors(issues): - return (), issues - planned = [] - for rel in pre.discovery.images: - source = Fingerprint.of(root / rel) - if rel in to_convert: - planned.append(_converted(headers[rel], review.file_mappings[rel], source)) - else: - reason = "excluded" if rel in pre.excluded else None - meta = headers.get(rel) - entry = CopyEntry(rel, COPIED, reason, source, source_axes=meta.axes if meta else "") - planned.append(PlannedCopy(entry, nbytes=source.size)) - return tuple(planned), issues - - -def _cannot_convert( - root: Path, - to_convert: set[str], - headers: dict[str, TiffMetadata], - mappings: Mapping[str, AxisMapping], - memory_limit: int, -) -> list[ValidationIssue]: - """Return why images cannot be converted (errors) and the planes read in blocks.""" - wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) - if wide: - return [pixel_type_issue(wide, headers[wide[0]].dtype.name)] - return check_planes(root, {rel: mappings[rel] for rel in to_convert}, memory_limit=memory_limit) - - -def _converted(meta: TiffMetadata, mapping: AxisMapping, source: Fingerprint) -> PlannedCopy: - """Plan the named-axis copy of one image read under its confirmed roles.""" - layout = copy_layout(meta, mapping) - entry = CopyEntry( - relative_path=meta.relative_path, - action=CONVERTED, - reason=header_axis_issue(meta, mapping), - source=source, - source_axes=meta.axes, - roles=mapping.roles, - copy_axes=layout.axes, - pixel_sizes=layout.sizes, - unknown_sizes=layout.unknown, - ) - return PlannedCopy(entry, layout, mapping, copy_nbytes(layout, meta.dtype)) - - def _locked_conversion( root: Path, dest: Path, @@ -358,6 +232,9 @@ def _locked_conversion( reused=tuple(e.relative_path for e in reused), record_sha256=sha256, suggested_axes=_suggested_axes(files), + unknown_sizes={ + e.relative_path: tuple(sorted(e.unknown_sizes)) for e in files if e.unknown_sizes + }, ) @@ -565,15 +442,6 @@ def _suggested_axes(files: list[CopyEntry]) -> str | None: return None -def _folder_issues(root: Path, dest: Path) -> list[ValidationIssue]: - """Refuse a copy tree inside the experiment, holding it, or that is a file.""" - if dest == root or root in dest.parents or dest in root.parents: - return [overlap_issue(root, dest)] - if dest.exists() and not dest.is_dir(): - return [not_a_folder_issue(dest)] - return [] - - def _space_issue(dest: Path, to_write: list[PlannedCopy]) -> ValidationIssue | None: """Refuse a conversion that would not fit on the drive of the copy tree.""" needed, free = sum(item.nbytes for item in to_write), free_space(dest) diff --git a/src/morphobricks_preprocessing/conversion_plan.py b/src/morphobricks_preprocessing/conversion_plan.py new file mode 100644 index 00000000..040aa6ad --- /dev/null +++ b/src/morphobricks_preprocessing/conversion_plan.py @@ -0,0 +1,284 @@ +"""Plan a conversion: what each image's copy will be, before anything is written (§17). + +:func:`plan_conversion` checks the experiment with the confirmed roles, from the headers +only, and plans each image's copy: a named-axis OME-TIFF for an image with an axis error or +unnamed axes, a byte-for-byte copy for every other image. :func:`.conversion.convert_experiment` +writes that plan. :func:`preview_conversion` gives the same plan, with the folder rule +(:func:`copy_folder_issues`), the space the copies need and the free space, so the notebook +can show what will happen before the user starts the conversion (PP-11b); it writes +nothing. +""" + +from __future__ import annotations + +import os +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from pathlib import Path + +from ._io import free_space +from .axes import AxisMapping +from .axis_check import ( + AXES_NOT_NAMED, + AXIS_ERROR_CODES, + ConfirmedAxes, + header_axis_issue, +) +from .axis_review import review_axes +from .conversion_issues import not_a_folder_issue, overlap_issue, pixel_type_issue +from .conversion_record import CONVERTED, COPIED, RECORD_FILENAME, CopyEntry +from .copy_axes import CopyLayout, copy_layout +from .fingerprint import Fingerprint +from .metadata import TiffMetadata +from .ome_tiff_copy import copy_nbytes +from .planes import DEFAULT_MEMORY_LIMIT, check_planes +from .run.preflight import preflight +from .validation import ValidationIssue, has_errors + +_NOT_IN_OME = frozenset({"int64", "uint64"}) +"""Pixel types the brick reads that OME-TIFF cannot hold: they are never changed to fit.""" + + +@dataclass(frozen=True) +class PlannedCopy: + """One image of the copy tree, before it is written. + + Attributes + ---------- + entry + Its conversion-record entry, without the copy's fingerprint. + layout + The named-axis copy of a converted image, else None. + mapping + The roles a converted image is read with, else None. + nbytes + The bytes the copy takes (pixels only for a converted image). + """ + + entry: CopyEntry + layout: CopyLayout | None = None + mapping: AxisMapping | None = None + nbytes: int = 0 + + @property + def relative_path(self) -> str: + """The image, relative to the experiment and to the copy tree.""" + return self.entry.relative_path + + +def plan_conversion( + root: Path, + mapping: AxisMapping | ConfirmedAxes, + destination: Path, + exclude: Iterable[str] = (), + missing_as_singleton: bool = False, + memory_limit: int = DEFAULT_MEMORY_LIMIT, +) -> tuple[tuple[PlannedCopy, ...], list[ValidationIssue]]: + """Check the experiment with the confirmed roles and plan each image's copy. + + Only the headers are read. The axis errors do not stop the conversion: they are what + it corrects. Any other error does, and nothing is planned: so does an image to convert + of a pixel type OME-TIFF cannot hold, or whose planes cannot be read within the memory + limit, not even in blocks (checked as the run checks them, before anything is written). + + Parameters + ---------- + root + The experiment folder (absolute). + mapping + The confirmed mapping. + destination + The copy tree (absolute). + exclude + Images to leave unread. + missing_as_singleton + As :func:`convert_experiment`. + memory_limit + As :func:`convert_experiment`. + + Returns + ------- + tuple + The planned copies, sorted by path (empty when an error stops the conversion), + and the issues: the errors that stop it, and the warnings. + """ + pre = preflight(root, destination, exclude=exclude) + review = review_axes(pre.metadata, mapping, missing_as_singleton=missing_as_singleton) + others = [issue for issue in pre.issues if issue not in pre.metadata.issues] + issues = [*others, *review.combined_issues(pre.metadata)] + if any(i.is_error and i.code not in AXIS_ERROR_CODES for i in issues): + return (), [i for i in issues if not (i.is_error and i.code in AXIS_ERROR_CODES)] + # The axis errors and unnamed axes are what is corrected (each entry records its code). + issues = [i for i in issues if not i.is_error and i.code != AXES_NOT_NAMED] + to_convert = set(review.axis_error_files) | set(review.confirmed_files) + headers = {meta.relative_path: meta for meta in pre.metadata.files} + issues += _cannot_convert(root, to_convert, headers, review.file_mappings, memory_limit) + if has_errors(issues): + return (), issues + planned = [] + for rel in pre.discovery.images: + source = Fingerprint.of(root / rel) + if rel in to_convert: + planned.append(_converted(headers[rel], review.file_mappings[rel], source)) + else: + reason = "excluded" if rel in pre.excluded else None + meta = headers.get(rel) + entry = CopyEntry(rel, COPIED, reason, source, source_axes=meta.axes if meta else "") + planned.append(PlannedCopy(entry, nbytes=source.size)) + return tuple(planned), issues + + +def _cannot_convert( + root: Path, + to_convert: set[str], + headers: dict[str, TiffMetadata], + mappings: Mapping[str, AxisMapping], + memory_limit: int, +) -> list[ValidationIssue]: + """Return why images cannot be converted (errors) and the planes read in blocks.""" + wide = sorted(rel for rel in to_convert if headers[rel].dtype.name in _NOT_IN_OME) + if wide: + return [pixel_type_issue(wide, headers[wide[0]].dtype.name)] + return check_planes(root, {rel: mappings[rel] for rel in to_convert}, memory_limit=memory_limit) + + +def _converted(meta: TiffMetadata, mapping: AxisMapping, source: Fingerprint) -> PlannedCopy: + """Plan the named-axis copy of one image read under its confirmed roles.""" + layout = copy_layout(meta, mapping) + entry = CopyEntry( + relative_path=meta.relative_path, + action=CONVERTED, + reason=header_axis_issue(meta, mapping), + source=source, + source_axes=meta.axes, + roles=mapping.roles, + copy_axes=layout.axes, + pixel_sizes=layout.sizes, + unknown_sizes=layout.unknown, + ) + return PlannedCopy(entry, layout, mapping, copy_nbytes(layout, meta.dtype)) + + +def copy_folder_issues( + experiment_root: str | os.PathLike, destination: str | os.PathLike +) -> list[ValidationIssue]: + """Refuse a folder for the copies inside the experiment, holding it, or that is a file. + + The rule :func:`.conversion.convert_experiment` applies first, before anything is read. + + Parameters + ---------- + experiment_root + The experiment folder. + destination + The folder for the copies. + + Returns + ------- + list of ValidationIssue + ``copy_folder_overlaps_experiment`` or ``copy_folder_not_a_folder``; empty when the + folder can hold the copies. + """ + root, dest = Path(experiment_root).resolve(), Path(destination).resolve() + if dest == root or root in dest.parents or dest in root.parents: + return [overlap_issue(root, dest)] + if dest.exists() and not dest.is_dir(): + return [not_a_folder_issue(dest)] + return [] + + +@dataclass(frozen=True) +class ConversionPreview: + """What a conversion would write, worked out before anything is written. + + Attributes + ---------- + destination + The folder for the copies (absolute). + issues + The errors that would stop the conversion (the folder rule, then the checks of + :func:`plan_conversion`), and the warnings. + converted + Images that would be written as named-axis OME-TIFF, with their axis code, sorted. + copied + Images that would be copied byte for byte, sorted (``"excluded"`` or None). + needed_bytes + The space all the copies take; less when unchanged copies of an earlier conversion + into the folder are reused. + free_bytes + The free space on the drive of the folder, or None when unknown. + earlier_conversion + True when the folder already holds a conversion record: its unchanged copies are + reused, and anything else that differs is refused. + """ + + destination: Path + issues: tuple[ValidationIssue, ...] = () + converted: tuple[tuple[str, str], ...] = () + copied: tuple[tuple[str, str | None], ...] = () + needed_bytes: int = 0 + free_bytes: int | None = None + earlier_conversion: bool = False + + @property + def ok(self) -> bool: + """True when nothing found so far stops the conversion.""" + return not has_errors(self.issues) + + @property + def fits(self) -> bool: + """True when the copies fit in the free space, or the free space is unknown.""" + return self.free_bytes is None or self.needed_bytes <= self.free_bytes + + +def preview_conversion( + experiment_root: str | os.PathLike, + mapping: AxisMapping | ConfirmedAxes, + destination: str | os.PathLike, + *, + exclude: Iterable[str] = (), + missing_as_singleton: bool = False, + memory_limit: int = DEFAULT_MEMORY_LIMIT, +) -> ConversionPreview: + """Work out what :func:`.conversion.convert_experiment` would write; write nothing. + + The same folder rule and plan as the conversion, from the headers only. The conversion + checks everything again under the folder's lock, which decides (a copy already in the + folder that differs, the lock of another conversion, the space actually needed). + + Parameters + ---------- + experiment_root + The experiment folder. + mapping + The roles the user confirmed. + destination + The folder for the copies. + exclude + Images to leave unread (copied byte for byte). + missing_as_singleton + As :func:`.conversion.convert_experiment`. + memory_limit + As :func:`.conversion.convert_experiment`. + + Returns + ------- + ConversionPreview + """ + root, dest = Path(experiment_root).resolve(), Path(destination).resolve() + refused = copy_folder_issues(root, dest) + if refused: + return ConversionPreview(dest, tuple(refused)) + planned, issues = plan_conversion( + root, mapping, dest, exclude, missing_as_singleton, memory_limit + ) + entries = [item.entry for item in planned] + return ConversionPreview( + destination=dest, + issues=tuple(sorted(issues, key=lambda issue: not issue.is_error)), + converted=tuple((e.relative_path, e.reason) for e in entries if e.action == CONVERTED), + copied=tuple((e.relative_path, e.reason) for e in entries if e.action == COPIED), + needed_bytes=sum(item.nbytes for item in planned), + free_bytes=free_space(dest), + earlier_conversion=(dest / RECORD_FILENAME).is_file(), + ) diff --git a/src/morphobricks_preprocessing/notebook_axis_step.py b/src/morphobricks_preprocessing/notebook_axis_step.py index 1a17f773..0fcc4aed 100644 --- a/src/morphobricks_preprocessing/notebook_axis_step.py +++ b/src/morphobricks_preprocessing/notebook_axis_step.py @@ -178,6 +178,7 @@ def _refresh_axis_step(self) -> None: ), self.missing_card, self.axis_status, + self.convert_card, self.preview_card, self.confirm_button, self.confirmed_status, @@ -328,6 +329,7 @@ def _mapping_changed(self) -> None: "Click Show preview and inspect a few planes before confirming.", ) self._review_ok = review.ok + self._refresh_convert_card(review) if self._preview_shown: self._configure_sliders(reset=False) self._render_preview() diff --git a/src/morphobricks_preprocessing/notebook_convert.py b/src/morphobricks_preprocessing/notebook_convert.py new file mode 100644 index 00000000..782976b5 --- /dev/null +++ b/src/morphobricks_preprocessing/notebook_convert.py @@ -0,0 +1,433 @@ +"""Step 2 of the notebook: **Make named-axis copies**, the conversion card (PP-11b, §17). + +When the axis check of step 2 reports images whose header names their axes wrongly +(``axes_yx_swapped``, ``axes_contradict_header``, ``axes_x_before_y``) or not at all +(``axes_not_named``), this card offers what the command line's ``convert`` does, with the +same functions: :func:`.conversion_plan.preview_conversion` (the folder rule and the plan, +from the headers only) and :func:`.conversion.convert_experiment` (the copies, under the +copy folder's lock). + +- The folder for the copies is required (no default): pasted or chosen with **Browse…** + (on Google Colab, the Files panel's **Copy path**), outside the experiment and not + holding it. +- The roles are the user's: those confirmed with **Confirm axis roles**, or, while the + axis errors keep that button disabled, the roles shown once the preview is checked and + the card's own question is answered. Nothing is taken from the headers. +- Before anything is written the card shows which images are written as OME-TIFF and + which are copied byte for byte, the space needed and the free space. +- A refusal is shown with the existing cards (the locked copy folder with + :class:`.notebook_output._LockCard`), and after the copies are made, the record and the + next step: check the copies in step 1 (one button fills the experiment folder, keeps the + output folder and checks again). + +An embedding brick that hides the output folder (``show_output=False``) does not get it. +""" + +from __future__ import annotations + +import html +import traceback +from pathlib import Path + +import ipywidgets as widgets + +from . import notebook_style as ui +from .axes import AxisMapping, AxisReview, axis_name +from .axis_check import AXIS_CHECK_CODES +from .conversion import ConversionResult, convert_experiment +from .conversion_plan import ConversionPreview, preview_conversion +from .conversion_record import RECORD_FILENAME +from .notebook_options import _WIDE +from .notebook_output import _LockCard +from .notebook_render import issues_html +from .planes import format_bytes +from .validation import plural + +_LABEL = " Make named-axis copies" +_ICON = "copy" +_RECHECK_LABEL = " Check the copies in step 1" +_COPY_HOLDERS = ( + "Another conversion is writing into this folder, or one was stopped (for example Colab " + "disconnected) and left its lock behind. Wait for a conversion that is still going." +) + + +class _ConvertCard: + """Step 2's conversion card: the folder, the roles, the plan, the copies, what next.""" + + def _build_convert_card(self) -> None: + """Build the card, hidden until the axis check reports images to convert.""" + self._convert_preview: ConversionPreview | None = None + self._converted_to: Path | None = None + self.convert_folder = widgets.Text( + placeholder="a new folder outside the experiment, e.g. D:/microscopy/exp_named_axes", + continuous_update=False, + layout=widgets.Layout(flex="1 1 auto", width="auto"), + ) + self.convert_browse = ui.button( + "Browse…", icon="folder-open", tooltip="Choose the folder for the copies in a dialog" + ) + self.convert_roles = widgets.HTML() + self.convert_roles_checked = ui.ConfirmToggle( + "Are these the roles of every image, for the copies?", + "The copies are written with exactly these roles, and their metadata then names " + "every axis. Check them with the preview first: the image header is not used.", + "Yes, write the copies with these roles", + "Roles confirmed for the copies.", + "Undo: the roles are not right", + ) + self.convert_plan = widgets.HTML() + self.convert_button = ui.button( + _LABEL, + style="success", + icon=_ICON, + tooltip="Write the copies into the folder above (the originals are only read)", + main=True, + ) + self.convert_progress = widgets.IntProgress( + value=0, min=0, max=1, bar_style="info", layout=widgets.Layout(width="99%") + ) + self.convert_status = widgets.HTML() + self.convert_lock = _LockCard( + "Then click Make named-axis copies again.", + "The folder for the copies is locked", + _COPY_HOLDERS, + "Another conversion", + ) + self.convert_result = widgets.HTML() + self.convert_recheck_button = ui.button( + _RECHECK_LABEL, + style="primary", + icon="search", + tooltip="Set step 1's experiment folder to the copies, keep the output folder, " + "and check again", + main=True, + ) + self.convert_card = ui.card("Make named-axis copies (optional)", self._convert_children()) + ui.set_visible(self.convert_card, False) + self._connect_convert_card() + + def _connect_convert_card(self) -> None: + """Connect the card's controls, and the step 2 answers its roles depend on.""" + self.convert_browse.on_click(lambda _: self._browse_copy_folder()) + self.convert_folder.observe(lambda _: self._plan_copies(), names="value") + self.convert_roles_checked.observe(lambda _: self._convert_roles_changed()) + self.preview_checked.observe(lambda _: self._convert_roles_changed()) + self.confirm_button.on_click(lambda _: self._convert_roles_changed()) + self.convert_button.on_click(lambda _: self._on_convert()) + self.convert_recheck_button.on_click(lambda _: self._check_the_copies()) + + def _convert_children(self) -> list[widgets.Widget]: + """Return the card's widgets, top to bottom.""" + return [ + ui.help_text( + "Saves the images whose metadata names their axes wrongly, or not at all, " + "again as named-axis OME-TIFF copies with the roles above, in a new folder; " + "every other image is copied as it is, so the folder holds the whole " + "experiment with the same folders and file names. The originals are only " + "read, and masks are never converted. Then you check and run preprocessing " + "on the copies, with the same output folder." + ), + widgets.HBox( + [self.convert_folder, self.convert_browse], layout=widgets.Layout(**_WIDE) + ), + ui.help_text( + "Required: a new or empty folder, outside the experiment folder (or the folder " + "of an earlier conversion of this experiment, whose unchanged copies are " + "kept). Paste the path, or use Browse…; on Google Colab, right-click " + "a folder in the Files panel, choose Copy path and paste it." + ), + self.convert_roles, + self.convert_roles_checked.widget, + self.convert_plan, + self.convert_button, + self.convert_progress, + self.convert_status, + self.convert_lock.card, + self.convert_result, + self.convert_recheck_button, + ] + + def _refresh_convert_card(self, review: AxisReview) -> None: + """Offer the card when the axis check of these roles finds images to convert. + + Called whenever the roles, the missing-axes answer or the checked folder change: + the roles must be confirmed again, and an earlier plan or result no longer applies. + """ + if not hasattr(self, "convert_card"): + return + offered = self._show_output and any(i.code in AXIS_CHECK_CODES for i in review.issues) + ui.set_visible(self.convert_card, offered) + self.convert_roles_checked.set(False, notify=False) + self._convert_roles_changed() + + def _convert_roles_changed(self) -> None: + """Show the roles the copies get, clear an earlier result and plan again.""" + if not hasattr(self, "convert_card"): + return # the axis step's own handlers run while the card is being built + self._clear_conversion() + roles = self._copy_roles() + confirmed = self.confirmed_mapping is not None + self.convert_roles_checked.disabled = not self.preview_checked.value + ui.set_visible( + self.convert_roles_checked.widget, not confirmed and self.mapping is not None + ) + self.convert_roles.value = _roles_html(self.mapping, roles, confirmed) + self._plan_copies() + + def _copy_roles(self) -> AxisMapping | None: + """Return the roles the copies are written with, or None until the user confirmed them.""" + if self.confirmed_mapping is not None: + return self.confirmed_mapping + if self.convert_roles_checked.value and self.preview_checked.value: + return self.mapping + return None + + def _clear_conversion(self) -> None: + """Clear the progress, status, lock card and result of an earlier conversion.""" + self._converted_to = None + ui.set_visible(self.convert_progress, False) + ui.set_visible(self.convert_recheck_button, False) + self.convert_status.value = self.convert_result.value = "" + self.convert_lock.hide() + + def _plan_copies(self) -> ConversionPreview | None: + """Show what the conversion would write, from the headers only; write nothing.""" + self._convert_preview = None + roles, folder = self._copy_roles(), ui.clean_path(self.convert_folder.value) + if self.preflight is None or roles is None or not folder: + self.convert_plan.value = ui.status( + "info", + "Waiting for the roles and the folder for the copies.", + [ + "Confirm the roles above and give the folder for the copies; the card " + "then shows what will be written." + ], + ) + self._update_convert_button() + return None + with self.log.attached(): + preview = preview_conversion( + self.preflight.discovery.root, + roles, + folder, + exclude=self.preflight.excluded, + missing_as_singleton=self.missing_as_singleton.value, + memory_limit=self._memory_limit(), + ) + self._convert_preview = preview + self.convert_plan.value = _plan_html(preview) + self._update_convert_button() + return preview + + def _update_convert_button(self) -> None: + """Enable the button once the plan shows nothing that stops the conversion.""" + preview = self._convert_preview + ready = preview is not None and preview.ok and (preview.fits or preview.earlier_conversion) + self.convert_button.disabled = not ready + self.convert_button.tooltip = ( + "Write the copies into the folder above (the originals are only read)" + if ready + else "Confirm the roles and give a folder the copies can be written to first" + ) + + def _browse_copy_folder(self) -> None: + chosen = ui.ask_directory(ui.clean_path(self.convert_folder.value)) + if chosen is None: + self.convert_status.value = ui.no_dialog_status("folder", "the folder for the copies") + elif chosen: + self.convert_folder.value = chosen + + def _on_convert(self) -> ConversionResult | None: + """Write the copies with the confirmed roles, under the copy folder's lock. + + Returns + ------- + ConversionResult or None + None when the button is disabled or an unexpected problem stopped it (the card + says why). + """ + self._clear_conversion() + preview = self._plan_copies() # the experiment or the folder may have changed since + if preview is None or self.convert_button.disabled: + return None + self._busy(self.convert_button, "Making the copies…") + ui.set_visible(self.convert_progress, True) + self.convert_progress.value, self.convert_progress.bar_style = 0, "info" + self.convert_status.value = ui.working("Checking the images and the folder…") + try: + with self.log.attached(): + result = convert_experiment( + self.preflight.discovery.root, + self._copy_roles(), + preview.destination, + exclude=self.preflight.excluded, + missing_as_singleton=self.missing_as_singleton.value, + memory_limit=self._memory_limit(), + progress=self._convert_progressed, + ) + except Exception as exc: # unexpected failures still get a readable message + self.log.lines.append(traceback.format_exc()) + self.convert_progress.bar_style = "danger" + self.convert_status.value = ui.status( + "error", + "The copies could not be made because of an unexpected problem.", + [f"{type(exc).__name__}: {exc}", "Open Technical details for the full report."], + ) + return None + finally: + self._idle(self.convert_button, _LABEL, _ICON) + self._show_log() + self._show_conversion(result) + return result + + def _convert_progressed(self, done: int, total: int, image: str) -> None: + self.convert_progress.max = max(total, 1) + self.convert_progress.value = done + if image: + self.convert_status.value = ui.working(f"Copying image {done + 1} of {total}: {image}") + + def _show_conversion(self, result: ConversionResult) -> None: + """Show a refusal with its card, or the copies, the record and the next step.""" + if not result.written: + self.convert_progress.bar_style = "danger" + locked = any(issue.code == "output_locked" for issue in result.issues) + self.convert_status.value = ui.status( + "error", + "No copies were made: the folder for the copies is locked." + if locked + else "No copies were made.", + ["Nothing was written. Read the messages below; each says what to do."], + ) + if locked: + self.convert_lock.show(result.destination) + self.convert_result.value = issues_html(result.issues) + return + self.convert_progress.bar_style = "success" + self._converted_to = result.destination + output = self._output_path(self.preflight.discovery.root) + self.convert_status.value = _done_html(result) + self.convert_result.value = _next_html(result, output, self.missing_as_singleton.value) + ( + issues_html(result.issues) if result.issues else "" + ) + ui.set_visible(self.convert_recheck_button, True) + + def _check_the_copies(self) -> None: + """Check the copies in step 1: the copy folder as experiment, the same output folder.""" + if self._converted_to is None or self.preflight is None: + return + copies = self._converted_to + # The run on the copies keeps the output folder (its design, image type and + # calibration), so an empty box, meaning the originals' default, is filled in. + self.output.value = str(self._output_path(self.preflight.discovery.root)) + self.folder.value = str(copies) + self.check_folder() + + +def _roles_html(mapping: AxisMapping | None, roles: AxisMapping | None, confirmed: bool) -> str: + """Say which roles the copies get, or what confirms them.""" + if mapping is None: + return "" + shown = roles or mapping + lines = [ + f"{html.escape(axis_name(axis))} in the file → {html.escape(role or 'no role')}" + for axis, role in zip(shown.axes, shown.roles, strict=True) + ] + if roles is None: + return ui.status( + "info", + "The roles for the copies are not confirmed yet.", + [ + *lines, + "Check them with Show preview and press Yes, the preview looks right; then " + "answer the question below.", + ], + raw=True, + ) + how = "confirmed in step 2" if confirmed else "confirmed for the copies" + return ui.status("ok", f"The copies are written with these roles ({how}).", lines, raw=True) + + +def _plan_html(preview: ConversionPreview) -> str: + """Show what the conversion would write, or what stops it.""" + errors = [issue for issue in preview.issues if issue.is_error] + if errors: + return ui.status( + "error", + "These copies cannot be made.", + ["Nothing was written. Read the messages below; each says what to do."], + ) + issues_html(preview.issues) + converted = [f"{path} ({code})" for path, code in preview.converted] + copied = [path + (" (excluded)" if why else "") for path, why in preview.copied] + free = "unknown" if preview.free_bytes is None else format_bytes(preview.free_bytes) + lines = [ + f"Written as named-axis OME-TIFF with your roles: {plural(len(converted), 'image')}" + + ui.path_list(converted, limit=5), + "Copied byte for byte (their metadata names their axes as confirmed, OME-Zarr, or " + f"excluded): {plural(len(copied), 'image')}" + ui.path_list(copied, limit=5), + f"Space needed: about {format_bytes(preview.needed_bytes)}. Free on that drive: {free}.", + ] + if preview.earlier_conversion: + lines.append( + "This folder holds an earlier conversion of this experiment: unchanged copies are " + "kept, and copies that differ are refused (nothing is overwritten)." + ) + kind = "info" + if not preview.fits: + kind = "warn" if preview.earlier_conversion else "error" + lines.append( + "The drive does not have enough free space. Free some space, or choose a " + "folder on another drive." + ) + title = f"What will happen in {html.escape(str(preview.destination))}" + warnings = issues_html(preview.issues) if preview.issues else "" + return ui.status(kind, title, lines, raw=True) + warnings + + +def _done_html(result: ConversionResult) -> str: + """Say what the conversion wrote and where its record is.""" + record = result.destination / RECORD_FILENAME + return ui.status( + "ok", + "The named-axis copies are made.", + [ + f"Folder: {html.escape(str(result.destination))}", + f"Written as OME-TIFF with your roles: {plural(len(result.converted), 'image')}", + f"Copied as they are: {plural(len(result.copied), 'image')}", + f"Already there and unchanged: {plural(len(result.reused), 'image')}", + f"Conversion record: {html.escape(str(record))}", + ], + raw=True, + ) + + +def _next_html(result: ConversionResult, output: Path, missing_as_singleton: bool) -> str: + """Say what to do next: check the copies with the same output folder, and what to redo.""" + lines = [ + f"Press {_RECHECK_LABEL.strip()}: step 1's experiment folder becomes " + f"{html.escape(str(result.destination))}, the output folder stays " + f"{html.escape(str(output))}, and the folder is checked again. Then confirm " + "the labels, the axis roles (the copies' metadata names their axes" + + (f": {html.escape(result.suggested_axes)}" if result.suggested_axes else "") + + ") and the image type again. The experiment design and the stated pixel sizes are kept " + "(same output folder), and the run recomputes every row from the copies.", + ] + if missing_as_singleton: + lines.append( + "You confirmed that images lacking some axes have a single plane there: the copies " + "do not add those axes, so press Yes, they have a single plane there again " + "in step 2 (on the command line, give --missing-as-singleton again)." + ) + if result.unknown_sizes: + sizes = [f"{path}: {', '.join(roles)}" for path, roles in result.unknown_sizes.items()] + lines.append( + f"Pixel sizes: {plural(len(sizes), 'copy', 'copies')} do not carry a size their " + "header gave, because the axis was relabelled: its size is unknown in the copy " + "(a swapped Y and X keep theirs only when the header gives X and Y equal). If you " + "know them, state them " + "under Pixel sizes in step 1 after checking the copies." + + ui.path_list(sizes, limit=5) + ) + return ui.status( + "info", "What next: check the copies in step 1, then run on them.", lines, raw=True + ) diff --git a/src/morphobricks_preprocessing/notebook_output.py b/src/morphobricks_preprocessing/notebook_output.py index bdeb6032..373ad83b 100644 --- a/src/morphobricks_preprocessing/notebook_output.py +++ b/src/morphobricks_preprocessing/notebook_output.py @@ -39,6 +39,13 @@ """Why the lock is not enough on Google Drive (plan §12 question 58).""" +_HOLDERS = ( + "Another run, check or notebook is writing to this output folder, or one was stopped (for " + "example Colab disconnected) and left its lock behind. Wait for a run that is still going." +) +"""Who may hold the output folder's lock, for :class:`_LockCard`.""" + + def holder_lines(holder: LockHolder) -> list[str]: """Name the holder of a lock: its computer, process ID and start time.""" if not holder.pid: @@ -53,9 +60,20 @@ def holder_lines(holder: LockHolder) -> list[str]: class _LockCard: """The refusal ``output_locked`` and the button that removes the lock of a stopped run.""" - def __init__(self, again: str) -> None: - """Build the card, hidden; ``again`` names the button to press after a removal.""" - self.again = again + def __init__( + self, + again: str, + header: str = "The output folder is locked", + explanation: str = _HOLDERS, + who: str = "Another run", + ) -> None: + """Build the card, hidden; ``again`` names the button to press after a removal. + + ``header`` and ``explanation`` (HTML) name the folder and who may hold its lock; + the defaults are the output folder's (a run, a check or a notebook); ``who`` names + the holder in the refusal. + """ + self.again, self.who = again, who self.output: Path | None = None self.shown: LockHolder | None = None self.holder = widgets.HTML() @@ -67,15 +85,13 @@ def __init__(self, again: str) -> None: ) self.result = widgets.HTML() self.card = ui.card( - "The output folder is locked", + header, [ self.holder, ui.help_text( - "Another run, check or notebook is writing to this output folder, or one " - "was stopped (for example Colab disconnected) and left its lock behind. " - "Wait for a run that is still going. Click the button only if you are " - "sure none is: it never removes the lock of a process still running on " - f"this computer. {DRIVE_LIMIT}" + f"{explanation} Click the button only if you are sure none is going on: " + "it never removes the lock of a process still running on this computer. " + f"{DRIVE_LIMIT}" ), self.button, self.result, @@ -94,7 +110,7 @@ def show(self, output: Path) -> None: return self.holder.value = ui.status( "error", - f"Another run holds the lock of {output}. Nothing was written.", + f"{self.who} holds the lock of {output}. Nothing was written.", holder_lines(holder), ) self.button.disabled = False diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index f80b54b7..b9f39739 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -52,6 +52,7 @@ from .notebook_axis_preview import _AxisPreview from .notebook_axis_step import _AxisStep from .notebook_calibration import _CalibrationCard +from .notebook_convert import _ConvertCard from .notebook_design import _DesignCard from .notebook_folder import _FolderStep from .notebook_mask_step import _MaskStepCard @@ -124,6 +125,7 @@ class PreprocessingNotebook( _OutputCards, _AxisStep, _AxisPreview, + _ConvertCard, _RunStep, _MaskStepCard, _RunPlan, @@ -181,6 +183,7 @@ def __init__(self, experiment_folder: str = "", output_folder: str = ""): # noq self._build_masks_cards() self._build_folder_step(str(experiment_folder), str(output_folder)) self._build_axis_step() + self._build_convert_card() self._build_mask_step_card() self._build_run_step() self._build_results_step() diff --git a/tests/test_notebook_convert.py b/tests/test_notebook_convert.py new file mode 100644 index 00000000..def884cd --- /dev/null +++ b/tests/test_notebook_convert.py @@ -0,0 +1,397 @@ +"""PP-11b: the notebook's **Make named-axis copies** card (contract §17, plan §10). + +When step 2's axis check reports images whose header names their axes wrongly or not at +all, the card offers what the command line's ``convert`` does: a required folder for the +copies (the CLI's folder rule), the roles the user confirmed (nothing taken from the +headers; disabled until confirmed), what will be converted and copied byte for byte (the +CLI's planning), the space, then the copies under the copy folder's lock, with refusals and +the stale-lock button on the existing cards. After the copies, the record and the next +step: the copies checked in step 1 with the same output folder, then the run. +""" + +import importlib +import json +import re + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import ( + CONVERSION_RECORD_FILENAME, + AxisMapping, + convert_experiment, + read_conversion_record, +) +from morphobricks_preprocessing.conversion import plan_conversion +from morphobricks_preprocessing.run.lock import LOCK_FILENAME, output_lock +from test_convert_run import plain_experiment # plain stacks: axes_not_named + +pytest.importorskip("ipywidgets") +pytest.importorskip("matplotlib") +ui = importlib.import_module("morphobricks_preprocessing.notebook_style") +PreprocessingNotebook = importlib.import_module( + "morphobricks_preprocessing.notebook_ui" +).PreprocessingNotebook + +NAMES = ("ctrl/r1/a.tif", "ctrl/r1/b.tif", "drug/r1/a.tif") +SWAPPED = AxisMapping("ZYX", ("Z", "X", "Y")) + + +def swapped_experiment(root, names=NAMES, sizes=(0.5, 0.7)): + """OME images whose header names Y and X the other way round from the truth.""" + rng = np.random.default_rng(1) + for name in names: + path = root / name + path.parent.mkdir(parents=True, exist_ok=True) + data = rng.integers(0, 255, (2, 6, 9), dtype=np.uint8) + metadata = {"axes": "ZYX", "PhysicalSizeX": sizes[0], "PhysicalSizeY": sizes[1]} + tifffile.imwrite(path, data, ome=True, photometric="minisblack", metadata=metadata) + return root + + +def text(widget): + return re.sub(r"\s+", " ", re.sub("<[^>]+>", " ", widget.value)) + + +def output_of(root): + return root.parent / f"{root.name}_MorphoBricks_Preprocessing" + + +def checked(root, *, show_output=True, exclude=""): + nb = PreprocessingNotebook(str(root)) + nb.welcome() + nb.folder_step(show_output=show_output) + page = nb.axis_step() + nb.run_step() + nb.exclude.value = exclude + nb.check_button.click() + return nb, page + + +def swap_roles(nb): + """Choose the true roles: the header's Y is X and its X is Y.""" + nb.role_dropdowns[1].value = "X" + nb.role_dropdowns[2].value = "Y" + + +def confirm_copy_roles(nb): + nb.preview_button.click() + nb.preview_checked.button.click() + nb.convert_roles_checked.button.click() + + +def ready_to_convert(tmp_path, copies=None, **kwargs): + root = swapped_experiment(tmp_path / "exp") + nb, page = checked(root, **kwargs) + swap_roles(nb) + confirm_copy_roles(nb) + nb.convert_folder.value = str(copies or tmp_path / "copies") + return nb, root + + +# Offered only for the axis check's images ----------------------------------------------- + + +def test_the_card_is_offered_under_the_axis_check_for_swapped_axes(tmp_path): + root = swapped_experiment(tmp_path / "exp") + nb, page = checked(root) + assert nb.convert_card in nb.axis_box.children + assert not ui.is_visible(nb.convert_card) # the header's roles agree: nothing to convert + swap_roles(nb) + assert "swap the Y and X" in text(nb.axis_status) + assert ui.is_visible(nb.convert_card) + children = list(nb.axis_box.children) + assert children.index(nb.convert_card) == children.index(nb.axis_status) + 1 + nb.role_dropdowns[1].value, nb.role_dropdowns[2].value = "Y", "X" + assert not ui.is_visible(nb.convert_card) + + +def test_the_card_is_offered_for_plain_stacks_and_uses_the_confirmed_roles(tmp_path): + root = plain_experiment(tmp_path / "exp") + nb, _ = checked(root) + nb.role_dropdowns[0].value = "Z" + assert ui.is_visible(nb.convert_card) # axes_not_named: a warning, the roles can be confirmed + nb.preview_button.click() + nb.preview_checked.button.click() + nb.confirm_button.click() + assert nb.confirmed_mapping is not None + # Roles confirmed in step 2 need no second answer on the card. + assert not ui.is_visible(nb.convert_roles_checked.widget) + assert "confirmed in step 2" in text(nb.convert_roles) + nb.convert_folder.value = str(tmp_path / "copies") + assert not nb.convert_button.disabled + nb.convert_button.click() + record, _ = read_conversion_record(tmp_path / "copies") + assert {e.reason for e in record.files} == {"axes_not_named"} + assert record.mapping.roles == ("Z", "Y", "X") + + +def test_an_embedding_brick_that_hides_the_output_folder_does_not_get_the_card(tmp_path): + root = swapped_experiment(tmp_path / "exp") + nb, _ = checked(root, show_output=False) + swap_roles(nb) + assert "swap the Y and X" in text(nb.axis_status) + assert not ui.is_visible(nb.convert_card) + + +# Nothing guessed: the roles must be confirmed --------------------------------------------- + + +def test_the_button_waits_for_the_roles_and_the_folder(tmp_path): + root = swapped_experiment(tmp_path / "exp") + nb, _ = checked(root) + swap_roles(nb) + nb.convert_folder.value = str(tmp_path / "copies") + assert nb.convert_button.disabled + assert nb.convert_roles_checked.disabled # the preview comes first + assert "not confirmed yet" in text(nb.convert_roles) + assert "Waiting for the roles" in text(nb.convert_plan) + nb.preview_button.click() + nb.preview_checked.button.click() + assert not nb.convert_roles_checked.disabled and nb.convert_button.disabled + nb.convert_roles_checked.button.click() + assert not nb.convert_button.disabled + shown = text(nb.convert_roles) + assert "Y (height) in the file → X" in shown and "X (width) in the file → Y" in shown + # Changing a role takes the answer back: the copies are never written with other roles. + nb.role_dropdowns[0].value = "T" + assert not nb.convert_roles_checked.value and nb.convert_button.disabled + assert not (tmp_path / "copies").exists() + + +def test_the_folder_is_required_and_checked_with_the_commands_rule(tmp_path): + nb, root = ready_to_convert(tmp_path, copies=tmp_path / "exp" / "copies") + assert "copy folder overlaps experiment" in text(nb.convert_plan).lower() + assert nb.convert_button.disabled + nb.convert_folder.value = str(tmp_path) # holds the experiment + assert "copy folder overlaps experiment" in text(nb.convert_plan).lower() + (tmp_path / "a_file").write_text("x") + nb.convert_folder.value = str(tmp_path / "a_file") + assert "copy folder not a folder" in text(nb.convert_plan).lower() + nb.convert_folder.value = "" + assert "Waiting for the roles and the folder" in text(nb.convert_plan) + assert nb.convert_button.disabled + nb.convert_folder.value = str(tmp_path / "copies") + assert not nb.convert_button.disabled + + +def test_browse_without_a_dialog_names_colabs_copy_path(tmp_path, monkeypatch): + nb, _ = ready_to_convert(tmp_path) + monkeypatch.setattr(ui, "ask_directory", lambda initial="": None) + nb.convert_browse.click() + assert "Copy path" in nb.convert_status.value + monkeypatch.setattr(ui, "ask_directory", lambda initial="": str(tmp_path / "chosen")) + nb.convert_browse.click() + assert nb.convert_folder.value == str(tmp_path / "chosen") + assert str(tmp_path / "chosen") in text(nb.convert_plan) + + +# What will happen, from the command's planning -------------------------------------------- + + +def test_the_plan_shows_what_is_converted_and_copied_and_the_space(tmp_path): + root = swapped_experiment(tmp_path / "exp") + nb, _ = checked(root, exclude="drug/r1/a.tif") + swap_roles(nb) + confirm_copy_roles(nb) + nb.convert_folder.value = str(tmp_path / "copies") + plan = text(nb.convert_plan) + assert "Written as named-axis OME-TIFF with your roles: 2 images" in plan + assert "ctrl/r1/a.tif (axes_yx_swapped)" in plan + assert "Copied byte for byte" in plan and "drug/r1/a.tif (excluded)" in plan + assert "Space needed" in plan and "Free on that drive" in plan + assert not (tmp_path / "copies").exists() # planning writes nothing + from morphobricks_preprocessing import preview_conversion # new in PP-11b + + expected = preview_conversion(root, SWAPPED, tmp_path / "copies", exclude=["drug/r1/a.tif"]) + assert nb._convert_preview == expected + + +def test_not_enough_free_space_disables_the_button(tmp_path, monkeypatch): + plan_module = importlib.import_module("morphobricks_preprocessing.conversion_plan") + monkeypatch.setattr(plan_module, "free_space", lambda path: 10) + nb, _ = ready_to_convert(tmp_path) + assert "does not have enough free space" in text(nb.convert_plan) + assert nb.convert_button.disabled + + +def test_preview_conversion_is_the_commands_plan_and_writes_nothing(tmp_path): + from morphobricks_preprocessing import preview_conversion # new in PP-11b + + root = swapped_experiment(tmp_path / "exp") + dest = tmp_path / "copies" + preview = preview_conversion(root, SWAPPED, dest) + planned, issues = plan_conversion(root.resolve(), SWAPPED, dest.resolve()) + assert preview.ok and preview.fits and not preview.earlier_conversion + assert preview.converted == tuple((p.relative_path, "axes_yx_swapped") for p in planned) + assert preview.copied == () + assert preview.needed_bytes == sum(p.nbytes for p in planned) == 3 * 2 * 6 * 9 + assert preview.free_bytes is not None and not dest.exists() + assert [i.code for i in preview_conversion(root, SWAPPED, root / "x").issues] == [ + "copy_folder_overlaps_experiment" + ] + convert_experiment(root, SWAPPED, dest) + assert preview_conversion(root, SWAPPED, dest).earlier_conversion + + +# The copies, refusals and the lock ---------------------------------------------------------- + + +def test_the_copies_are_made_with_a_progress_bar_and_the_record_is_shown(tmp_path): + nb, root = ready_to_convert(tmp_path) + nb.convert_button.click() + copies = tmp_path / "copies" + record, sha256 = read_conversion_record(copies) + assert [e.relative_path for e in record.files] == sorted(NAMES) + assert record.mapping == SWAPPED and record.source_root == root.as_posix() + assert nb.convert_progress.bar_style == "success" and ui.is_visible(nb.convert_progress) + assert nb.convert_progress.value == nb.convert_progress.max == 3 + done = text(nb.convert_status) + assert "The named-axis copies are made" in done + assert str(copies / CONVERSION_RECORD_FILENAME) in done + assert "Written as OME-TIFF with your roles: 3 images" in done + assert ui.is_visible(nb.convert_recheck_button) + assert nb.convert_button.description.strip() == "Make named-axis copies" + # Repeating it reuses the copies and keeps the record. + nb.convert_button.click() + assert "Already there and unchanged: 3 images" in text(nb.convert_status) + assert read_conversion_record(copies)[1] == sha256 + + +def test_pixel_sizes_of_a_relabelled_y_and_x_are_stated_as_unknown(tmp_path): + nb, _ = ready_to_convert(tmp_path) + nb.convert_button.click() + after = text(nb.convert_result) + assert "Pixel sizes: 3 copies" in after and "ctrl/r1/a.tif: X, Y" in after + assert "under Pixel sizes in step 1" in after + + +def test_equal_x_and_y_sizes_are_carried_so_nothing_is_said_about_them(tmp_path): + root = swapped_experiment(tmp_path / "exp", sizes=(0.5, 0.5)) + nb, _ = checked(root) + swap_roles(nb) + confirm_copy_roles(nb) + nb.convert_folder.value = str(tmp_path / "copies") + nb.convert_button.click() + assert "The named-axis copies are made" in text(nb.convert_status) + assert "Pixel sizes:" not in text(nb.convert_result) + + +def test_a_folder_that_is_not_empty_is_refused_on_the_card(tmp_path): + copies = tmp_path / "copies" + copies.mkdir() + (copies / "notes.txt").write_text("mine") + nb, _ = ready_to_convert(tmp_path) + nb.convert_button.click() + assert "No copies were made." in text(nb.convert_status) + assert "copy folder not empty" in text(nb.convert_result).lower() + assert nb.convert_progress.bar_style == "danger" + assert sorted(p.name for p in copies.iterdir()) == ["notes.txt"] + assert not ui.is_visible(nb.convert_recheck_button) + + +def test_a_conversion_going_on_shows_the_lock_card_and_writes_nothing(tmp_path): + copies = tmp_path / "copies" + copies.mkdir() + nb, _ = ready_to_convert(tmp_path) + with output_lock(copies): + nb.convert_button.click() + assert "the folder for the copies is locked" in text(nb.convert_status) + assert ui.is_visible(nb.convert_lock.card) + assert "Another conversion holds the lock" in text(nb.convert_lock.holder) + assert "The folder for the copies is locked" in nb.convert_lock.card.children[0].value + nb.convert_lock.button.click() # a process running here: its lock is kept + assert "The lock was kept" in nb.convert_lock.result.value + assert sorted(p.name for p in copies.iterdir()) == [] + + +def test_the_stale_lock_of_a_stopped_conversion_is_removed_only_on_the_button(tmp_path): + copies = tmp_path / "copies" + copies.mkdir() + holder = {"host": "another-pc", "pid": 4242, "started_at": "2026-11-02T10:00:00+00:00"} + (copies / LOCK_FILENAME).write_text(json.dumps(holder), encoding="utf-8") + nb, _ = ready_to_convert(tmp_path) + nb.convert_button.click() + assert ui.is_visible(nb.convert_lock.card) + assert "another-pc" in nb.convert_lock.holder.value + assert (copies / LOCK_FILENAME).exists() + nb.convert_lock.button.click() + assert not (copies / LOCK_FILENAME).exists() + assert "Then click Make named-axis copies again." in nb.convert_lock.result.value + nb.convert_button.click() + assert "The named-axis copies are made" in text(nb.convert_status) + assert not ui.is_visible(nb.convert_lock.card) + + +def test_an_unexpected_failure_becomes_a_red_card(tmp_path, monkeypatch): + nb, _ = ready_to_convert(tmp_path) + notebook_convert = importlib.import_module("morphobricks_preprocessing.notebook_convert") + + def broken(*args, **kwargs): + raise RuntimeError("disk on fire") + + monkeypatch.setattr(notebook_convert, "convert_experiment", broken) + nb.convert_button.click() + assert "unexpected problem" in text(nb.convert_status) + assert "RuntimeError: disk on fire" in text(nb.convert_status) + assert not nb.convert_button.disabled # restored for another try + + +# What next: check the copies, then run on them -------------------------------------------- + + +def test_missing_axes_must_be_confirmed_again_on_the_copies(tmp_path): + root = tmp_path / "exp" + swapped_experiment(root, names=("ctrl/r1/a.tif",)) + path = root / "drug" / "r1" / "a.tif" + path.parent.mkdir(parents=True) + tifffile.imwrite( + path, np.zeros((6, 9), np.uint8), ome=True, photometric="minisblack", + metadata={"axes": "YX"}, + ) # fmt: skip + nb, _ = checked(root) + swap_roles(nb) + nb.missing_as_singleton.button.click() + confirm_copy_roles(nb) + nb.convert_folder.value = str(tmp_path / "copies") + nb.convert_button.click() + after = text(nb.convert_result) + assert "press Yes, they have a single plane there again" in after + assert "--missing-as-singleton" in after + assert read_conversion_record(tmp_path / "copies")[0].missing_as_singleton + + +def test_end_to_end_convert_check_the_copies_with_header_axes_and_run(tmp_path): + nb, root = ready_to_convert(tmp_path) + output = output_of(root) + nb.labels_confirmed.button.click() + for toggle in nb.group_confirmations.values(): + if not toggle.value: + toggle.button.click() + assert nb.output.value == "" # the default output folder of the originals + nb.convert_button.click() + after = text(nb.convert_result) + assert "Check the copies in step 1" in after and str(output) in after + copies = tmp_path / "copies" + nb.convert_recheck_button.click() + # Step 1 now checks the copies, with the originals' output folder kept. + assert nb.folder.value == str(copies) and nb.output.value == str(output) + assert nb.preflight is not None and nb.preflight.discovery.root == copies + assert nb.preflight.ok + assert "".join(nb.mapping.roles) == "ZYX" # suggested from the copies' named axes + assert not ui.is_visible(nb.convert_card) # nothing to convert any more + nb.labels_confirmed.button.click() + assert not nb._design_pending() # the design answered on the originals is kept + nb.preview_button.click() + nb.preview_checked.button.click() + nb.confirm_button.click() + assert "Axes checked" in nb.confirmed_status.value + nb.image_type_checked.button.click() + result = nb.run() + assert result.written, result.issues + manifest = json.loads((output / "run_manifest.json").read_text()) + assert manifest["experiment_root"] == copies.as_posix() + assert {f["axes_from"] for f in manifest["files"]} == {"header"} + assert manifest["conversion_record_sha256"] == read_conversion_record(copies)[1] + assert sorted(f["relative_path"] for f in manifest["files"]) == sorted(NAMES) diff --git a/tests/test_notebook_file.py b/tests/test_notebook_file.py index f43b9456..4f77fac7 100644 --- a/tests/test_notebook_file.py +++ b/tests/test_notebook_file.py @@ -151,6 +151,9 @@ def test_notebook_names_buttons_that_exist(): nb.preview_checked.button, nb.missing_as_singleton.button, nb.confirm_button, + nb.convert_roles_checked.button, + nb.convert_button, + nb.convert_recheck_button, nb.image_type_checked.button, nb.run_button, nb.mask_step_button, From 28ddd91594a600fcefa1ef8eef930c7367e2fde5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:40:07 +0000 Subject: [PATCH 2/3] PP-11b: compare the plan without the free space, which may change between calls Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_notebook_convert.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_notebook_convert.py b/tests/test_notebook_convert.py index def884cd..916330ad 100644 --- a/tests/test_notebook_convert.py +++ b/tests/test_notebook_convert.py @@ -12,6 +12,7 @@ import importlib import json import re +from dataclasses import replace import numpy as np import pytest @@ -205,7 +206,9 @@ def test_the_plan_shows_what_is_converted_and_copied_and_the_space(tmp_path): from morphobricks_preprocessing import preview_conversion # new in PP-11b expected = preview_conversion(root, SWAPPED, tmp_path / "copies", exclude=["drug/r1/a.tif"]) - assert nb._convert_preview == expected + # The same plan as the command's; the free space may change between the two calls. + shown = nb._convert_preview + assert replace(shown, free_bytes=None) == replace(expected, free_bytes=None) def test_not_enough_free_space_disables_the_button(tmp_path, monkeypatch): From 083597317fe27bcb8f68853b5f6515c7b28bcb94 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:55:36 +0000 Subject: [PATCH 3/3] PP-11b: check the kept lock of a running conversion on POSIX only, as the other lock tests Windows cannot tell whether a process runs, so remove_stale_lock leaves the decision to the user. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- tests/test_notebook_convert.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_notebook_convert.py b/tests/test_notebook_convert.py index 916330ad..7691fb5b 100644 --- a/tests/test_notebook_convert.py +++ b/tests/test_notebook_convert.py @@ -11,6 +11,7 @@ import importlib import json +import os import re from dataclasses import replace @@ -304,9 +305,19 @@ def test_a_conversion_going_on_shows_the_lock_card_and_writes_nothing(tmp_path): assert ui.is_visible(nb.convert_lock.card) assert "Another conversion holds the lock" in text(nb.convert_lock.holder) assert "The folder for the copies is locked" in nb.convert_lock.card.children[0].value + assert sorted(p.name for p in copies.iterdir()) == [] + + +@pytest.mark.skipif(os.name != "posix", reason="only POSIX can tell a process runs") +def test_the_lock_of_a_conversion_running_here_is_kept(tmp_path): + copies = tmp_path / "copies" + copies.mkdir() + nb, _ = ready_to_convert(tmp_path) + with output_lock(copies): + nb.convert_button.click() nb.convert_lock.button.click() # a process running here: its lock is kept assert "The lock was kept" in nb.convert_lock.result.value - assert sorted(p.name for p in copies.iterdir()) == [] + assert (copies / LOCK_FILENAME).exists() def test_the_stale_lock_of_a_stopped_conversion_is_removed_only_on_the_button(tmp_path):