diff --git a/CHANGELOG.md b/CHANGELOG.md index be71a37a..de39ae69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,32 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Audit batches 33 and 34a: what is published is what was answered, of the image checked.** + *Yes, these are right* saves exactly the tags a block shows (a hidden proposal is never + saved), and a unit tagging one label twice is refused before it is saved + (`E_DUPLICATE_CHANNEL_IN_UNIT`). An unanswered image type is an open question + (`unit_image_type_unanswered`; "channels" is still published as an absent `image_type`). + `publish_draft` now requires `channel_counts`, each unit's channel count read through the + confirmed axis roles: tags saved for another count (`unit_channels_stale`) or a unit without + a count (`unit_channel_count_unknown`) are refused. The notebook's folder check binds every + header to its file (`bind_headers=True`); the tag pictures, the Step 1 pictures and the mask + evidence are drawn only of that revision (masks paired again without a new check are bound + too, and evidence already read is not shown once its image or mask changed), and the save is + refused when an image changed since the check. The axis step's preview is not bound yet. + *Check and save* shows tags saved for another channel count as stale, not as the folder's + channels. An answer the draft file cannot take (lock, write error) is + refused and not shown as answered. `make_described_run` refuses a member with no technical + repeat or fewer images than technical repeats before writing anything. A new record the + records reader would refuse is never written (`R_RECORDS_FILE_INVALID`), and a run refused + for its records file reports that code with its `E_RECORDS_*` reasons instead of + `description_unreadable`. `all_colour` counts a unit only when its tags are exactly R, G, B. + **API changes:** `publish_draft(..., channel_counts=...)` is a new required keyword; + `missing_confirmations` and `description_from_draft` take an optional `channel_counts`; + `header_preview(..., bound=...)` is a new required keyword; `mask_evidence` takes + `options: StripOptions, bound` in place of `cache, memory_limit`; `Thumbnails` takes the + check's `header_fingerprints`. A draft saved before this change with no image type reopens + with the image type asked, never as "channels". + - **Notebook: folder pictures with a 1:1 crop, "Tag like the saved group", colour mask evidence (PR 17c).** *Images found* adds a picture of each folder's first image to the labels table, from the file header only and display only (`header_preview`, `notebook_folder_previews`): @@ -52,7 +78,9 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the (`inconsistent_mapping`, `missing_axes`, `ambiguous_missing_axes`) no longer advise exporting the images the same way: they stay errors and say they follow from the layouts. The notebook's axis step says that no roles are suggested because the units declare different layouts (fluorescence - and RGB). The refusal codes are unchanged: nothing refused before is accepted. + and RGB). A previously blocking input stays blocked; diagnostic codes may change (a described + run reports the per-type checks and the Spec's codes in place of the global `mixed_dtypes` and + `inconsistent_dimensions`). - **Notebook: one step at a time (UX proposal 2026-11-09, PR 17a).** `PreprocessingNotebook.steps()` shows the shipped notebook's 13 steps one at a time, with a strip of chips (done green with a tick, current blue, to do grey, changed by a reopen orange; a chip opens its step) and Back/Next (Next is diff --git a/docs/contract.md b/docs/contract.md index 489e9cf8..804bd591 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -73,7 +73,10 @@ planes of an experiment use the same bin count. In a described run (a published `morphobricks.toml`) the pixel type is compared among the units that declare the same `image_type`, and the notebook's folder check compares images inside each unit; units of different image types (RGB and fluorescence) are refused as `R_HETEROGENEOUS_LAYOUT_NOT_ENABLED` -(Stages 1-2), not as `mixed_dtypes`. An undescribed run compares every image of the experiment. +(Stages 1-2), not as `mixed_dtypes`. An undescribed run compares every image of the experiment. The +invariant is that a previously blocking input stays blocked; the diagnostic codes may change +(the per-type checks and the Spec's `R_*` codes replace the global `mixed_dtypes` and +`inconsistent_dimensions` of a described run). ## 4. Histogram semantics @@ -687,5 +690,5 @@ Rules. - **Saving** (`save_draft`) is atomic (temporary file, one replace) and under the output folder's lock, after each answer. The answers are applied to the draft and the exact bytes are read back with the strict reader first, so an answer that would be refused on resume (a `spec_commit` other than the pinned one, `spec_commit_mismatch`; an ID the reader would refuse, a folder the draft does not have, a date that is not a real day) is refused when saved and nothing is written. An incomplete draft can be saved. The file is replaced, not merged. A symbolic link at the draft's path is refused and never replaced. - **Reading** is strict and never repairs: bytes that are not UTF-8 JSON, a byte-order mark, a duplicate key, `NaN`, another `format`, an unknown `version`, an unknown key at any level, a missing key, a value of the wrong type or a date that is not a real `YYYY-MM-DD` day are `DraftFileError` (`code`: `malformed`, `unknown_version`, `unknown_key`, `R_DRAFT_BASE_MISSING`, `R_FILE_SYMLINK`). Reading accepts any 40-digit `spec_commit`; whether it is the current one is decided at resume and save, below. - **Resume** (`load_draft`): the file's `spec_commit` must equal the pinned `SPEC_COMMIT`, else `DraftFileError` with code `spec_commit_mismatch` (owner decision, 2026-11-08): the confirmations in the file were given under other rules, so none is carried forward. The message names both commits, says the published `morphobricks.toml` is untouched and says to start a fresh draft from the folders. Library code never rewrites the stored commit and never deletes or replaces the stale draft; the caller derives a fresh draft and saves it, which replaces the file. `publish_draft` resumes first, so it refuses the same way and writes nothing. Then the file's `base_sha256` must equal the SHA-256 of the file published now (or `absent` if there is none), else `R_PUBLISHED_FILE_CHANGED` (`PublicationRefused`); the draft is never merged with the new file, it is derived again from it. An answer keyed by a folder or repeat the draft no longer has is `DraftError` (`unknown_key`), never moved to another folder. -- **Draft to description** (`description_from_draft(draft, answers)`, pure): refuses with `DraftIncomplete`, listing **every** missing confirmation once (`id_pending`, `repeat_not_confirmed`, `confirmed_on_missing`, `control_unresolved`, `legacy_folder_missing`, `unit_channels_missing` for a unit with no channel list, `unit_channel_untagged` for a position still `null`, `channel_undeclared` for a channel a unit names that is not declared), otherwise returns the `Description`. Nothing is confirmed for the user. `used_ids` is the draft's retired IDs plus every ID confirmed now. The description is then validated by the strict reader at publication; the function does not repeat the reader's rules. -- **Publication** (`publish_draft`): under one hold of the lock the draft is resumed, the description built, published with all the checks of `publish_description` (symbolic link, base, transition, strict reader on the exact bytes), and then the draft file is deleted. Any refusal writes nothing: the draft and the published file stay byte for byte as they were. If the draft cannot be deleted after a successful publication, the error is raised and the publication stands; the leftover draft records a base that no longer matches and is refused on resume. +- **Draft to description** (`description_from_draft(draft, answers, *, channel_counts=None)`, pure): refuses with `DraftIncomplete`, listing **every** missing confirmation once (`id_pending`, `repeat_not_confirmed`, `confirmed_on_missing`, `control_unresolved`, `legacy_folder_missing`, `unit_channels_missing` for a unit with no channel list, `unit_image_type_unanswered` for a unit whose image type is not answered `channels` or `rgb` (an old draft without one reopens with the question asked, never as `channels`), `unit_channel_untagged` for a position still `null`, `channel_undeclared` for a channel a unit names that is not declared; with `channel_counts`, the channel count of each unit's image now read through the confirmed axis roles, also `unit_channels_stale` for tags saved for another count and `unit_channel_count_unknown` for a unit without a count), otherwise returns the `Description`. A unit tagging one label at two positions is refused by `apply_answers` before anything is saved (`E_DUPLICATE_CHANNEL_IN_UNIT`). Nothing is confirmed for the user. `used_ids` is the draft's retired IDs plus every ID confirmed now. The description is then validated by the strict reader at publication; the function does not repeat the reader's rules. +- **Publication** (`publish_draft(..., channel_counts=...)`, the counts required): under one hold of the lock the draft is resumed, the description built with the counts, published with all the checks of `publish_description` (symbolic link, base, transition, strict reader on the exact bytes), and then the draft file is deleted. Any refusal writes nothing: the draft and the published file stay byte for byte as they were. If the draft cannot be deleted after a successful publication, the error is raised and the publication stands; the leftover draft records a base that no longer matches and is refused on resume. diff --git a/src/morphobricks_preprocessing/channel_answers.py b/src/morphobricks_preprocessing/channel_answers.py index c9257094..6a0a2bcf 100644 --- a/src/morphobricks_preprocessing/channel_answers.py +++ b/src/morphobricks_preprocessing/channel_answers.py @@ -37,12 +37,17 @@ def all_colour(layouts: Sequence[UnitLayout], units: Mapping[UnitKey, UnitAnswer def _declared_colour(layout: UnitLayout, answer: UnitAnswer | None) -> bool: - """Return True for a readable unit with a saved ``rgb`` answer that still fits its image.""" + """Return True for a readable unit with a saved ``rgb`` answer that still fits its image. + + The tags must be exactly ``R``, ``G``, ``B`` in this order (experiment-v1 section 4.4b): a + stale or malformed answer (a resumed draft with other tokens) never counts. + """ return ( layout.usable and answer is not None and answer.image_type == "rgb" - and layout.channels == len(RGB_IDS) == len(answer.channels) # a stale answer never counts + and layout.channels == len(RGB_IDS) + and tuple(answer.channels) == RGB_IDS ) diff --git a/src/morphobricks_preprocessing/channel_labels.py b/src/morphobricks_preprocessing/channel_labels.py index dc28e2d5..3667eb44 100644 --- a/src/morphobricks_preprocessing/channel_labels.py +++ b/src/morphobricks_preprocessing/channel_labels.py @@ -204,7 +204,9 @@ class UnitLayout: ``image`` is the first image of the unit by sorted path, whose header gives ``channels`` (the number of C indices, 3 for a colour image) and ``names`` (the raw header names by C index, None where it has none). ``problem`` is why the unit cannot be tagged (no readable first - image, roles that do not fit it); then ``channels`` is 0. + image, roles that do not fit it); then ``channels`` is 0. ``uniform`` is False when some + other image of the unit has another header signature (channel count, raw names or colour): + such a unit is never put in a block with others. """ key: UnitKey @@ -214,6 +216,7 @@ class UnitLayout: rgb: bool = False problem: str = "" images: int = 0 + uniform: bool = True @property def usable(self) -> bool: diff --git a/src/morphobricks_preprocessing/channel_units.py b/src/morphobricks_preprocessing/channel_units.py index 17417097..244badb0 100644 --- a/src/morphobricks_preprocessing/channel_units.py +++ b/src/morphobricks_preprocessing/channel_units.py @@ -67,7 +67,9 @@ def unit_layouts( """Return one layout per unit key, from the unit's first image by sorted path. A unit whose first image cannot be read, or does not fit the confirmed roles, is returned - with a ``problem`` and no channels: the next image is **not** used instead. + with a ``problem`` and no channels: the next image is **not** used instead. A unit whose + other images do not all have the first one's header signature is ``uniform`` False, so the + tag grid never puts it in a block with other units on its first image alone. """ images = unit_images(discovery, excluded) headers = {f.relative_path: f for f in metadata.files} @@ -90,12 +92,18 @@ def unit_layouts( ) ) continue - out.append( - UnitLayout( - key, first, count, tuple(meta.raw_channel_names), meta.rgb_samples, "", len(found) - ) - ) + signature = (count, tuple(meta.raw_channel_names), meta.rgb_samples) + uniform = all(_signature(headers.get(i), review) == signature for i in found[1:]) + out.append(UnitLayout(key, first, *signature, "", len(found), uniform)) return tuple(out) +def _signature(meta: TiffMetadata | None, review: AxisReview) -> tuple | None: + """Return an image's header signature (count, raw names, colour); None when unreadable.""" + if meta is None or not meta.readable: + return None + count = channel_count(meta, review) + return None if count is None else (count, tuple(meta.raw_channel_names), meta.rgb_samples) + + __all__ = ["channel_count", "header_names", "slot_count", "unit_images", "unit_layouts"] diff --git a/src/morphobricks_preprocessing/draft_answers.py b/src/morphobricks_preprocessing/draft_answers.py index 1d2ce9a6..5fb6009d 100644 --- a/src/morphobricks_preprocessing/draft_answers.py +++ b/src/morphobricks_preprocessing/draft_answers.py @@ -28,6 +28,11 @@ from .description_transition import DATE from .draft import Draft, DraftError, OpenQuestion +CHANNELS = "channels" +"""The answer "an ordinary channel image": published as an absent ``image_type`` (§4.4b).""" +ANSWERED_TYPES = (CHANNELS, "rgb") +"""The image-type answers that count: only an explicit one is published.""" + UnitKey = tuple[str, str, str] """``(condition folder, source repeat folder, technical-repeat folder or "single")``.""" @@ -214,8 +219,10 @@ def apply_answers(draft: Draft, answers: DraftAnswers) -> Draft: answer for a folder or repeat the draft does not have is refused. Applying the same answers twice gives the same draft. The pairing is applied first (:meth:`.draft.Draft.paired`); a reference to the condition itself or one that closes a loop is refused (``E_REFERENCE_SELF``, - ``E_REFERENCE_CYCLE``). Channels, units and masks are not part of the draft and are not - checked here (:func:`missing_confirmations`, then the strict reader at publication). + ``E_REFERENCE_CYCLE``). A unit that tags one label at two positions is refused + (``E_DUPLICATE_CHANNEL_IN_UNIT``): such an answer is never saved. The rest of the + channels, units and masks are not part of the draft and are not checked here + (:func:`missing_confirmations`, then the strict reader at publication). Raises ------ @@ -226,6 +233,12 @@ def apply_answers(draft: Draft, answers: DraftAnswers) -> Draft: unknown = _unknown_keys(draft, answers) if unknown: raise DraftError(f"the draft has no {', '.join(unknown)}", ("unknown_key",)) + twice = _repeated_tags(answers.units) + if twice: + raise DraftError( + f"one label cannot stand at two positions of a unit: {'; '.join(twice)}", + ("E_DUPLICATE_CHANNEL_IN_UNIT",), + ) for folder, answer in sorted(answers.conditions.items()): if answer.id is not None: draft = draft.confirm_id("conditions", folder, answer.id) @@ -291,29 +304,78 @@ def carry_over( return kept, tuple(dropped) -def missing_confirmations(draft: Draft, answers: DraftAnswers) -> tuple[OpenQuestion, ...]: +def _repeated_tags(units: Mapping[UnitKey, UnitAnswer]) -> list[str]: + """Name every unit that tags one label at more than one position, with the label.""" + out = [] + for key, unit in sorted(units.items()): + tags = [t for t in unit.channels if t is not None] + out += [f"{' / '.join(key)}: {t}" for t in sorted({t for t in tags if tags.count(t) > 1})] + return out + + +def _unit_questions( + where: str, answer: UnitAnswer, count: int | None, checked: bool +) -> list[OpenQuestion]: + """Return what is still open about one unit that has saved channels. + + ``count`` is the channel count of the unit's image now, from its header read through the + confirmed axis roles (None when the caller does not know it); ``checked`` says whether the + caller gave the counts at all. + """ + out = [] + if answer.image_type not in ANSWERED_TYPES: + out.append(OpenQuestion("unit_image_type_unanswered", where)) + if checked and count is None: + out.append(OpenQuestion("unit_channel_count_unknown", where)) + elif checked and count != len(answer.channels): + # Tags saved for another channel count: never stretched, cut or kept; retag. + out.append( + OpenQuestion( + "unit_channels_stale", + f"{where} ({len(answer.channels)} tags saved, the image has {count} channels)", + ) + ) + return out + out += [ + OpenQuestion("unit_channel_untagged", f"{where} / C{i}") + for i, c in enumerate(answer.channels) + if c is None + ] + return out + + +def missing_confirmations( + draft: Draft, + answers: DraftAnswers, + *, + channel_counts: Mapping[UnitKey, int] | None = None, +) -> tuple[OpenQuestion, ...]: """Return every confirmation still missing for ``draft`` with ``answers`` applied. The draft's own open questions (IDs, repeats, controls, legacy folders), plus the channels - of every imaging unit that has none (``unit_channels_missing``), every position not tagged - (``unit_channel_untagged``) and every channel a unit names that is not declared + of every imaging unit that has none (``unit_channels_missing``), every unit whose image + type is not answered ``"channels"`` or ``"rgb"`` (``unit_image_type_unanswered``: an + unanswered type is never published as an ordinary channel image), every position not + tagged (``unit_channel_untagged``) and every channel a unit names that is not declared (``channel_undeclared``). + + ``channel_counts`` are the channel counts of each unit's image now, from its header read + through the confirmed axis roles. With them, a unit whose saved tags are not one per + channel is ``unit_channels_stale`` (the user retags it) and a unit without a count is + ``unit_channel_count_unknown``. :func:`.run.draft_file.publish_draft` always gives them. """ out = list(draft.open_questions()) declared = set(answers.channels) undeclared: set[str] = set() + checked = channel_counts is not None for row in draft.unit_rows(): key = (row.condition, row.biological_repeat, row.technical_repeat) answer = answers.units.get(key) if answer is None or not answer.channels: out.append(OpenQuestion("unit_channels_missing", " / ".join(key))) continue - where = " / ".join(key) - out += [ - OpenQuestion("unit_channel_untagged", f"{where} / C{i}") - for i, c in enumerate(answer.channels) - if c is None - ] + count = channel_counts.get(key) if checked else None + out += _unit_questions(" / ".join(key), answer, count, checked) undeclared.update(c for c in answer.channels if c and c not in declared) return tuple(out + [OpenQuestion("channel_undeclared", c) for c in sorted(undeclared)]) @@ -346,7 +408,7 @@ def _repeat(draft: Draft, label: str, answers: DraftAnswers) -> Repeat: units = {} for u in member.units: given = answers.units[(member.condition, member.source_repeat, u.source_folder)] - kind = None if given.image_type == "channels" else given.image_type + kind = None if given.image_type == CHANNELS else given.image_type units[draft.technical_repeats[u.source_folder].confirmed] = Unit( u.source_folder, tuple(given.channels), given.display_name, kind ) @@ -355,12 +417,18 @@ def _repeat(draft: Draft, label: str, answers: DraftAnswers) -> Repeat: return Repeat(True, repeat.confirmed_on, members, shown) -def description_from_draft(draft: Draft, answers: DraftAnswers) -> Description: +def description_from_draft( + draft: Draft, + answers: DraftAnswers, + *, + channel_counts: Mapping[UnitKey, int] | None = None, +) -> Description: """Build the description of a draft whose every science decision the user confirmed. Pure: no file is read or written and no clock is read. Each ID and ``confirmed_on`` is the user's, from ``answers`` (or carried by a migrated ``draft``); ``used_ids`` is the draft's - retired IDs plus every ID confirmed now. + retired IDs plus every ID confirmed now. ``channel_counts`` are as + :func:`missing_confirmations` takes them. Raises ------ @@ -371,7 +439,7 @@ def description_from_draft(draft: Draft, answers: DraftAnswers) -> Description: repeat holding two folders of one condition. """ applied = apply_answers(draft, answers) - questions = missing_confirmations(applied, answers) + questions = missing_confirmations(applied, answers, channel_counts=channel_counts) if questions: raise DraftIncomplete(questions) conditions = {} @@ -408,6 +476,8 @@ def description_from_draft(draft: Draft, answers: DraftAnswers) -> Description: __all__ = [ + "ANSWERED_TYPES", + "CHANNELS", "ChannelAnswer", "ConditionAnswer", "DraftAnswers", diff --git a/src/morphobricks_preprocessing/fingerprint.py b/src/morphobricks_preprocessing/fingerprint.py index c14240c3..46fc3cf3 100644 --- a/src/morphobricks_preprocessing/fingerprint.py +++ b/src/morphobricks_preprocessing/fingerprint.py @@ -242,6 +242,22 @@ def current_fingerprint(path: Path) -> Fingerprint | None: return None +def changed_since_check( + root: Path, bound: Mapping[str, Fingerprint | None], paths: Iterable[str] +) -> tuple[str, ...]: + """Return the images that are not, now, the revision whose header the check read. + + ``bound`` are the fingerprints the folder check took with each header + (:attr:`.run.preflight.Preflight.header_fingerprints`); an image without one, changed or + gone is listed, sorted. + """ + return tuple( + rel + for rel in sorted(paths) + if bound.get(rel) is None or current_fingerprint(root / rel) != bound[rel] + ) + + def _frozen(value: object) -> object: """Return ``value`` with lists and dicts made hashable (tuples), for comparing keys.""" if isinstance(value, Mapping): diff --git a/src/morphobricks_preprocessing/header_preview.py b/src/morphobricks_preprocessing/header_preview.py index 0209c6da..4f5df332 100644 --- a/src/morphobricks_preprocessing/header_preview.py +++ b/src/morphobricks_preprocessing/header_preview.py @@ -16,6 +16,9 @@ the planes (the caption says so). OME-Zarr images are read from level 0 by the same rule. - **A failure is a sentence in the cell** (:class:`HeaderPreview.problem`), never an exception that stops the folder check. +- **Only the image as the check read it:** the picture is drawn only when the file still has the + fingerprint the folder check took with its header; another revision (or one not bound) says + "check the folder again" and is not drawn. The projection reads the planes through :func:`.planes.read_plane` with a reading layout made from the header's own letters. Its role slots only say which dimension to step through: every @@ -27,15 +30,23 @@ import math import os +from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path import numpy as np from .axes import AxisMapping +from .fingerprint import Fingerprint from .metadata import TiffMetadata, named_axes from .planes import PlaneMemoryError, PlaneReadError, read_plane -from .preview import READ_BUDGET, THUMBNAIL_SIDE +from .preview import ( + READ_BUDGET, + THUMBNAIL_SIDE, + PreviewRefused, + check_unchanged, + checked_fingerprint, +) from .preview_crop import TilePictures, tile_pictures PLANE_CAP = 8 @@ -183,7 +194,11 @@ def header_planes(path: str | os.PathLike, meta: TiffMetadata) -> HeaderPlanes: def header_preview( - root: Path, meta: TiffMetadata | None, side: int = THUMBNAIL_SIDE + root: Path, + meta: TiffMetadata | None, + side: int = THUMBNAIL_SIDE, + *, + bound: Mapping[str, Fingerprint | None], ) -> HeaderPreview: """Return a folder's picture from its first image's header, or the reason there is none. @@ -195,13 +210,22 @@ def header_preview( The header of the folder's first image by path; None when it has no image to analyse. side The thumbnail's largest side in pixels. + bound + The fingerprints the folder check took with each header + (:attr:`.run.preflight.Preflight.header_fingerprints`): the image is drawn only as that + revision. """ if meta is None: return HeaderPreview(None, problem="no image to analyse in this folder") try: + check_unchanged( + root / meta.relative_path, + checked_fingerprint(bound, meta.relative_path), + meta.relative_path, + ) found = header_planes(root / meta.relative_path, meta) pictures = tile_pictures(found.planes, found.colour, side) - except PreviewUnavailable as exc: + except (PreviewUnavailable, PreviewRefused) as exc: return HeaderPreview(meta.relative_path, problem=str(exc)) except READ_FAILURES as exc: return HeaderPreview(meta.relative_path, problem=f"cannot be read for a preview: {exc}") diff --git a/src/morphobricks_preprocessing/mask_evidence.py b/src/morphobricks_preprocessing/mask_evidence.py index 9b8f6ce8..ba9ae979 100644 --- a/src/morphobricks_preprocessing/mask_evidence.py +++ b/src/morphobricks_preprocessing/mask_evidence.py @@ -10,24 +10,31 @@ "separate objects" or "regions"; the overlay is drawn only as the user declared (:func:`.preview_render.mask_overlay_png`). For a colour (RGB) folder the image is drawn in colour (:meth:`MaskEvidence.in_colour`, :func:`.preview_render.colour_composite`), display only. + +The evidence is of the files as the folder check read them: the image is read only with the +fingerprint the check took with its header, and the mask only with the fingerprint taken with +its own header (:attr:`.masks.MaskFile.bound`); a mask found without one must not change while it +is read. Another revision is refused ("check the folder again"), never shown as the one checked. """ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from dataclasses import dataclass, replace from pathlib import Path import numpy as np from .axes import AxisReview +from .fingerprint import Fingerprint, current_fingerprint from .masks import MaskFile, MaskInventory, read_mask from .preview import ( - THUMBNAIL_SIDE, - PreviewCache, + FILE_CHANGED, PreviewRefused, StripOptions, channel_strip, + check_unchanged, + checked_fingerprint, labels_thumbnail, ) from .preview_render import ( @@ -119,8 +126,8 @@ def mask_evidence( analysed: Sequence[str], review: AxisReview, channel: int | None, - cache: PreviewCache, - memory_limit: int, + options: StripOptions, + bound: Mapping[str, Fingerprint | None], ) -> MaskEvidence: """Gather the evidence about one mask type (see the module docstring). @@ -138,8 +145,12 @@ def mask_evidence( The axis review of the confirmed roles: the image is read only through them. channel The C index to draw the mask on (None or 0 for an image without channels). - cache, memory_limit - The preview cache and the memory limit of one plane. + options + The size, preview cache and memory limit of the image's picture (its ``expected`` is + set here, from ``bound``). + bound + The fingerprints the folder check took with each image header + (:attr:`.run.preflight.Preflight.header_fingerprints`). """ files = [f for f in masks.files if f.mask_type == source_type and f.image in set(analysed)] dtypes = tuple(sorted({str(f.dtype) for f in files if f.dtype is not None})) @@ -147,13 +158,14 @@ def mask_evidence( first = representative(masks, source_type, analysed) if first is None: return found - options = StripOptions(side=THUMBNAIL_SIDE, cache=cache, memory_limit=memory_limit) try: + options = replace(options, expected=checked_fingerprint(bound, first.image)) + check_unchanged(root / first.image, options.expected, first.image) strip = channel_strip(root / first.image, first.image, review, options) panel = strip.panels[channel or 0] z = strip.z if "Z" in first.axes else None t = strip.t if "T" in first.axes else None - plane = read_mask(first, masks.folder, root, z=z, t=t) + plane = _read_bound(first, masks.folder, root, (z, t)) values = int(np.unique(plane).size) labels = labels_thumbnail(plane, panel.plane).pixels except READ_ERRORS as exc: @@ -178,6 +190,29 @@ def mask_evidence( ) +def _read_bound( + mask: MaskFile, folder: Path | None, root: Path, where: tuple[int | None, int | None] +) -> np.ndarray: + """Read the mask plane only as the revision whose header was read; refuse another. + + A mask bound by the check (:attr:`.masks.MaskFile.bound`) must still have that fingerprint; + one found without binding must keep one fingerprint across the read. + + Raises + ------ + PreviewRefused + ``file_changed_while_shown``. + """ + path = mask.location(folder, root) + before = mask.bound or current_fingerprint(path) + if before is None: + raise PreviewRefused(FILE_CHANGED, f"the mask {mask.path} is gone: check the folder again.") + check_unchanged(path, before, f"the mask {mask.path}") + plane = read_mask(mask, folder, root, z=where[0], t=where[1]) + check_unchanged(path, before, f"the mask {mask.path}") + return plane + + def _missing(found: MaskEvidence, image: str, why: str) -> MaskEvidence: """Return the evidence without pictures or values, saying why.""" return MaskEvidence( diff --git a/src/morphobricks_preprocessing/notebook_channel_thumbs.py b/src/morphobricks_preprocessing/notebook_channel_thumbs.py index 69c0dbaa..1308b4bf 100644 --- a/src/morphobricks_preprocessing/notebook_channel_thumbs.py +++ b/src/morphobricks_preprocessing/notebook_channel_thumbs.py @@ -12,15 +12,22 @@ When a thumbnail keeps fewer than one pixel in :data:`.preview_crop.CROP_FACTOR` per side, :meth:`Thumbnails.tile` adds a crop at native pixels (:func:`.preview_crop.tile_pictures`), read through the same confirmed roles and plane. + +Every picture is bound to the image as the folder check read its header: each read is given +that fingerprint (``StripOptions.expected``), so an image changed since the check (same name, +same shape or not) is refused with "check the folder again", never drawn as if it were the one +checked. """ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, replace from pathlib import Path from .axes import AxisReview from .channel_labels import UnitLayout +from .fingerprint import Fingerprint from .preview import ( DETAIL_SIDE, THUMBNAIL_SIDE, @@ -28,6 +35,8 @@ PreviewRefused, StripOptions, channel_strip, + check_unchanged, + checked_fingerprint, confirmed_mapping, read_plane, ) @@ -57,10 +66,22 @@ class Thumb: class Thumbnails: - """Reads and keeps the channel pictures of the units' representative images.""" - - def __init__(self, root: Path, review: AxisReview, memory_limit: int) -> None: + """Reads and keeps the channel pictures of the units' representative images. + + ``bound`` are the fingerprints the folder check took with each header + (:attr:`.run.preflight.Preflight.header_fingerprints`): an image is read only as that + revision. + """ + + def __init__( + self, + root: Path, + review: AxisReview, + memory_limit: int, + bound: Mapping[str, Fingerprint | None], + ) -> None: self.root, self.review, self.memory_limit = root, review, memory_limit + self.bound = bound self._cache = PreviewCache() self._strips: dict[tuple[str, int], tuple[Panel, ...] | Thumb] = {} self._planes: dict[str, tuple[tuple[int, int], int | None, int | None]] = {} @@ -70,9 +91,18 @@ def __init__(self, root: Path, review: AxisReview, memory_limit: int) -> None: def _read(self, layout: UnitLayout, side: int) -> tuple[Panel, ...] | Thumb: """Read every channel of the unit's image once per size; a refusal becomes one note.""" key = (layout.image or "", side) + try: # a picture already read is shown only while the image is still that revision + check_unchanged(self.root / key[0], checked_fingerprint(self.bound, key[0]), key[0]) + except READ_ERRORS as exc: + return Thumb(None, str(exc)) if key not in self._strips: - options = StripOptions(side=side, cache=self._cache, memory_limit=self.memory_limit) try: + options = StripOptions( + side=side, + expected=checked_fingerprint(self.bound, key[0]), + cache=self._cache, + memory_limit=self.memory_limit, + ) strip = channel_strip(self.root / key[0], key[0], self.review, options) except READ_ERRORS as exc: self._strips[key] = Thumb(None, str(exc)) @@ -150,6 +180,7 @@ def _native_tile( has_c = mapping.index_of("C") is not None planes = [] try: + check_unchanged(self.root / image, checked_fingerprint(self.bound, image), image) for c in range(3 if colour else 1): plane = read_plane( self.root / image, diff --git a/src/morphobricks_preprocessing/notebook_channels.py b/src/morphobricks_preprocessing/notebook_channels.py index bb3b7531..c7e49457 100644 --- a/src/morphobricks_preprocessing/notebook_channels.py +++ b/src/morphobricks_preprocessing/notebook_channels.py @@ -128,17 +128,34 @@ def _refresh_channels(self) -> None: ] layouts = unit_layouts(keys, pre.discovery, pre.excluded, pre.metadata, review) slots, names = slot_count(pre.metadata, review), header_names(pre.metadata) - key = (str(pre.discovery.root), self.confirmed_mapping, layouts, slots, tuple(names)) + # The pictures are of the images as this check read them: a new check, or another + # revision of a representative image, is another key and its pictures are read again. + bound = tuple( + sorted((u.image, pre.header_fingerprints.get(u.image)) for u in layouts if u.image) + ) + key = (str(pre.discovery.root), self.confirmed_mapping, layouts, slots, tuple(names), bound) if key == self._channels_key: self._refresh_masks_screen() return self._channels_key = key self._channel_layouts = layouts - self._channel_thumbs = Thumbnails(pre.discovery.root, review, self._memory_limit()) + self._channel_thumbs = Thumbnails( + pre.discovery.root, review, self._memory_limit(), pre.header_fingerprints + ) self.channels_waiting.value = "" self.channel_labels.open(slots, names, layouts) self._channel_labels_changed() + def _channel_counts(self) -> dict[tuple[str, str, str], int]: + """Return each unit's channel count, read through the confirmed axis roles, by key. + + Empty while the screen is not open for the current check and roles; a unit whose image + cannot be tagged has none. The save is gated on these counts. + """ + if self._channels_key is None: + return {} + return {u.key: u.channels for u in self._channel_layouts if u.usable} + @property def _stains_not_applicable(self) -> bool: """True when the user declared every folder colour (RGB): R, G, B need no label.""" diff --git a/src/morphobricks_preprocessing/notebook_channels_grid.py b/src/morphobricks_preprocessing/notebook_channels_grid.py index 78f5eac2..a7aac0e4 100644 --- a/src/morphobricks_preprocessing/notebook_channels_grid.py +++ b/src/morphobricks_preprocessing/notebook_channels_grid.py @@ -73,8 +73,12 @@ def folder_order(layout: UnitLayout) -> tuple: def named(layout: UnitLayout) -> bool: - """Return True when the header names every channel of a usable unit (a block can hold it).""" - return layout.usable and all(layout.name(i) for i in range(layout.channels)) + """Return True when the header names every channel of a usable unit (a block can hold it). + + Every image of the unit must have that header: a unit whose later images differ is never + grouped on its first image alone. + """ + return layout.usable and layout.uniform and all(layout.name(i) for i in range(layout.channels)) class TagGrid: @@ -172,7 +176,8 @@ def _block_key(self, layout: UnitLayout) -> tuple: """Return what the folders of one block share exactly; a folder alone has its own key.""" colour = self._colour(layout) tags = tags_of(layout, self.store.answers().units.get(layout.key)) - together = (colour or named(layout)) and layout.key not in self.alone + grouped = (colour and layout.uniform) or named(layout) + together = grouped and layout.key not in self.alone return (layout.signature, colour, tags, None if together else layout.key) def block_list(self) -> list[list[UnitLayout]]: @@ -236,19 +241,18 @@ def _refuse_twice(self, value: str, other: int) -> None: self.refresh() def _confirm_layout(self, units: Sequence[UnitLayout]) -> None: - """Save the header's proposal of these units as the answer. + """Save the tags these units show as the answer: exactly what is displayed. - The colour pre-tick is saved too only when the image type is answered here. + Only the proposals shown (:meth:`effective`) are saved; a position whose proposal is + not shown (its label is already tagged elsewhere in the unit) stays untagged. The colour + pre-tick is saved too only when the image type is answered here. """ answers = self.store.answers() rgb = [u.key for u in units if self.place_types and self.types.proposed(u)] for layout in units: - proposal = self._proposal(layout) - if proposal is None: - continue - tags = tags_of(layout, answers.units.get(layout.key)) - filled = tuple(t if t is not None else proposal[i] for i, t in enumerate(tags)) - answers = with_tags(answers, layout, filled) + shown, proposed = self.effective(layout) + if proposed: + answers = with_tags(answers, layout, shown) ok = self.store.update(synced(answers, self.layouts)) self.message = "" if ok else ui.status("error", "Not saved.", [self.store.refusal()]) if ok and rgb: @@ -381,6 +385,8 @@ def _block_text(self, units: Sequence[UnitLayout], colour: bool, apart: bool) -> text += f". {self._type_text(first)} Pictures: {COLOUR_TEXT}." elif len(units) > 1: text += " with this header: tagged once, for all of them." + elif not first.uniform: + text += ": its images do not all have this header, so it is tagged on its own." elif not named(first): text += ": a header without every channel name is tagged folder by folder." if apart: diff --git a/src/morphobricks_preprocessing/notebook_check_save.py b/src/morphobricks_preprocessing/notebook_check_save.py index f02bbf10..f72256f1 100644 --- a/src/morphobricks_preprocessing/notebook_check_save.py +++ b/src/morphobricks_preprocessing/notebook_check_save.py @@ -21,6 +21,7 @@ import html from dataclasses import replace from datetime import date +from pathlib import Path import ipywidgets as widgets @@ -39,6 +40,7 @@ missing_confirmations, ) from .draft_format import DraftFileError +from .fingerprint import changed_since_check from .mask_answers import masks_unanswered from .notebook_channel_thumbs import Thumb from .notebook_check_view import ( @@ -61,6 +63,7 @@ from .run import OutputLockedError from .run.description_file import PublicationRefused from .run.draft_file import publish_draft +from .validation import plural INTRO = ( "What happens here: your experiment is restated in sentences. Answer, for each " @@ -218,7 +221,8 @@ def _refresh_check_save(self) -> None: def _todo(self, applied: object, answers: object, problem: str) -> tuple[list, list[str]]: """Return the open questions and the other answers still missing before saving.""" - questions = list(missing_confirmations(applied, answers)) + counts = self._channel_counts() + questions = list(missing_confirmations(applied, answers, channel_counts=counts)) extra = [problem] if problem else [] left = masks_unanswered(answers, self._mask_types()) extra += [f"mask {m}: separate objects or regions? (Masks, step 10)" for m in left] @@ -243,7 +247,9 @@ def _todo(self, applied: object, answers: object, problem: str) -> tuple[list, l def _candidate_check(self, answers: object) -> CandidateCheck | None: """Build the candidate from the answers and check its exact bytes; None if refused.""" try: - candidate = description_from_draft(self._draft_base, answers) + candidate = description_from_draft( + self._draft_base, answers, channel_counts=self._channel_counts() + ) except DraftError as exc: self.todo.value = ui.status("error", "The answers cannot be saved.", [str(exc)]) return None @@ -253,7 +259,7 @@ def _candidate_check(self, answers: object) -> CandidateCheck | None: def _show_replicates(self, applied: object, answers: object, labels: list[str]) -> None: """One box per replicate: folders, pictures, channels, and its own question.""" boxes, toggles = [], {} - units = self._reopen_units() + units, counts = self._reopen_units(), self._channel_counts() for view in replicates(applied, answers): toggle = ui.ConfirmToggle( SAME_WAY, @@ -271,7 +277,7 @@ def _show_replicates(self, applied: object, answers: object, labels: list[str]) self._member_picture( (m.condition, m.source_repeat), self.replicates_detail ), - widgets.HTML(member_html(m, labels)), + widgets.HTML(member_html(m, labels, counts)), ] ) for m in view.members @@ -401,7 +407,24 @@ def save_experiment(self) -> bool: ["See Conditions and Replicates (steps 4 and 5)."], ) return False - self._save_description() + if not self._save_description(): + self.save_status.value = ui.status( + "error", "Not saved: your answers could not be saved first.", [self._save_problem] + ) + return False + changed = self._changed_images() + if changed: + self.save_status.value = ui.status( + "error", + f"Not saved: {plural(len(changed), 'image')} changed since the folder check.", + [ + "Check the folder again (step 1): the channels and pictures were of the " + "images as the check read them. Changed: " + + ", ".join(changed[:5]) + + (" …" if len(changed) > 5 else "") + ], + ) + return False try: publication = self._publish() except OutputLockedError: @@ -426,6 +449,12 @@ def save_experiment(self) -> bool: self._refresh_check_save() return True + def _changed_images(self) -> tuple[str, ...]: + """Return the images to analyse that are no longer as the folder check read them.""" + pre = self.preflight + analysed = [m.relative_path for m in pre.metadata.files] + return changed_since_check(Path(pre.discovery.root), pre.header_fingerprints, analysed) + def _publish(self) -> object: """Publish the saved draft with the removals the user confirmed.""" state, today = self._published(), date.today().isoformat() @@ -436,7 +465,9 @@ def _publish(self) -> object: for q in state.removals if q.condition_id in self._removals ) - candidate = description_from_draft(self._draft_base, self.draft_answers) + candidate = description_from_draft( + self._draft_base, self.draft_answers, channel_counts=self._channel_counts() + ) confirmations = reconfirmations( state.description, candidate, state.reconciliation, removals ).accepted @@ -444,6 +475,7 @@ def _publish(self) -> object: return publish_draft( self._design_folder(), self._draft_base, + channel_counts=self._channel_counts(), confirmations=confirmations, removals=removals, recorded_on=recorded, diff --git a/src/morphobricks_preprocessing/notebook_check_view.py b/src/morphobricks_preprocessing/notebook_check_view.py index 0fd5a9c0..8a088ff9 100644 --- a/src/morphobricks_preprocessing/notebook_check_view.py +++ b/src/morphobricks_preprocessing/notebook_check_view.py @@ -7,7 +7,7 @@ from __future__ import annotations import html -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from . import notebook_style as ui from .description_candidate import CandidateCheck @@ -15,6 +15,7 @@ from .description_removal import RemovalQuestion from .description_review import MemberView, ReplicateView, unit_text from .draft import OpenQuestion +from .draft_answers import UnitKey from .notebook_channels_view import ORANGE, RGB_COLOURS, blank_chip, chip, rgb_chip from .validation import ValidationIssue, plural @@ -31,6 +32,14 @@ "unit_channels_missing": "the channels of {subject} are not tagged (Tag channels)", "unit_channel_untagged": "{subject} is not tagged (Tag channels)", "channel_undeclared": "the label {subject} is used but not declared (Stains)", + "unit_image_type_unanswered": "the image type of {subject} is not answered (Image type)", + "unit_channels_stale": ( + "the tags of {subject} were saved for another image: tag its channels again (Tag channels)" + ), + "unit_channel_count_unknown": ( + "the channels of {subject} are not known from its image: confirm the axis roles " + "(Axes) and check that its first image can be read" + ), "control_unresolved": "say what {subject} is compared against", "legacy_folder_missing": "{subject} is gone: confirm the removal", } @@ -49,13 +58,27 @@ def _channels(channels: Sequence[str | None], labels: Sequence[str]) -> str: ) -def member_html(member: MemberView, labels: Sequence[str]) -> str: - """Return one folder of a replicate: where, how many images, and its channels.""" +def member_html( + member: MemberView, labels: Sequence[str], counts: Mapping[UnitKey, int] | None = None +) -> str: + """Return one folder of a replicate: where, how many images, and its channels. + + ``counts`` are the channel counts of each unit's image now (as *Check and save* gates the + save on them): tags saved for another count are not shown as the folder's channels, only + said to be stale, as the to-do list says (``unit_channels_stale``). + """ name = html.escape(f"{member.condition}/{member.source_repeat}") lines = [f"{name} {plural(member.image_count, 'image')}"] for unit in member.units: where = "" if unit.key[2] == "single" else f"{html.escape(unit.key[2])}: " - tags = _channels(unit.channels, labels) if unit.channels else blank_chip("not tagged") + now = None if counts is None else counts.get(unit.key) + if unit.channels and now is not None and now != len(unit.channels): + tags = ( + f"{len(unit.channels)} tags saved for another " + f"image (it has {now} channels now): tag its channels again" + ) + else: + tags = _channels(unit.channels, labels) if unit.channels else blank_chip("not tagged") lines.append(f"
{where}{tags}
") return "".join(lines) diff --git a/src/morphobricks_preprocessing/notebook_description.py b/src/morphobricks_preprocessing/notebook_description.py index 35ff6cd2..d6d65575 100644 --- a/src/morphobricks_preprocessing/notebook_description.py +++ b/src/morphobricks_preprocessing/notebook_description.py @@ -58,6 +58,13 @@ ) +class AnswerNotSaved(DraftError): + """An answer refused because it could not be saved; the message says why.""" + + def __init__(self, reason: str) -> None: + super().__init__(f"Not saved: {reason}.", ("not_saved",)) + + class _DescriptionCard(_NameRows, _SavedDraft): """Step 1's names and references cards: the user's answers to the draft description.""" @@ -73,6 +80,7 @@ def _build_description_cards(self) -> None: self._keep_old_draft = False self._refreshing = False self._refusal: DraftError | None = None + self._save_problem = "" self.id_inputs: dict[tuple[str, str], widgets.Text] = {} self.name_inputs: dict[tuple[str, str], widgets.Text] = {} self.name_hints: dict[tuple[str, str], widgets.HTML] = {} @@ -193,51 +201,73 @@ def _sync_description(self) -> None: def _answer(self, answers: DraftAnswers, *, rebuild: bool = True) -> bool: """Apply new answers to the draft and save them; False when they are refused. - A refused answer changes nothing and is shown by the card that asked for it. With - ``rebuild`` False the boxes are left as they are (a name being typed keeps its focus). + A refused answer changes nothing and is shown by the card that asked for it. An answer + that cannot be saved (the output folder's lock, the draft file, a write error) is + refused too: the answers before it are kept, so nothing is shown as answered that is + not on disk. With ``rebuild`` False the boxes are left as they are (a name being typed + keeps its focus). """ try: draft = apply_answers(self._draft_base, answers) except DraftError as exc: self._refusal = exc return False - self._refusal = None + before = self.draft, self.draft_answers self.draft, self.draft_answers = draft, answers - self._save_description() + saved = self._save_description() + if not saved: + self.draft, self.draft_answers = before + self._refusal = AnswerNotSaved(self._save_problem) + else: + self._refusal = None if rebuild: self._refresh_description() else: self._show_names_status() - return True + return saved + + def _save_description(self) -> bool: + """Save the answers to ``morphobricks.draft.json`` under the output folder's lock. - def _save_description(self) -> None: - """Save the answers to ``morphobricks.draft.json`` under the output folder's lock.""" + Returns + ------- + bool + False when the save was refused or failed (``_save_problem`` says why); True when + the draft was written, or when no output folder is set (the answers are then kept + while the notebook is open, and the page says they are not saved). + """ output = self._design_folder() - self._draft_saved = False + self._draft_saved, self._save_problem = False, "" if output is None: - return + return True if self._keep_old_draft and not self._keep_unused_draft(output): - return + self._save_problem = "the unused draft could not be kept" + return False with self.log.attached(): try: save_draft(output, self._draft_base, self.draft_answers) except OutputLockedError: self.folder_lock.show(output) + self._save_problem = "another run holds the lock of the output folder" self._names_refusal = ui.status( "error", "Not saved: another run holds the lock of the output folder.", ["Answer again once it is free."], ) - return + return False except (OutputInsideMasksError, DraftFileError, DraftError) as exc: + self._save_problem = str(exc) self._names_refusal = ui.status("error", "Not saved.", [str(exc)]) - return - except OSError: + return False + except OSError as exc: logging.getLogger(__name__).warning("Cannot save the draft", exc_info=True) - return + self._save_problem = f"the draft could not be written: {exc}" + self._names_refusal = ui.status("error", "Not saved.", [self._save_problem]) + return False finally: self._show_log() self._draft_saved = True + return True # The names card ---------------------------------------------------------------------- @@ -464,4 +494,4 @@ def _with_ids(answers: DraftAnswers, ids: dict[tuple[str, str], str]) -> DraftAn ) -__all__ = ["_DescriptionCard"] +__all__ = ["AnswerNotSaved", "_DescriptionCard"] diff --git a/src/morphobricks_preprocessing/notebook_folder.py b/src/morphobricks_preprocessing/notebook_folder.py index 2098ed7a..49fe5f6d 100644 --- a/src/morphobricks_preprocessing/notebook_folder.py +++ b/src/morphobricks_preprocessing/notebook_folder.py @@ -213,6 +213,9 @@ def check_folder(self) -> Preflight | None: exclude=self._excluded(), masks_folder=masks, mask_axes=mask_axes, + # Each header is bound to the file's fingerprint: every picture and the + # save are of the images as this check read them (audit batch 33). + bind_headers=True, ) if self._describe: pre = unit_checked(pre) # a described run checks inside each unit diff --git a/src/morphobricks_preprocessing/notebook_folder_previews.py b/src/morphobricks_preprocessing/notebook_folder_previews.py index f4a09ec6..a9f9e404 100644 --- a/src/morphobricks_preprocessing/notebook_folder_previews.py +++ b/src/morphobricks_preprocessing/notebook_folder_previews.py @@ -19,6 +19,7 @@ from . import notebook_style as ui from .discovery import LeafFolder +from .fingerprint import Fingerprint from .header_preview import HEADER_NOTE, HeaderPreview, header_preview from .metadata import TiffMetadata from .notebook_render import label_cells @@ -86,6 +87,7 @@ def __init__(self) -> None: self._leaves: dict[str, LeafFolder] = {} self._root: Path | None = None self._headers: Mapping[str, TiffMetadata | None] = {} + self._bound: Mapping[str, Fingerprint | None] = {} self.widget = widgets.VBox([self.rows, self.more, self.detail]) def clear(self) -> None: @@ -99,6 +101,7 @@ def show(self, pre: Preflight) -> None: """Draw the table of a new check: labels, image count, folder and picture per folder.""" self.clear() self._root, self._headers = Path(pre.discovery.root), first_headers(pre) + self._bound = pre.header_fingerprints # drawn only as the check read them header = widgets.HTML(_cells([f"{h}" for h in HEADERS])) top = widgets.GridBox([header, widgets.HTML("Preview")], layout=self._grid()) leaves = pre.discovery.leaves @@ -136,7 +139,7 @@ def draw(self, folder: str) -> None: """Draw one folder's picture into its row (read once).""" if folder in self.tiles: return - found = header_preview(self._root, self._headers[folder]) + found = header_preview(self._root, self._headers[folder], bound=self._bound) self.found[folder] = found leaf = self._leaves[folder] tile = ThumbTile( @@ -162,7 +165,9 @@ def draw_all(self) -> None: def enlarge(self, leaf: LeafFolder) -> None: """Show one folder's picture at the detail size under the table (read again).""" - found = header_preview(self._root, self._headers[leaf.relative_path], DETAIL_SIDE) + found = header_preview( + self._root, self._headers[leaf.relative_path], DETAIL_SIDE, bound=self._bound + ) folder = "experiment folder" if leaf.relative_path == "." else leaf.relative_path title = f"{html.escape(folder)} · {html.escape(_caption(found))}" show_detail(self.detail, title, found.pictures, found.problem) diff --git a/src/morphobricks_preprocessing/notebook_masks_screen.py b/src/morphobricks_preprocessing/notebook_masks_screen.py index c51c878a..467de41a 100644 --- a/src/morphobricks_preprocessing/notebook_masks_screen.py +++ b/src/morphobricks_preprocessing/notebook_masks_screen.py @@ -14,6 +14,7 @@ import html from collections.abc import Callable, Sequence from dataclasses import replace +from pathlib import Path import ipywidgets as widgets @@ -21,6 +22,7 @@ from .axis_review import review_axes from .channel_answers import label_ids from .channel_units import unit_images +from .fingerprint import changed_since_check, current_fingerprint from .mask_answers import ( drawn_on, dropped_channels, @@ -31,10 +33,11 @@ without_mask, ) from .mask_evidence import MaskEvidence, mask_evidence, representative +from .masks import MaskFile from .notebook_channels import _NotebookStore from .notebook_channels_view import ORANGE, AnswerStore from .notebook_mask_question import MaskQuestion -from .preview import PreviewCache +from .preview import PreviewCache, StripOptions from .validation import plural INTRO = ( @@ -236,6 +239,8 @@ def _mask_evidence(self, source_type: str) -> MaskEvidence: first = representative(pre.masks, source_type, analysed) unit = next((k for k, found in images.items() if first and first.image in found), None) channel = drawn_on(self.draft_answers, source_type, unit) + if first is not None and _changed(pre, first): # never the evidence of another revision + self._mask_found.pop((source_type, channel), None) if (source_type, channel) not in self._mask_found: review = review_axes( pre.metadata, @@ -249,8 +254,8 @@ def _mask_evidence(self, source_type: str) -> MaskEvidence: analysed, review, channel, - self._mask_cache, - self._memory_limit(), + StripOptions(cache=self._mask_cache, memory_limit=self._memory_limit()), + pre.header_fingerprints, ) found = self._mask_found[(source_type, channel)] if self._colour_unit(unit): @@ -284,4 +289,17 @@ def _masks_answered(self) -> None: self._refresh_check_save() +def _changed(pre: object, mask: MaskFile) -> bool: + """Return True when the image or the bound mask is no longer as the check read it. + + Evidence kept from an earlier read is then read again (and refused: "check the folder + again"), never shown as the files checked. + """ + root = Path(pre.discovery.root) + if changed_since_check(root, pre.header_fingerprints, [mask.image]): + return True + path = mask.location(pre.masks.folder, root) + return mask.bound is not None and current_fingerprint(path) != mask.bound + + __all__ = ["INTRO", "MasksCard", "_MasksScreen"] diff --git a/src/morphobricks_preprocessing/preview.py b/src/morphobricks_preprocessing/preview.py index dcaf236f..13fe7580 100644 --- a/src/morphobricks_preprocessing/preview.py +++ b/src/morphobricks_preprocessing/preview.py @@ -24,7 +24,7 @@ import math import os from collections import OrderedDict -from collections.abc import Hashable, Sequence +from collections.abc import Hashable, Mapping, Sequence from dataclasses import dataclass import numpy as np @@ -223,15 +223,58 @@ def confirmed_mapping(review: AxisReview | None, relative_path: str) -> AxisMapp return mapping -def _check_fingerprint(path: str | os.PathLike, expected: Fingerprint | None) -> Fingerprint: +def _check_fingerprint( + path: str | os.PathLike, expected: Fingerprint | None, shown: str | None = None +) -> Fingerprint: now = Fingerprint.of(path) if expected is not None and now != expected: + name = os.fspath(path) if shown is None else shown raise PreviewRefused( - FILE_CHANGED, f"{os.fspath(path)} changed while it was shown: check the folder again." + FILE_CHANGED, f"{name} changed since the folder check read it: check the folder again." ) return now +def check_unchanged( + path: str | os.PathLike, expected: Fingerprint, shown: str | None = None +) -> None: + """Refuse when the file is not, now, the revision whose header was read (``expected``). + + ``shown`` is the name the refusal gives the file (its path relative to the experiment); + the full path otherwise. + + Raises + ------ + PreviewRefused + ``file_changed_while_shown``. + OSError + When the file is gone. + """ + _check_fingerprint(path, expected, shown) + + +def checked_fingerprint(bound: Mapping[str, Fingerprint | None], relative_path: str) -> Fingerprint: + """Return the fingerprint the folder check bound to an image's header, or refuse. + + A picture is evidence about the image as the check read its header. An image that the + check did not bind (it changed while its header was read, or it was not fingerprinted) has + no such revision, so no picture is drawn for it: the user checks the folder again. + + Raises + ------ + PreviewRefused + ``file_changed_while_shown``. + """ + found = bound.get(relative_path) + if found is None: + raise PreviewRefused( + FILE_CHANGED, + f"{relative_path} is not bound to the header the folder check read (it changed " + "while it was read): check the folder again.", + ) + return found + + def _label(c: int, names: Sequence[str | None] | None) -> str: name = names[c] if names is not None and c < len(names) else None return f"C{c}" if not name else f"C{c} '{name}'" @@ -390,6 +433,8 @@ def labels_thumbnail(labels: np.ndarray, image: ReducedPlane) -> ReducedPlane: "StripOptions", "ReducedPlane", "channel_strip", + "check_unchanged", + "checked_fingerprint", "confirmed_mapping", "labels_thumbnail", "mask_thumbnail", diff --git a/src/morphobricks_preprocessing/run/description_file.py b/src/morphobricks_preprocessing/run/description_file.py index 47610bf0..87361734 100644 --- a/src/morphobricks_preprocessing/run/description_file.py +++ b/src/morphobricks_preprocessing/run/description_file.py @@ -167,18 +167,35 @@ def _replace_file_durably(path: Path, data: bytes) -> None: raise +def _candidate_records(folder: Path, records: tuple[dict, ...]) -> bytes: + """Return the records file's bytes with ``records`` added, checked as the reader checks them. + + Raises + ------ + RecordsFileError + ``R_RECORDS_FILE_INVALID`` when the existing file (a symbolic link, a torn or foreign + file) or the complete candidate bytes would be refused by the reader: a new record with + a wrong key set, a bad date or a malformed SHA-256 is never written. + """ + read_records(folder) # refuses a symbolic link and a torn or foreign file before writing + current = read_file_bytes(folder / RECORDS_FILENAME) or b"" + data = current + b"".join(encode_record(r) for r in records) + _, issues = check_records(data) + if issues: + raise RecordsFileError(issues) + return data + + def _append_records(folder: Path, records: tuple[dict, ...]) -> None: """Add the records as lines after the existing ones, in one atomic replace of the file. - The live file is replaced only once the old bytes and the new lines are all on disk in a - temporary file, so a failure leaves it exactly as it was. + The complete new bytes are checked first (:func:`_candidate_records`); the live file is + replaced only once the old bytes and the new lines are all on disk in a temporary file, so a + refusal or a failure leaves it exactly as it was. """ if not records: return - read_records(folder) # refuses a symbolic link and a torn or foreign file before writing - path = folder / RECORDS_FILENAME - current = read_file_bytes(path) or b"" - _replace_file_durably(path, current + b"".join(encode_record(r) for r in records)) + _replace_file_durably(folder / RECORDS_FILENAME, _candidate_records(folder, records)) def _compare( @@ -297,6 +314,8 @@ def publish_locked( records += image_count_records(old, candidate, recorded_on, confirmations) elif removals: raise ValueError("a removal needs a published file to remove from") + if records: + _candidate_records(folder, records) # a record the reader refuses: nothing is written write_text_atomic(path, data.decode("utf-8")) try: _append_records(folder, records) diff --git a/src/morphobricks_preprocessing/run/description_run.py b/src/morphobricks_preprocessing/run/description_run.py index 23708c11..e51a8aa2 100644 --- a/src/morphobricks_preprocessing/run/description_run.py +++ b/src/morphobricks_preprocessing/run/description_run.py @@ -71,7 +71,11 @@ def load_description(output: Path) -> tuple[RunDescription | None, list[Validati records = read_records(output) except DescriptionError as exc: return None, [reader_issue(item) for item in exc.issues] - except (RecordsFileError, OSError) as exc: + except RecordsFileError as exc: # the Spec code and its E_RECORDS_* reasons, kept + reasons = ", ".join(sorted(exc.reasons)) + message = f"The records file of the experiment description is refused ({reasons}): {exc}." + return None, [error(exc.code, message, (), "Fix or remove the records file.")] + except OSError as exc: message = f"The experiment description or its records file cannot be read: {exc}." return None, [error("description_unreadable", message, (), "Fix or remove the file.")] return RunDescription(description, sha256_hex(data), records), [] diff --git a/src/morphobricks_preprocessing/run/draft_file.py b/src/morphobricks_preprocessing/run/draft_file.py index d9c7bcda..0a61fb89 100644 --- a/src/morphobricks_preprocessing/run/draft_file.py +++ b/src/morphobricks_preprocessing/run/draft_file.py @@ -28,6 +28,7 @@ from __future__ import annotations import os +from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path @@ -39,7 +40,7 @@ from ..design import DESIGN_FILENAME from ..design_derive import write_derived_design from ..draft import Draft -from ..draft_answers import DraftAnswers, apply_answers, description_from_draft +from ..draft_answers import DraftAnswers, UnitKey, apply_answers, description_from_draft from ..draft_format import ( ABSENT, SPEC_COMMIT, @@ -204,6 +205,7 @@ def publish_draft( output_folder: str | os.PathLike, draft: Draft, *, + channel_counts: Mapping[UnitKey, int], confirmations: tuple[Reconfirmation, ...] = (), removals: tuple[Removal, ...] = (), recorded_on: str | None = None, @@ -218,6 +220,12 @@ def publish_draft( ``removals`` and ``recorded_on`` are the user's, as :func:`.description_file.publish_locked` takes them (a removed folder, a changed image count). + ``channel_counts`` are the channel counts of each unit's image now, from its header read + through the axis roles the user confirmed (one entry per unit). It is required: a unit + whose saved tags are not one per channel, or that has no count, is refused + (:func:`.draft_answers.missing_confirmations`), so tags saved for another image are never + published. + Raises ------ OutputLockedError @@ -232,7 +240,7 @@ def publish_draft( folder = Path(output_folder) with output_lock(folder): resumed = _resume(folder, draft) - candidate = description_from_draft(draft, resumed.answers) + candidate = description_from_draft(draft, resumed.answers, channel_counts=channel_counts) publication = publish_locked( folder, candidate, diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py index b9d8ccdf..fdfe65ab 100644 --- a/src/morphobricks_preprocessing/run/preflight.py +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -303,7 +303,10 @@ def remasked( The notebook calls it when the user changes the masks folder or states the axes of a mask type, so the masks follow without checking the folder again (the answers given - so far are kept). Only the masks' headers are read. + so far are kept). Only the masks' headers are read. A preflight whose headers were bound + (``bind_headers``: :attr:`Preflight.header_fingerprints` is not empty) binds the masks + paired again the same way (:attr:`.masks.MaskFile.bound`), so their evidence is of the + masks as this pairing read them. Parameters ---------- @@ -327,6 +330,7 @@ def remasked( masks_folder, mask_axes, Path(output_folder), + bind=bool(pre.header_fingerprints), with_description=with_description, ) return replace(pre, masks=masks) diff --git a/src/morphobricks_preprocessing/testing_described.py b/src/morphobricks_preprocessing/testing_described.py index 57272da7..6f00da79 100644 --- a/src/morphobricks_preprocessing/testing_described.py +++ b/src/morphobricks_preprocessing/testing_described.py @@ -92,27 +92,35 @@ def _units(description: Description) -> Iterator[tuple[Path, Unit, int]]: A member's ``image_count`` is its total over its technical repeats, so it is spread over them as evenly as possible: every unit gets ``image_count // n`` images and the - first ``image_count % n`` units, by sorted unit ID, one more. + first ``image_count % n`` units, by sorted unit ID, one more. Every member is checked + before the first unit is yielded, so a refused description writes nothing. Raises ------ ValueError - If a member has fewer images than technical repeats (a unit would be empty). + If a member has no technical repeat, or fewer images than technical repeats (a unit + would be empty; a negative count included). """ - for repeat_id, repeat in description.repeats.items(): - for condition_id, member in repeat.members.items(): - unit_ids = sorted(member.technical_repeats) - base, extra = divmod(member.image_count, len(unit_ids)) - if base == 0: - raise ValueError( - f"member {condition_id!r} of repeat {repeat_id!r} has image_count " - f"{member.image_count} for {len(unit_ids)} technical repeats; every " - "technical repeat needs at least one image" - ) - for position, unit_id in enumerate(unit_ids): - unit = member.technical_repeats[unit_id] - folder = unit_folder(description, repeat_id, condition_id, unit) - yield folder, unit, base + (1 if position < extra else 0) + members = [ + (repeat_id, condition_id, member) + for repeat_id, repeat in description.repeats.items() + for condition_id, member in repeat.members.items() + ] + for repeat_id, condition_id, member in members: + units = len(member.technical_repeats) + if units == 0 or member.image_count < units: + raise ValueError( + f"member {condition_id!r} of repeat {repeat_id!r} has image_count " + f"{member.image_count} for {units} technical repeats; it needs at least one " + "technical repeat, and every technical repeat at least one image" + ) + for repeat_id, condition_id, member in members: + unit_ids = sorted(member.technical_repeats) + base, extra = divmod(member.image_count, len(unit_ids)) + for position, unit_id in enumerate(unit_ids): + unit = member.technical_repeats[unit_id] + folder = unit_folder(description, repeat_id, condition_id, unit) + yield folder, unit, base + (1 if position < extra else 0) def _mapping(description: Description) -> AxisMapping: @@ -207,7 +215,8 @@ def make_described_run( ------ ValueError For an unknown mask ID, a description that mixes rgb and fluorescence units, or a - member whose ``image_count`` is below its number of technical repeats. + member with no technical repeat or whose ``image_count`` is below its number of + technical repeats (nothing is written then). RuntimeError If the run or the mask step does not write its outputs (its issues are in the message). """ diff --git a/src/morphobricks_preprocessing/unit_consistency.py b/src/morphobricks_preprocessing/unit_consistency.py index e2c401e9..d8bacbf6 100644 --- a/src/morphobricks_preprocessing/unit_consistency.py +++ b/src/morphobricks_preprocessing/unit_consistency.py @@ -16,6 +16,9 @@ the axis review's (``inconsistent_mapping``). An experiment whose units all declare one image type gets the same pixel-type refusal as before. +The invariant is that a previously blocking input stays blocked; the diagnostic codes may change +(the per-type checks and the Spec's ``R_*`` codes replace the global ``mixed_dtypes`` and +``inconsistent_dimensions``). """ from __future__ import annotations diff --git a/tests/draft_file_support.py b/tests/draft_file_support.py index fcad27d3..0f5ae75d 100644 --- a/tests/draft_file_support.py +++ b/tests/draft_file_support.py @@ -19,6 +19,8 @@ DAY = "2026-11-06" PATHS = [f"{c}/{r}/{n}.tif" for c in ("CTRL", "DrugA") for r in ("rep1", "rep2") for n in "ab"] UNITS = [(c, r, "single") for c in ("CTRL", "DrugA") for r in ("rep1", "rep2")] +COUNTS = dict.fromkeys(UNITS, 2) +"""The channel count of every unit's image, as the confirmed axis roles read it (2 here).""" def complete(base=ABSENT): @@ -35,7 +37,7 @@ def complete(base=ABSENT): "rep1": RepeatAnswer(id="rep1", confirmed_on=DAY), "rep2": RepeatAnswer(id="rep2", confirmed_on="2026-11-05", display_name="Second"), }, - units={key: UnitAnswer(("DNA", "ACTIN")) for key in UNITS}, + units={key: UnitAnswer(("DNA", "ACTIN"), "channels") for key in UNITS}, channels={ "DNA": ChannelAnswer("DAPI", ("DAPI",)), "ACTIN": ChannelAnswer(None, None), diff --git a/tests/test_batch33_gate.py b/tests/test_batch33_gate.py new file mode 100644 index 00000000..4f2bde3f --- /dev/null +++ b/tests/test_batch33_gate.py @@ -0,0 +1,166 @@ +"""Audit batches 33 and 34a, the publication gate: nothing unanswered, stale or doubled is saved. + +- B2: a unit whose image type is not answered (None) is an open question + (``unit_image_type_unanswered``) and is never published as an ordinary channel image, although + the published file writes "channels" as an absent ``image_type`` (experiment-v1 §4.4b). +- B3: ``publish_draft`` requires the channel count of each unit's image now; tags saved for + another count (``unit_channels_stale``) or a unit without a count are refused. +- B1: a unit tagging one label at two positions is refused before it is saved + (``E_DUPLICATE_CHANNEL_IN_UNIT``). +- FILE 1: ``make_described_run`` refuses a member with fewer images than technical repeats + (negative included) or none, and writes nothing. +- 34a item 5: only the exact tokens R, G, B make a colour unit count for "every folder is colour". +""" + +from dataclasses import replace + +import pytest + +from draft_file_support import COUNTS, UNITS, complete, snapshot +from morphobricks_preprocessing.channel_answers import all_colour +from morphobricks_preprocessing.channel_labels import UnitLayout +from morphobricks_preprocessing.description import Description, description_from_bytes +from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, DRAFT_FILENAME +from morphobricks_preprocessing.draft import DraftError, draft_from_discovery +from morphobricks_preprocessing.draft_answers import ( + DraftIncomplete, + UnitAnswer, + apply_answers, + description_from_draft, + missing_confirmations, +) +from morphobricks_preprocessing.draft_format import answers_from_bytes, answers_to_bytes +from morphobricks_preprocessing.run.draft_file import publish_draft, save_draft +from morphobricks_preprocessing.testing import make_described_run +from spec_pin import fixtures +from test_draft_file import exp, out # noqa: F401 (the same folders) + +VALID = {item.name.split("_")[0]: item for item in fixtures("valid")} +KEY = UNITS[0] + + +def _with_unit(answers, unit): + return replace(answers, units={**answers.units, KEY: unit}) + + +def _codes(draft, answers, counts=COUNTS): + applied = apply_answers(draft, answers) + return {q.code for q in missing_confirmations(applied, answers, channel_counts=counts)} + + +# --- B2 ----------------------------------------------------------------------------------------- + + +def test_an_unanswered_image_type_is_an_open_question(exp): # noqa: F811 + draft = draft_from_discovery(exp) + unanswered = _with_unit(complete(), UnitAnswer(("DNA", "ACTIN"), None)) + questions = missing_confirmations(apply_answers(draft, unanswered), unanswered) + assert [(q.code, q.subject) for q in questions] == [ + ("unit_image_type_unanswered", " / ".join(KEY)) + ] + with pytest.raises(DraftIncomplete) as refusal: + description_from_draft(draft, unanswered, channel_counts=COUNTS) + assert refusal.value.codes == ("unit_image_type_unanswered",) + + +def test_only_an_explicit_type_answers_it_and_channels_is_published_absent(exp): # noqa: F811 + draft = draft_from_discovery(exp) + assert not _codes(draft, complete()) # "channels" answers it + described = description_from_draft(draft, complete(), channel_counts=COUNTS) + members = [m for r in described.repeats.values() for m in r.members.values()] + assert all(u.image_type is None for m in members for u in m.technical_repeats.values()) + + +def test_publish_refuses_an_unanswered_type_and_writes_nothing(exp, out): # noqa: F811 + draft = draft_from_discovery(exp) + save_draft(out, draft, _with_unit(complete(), UnitAnswer(("DNA", "ACTIN"), None))) + before = snapshot(out) + with pytest.raises(DraftIncomplete) as refusal: + publish_draft(out, draft, channel_counts=COUNTS) + assert refusal.value.codes == ("unit_image_type_unanswered",) + assert snapshot(out) == before and not (out / DESCRIPTION_FILENAME).exists() + + +# --- B3 ----------------------------------------------------------------------------------------- + + +def test_tags_saved_for_another_channel_count_are_refused_until_retagged(exp, out): # noqa: F811 + draft = draft_from_discovery(exp) + save_draft(out, draft, complete()) # two tags per unit + before = snapshot(out) + now = {**COUNTS, KEY: 3} # the image at the same path now has three channels + for counts, code in ((now, "unit_channels_stale"), ({}, "unit_channel_count_unknown")): + with pytest.raises(DraftIncomplete) as refusal: + publish_draft(out, draft, channel_counts=counts) + assert code in refusal.value.codes + assert snapshot(out) == before # nothing written, the draft kept + with pytest.raises(TypeError): + publish_draft(out, draft) # the current counts are required + retagged = _with_unit(complete(), UnitAnswer(("DNA", "ACTIN", "DNA2"), "channels")) + channels = {**retagged.channels, "DNA2": retagged.channels["ACTIN"]} + save_draft(out, draft, replace(retagged, channels=channels)) + assert publish_draft(out, draft, channel_counts=now).path.exists() + + +# --- B1 ----------------------------------------------------------------------------------------- + + +def test_a_label_at_two_positions_of_a_unit_is_never_saved(exp, out): # noqa: F811 + draft = draft_from_discovery(exp) + doubled = _with_unit(complete(), UnitAnswer(("DNA", "DNA"), "channels")) + with pytest.raises(DraftError) as refusal: + apply_answers(draft, doubled) + assert refusal.value.codes == ("E_DUPLICATE_CHANNEL_IN_UNIT",) + with pytest.raises(DraftError): + save_draft(out, draft, doubled) + assert not (out / DRAFT_FILENAME).exists() + + +# --- FILE 1 ------------------------------------------------------------------------------------- + + +def _member_count(description: Description, count: int, units: dict | None = None): + repeat_id = next(iter(description.repeats)) + repeat = description.repeats[repeat_id] + cid = next(iter(repeat.members)) + member = repeat.members[cid] + member = replace( + member, + image_count=count, + technical_repeats=member.technical_repeats if units is None else units, + ) + repeats = { + **description.repeats, + repeat_id: replace(repeat, members={**repeat.members, cid: member}), + } + return replace(description, repeats=repeats) + + +@pytest.mark.parametrize(("count", "units"), [(-1, None), (-3, None), (0, None), (2, {})]) +def test_make_described_run_refuses_impossible_counts_and_writes_nothing(tmp_path, count, units): + description = _member_count(description_from_bytes(VALID["v01"].data), count, units) + with pytest.raises(ValueError, match="technical repeat"): + make_described_run(tmp_path / "run", description) + assert not (tmp_path / "run").exists() or not any((tmp_path / "run").rglob("*")) + + +# --- batch 34a, item 5 ----------------------------------------------------------------------- + + +def test_only_the_exact_colour_tokens_make_every_folder_colour(): + layout = UnitLayout(KEY, "a.tif", 3, (None, None, None), rgb=True) + assert all_colour([layout], {KEY: UnitAnswer(("R", "G", "B"), "rgb")}) + for tags in (("G", "R", "B"), ("R", "G", "G"), ("DNA", "G", "B"), (None, None, None)): + assert not all_colour([layout], {KEY: UnitAnswer(tags, "rgb")}) + + +def test_a_malformed_resumed_draft_does_not_make_the_stains_not_applicable(): + answers = _with_unit(complete(), UnitAnswer(("R", "G", "B"), "rgb")) + raw = b'"R",\n "G",\n "B"' + doc = answers_to_bytes(answers).replace( + raw, raw.replace(b'"R"', b'"X"').replace(b'"B"', b'"R"').replace(b'"X"', b'"B"') + ) + resumed = answers_from_bytes(doc) # the file format reads it; the tokens are wrong + assert resumed.units[KEY].channels == ("B", "G", "R") + layout = UnitLayout(KEY, "a.tif", 3, (None, None, None), rgb=True) + assert not all_colour([layout], {KEY: resumed.units[KEY]}) diff --git a/tests/test_described_unit_checks.py b/tests/test_described_unit_checks.py index 626c7049..203c08b1 100644 --- a/tests/test_described_unit_checks.py +++ b/tests/test_described_unit_checks.py @@ -5,6 +5,10 @@ changes is which refusal the user meets: not ``mixed_dtypes`` and ``inconsistent_dimensions`` from comparing an RGB image with a fluorescence one, but the Spec's own. A unit mixed inside itself and an undescribed (v0.4) run are refused as before. + +The invariant is that a previously blocking input stays blocked; the diagnostic codes may change +(the per-type checks and the Spec's codes replace the global ``mixed_dtypes`` and +``inconsistent_dimensions``). It is not that the set of refusal codes is unchanged. """ from pathlib import Path @@ -187,6 +191,7 @@ def test_units_of_one_layout_that_disagree_are_still_refused_by_the_run( for image in unit: _rewrite(root / image, data, axes) pre = unit_checked(preflight(root, out)) + assert pre.ok, [i.code for i in pre.issues if i.is_error] # only the run can refuse it assert not {"inconsistent_dimensions", "different_channel_counts"} & codes(pre.issues) result = run_experiment(root, prefill_mapping(mapping), output_folder=out) assert not result.written diff --git a/tests/test_description_file.py b/tests/test_description_file.py index 8b97ef75..1c50bc78 100644 --- a/tests/test_description_file.py +++ b/tests/test_description_file.py @@ -388,3 +388,46 @@ def test_the_records_are_written_with_the_same_hashes_as_the_files(tmp_path): assert {r["new_sha256"] for r in read_records(tmp_path)} == {done.sha256} assert canonical_bytes(V01) != (tmp_path / DESCRIPTION_FILENAME).read_bytes() assert os.path.getsize(tmp_path / RECORDS_FILENAME) > 0 + + +# --- batch 34a: a record the reader would refuse is never written ------------------------------ + +GOOD = { + "type": "reconfirmation", + "id_kind": "repeats", + "old_id": "rep1", + "new_id": "Day1", + "confirmed_on": DAY, + "old_sha256": "absent", + "new_sha256": "0" * 64, +} +BAD_RECORDS = { + "missing_keys": {"type": "reconfirmation"}, + "bad_date": {**GOOD, "confirmed_on": "2026-02-30"}, + "malformed_sha": {**GOOD, "new_sha256": "ABC"}, +} + + +@pytest.mark.parametrize("name", sorted(BAD_RECORDS)) +def test_a_new_record_the_reader_refuses_is_not_appended(tmp_path, name): + with pytest.raises(RecordsFileError) as refusal: + publication._append_records(tmp_path, (BAD_RECORDS[name],)) + assert refusal.value.code == "R_RECORDS_FILE_INVALID" + assert not (tmp_path / RECORDS_FILENAME).exists() + + +@pytest.mark.parametrize("name", sorted(BAD_RECORDS)) +def test_a_publication_with_a_refused_record_writes_neither_file(tmp_path, monkeypatch, name): + base = _published(tmp_path) + publication._append_records(tmp_path, (GOOD,)) # a records file that must stay as it is + before = _snapshot(tmp_path) + monkeypatch.setattr(publication, "_record", lambda *_: BAD_RECORDS[name]) + with pytest.raises(RecordsFileError) as refusal: + publish_description( + tmp_path, + description_from_bytes(BY_NAME["t01"].data), + base_sha256=base, + confirmations=RENAMES, + ) + assert refusal.value.code == "R_RECORDS_FILE_INVALID" + assert _snapshot(tmp_path) == before and not (tmp_path / LOCK_FILENAME).exists() diff --git a/tests/test_draft_answers.py b/tests/test_draft_answers.py index 0788c7ae..588f1509 100644 --- a/tests/test_draft_answers.py +++ b/tests/test_draft_answers.py @@ -73,7 +73,7 @@ def test_nothing_is_confirmed_for_the_user_and_every_missing_item_is_listed(exp) def test_a_partly_answered_draft_lists_only_what_is_missing(exp): answers = complete() units = dict(answers.units) - units[("DrugA", "rep2", "single")] = UnitAnswer(("DNA", "GFP")) # GFP is not declared + units[("DrugA", "rep2", "single")] = UnitAnswer(("DNA", "GFP"), "channels") # not declared units.pop(("CTRL", "rep1", "single")) repeats = dict(answers.repeats) repeats["rep2"] = RepeatAnswer(id="rep2") # no confirmation day diff --git a/tests/test_draft_file.py b/tests/test_draft_file.py index ca427433..28c129ea 100644 --- a/tests/test_draft_file.py +++ b/tests/test_draft_file.py @@ -5,6 +5,7 @@ import pytest from draft_file_support import ( + COUNTS, DAY, PATHS, REFUSED, @@ -101,7 +102,7 @@ def test_a_draft_file_that_is_not_exactly_the_format_is_refused_never_repaired(n load_draft(out, draft_from_discovery(exp)) assert on_disk.value.code == code with pytest.raises(DraftFileError): - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert (out / DRAFT_FILENAME).read_bytes() == data # not repaired, not deleted assert not (out / DESCRIPTION_FILENAME).exists() @@ -135,7 +136,7 @@ def test_resume_is_refused_when_the_published_file_changed_since_the_base(exp, o load_draft(out, base) assert refusal.value.codes == {Refusal.R_PUBLISHED_FILE_CHANGED} with pytest.raises(PublicationRefused): - publish_draft(out, base) + publish_draft(out, base, channel_counts=COUNTS) assert snapshot(out) == kept @@ -260,7 +261,7 @@ def test_an_incomplete_draft_can_be_saved_after_each_answer_and_resumed(exp, out def test_the_date_is_the_users_never_the_clock(exp, out): save_draft(out, draft_from_discovery(exp), complete()) - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) text = (out / DESCRIPTION_FILENAME).read_text(encoding="utf-8") assert date.today().isoformat() not in text assert f'confirmed_on = "{DAY}"' in text @@ -272,7 +273,7 @@ def test_the_date_is_the_users_never_the_clock(exp, out): def test_a_complete_draft_is_published_and_the_draft_is_deleted(exp, out): save_draft(out, draft_from_discovery(exp), complete()) - publication = publish_draft(out, draft_from_discovery(exp)) + publication = publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert publication.base_sha256 == ABSENT # draft and lock gone; the derived experiment_design.csv is written only here (v1 11) assert sorted(p.name for p in out.iterdir()) == [DESIGN_FILENAME, DESCRIPTION_FILENAME] @@ -289,7 +290,7 @@ def test_a_draft_based_on_a_published_file_publishes_over_it(exp, out): first = publish_description(out, description, base_sha256=ABSENT) answers = complete(first.sha256) save_draft(out, draft_from_discovery(exp), answers) - publication = publish_draft(out, draft_from_discovery(exp)) + publication = publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert publication.base_sha256 == first.sha256 assert not (out / DRAFT_FILENAME).exists() @@ -309,7 +310,9 @@ def test_an_incomplete_draft_is_refused_and_nothing_is_written(exp, out): out, draft_from_discovery(exp), DraftAnswers(ABSENT, SPEC_COMMIT, answers.conditions) ) refusal = refused_leaves_everything( - out, lambda: publish_draft(out, draft_from_discovery(exp)), DraftIncomplete + out, + lambda: publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS), + DraftIncomplete, ) assert ("unit_channels_missing", "CTRL / rep1 / single") in { (q.code, q.subject) for q in refusal.questions @@ -324,7 +327,9 @@ def test_an_incomplete_draft_leaves_an_earlier_published_file_alone(exp, out): ) save_draft(out, draft_from_discovery(exp), DraftAnswers(first.sha256, SPEC_COMMIT)) refused_leaves_everything( - out, lambda: publish_draft(out, draft_from_discovery(exp)), DraftIncomplete + out, + lambda: publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS), + DraftIncomplete, ) assert sha256_hex((out / DESCRIPTION_FILENAME).read_bytes()) == first.sha256 @@ -332,7 +337,7 @@ def test_an_incomplete_draft_leaves_an_earlier_published_file_alone(exp, out): def test_a_description_the_strict_reader_refuses_writes_nothing(exp, out): answers = complete() channels = {**answers.channels, "bad id": ChannelAnswer()} - units = {key: UnitAnswer(("bad id", "ACTIN")) for key in UNITS} + units = {key: UnitAnswer(("bad id", "ACTIN"), "channels") for key in UNITS} bad = DraftAnswers( ABSENT, SPEC_COMMIT, @@ -347,7 +352,9 @@ def test_a_description_the_strict_reader_refuses_writes_nothing(exp, out): from morphobricks_preprocessing.description import DescriptionError refusal = refused_leaves_everything( - out, lambda: publish_draft(out, draft_from_discovery(exp)), DescriptionError + out, + lambda: publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS), + DescriptionError, ) assert Code.E_ID_SYNTAX in {issue.code for issue in refusal.issues} @@ -356,7 +363,9 @@ def test_a_changed_published_file_refuses_publication_and_keeps_the_draft(exp, o save_draft(out, draft_from_discovery(exp), complete()) (out / DESCRIPTION_FILENAME).write_text("changed by someone else") refusal = refused_leaves_everything( - out, lambda: publish_draft(out, draft_from_discovery(exp)), PublicationRefused + out, + lambda: publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS), + PublicationRefused, ) assert refusal.codes == {Refusal.R_PUBLISHED_FILE_CHANGED} @@ -365,12 +374,12 @@ def test_a_held_lock_refuses_publication_and_keeps_the_draft(exp, out): save_draft(out, draft_from_discovery(exp), complete()) before = snapshot(out) with output_lock(out), pytest.raises(OutputLockedError): - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert snapshot(out) == before def test_publication_without_a_draft_is_refused(exp, out): with pytest.raises(DraftFileError) as refusal: - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert refusal.value.code == "no_draft" assert list(out.iterdir()) == [] diff --git a/tests/test_draft_spec_commit.py b/tests/test_draft_spec_commit.py index 318adf17..f2d57e3e 100644 --- a/tests/test_draft_spec_commit.py +++ b/tests/test_draft_spec_commit.py @@ -5,7 +5,7 @@ import pytest -from draft_file_support import PATHS, complete, snapshot +from draft_file_support import COUNTS, PATHS, complete, snapshot from morphobricks_preprocessing import discover_experiment from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, DRAFT_FILENAME from morphobricks_preprocessing.draft import draft_from_discovery @@ -63,7 +63,7 @@ def test_publish_refuses_and_leaves_an_absent_description_absent(exp, out): stale(out, exp) before = snapshot(out) with pytest.raises(DraftFileError) as refusal: - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert refusal.value.code == "spec_commit_mismatch" assert not (out / DESCRIPTION_FILENAME).exists() assert snapshot(out) == before @@ -71,7 +71,7 @@ def test_publish_refuses_and_leaves_an_absent_description_absent(exp, out): def test_publish_refuses_and_keeps_an_existing_description_byte_identical(exp, out): save_draft(out, draft_from_discovery(exp), complete()) - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) published = (out / DESCRIPTION_FILENAME).read_bytes() answers = new_answers(out) assert answers.base_sha256 != "absent" @@ -82,7 +82,7 @@ def test_publish_refuses_and_keeps_an_existing_description_byte_identical(exp, o path.write_text(json.dumps(doc, indent=2, sort_keys=True) + "\n") before = snapshot(out) with pytest.raises(DraftFileError) as refusal: - publish_draft(out, draft_from_discovery(exp)) + publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert refusal.value.code == "spec_commit_mismatch" assert (out / DESCRIPTION_FILENAME).read_bytes() == published assert snapshot(out) == before @@ -108,7 +108,7 @@ def test_save_refusal_keeps_an_earlier_draft_byte_identical(exp, out): def test_a_draft_with_the_current_commit_round_trips_and_publishes(exp, out): save_draft(out, draft_from_discovery(exp), complete()) assert load_draft(out, draft_from_discovery(exp)).answers.spec_commit == SPEC_COMMIT - publication = publish_draft(out, draft_from_discovery(exp)) + publication = publish_draft(out, draft_from_discovery(exp), channel_counts=COUNTS) assert publication is not None assert (out / DESCRIPTION_FILENAME).exists() assert not (out / DRAFT_FILENAME).exists() diff --git a/tests/test_mask_answers.py b/tests/test_mask_answers.py index d261bd53..360673d9 100644 --- a/tests/test_mask_answers.py +++ b/tests/test_mask_answers.py @@ -14,6 +14,7 @@ ) from morphobricks_preprocessing.draft_format import ABSENT, SPEC_COMMIT from morphobricks_preprocessing.draft_ids import id_problems, propose_id +from morphobricks_preprocessing.fingerprint import Fingerprint from morphobricks_preprocessing.mask_answers import ( MaskAnswerRefused, drawn_on, @@ -31,6 +32,7 @@ THUMBNAIL_SIDE, PreviewCache, PreviewRefused, + StripOptions, labels_thumbnail, read_plane, ) @@ -149,9 +151,17 @@ def test_the_evidence_counts_values_and_draws_nothing_undeclared(tmp_path): inventory = find_masks(masks, pre.discovery, pre.metadata.files) mapping = AxisMapping("CYX", "CYX") review = AxisReview(mapping, dict.fromkeys(images, mapping), ()) + bound = {rel: Fingerprint.of(root / rel) for rel in images} assert representative(inventory, "Nuclei", sorted(images)).image == "ctrl/rep1/a.tif" found = mask_evidence( - inventory, "Nuclei", root, sorted(images), review, 1, PreviewCache(), 2**30 + inventory, + "Nuclei", + root, + sorted(images), + review, + 1, + StripOptions(cache=PreviewCache(), memory_limit=2**30), + bound, ) assert (found.images_with, found.images, found.dtypes) == (2, 2, ("uint16",)) assert found.image == "ctrl/rep1/a.tif" and found.values == 8 and found.channel == 1 @@ -159,6 +169,13 @@ def test_the_evidence_counts_values_and_draws_nothing_undeclared(tmp_path): assert found.overlay_png("instances").startswith(b"\x89PNG") assert found.mask_png().startswith(b"\x89PNG") and found.image_png().startswith(b"\x89PNG") missing = mask_evidence( - inventory, "Nuclei", root, sorted(images), review, 5, PreviewCache(), 2**30 + inventory, + "Nuclei", + root, + sorted(images), + review, + 5, + StripOptions(cache=PreviewCache(), memory_limit=2**30), + bound, ) assert missing.picture is None and missing.problem # no channel 5: said, not guessed diff --git a/tests/test_notebook_batch33.py b/tests/test_notebook_batch33.py new file mode 100644 index 00000000..70a3f183 --- /dev/null +++ b/tests/test_notebook_batch33.py @@ -0,0 +1,228 @@ +"""Audit batch 33, the notebook side: what is saved is what is shown, of the image checked. + +- B1: *Yes, these are right* saves exactly the tags the block shows; a proposal that is not + shown (its label is tagged elsewhere in the folder) is never saved, and a label is never + saved at two positions of a folder. +- B3: tags saved for a 3-channel image are not published once the image has 2 channels: the + save is refused before a new check (the image changed) and after it (the tags are stale), + until the folder is tagged again. +- B4: a picture is drawn only of the image as the folder check read its header; an image + changed since (same shape, same header names) is refused with "check the folder again". +- FILE 2: an answer the draft file cannot take (the output folder's lock) is refused, and the + answers before it are kept: nothing is acknowledged that is not on disk. +""" + +import os +from dataclasses import replace +from pathlib import Path + +import pytest + +from channel_images import write_channels +from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, DRAFT_FILENAME +from morphobricks_preprocessing.run.lock import output_lock +from test_notebook_channels import ACTIN, DNA, PHASE, open_screen, set_labels, tree +from test_notebook_check_save import FOLDERS, KINDS, answer_all, exp, opened, tag # noqa: F401 + +pytest.importorskip("ipywidgets") +pytest.importorskip("matplotlib") + +KEYS = [(*f.split("/"), "single") for f in FOLDERS] + + +def _rewrite(path: Path, names: list[str], seed: int) -> None: + """Write another image at the same path, with a later modification time.""" + before = path.stat().st_mtime_ns + write_channels(path, names, KINDS[: len(names)], seed=seed) + os.utime(path, ns=(before + 5 * 10**9, before + 5 * 10**9)) + + +# --- B1 ----------------------------------------------------------------------------------------- + + +def test_confirm_saves_only_the_tags_the_block_shows(tmp_path): + root = tree(tmp_path, lambda f: ["DNA", "ACTIN", "PHASE"]) + nb = open_screen(root, tmp_path / "out") + set_labels(nb, [ACTIN, DNA, PHASE]) + grid = nb.channel_grid + grid.cells[(KEYS[0], 1)].value = DNA # the user tags C1 = DNA: the C0 proposal is hidden + shown, proposed = grid.effective(grid.layouts[0]) + assert shown == (None, DNA, PHASE) and proposed == {2} + grid.confirm_buttons[0].click() + for key in KEYS: # before the fix: (DNA, DNA, PHASE), a tag never shown + assert nb.draft_answers.units[key].channels == (None, DNA, PHASE) + assert b'"DNA", "DNA"' not in (tmp_path / "out" / DRAFT_FILENAME).read_bytes() + assert not grid.complete # C0 stays to tag: the block is not answered + assert "1 channel(s) still to tag" not in grid.status.value # one per folder, four in all + assert "4 channel(s) still to tag" in grid.status.value + + +def test_a_folder_whose_later_image_differs_is_never_grouped_on_its_first_image(tmp_path): + root = tree(tmp_path, lambda f: ["DAPI", "Actin", "Ch3"]) + for i, folder in enumerate(FOLDERS): # a second image per folder; one header disagrees + names = ["DAPI", "Actin", "Phase"] if folder == "ctrl/rep2" else ["DAPI", "Actin", "Ch3"] + write_channels(root / folder / "b.tif", names, KINDS, seed=i + 20) + nb = open_screen(root, tmp_path / "out") + set_labels(nb, [DNA, ACTIN, PHASE]) + grid = nb.channel_grid + by_key = {u.key: u for u in grid.layouts} + assert not by_key[KEYS[1]].uniform and by_key[KEYS[0]].uniform + blocks = [[u.key for u in block] for block in grid.block_list()] + assert [KEYS[1]] in blocks # alone, although its first image's header is the others' + assert [KEYS[0], KEYS[2], KEYS[3]] in blocks + assert "its images do not all have this header" in repr(grid.blocks.children) + grid.cells[(KEYS[0], 0)].value = DNA + assert KEYS[1] not in nb.draft_answers.units # the block answer does not reach it + + +def test_a_label_at_two_positions_is_never_saved_whatever_writes_it(tmp_path): + from morphobricks_preprocessing.channel_answers import with_tags + + root = tree(tmp_path, lambda f: ["DAPI", "Actin", "Ch3"]) + nb = open_screen(root, tmp_path / "out") + set_labels(nb, [DNA, ACTIN, PHASE]) + layout = nb.channel_grid.layouts[0] + before = nb.draft_answers + assert not nb._answer(with_tags(before, layout, (DNA, DNA, PHASE))) + assert "E_DUPLICATE_CHANNEL_IN_UNIT" in nb._refusal.codes + assert nb.draft_answers is before + + +# --- B3 ----------------------------------------------------------------------------------------- + + +def _confirm_axes(nb): + nb.preview_checked.set(True) + nb.show_preview() + nb.confirm_button.click() + + +def test_tags_saved_for_another_channel_count_are_not_published(exp): # noqa: F811 + root, masks, out = exp + nb = opened(*exp) + answer_all(nb) + assert nb._check is not None and nb._check.can_save # a valid 3-channel draft + for seed, folder in enumerate(FOLDERS): # now 2 channels, at the same paths + _rewrite(root / folder / "a.tif", ["DAPI", "Actin"], seed=seed + 9) + assert not nb.save_experiment() # before a new check: the image is not the one checked + assert "changed since the folder check" in nb.save_status.value + assert not (out / DESCRIPTION_FILENAME).exists() + nb.check_button.click() # the new check reads 2 channels; the 3 saved tags are kept on disk + _confirm_axes(nb) + assert nb.confirmed_mapping is not None + assert all(len(nb.draft_answers.units[key].channels) == 3 for key in KEYS) + nb._refresh_check_save() + assert "were saved for another image: tag its channels again" in nb.todo.value + shown = repr(nb.replicates_box.children) # the stale 3-tag chips are not shown as channels + assert "3 tags saved for another image (it has 2 channels now)" in shown + assert ">PHASE<" not in shown + assert nb.save_button.disabled and not nb.save_experiment() + assert not (out / DESCRIPTION_FILENAME).exists() + tag(nb) # the user retags the folders + assert all(nb.draft_answers.units[key].channels == (DNA, ACTIN) for key in KEYS) + nb._refresh_check_save() + assert "another image" not in nb.todo.value + assert "tags saved for another image" not in repr(nb.replicates_box.children) + answer_all(nb) + assert nb.save_experiment() and (out / DESCRIPTION_FILENAME).exists() + + +def test_an_unanswered_image_type_keeps_save_and_run_closed_until_answered(exp): # noqa: F811 + root, masks, out = exp + nb = opened(*exp) + answer_all(nb) + answers = nb.draft_answers # every other answer given; one image type not (as resumed) + unit = replace(answers.units[KEYS[2]], image_type=None) + assert nb._answer(replace(answers, units={**answers.units, KEYS[2]: unit})) + assert nb.draft_answers.units[KEYS[2]].image_type is None + nb._refresh_check_save() + assert "the image type of drug / rep1 is not answered" in nb.todo.value + assert nb.save_button.disabled and not nb.save_experiment() + assert not (out / DESCRIPTION_FILENAME).exists() and not nb._run_allowed() + radio = nb.channel_types.cell(next(u for u in nb._channel_layouts if u.key == KEYS[2])) + assert radio is not None and nb.channel_types.radios[KEYS[2]].value is None + nb.channel_types.radios[KEYS[2]].value = "channels" # the user's answer + nb._refresh_check_save() + assert not nb.save_button.disabled and nb.save_experiment() + assert (out / DESCRIPTION_FILENAME).exists() and nb._run_allowed() + + +# --- B4 ----------------------------------------------------------------------------------------- + + +def test_a_thumbnail_of_an_image_changed_after_the_check_is_refused(exp): # noqa: F811 + root = exp[0] + nb = opened(*exp) + thumbs, layout = nb._channel_thumbs, nb._channel_layouts[1] + _rewrite(root / layout.image, ["DAPI", "Actin", "Ch3"], seed=11) # same shape and names + for found in (thumbs.get(layout, 0), thumbs.colour(layout), thumbs.tile(layout, False)): + assert found.png is None and "check the folder again" in found.note + nb.check_button.click() # a new check binds the new revision: pictures again + _confirm_axes(nb) + assert nb._channel_thumbs is not thumbs + assert nb._channel_thumbs.get(nb._channel_layouts[1], 0).png is not None + + +def test_the_step_1_picture_of_an_image_changed_after_the_check_is_refused(exp): # noqa: F811 + root = exp[0] + nb = opened(*exp, axes=False) + previews = nb.label_previews + _rewrite(root / "ctrl/rep2/a.tif", ["DAPI", "Actin", "Ch3"], seed=12) + previews.enlarge(previews._leaves["ctrl/rep2"]) + assert "check the folder again" in repr(previews.detail.children) + previews.tiles.pop("ctrl/rep2") + previews.draw("ctrl/rep2") + assert previews.found["ctrl/rep2"].pictures is None + assert "check the folder again" in previews.found["ctrl/rep2"].problem + + +def test_the_mask_evidence_of_an_image_changed_after_the_check_is_refused(exp): # noqa: F811 + root = exp[0] + nb = opened(*exp) + tag(nb) + _rewrite(root / "ctrl/rep1/a.tif", ["DAPI", "Actin", "Ch3"], seed=13) + nb._mask_found = {} + found = nb._mask_evidence("Nuclei") + assert found.picture is None and "check the folder again" in found.problem + + +def test_mask_evidence_already_read_is_not_shown_once_its_image_changed(exp): # noqa: F811 + root = exp[0] + nb = opened(*exp) + tag(nb) + assert nb._mask_evidence("Nuclei").picture is not None # read once, kept + _rewrite(root / "ctrl/rep1/a.tif", ["DAPI", "Actin", "Ch3"], seed=14) + found = nb._mask_evidence("Nuclei") # before the fix: the kept picture of the old image + assert found.picture is None and "check the folder again" in found.problem + + +def test_masks_paired_again_are_bound_and_a_changed_mask_is_refused(exp): # noqa: F811 + root, masks, _ = exp + nb = opened(*exp) + tag(nb) + nb._check_masks() # the masks are paired again (remasked), without a new folder check + assert nb.preflight.masks.files and all(m.bound for m in nb.preflight.masks.files) + mask = masks / "Nuclei" / "ctrl/rep1/a.tif" + before = mask.stat().st_mtime_ns + mask.write_bytes(mask.read_bytes()) # another revision of the mask, same pixels + os.utime(mask, ns=(before + 5 * 10**9, before + 5 * 10**9)) + found = nb._mask_evidence("Nuclei") # before the fix: drawn, as if it were the one paired + assert found.picture is None and "check the folder again" in found.problem + + +# --- FILE 2 ------------------------------------------------------------------------------------- + + +def test_an_answer_refused_by_the_output_lock_is_not_acknowledged(tmp_path): + root = tree(tmp_path, lambda f: ["DAPI", "Actin", "Ch3"]) + out = tmp_path / "out" + nb = open_screen(root, out) + set_labels(nb, [DNA, ACTIN, PHASE]) + before, on_disk = nb.draft_answers, (out / DRAFT_FILENAME).read_bytes() + with output_lock(out): + nb.channel_grid.cells[(KEYS[0], 0)].value = DNA + assert nb.draft_answers == before # kept as it was: the answer was not saved + assert (out / DRAFT_FILENAME).read_bytes() == on_disk + assert "Not saved" in nb.channel_grid.problem.value + assert "lock" in nb.channel_grid.problem.value + assert nb.channel_grid.cells[(KEYS[0], 0)].value is None # the grid shows what is saved diff --git a/tests/test_notebook_masks.py b/tests/test_notebook_masks.py index c1c9e194..9061be82 100644 --- a/tests/test_notebook_masks.py +++ b/tests/test_notebook_masks.py @@ -79,7 +79,8 @@ def test_the_masks_field_pairs_the_masks_as_check_masks_does(tmp_path): for rel in IMAGES[:3]: # or the type after the image name; one image has none write_data(masks / rel.replace("a.tif", "a_nuclei.tif"), labels((16, 16))) nb = notebook(root, masks) - assert nb.preflight.masks == preflight(root, masks_folder=masks).masks + # The notebook binds each header to its file (audit batch 33): so does this check. + assert nb.preflight.masks == preflight(root, masks_folder=masks, bind_headers=True).masks assert ui.is_visible(nb.masks_input_card) and ui.is_visible(nb.masks_card) assert "2 mask types for 4 images" in nb.masks_status.value table = nb.masks_table.value diff --git a/tests/test_notebook_thumbnails.py b/tests/test_notebook_thumbnails.py index df6380fd..f66ed60c 100644 --- a/tests/test_notebook_thumbnails.py +++ b/tests/test_notebook_thumbnails.py @@ -12,6 +12,7 @@ from channel_images import blobs, large, write_channels, write_rgb from morphobricks_preprocessing import header_preview as hp +from morphobricks_preprocessing.fingerprint import Fingerprint from morphobricks_preprocessing.metadata import inspect_image from morphobricks_preprocessing.preview_crop import CROP_SIDE, busiest_square, tile_pictures from morphobricks_preprocessing.preview_render import display_panel @@ -74,12 +75,21 @@ def _meta(root, name): return inspect_image(root / name, name) +def _bound(root, name): + """The fingerprint the folder check takes with the header (the picture is of that file).""" + return {name: Fingerprint.of(root / name)} + + def test_a_picture_is_drawn_only_when_the_header_names_y_and_x(tmp_path): tifffile.imwrite(tmp_path / "plain.tif", np.stack([blobs(1), blobs(2)])) # QYX: nothing named write_channels(tmp_path / "named.tif", ["DAPI", "Actin"], ["blobs", "filaments"]) - plain = hp.header_preview(tmp_path, _meta(tmp_path, "plain.tif")) + plain = hp.header_preview( + tmp_path, _meta(tmp_path, "plain.tif"), bound=_bound(tmp_path, "plain.tif") + ) assert plain.pictures is None and plain.problem == hp.NO_YX - named = hp.header_preview(tmp_path, _meta(tmp_path, "named.tif")) + named = hp.header_preview( + tmp_path, _meta(tmp_path, "named.tif"), bound=_bound(tmp_path, "named.tif") + ) assert named.pictures is not None and named.text == "max projection over C (2): all 2 planes" @@ -104,17 +114,19 @@ def test_a_plane_over_the_pixel_cap_is_not_read_and_the_cap_is_said(tmp_path, mo write_channels(tmp_path / "a.tif", ["DAPI"], ["blobs"]) monkeypatch.setattr(hp, "PIXEL_CAP", 1000) monkeypatch.setattr(hp, "read_plane", lambda *a, **k: pytest.fail("read over the cap")) - found = hp.header_preview(tmp_path, _meta(tmp_path, "a.tif")) + found = hp.header_preview(tmp_path, _meta(tmp_path, "a.tif"), bound=_bound(tmp_path, "a.tif")) assert found.pictures is None and "over the 1,000-pixel cap" in found.problem def test_an_rgb_header_is_drawn_in_colour_and_a_read_failure_is_a_sentence(tmp_path): write_rgb(tmp_path / "rgb.tif") - found = hp.header_preview(tmp_path, _meta(tmp_path, "rgb.tif")) + found = hp.header_preview( + tmp_path, _meta(tmp_path, "rgb.tif"), bound=_bound(tmp_path, "rgb.tif") + ) assert found.colour and decode(found.pictures.png).shape == (64, 64, 3) meta = _meta(tmp_path, "rgb.tif") (tmp_path / "rgb.tif").write_bytes((tmp_path / "rgb.tif").read_bytes()[:400]) # truncated - broken = hp.header_preview(tmp_path, meta) + broken = hp.header_preview(tmp_path, meta, bound=_bound(tmp_path, "rgb.tif")) assert broken.pictures is None and broken.problem.startswith("cannot be read for a preview") diff --git a/tests/test_published_answers.py b/tests/test_published_answers.py index 2a365c59..5230869d 100644 --- a/tests/test_published_answers.py +++ b/tests/test_published_answers.py @@ -8,7 +8,7 @@ import pytest -from draft_file_support import DAY, PATHS, complete +from draft_file_support import COUNTS, DAY, PATHS, complete from morphobricks_preprocessing import discover_experiment, preflight from morphobricks_preprocessing.description_candidate import check_candidate from morphobricks_preprocessing.description_reopen import published_state @@ -66,8 +66,8 @@ def test_a_repeat_named_otherwise_is_paired_under_its_id(tmp_path): repeats={"rep1": complete().repeats["rep1"]}, pairing={("DrugA", "day2"): "rep1"}, units={ - ("CTRL", "rep1", "single"): UnitAnswer(("DNA", "ACTIN")), - ("DrugA", "day2", "single"): UnitAnswer(("DNA", "ACTIN")), + ("CTRL", "rep1", "single"): UnitAnswer(("DNA", "ACTIN"), "channels"), + ("DrugA", "day2", "single"): UnitAnswer(("DNA", "ACTIN"), "channels"), }, ) published = description_from_draft(draft, answers) @@ -108,7 +108,7 @@ def test_the_review_states_comparisons_in_words_and_missing_channels(exp): }, ) units = dict(answers.units) - units[("DrugA", "rep2", "single")] = UnitAnswer(("DNA",)) + units[("DrugA", "rep2", "single")] = UnitAnswer(("DNA",), "channels") answers = replace(answers, units=units) applied = apply_answers(draft, answers) assert comparisons(applied, answers) == [ @@ -158,5 +158,5 @@ def test_publish_draft_writes_the_derived_csv_only_with_the_description(exp, tmp out = tmp_path / "out" save_draft(out, draft, complete()) assert not (out / "experiment_design.csv").exists() # saving answers writes no CSV - publish_draft(out, draft) + publish_draft(out, draft, channel_counts=COUNTS) assert (out / "experiment_design.csv").read_bytes().startswith(b"condition,") diff --git a/tests/test_run_description.py b/tests/test_run_description.py index 2f458b1a..0901bfc2 100644 --- a/tests/test_run_description.py +++ b/tests/test_run_description.py @@ -120,8 +120,19 @@ def test_an_unreadable_records_file_stops_the_run(tmp_path): (out / "morphobricks.toml").write_bytes(V01.data) (out / "morphobricks.records.jsonl").write_text("not json\n", encoding="utf-8") result = run_experiment(root, MAPPING, output_folder=out) - assert not result.written - assert result.issues[0].code == "description_unreadable" + assert not result.written and not (out / "run_manifest.json").exists() + assert result.issues[0].code == "R_RECORDS_FILE_INVALID" # the Spec code (batch 34a) + assert "E_RECORDS_NOT_JSON_OBJECT" in result.issues[0].message + + +def test_a_malformed_record_stops_the_run_with_the_spec_code_and_its_reason(tmp_path): + root, out = _folder(tmp_path) + (out / "morphobricks.toml").write_bytes(V01.data) + (out / "morphobricks.records.jsonl").write_text('{"type": "reconfirmation"}\n', "utf-8") + result = run_experiment(root, MAPPING, output_folder=out) + assert not result.written and not (out / "run_manifest.json").exists() + assert [i.code for i in result.issues] == ["R_RECORDS_FILE_INVALID"] + assert "E_RECORDS_KEY_SET" in result.issues[0].message def test_a_description_that_lists_a_missing_unit_is_refused_with_its_code(tmp_path):