diff --git a/src/morphobricks_preprocessing/description_reconcile.py b/src/morphobricks_preprocessing/description_reconcile.py index 029188d9..aa9ae4a4 100644 --- a/src/morphobricks_preprocessing/description_reconcile.py +++ b/src/morphobricks_preprocessing/description_reconcile.py @@ -65,11 +65,17 @@ class ObservedUnit: ``channel_count`` and ``header_names`` (raw names by C index, None where there is none) are None when the headers were not read or do not state them: that is not evidence of a change. + ``uniform`` is False when the unit's headers disagree with each other (it is no longer one + layout): its channel evidence then no longer fits its declared channel list. ``rgb`` is + whether the headers state a colour (RGB) image, None when they were not read or do not say + (OME-Zarr): a declared image type the headers now contradict is a channel change. """ image_count: int channel_count: int | None = None header_names: tuple[str | None, ...] | None = None + uniform: bool = True + rgb: bool | None = None def __post_init__(self) -> None: if self.image_count < 1: @@ -224,6 +230,10 @@ def _channels_differ( ) -> bool: """Return whether the observed channel evidence no longer fits the declared channel list.""" unit = published.unit + if not seen.uniform: + return True + if seen.rgb is not None and seen.rgb != (unit.image_type == "rgb"): + return True # as the run's checks compare it (IMAGE_TYPE_CONTRADICTS_HEADER) if seen.channel_count is not None and seen.channel_count != len(unit.channels): return True if seen.header_names is None or unit.image_type == "rgb": diff --git a/src/morphobricks_preprocessing/description_reopen.py b/src/morphobricks_preprocessing/description_reopen.py index 9cfaea02..8cb7fd3d 100644 --- a/src/morphobricks_preprocessing/description_reopen.py +++ b/src/morphobricks_preprocessing/description_reopen.py @@ -9,7 +9,7 @@ from __future__ import annotations -from collections.abc import Collection +from collections.abc import Collection, Iterable from dataclasses import dataclass from datetime import date from pathlib import Path @@ -19,6 +19,7 @@ from .description_reconcile import Reconciliation, reconcile from .description_removal import RemovalQuestion, removal_questions from .discovery import DiscoveryResult +from .metadata import TiffMetadata from .reconcile_observe import observe_folders @@ -38,16 +39,27 @@ class PublishedState: @property def changed(self) -> bool: - """True when a folder is new or removed, or a member's image count changed.""" + """True when the folders or headers differ from the saved file. + + A folder is new or removed, a member's image count changed, or a unit's headers no + longer fit its saved channels (any reason that reopens a repeat). + """ rec = self.reconciliation - return bool(rec.new or rec.removed or rec.counts) + return bool(rec.new or rec.removed or rec.counts or rec.reopened) def published_state( - output: Path, discovery: DiscoveryResult, excluded: Collection[str] = () + output: Path, + discovery: DiscoveryResult, + excluded: Collection[str] = (), + metadata: Iterable[TiffMetadata] | None = None, ) -> tuple[PublishedState | None, str]: """Return the published file compared with the folders, or None and why it cannot be used. + ``metadata`` are the image headers already read (the preflight's): with them each unit's + channel count and header names are compared with its saved channels, so a header changed + under the same files is a change, never "nothing changed". + Returns ------- tuple @@ -65,7 +77,7 @@ def published_state( return None, f"{DESCRIPTION_FILENAME} cannot be used ({codes})" except OSError as exc: return None, f"{DESCRIPTION_FILENAME} cannot be read: {exc}" - rec = reconcile(published, observe_folders(discovery, excluded)) + rec = reconcile(published, observe_folders(discovery, excluded, metadata)) day = date.fromtimestamp(path.stat().st_mtime).isoformat() return PublishedState( published, sha256_hex(data), day, rec, removal_questions(published, rec) diff --git a/src/morphobricks_preprocessing/notebook_axes_quick.py b/src/morphobricks_preprocessing/notebook_axes_quick.py index ae2f95ad..8ec49040 100644 --- a/src/morphobricks_preprocessing/notebook_axes_quick.py +++ b/src/morphobricks_preprocessing/notebook_axes_quick.py @@ -25,6 +25,7 @@ from .axes import AxisMapping, axis_name from .axis_check import ConfirmedAxes from .axis_review import review_axes +from .description_reconcile import CHANNELS_CHANGED from .fingerprint import unchanged_confirmation DETAILS_OPEN, DETAILS_CLOSED = " Hide the role boxes", " Look closer, or change the roles" @@ -133,9 +134,25 @@ def _refresh_quick_axes(self) -> None: self._quick_filling = False self.quick_text.value = self._quick_text(offer) self.show_preview() - self.quick_image.value = self.preview_image.value - ui.set_visible(self.quick_image, bool(self.preview_image.value)) - self.quick_caption.value = self.preview_caption.value + shown = self._quick_preview_shown(offer) + self.quick_image.value = self.preview_image.value if shown else b"" + ui.set_visible(self.quick_image, shown) + self.quick_confirm.disabled = not shown + self.quick_caption.value = self.preview_caption.value + ( + "" + if shown + else ui.status( + "warn", + "Confirm these axes is not offered: no preview of these roles is shown.", + ["Open the role boxes below to look closer or change the roles."], + ) + ) + + def _quick_preview_shown(self, offer: AxisMapping) -> bool: + """Return whether the preview now shows a plane drawn with exactly the offered roles.""" + drawn = getattr(self, "_preview_drawn", None) + same = drawn is not None and (drawn.axes, drawn.roles) == (offer.axes, offer.roles) + return bool(self.preview_image.value) and same def _quick_text(self, offer: AxisMapping) -> str: """Return what the card says: the roles, where they come from, what the click does.""" @@ -153,6 +170,7 @@ def _quick_text(self, offer: AxisMapping) -> str: "info", f"Suggested: {roles}.", [ + *self._headers_changed_line(), f"These are {source}; the axis check finds nothing to ask about them.", "Look at the preview: if it looks like a normal image of your sample, confirm. " "Nothing is confirmed until you click.", @@ -160,6 +178,17 @@ def _quick_text(self, offer: AxisMapping) -> str: raw=True, ) + def _headers_changed_line(self) -> list[str]: + """Say that the headers changed since the save, so the earlier roles are not offered.""" + state = self._published() if self.preflight is not None else None + units = () if state is None else state.reconciliation.units + if not any(CHANNELS_CHANGED in unit.reasons for unit in units): + return [] + return [ + "Some image headers changed since the experiment was saved: the roles the earlier " + "run recorded are not offered. Review these afresh." + ] + # The click --------------------------------------------------------------------------- def _confirm_quick_axes(self) -> bool: @@ -176,6 +205,8 @@ def _confirm_quick_axes(self) -> bool: if self._current_mapping() != offer: self._refresh_quick_axes() return False + if not self._quick_preview_shown(offer): + return False # no preview of these roles was shown: the click cannot vouch for it self.preview_checked.set(True) # the user's click says the preview looks right self._toggle_confirmation() return self.confirmed_mapping is not None diff --git a/src/morphobricks_preprocessing/notebook_axis_preview.py b/src/morphobricks_preprocessing/notebook_axis_preview.py index 50ae8c82..bb39f439 100644 --- a/src/morphobricks_preprocessing/notebook_axis_preview.py +++ b/src/morphobricks_preprocessing/notebook_axis_preview.py @@ -11,7 +11,8 @@ from .notebook_options import IMAGE_TYPE_NAMES from .notebook_plots import plane_png from .notebook_render import preview_caption -from .planes import PlaneReadError, read_plane +from .planes import Plane, PlaneReadError, read_plane +from .preview import PreviewRefused, check_unchanged, checked_fingerprint from .validation import plural @@ -68,9 +69,16 @@ def _render_preview_if_shown(self) -> None: self._render_preview() def _render_preview(self) -> None: - """Draw the chosen plane with the roles shown, labelled confirmed or not (PP-10).""" + """Draw the chosen plane with the roles shown, labelled confirmed or not (PP-10). + + ``_preview_drawn`` is the mapping of the plane now shown, None when none is (no image + fits, or the plane cannot be read): the one-click confirmation needs it (BLOCKER 3). + """ + self._preview_drawn = None header = self._preview_header() if header is None: + self.preview_image.value = b"" + ui.set_visible(self.preview_image, False) self.preview_caption.value = ui.status("warn", "No image fits these axis roles.") return file_mapping = file_mapping_for(header, self.mapping) @@ -80,15 +88,8 @@ def _render_preview(self) -> None: if file_mapping.index_of(role) is not None } try: - with self.log.attached(): - plane = read_plane( - self.preflight.discovery.root / header.relative_path, - file_mapping, - memory_limit=self._memory_limit(), - preview=True, - **coords, - ) - except (PlaneReadError, IndexError, ValueError) as exc: + plane = self._bound_plane(header.relative_path, file_mapping, coords) + except (PreviewRefused, PlaneReadError, IndexError, ValueError, OSError) as exc: self.preview_image.value = b"" ui.set_visible(self.preview_image, False) self.preview_caption.value = ui.status( @@ -97,11 +98,36 @@ def _render_preview(self) -> None: return self.preview_image.value = plane_png(plane) ui.set_visible(self.preview_image, True) + self._preview_drawn = self.mapping self.preview_caption.value = self._preview_label() + preview_caption( header.relative_path, plane ) self._show_log() + def _bound_plane(self, rel: str, file_mapping: AxisMapping, coords: dict) -> Plane: + """Read the preview plane of the image only as the folder check read its header. + + The fingerprint the check bound to the header must hold before and after the read: an + image changed since the check (same name, same shape or not) is refused, never shown as + the image checked, so the one-click confirmation is not offered on it. The binding is + the one every other picture uses (:func:`.preview.checked_fingerprint`). + + Raises + ------ + PreviewRefused + When the image is not bound, or is no longer that revision ("check the folder + again"). + """ + path = self.preflight.discovery.root / rel + expected = checked_fingerprint(self.preflight.header_fingerprints, rel) + check_unchanged(path, expected, rel) + with self.log.attached(): + plane = read_plane( + path, file_mapping, memory_limit=self._memory_limit(), preview=True, **coords + ) + check_unchanged(path, expected, rel) + return plane + def _preview_label(self) -> str: """Say whether the preview shows the confirmed roles or roles still being edited.""" if self.confirmed_mapping is not None: diff --git a/src/morphobricks_preprocessing/notebook_axis_step.py b/src/morphobricks_preprocessing/notebook_axis_step.py index 8f4097bb..83c60744 100644 --- a/src/morphobricks_preprocessing/notebook_axis_step.py +++ b/src/morphobricks_preprocessing/notebook_axis_step.py @@ -156,6 +156,7 @@ def _refresh_axis_step(self) -> None: pre = self.preflight self.axis_next.value = "" self._preview_shown = False + self._preview_drawn = None self._close_image_type() waiting = self._axis_waiting() if waiting: diff --git a/src/morphobricks_preprocessing/notebook_check_save.py b/src/morphobricks_preprocessing/notebook_check_save.py index f72256f1..84c54f4f 100644 --- a/src/morphobricks_preprocessing/notebook_check_save.py +++ b/src/morphobricks_preprocessing/notebook_check_save.py @@ -162,6 +162,13 @@ def _run_allowed(self) -> bool: check = self._published_check() return check is not None and check.can_run + def _published_refused(self) -> bool: + """Return whether the run's checks ran on the published file and refuse it.""" + if self.confirmed_mapping is None: + return False # the physical checks wait for the roles: nothing is decided yet + check = self._published_check() + return check is not None and check.physical_run and not check.can_run + def _published_check(self) -> CandidateCheck | None: """Return the run's checks of the published file (None without one).""" state = self._published() @@ -345,7 +352,9 @@ def _show_removals(self, state: object) -> None: def _show_saved_state(self) -> None: """Show the green saved line (folded or not) and the run gate.""" state = self._published() - if self._saved_current(): + if self._published_refused(): + self._folded = False # a refusal of the run's checks is never folded away + if self._saved_current() and not self._published_refused(): when = self._saved_on or (state.saved_on if state else "") self.save_status.value = ui.status("ok", f"Saved {when}. The run (step 12) can start.") self.saved_line.value = ui.status("ok", f"Saved on {when}, nothing changed.") @@ -354,6 +363,7 @@ def _show_saved_state(self) -> None: if self.save_status.value.find("Saved") >= 0: self.save_status.value = "" ui.set_visible(self.fold_bar, self._folded) + ui.set_visible(self.check_card, not self._folded) for name in FOLDED: ui.set_visible(getattr(self, name), not self._folded) self._refresh_run_step() diff --git a/src/morphobricks_preprocessing/notebook_check_view.py b/src/morphobricks_preprocessing/notebook_check_view.py index 8a088ff9..8061eeb8 100644 --- a/src/morphobricks_preprocessing/notebook_check_view.py +++ b/src/morphobricks_preprocessing/notebook_check_view.py @@ -11,7 +11,7 @@ from . import notebook_style as ui from .description_candidate import CandidateCheck -from .description_reconcile import Reconciliation, UnitFolders +from .description_reconcile import CHANNELS_CHANGED, Reconciliation, UnitFolders from .description_removal import RemovalQuestion from .description_review import MemberView, ReplicateView, unit_text from .draft import OpenQuestion @@ -187,13 +187,19 @@ def _folders(unit: UnitFolders) -> str: def changes_html(rec: Reconciliation) -> str: - """Say in orange what changed in the folders since the description was saved.""" + """Say in orange what changed in the folders (or their headers) since the save.""" lines = [f"new: {_folders(u.folders)} · {plural(u.image_count or 0, 'image')}" for u in rec.new] lines += [f"removed: {_folders(u.folders)}" for u in rec.removed] lines += [ f"{c.repeat_id} / {c.condition_id}: {c.before} images saved, {c.after} now" for c in rec.counts ] + lines += [ + f"headers changed: {_folders(u.folders)} · its image headers no longer fit the saved " + "channels (the run's checks say which)" + for u in rec.unchanged + if CHANNELS_CHANGED in u.reasons + ] if not lines: return "" return ui.status( diff --git a/src/morphobricks_preprocessing/notebook_described.py b/src/morphobricks_preprocessing/notebook_described.py index 3d9f77aa..5ebc3bb6 100644 --- a/src/morphobricks_preprocessing/notebook_described.py +++ b/src/morphobricks_preprocessing/notebook_described.py @@ -64,7 +64,7 @@ def _published(self) -> PublishedState | None: if getattr(self, "_published_key", None) != key: self._published_key = key self._published_cache, self._published_problem = published_state( - output, pre.discovery, pre.excluded + output, pre.discovery, pre.excluded, pre.metadata.files ) return self._published_cache diff --git a/src/morphobricks_preprocessing/notebook_step_states.py b/src/morphobricks_preprocessing/notebook_step_states.py index a635151c..593af2de 100644 --- a/src/morphobricks_preprocessing/notebook_step_states.py +++ b/src/morphobricks_preprocessing/notebook_step_states.py @@ -9,6 +9,7 @@ from __future__ import annotations from .channel_labels import remaining +from .description_reconcile import CHANNELS_CHANGED from .mask_answers import mask_kinds, masks_unanswered from .notebook_stepper import StepState from .notebook_values import natural_key, size_rows @@ -68,6 +69,9 @@ def _images_state(self) -> StepState: if state is not None and state.changed: rec = state.reconciliation text += f" · since saved: {len(rec.new)} new, {len(rec.removed)} gone" + headers = sum(CHANNELS_CHANGED in u.reasons for u in rec.unchanged) + if headers: + text += f", {plural(headers, 'folder')} with changed headers" if not pre.ok: text += " · problems to fix" return StepState(pre.ok, text) diff --git a/src/morphobricks_preprocessing/notebook_ui.py b/src/morphobricks_preprocessing/notebook_ui.py index fd456e71..665e70f5 100644 --- a/src/morphobricks_preprocessing/notebook_ui.py +++ b/src/morphobricks_preprocessing/notebook_ui.py @@ -198,6 +198,7 @@ def __init__( self.log = _LogCapture() self._checked_folder = "" self._preview_shown = False + self._preview_drawn = None self._show_output = True self._show_design = True self._layouts_cache: tuple[tuple, dict[str, PlaneLayout]] | None = None diff --git a/src/morphobricks_preprocessing/preview_crop.py b/src/morphobricks_preprocessing/preview_crop.py index 381a402e..879e7e88 100644 --- a/src/morphobricks_preprocessing/preview_crop.py +++ b/src/morphobricks_preprocessing/preview_crop.py @@ -7,8 +7,9 @@ - **where:** the square of the native grid (squares of :data:`CROP_SIDE` pixels from the top left corner, whole squares only) whose thumbnail pixels have the highest standard deviation, on - the displayed thumbnail (for colour, the mean of red, green and blue). A tie goes to the - topmost square, then the leftmost. The caption gives its x and y range. + the displayed thumbnail (for colour, the mean of red, green and blue), counting finite pixels + only (a magenta non-finite pixel is not detail). A tie goes to the topmost square, then the + leftmost. The caption gives its x and y range, and the non-finite pixels inside it. - **contrast:** the thumbnail's own black and white values, so both pictures agree (a flat thumbnail gives the crop its own contrast); non-finite pixels are magenta, as everywhere. @@ -56,7 +57,13 @@ def downscale_factor(shape: tuple[int, ...], side: int) -> int: return max(1, math.ceil(max(shape[:2]) / side)) -def busiest_square(shown: np.ndarray, factor: int, shape: tuple[int, int], side: int) -> int: +def busiest_square( + shown: np.ndarray, + factor: int, + shape: tuple[int, int], + side: int, + finite: np.ndarray | None = None, +) -> int: """Return the index (row-major) of the native square whose thumbnail pixels vary most. Parameters @@ -70,6 +77,9 @@ def busiest_square(shown: np.ndarray, factor: int, shape: tuple[int, int], side: The native (Y, X) shape. side The native side of a square. Only whole squares count. + finite + The (y, x) mask of the thumbnail pixels drawn from finite values; the others (magenta) + are left out of the measure. None counts every pixel. Returns ------- @@ -87,6 +97,9 @@ def busiest_square(shown: np.ndarray, factor: int, shape: tuple[int, int], side: grey = grey[keep_y][:, keep_x] index = (ys[keep_y][:, None] * cols + xs[keep_x][None, :]).ravel() values = grey.ravel() + if finite is not None: + kept = finite[keep_y][:, keep_x].ravel() + index, values = index[kept], values[kept] count = np.bincount(index, minlength=rows * cols) total = np.bincount(index, values, minlength=rows * cols) squares = np.bincount(index, values * values, minlength=rows * cols) @@ -137,37 +150,52 @@ def tile_pictures( """ shape = planes[0].shape factor = downscale_factor(shape, side) - panels = [display_panel(np.ascontiguousarray(p[::factor, ::factor])) for p in planes] + reduced = [np.ascontiguousarray(p[::factor, ::factor]) for p in planes] + panels = [display_panel(p) for p in reduced] shown = _picture(panels, colour) + finite = np.logical_and.reduce([_finite(p) for p in reduced]) pictures = TilePictures(png_bytes(shown.pixels), factor, note=shown.note) if factor <= CROP_FACTOR: return pictures if min(shape[:2]) < CROP_SIDE: why = f"no 1:1 detail: the image is under {CROP_SIDE} pixels on a side" return replace(pictures, crop_text=why) - best = busiest_square(shown.pixels, factor, shape, CROP_SIDE) + best = busiest_square(shown.pixels, factor, shape, CROP_SIDE, finite) if best < 0: why = f"no 1:1 detail: the thumbnail keeps too few pixels of a {CROP_SIDE}-pixel square" return replace(pictures, crop_text=why) columns = shape[1] // CROP_SIDE y0, x0 = (best // columns) * CROP_SIDE, (best % columns) * CROP_SIDE box = (y0, y0 + CROP_SIDE, x0, x0 + CROP_SIDE) - crop = _crop(planes, panels, box) + try: + crop = _crop(planes, panels, box) + except ValueError as exc: # a colour detail with non-finite values: never coloured + return replace(pictures, crop_text=f"no 1:1 detail: {exc}") big = np.repeat(np.repeat(crop.pixels, zoom, axis=0), zoom, axis=1) - where = _where(shown.pixels, factor, box) + where = _where(shown.pixels, factor, box, finite) text = ( f"1:1 detail: x {x0}-{box[3] - 1}, y {y0}-{box[1] - 1} ({where}), " f"each pixel drawn {zoom}x{zoom}" ) + if crop.nonfinite: + text += f"; {crop.nonfinite} non-finite pixels in the detail, shown in magenta" return replace(pictures, crop_png=png_bytes(big), crop_box=box, crop_text=text) -def _where(shown: np.ndarray, factor: int, box: tuple[int, int, int, int]) -> str: +def _finite(plane: np.ndarray) -> np.ndarray: + """Return the mask of a plane's finite values (every pixel of an integer plane).""" + return np.isfinite(plane) if plane.dtype.kind in "fc" else np.ones(plane.shape, bool) + + +def _where( + shown: np.ndarray, factor: int, box: tuple[int, int, int, int], finite: np.ndarray +) -> str: """Say why the crop is where it is: it varies most, or nothing varies (the first square).""" y0, y1, x0, x1 = (-(-v // factor) for v in box) # the thumbnail pixels inside the box square = shown[y0:y1, x0:x1].astype(np.float64) grey = square.mean(axis=2) if square.ndim == 3 else square # as busiest_square measures - if grey.min() == grey.max(): + grey = grey[finite[y0:y1, x0:x1]] + if grey.size == 0 or grey.min() == grey.max(): return f"the thumbnail does not vary in any {CROP_SIDE}-pixel square: the first one" return f"the {CROP_SIDE}-pixel square that varies most" diff --git a/src/morphobricks_preprocessing/preview_render.py b/src/morphobricks_preprocessing/preview_render.py index 1f917f7c..7a3e1b42 100644 --- a/src/morphobricks_preprocessing/preview_render.py +++ b/src/morphobricks_preprocessing/preview_render.py @@ -112,9 +112,9 @@ def display_panel(pixels: np.ndarray, limits: tuple[float, float] | None = None) if low == high: # most values equal, a few others: show the full range instead low, high = float(values.min()), float(values.max()) scaled = np.clip((wide - low) / (high - low), 0.0, 1.0) * 255.0 - note = f"{missing} non-finite pixels shown in magenta" if missing else "" + # _with_missing names the non-finite pixels, once (they were named twice, batch 34b FILE 1). return _with_missing( - Panel(np.rint(scaled).astype(np.uint8), NORMAL, low, high, missing, note), finite + Panel(np.rint(scaled).astype(np.uint8), NORMAL, low, high, missing, ""), finite ) diff --git a/src/morphobricks_preprocessing/reconcile_observe.py b/src/morphobricks_preprocessing/reconcile_observe.py index f08b820a..7faee552 100644 --- a/src/morphobricks_preprocessing/reconcile_observe.py +++ b/src/morphobricks_preprocessing/reconcile_observe.py @@ -1,31 +1,53 @@ """The units a folder walk found, as :func:`.description_reconcile.reconcile` takes them. -Counts only: how many images each unit has now. Whether the headers still fit the declared -channels is the header evidence of the physical checks; when a caller has it (channel count, -raw header names) it builds the :class:`~.description_reconcile.ObservedUnit` itself. Nothing -is read from an image here, and no unit is renamed or matched by similarity. +How many images each unit has now and, when the caller gives the image headers already read +(the preflight's), the channel evidence those headers state: the channel count and the raw +header names by C index (:func:`.physical_check.layout_of`, without axis roles: a count the +header leaves open stays unknown) and whether they state a colour (RGB) image. A unit whose +readable headers disagree with each other is marked not uniform. Nothing is read from an image +here, and no unit is renamed or matched by similarity. """ from __future__ import annotations -from collections.abc import Collection +from collections.abc import Collection, Iterable from .description_reconcile import ObservedUnit, UnitFolders from .discovery import DiscoveryResult +from .metadata import OME_ZARR, TiffMetadata +from .physical_check import layout_of + + +def _evidence(paths: Iterable[str], metas: dict[str, TiffMetadata], count: int) -> ObservedUnit: + """Return one unit's observation from the headers of its readable images.""" + readable = [metas[p] for p in paths if p in metas and metas[p].readable] + layouts = {layout_of(meta) for meta in readable} + if not layouts: + return ObservedUnit(count) # no header read: no evidence, never assumed unchanged + if len(layouts) > 1: + return ObservedUnit(count, uniform=False) + (layout,) = layouts + rgb = None if any(meta.kind == OME_ZARR for meta in readable) else layout.rgb + return ObservedUnit(count, layout.channels, layout.names or None, rgb=rgb) def observe_folders( - discovery: DiscoveryResult, excluded: Collection[str] = () + discovery: DiscoveryResult, + excluded: Collection[str] = (), + metadata: Iterable[TiffMetadata] | None = None, ) -> dict[UnitFolders, ObservedUnit]: """Return each unit of the walk with its image count, leaving out the images ``excluded``. A folder whose images are all excluded is not a unit (it is a removed one if published). - The channel evidence of every unit is left unknown (None), never assumed. + Without ``metadata`` the channel evidence of every unit is left unknown (None), never + assumed; with it, each unit carries what its headers state. """ - found: dict[UnitFolders, int] = {} + found: dict[UnitFolders, list[str]] = {} for leaf in discovery.leaves: - count = sum(1 for image in leaf.images if image not in excluded) - if count: - key = UnitFolders(*leaf.labels) - found[key] = found.get(key, 0) + count - return {key: ObservedUnit(count) for key, count in sorted(found.items())} + images = [image for image in leaf.images if image not in excluded] + if images: + found.setdefault(UnitFolders(*leaf.labels), []).extend(images) + if metadata is None: + return {key: ObservedUnit(len(paths)) for key, paths in sorted(found.items())} + metas = {meta.relative_path: meta for meta in metadata} + return {key: _evidence(paths, metas, len(paths)) for key, paths in sorted(found.items())} diff --git a/tests/test_notebook_axes_quick.py b/tests/test_notebook_axes_quick.py index ba588847..ff236a20 100644 --- a/tests/test_notebook_axes_quick.py +++ b/tests/test_notebook_axes_quick.py @@ -107,3 +107,115 @@ def test_a_role_edited_on_a_reopen_is_kept_not_put_back_to_the_run_s_roles(exp): assert nb.confirmed_mapping is None nb.role_dropdowns[0].value = "C" # back to the recorded roles: offered again, not confirmed assert nb._quick_offer is not None and nb.confirmed_mapping is None + + +def test_a_preview_that_cannot_be_read_is_not_confirmed_by_one_click(exp, monkeypatch): # noqa: F811 + # Regression (batch 34b BLOCKER 3): the headers are readable but the pixels are not; the + # card still offered the click, and the click set "the preview looks right" on no preview. + from morphobricks_preprocessing import notebook_axis_preview + from morphobricks_preprocessing.planes import PlaneReadError + + def broken(*_args, **_kwargs): + raise PlaneReadError("the pixels cannot be decoded") + + monkeypatch.setattr(notebook_axis_preview, "read_plane", broken) + nb = stepper(*exp) + assert nb._quick_offer is not None and ui.is_visible(nb.quick_card) + assert nb.preview_image.value == b"" and nb.quick_image.value == b"" + assert not ui.is_visible(nb.quick_image) # no image is shown + assert "the pixels cannot be decoded" in nb.quick_caption.value # the failure stays visible + assert "This plane cannot be shown with these roles." in nb.quick_caption.value + assert nb.quick_confirm.disabled + assert not nb._confirm_quick_axes() # even a click that reaches it is refused + assert nb.confirmed_mapping is None and not nb.preview_checked.value + assert "the pixels cannot be decoded" in nb.quick_caption.value + + +def test_a_preview_that_fails_later_withdraws_the_click(exp, monkeypatch): # noqa: F811 + from morphobricks_preprocessing import notebook_axis_preview + from morphobricks_preprocessing.planes import PlaneReadError + + nb = stepper(*exp) + assert not nb.quick_confirm.disabled # a preview of these roles is shown + monkeypatch.setattr( + notebook_axis_preview, + "read_plane", + lambda *_a, **_k: (_ for _ in ()).throw(PlaneReadError("gone")), + ) + nb._render_preview() # e.g. another plane chosen: it cannot be read + assert not nb._confirm_quick_axes() + assert nb.confirmed_mapping is None and not nb.preview_checked.value + + +def test_a_header_change_does_not_offer_the_earlier_run_s_roles(exp): # noqa: F811 + # Regression (batch 34b FILE 3): headers rewritten with other channel names (same files, + # shape and count) still offered "the roles the earlier run recorded". + from channel_images import write_channels + from test_notebook_check_save import KINDS + + first = stepper(*exp) + first.quick_confirm.click() + answer_all(first) + assert first.save_experiment() + assert first.run().written + root, _, _ = exp + write_channels(root / "ctrl/rep1/a.tif", ["Actin", "DAPI", "Ch3"], KINDS, seed=1) + nb = stepper(*exp) + assert nb._saved_axes() is None + assert "the earlier run of this experiment recorded" not in nb.quick_text.value + assert "image headers changed since the experiment was saved" in nb.quick_text.value + assert nb.confirmed_mapping is None and not nb.preview_checked.value # never automatic + + +def test_an_image_changed_since_the_folder_check_is_not_previewed_or_confirmed(exp): # noqa: F811 + # Regression (batch 34b, with batch 33's bound pictures): the axis preview read the file as + # it is now, not as the folder check read its header, and one click vouched for it. + import os + + from channel_images import write_channels + from test_notebook_check_save import KINDS + + nb = stepper(*exp) + rel = nb.preview_file.value + assert nb.preflight.header_fingerprints.get(rel) is not None # the check bound the header + assert not nb.quick_confirm.disabled + root, _, _ = exp + path = root / rel + write_channels(path, ["DAPI", "Actin", "Ch3"], KINDS, seed=7) # same name, header, shape + stat = path.stat() + os.utime(path, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**10)) + nb._refresh_quick_axes() + assert nb.preview_image.value == b"" and not ui.is_visible(nb.quick_image) + assert "changed since the folder check read it" in nb.quick_caption.value + assert nb.quick_confirm.disabled + assert not nb._confirm_quick_axes() + assert nb.confirmed_mapping is None and not nb.preview_checked.value + + +def test_an_image_changed_during_the_preview_read_is_not_shown(exp, monkeypatch): # noqa: F811 + # Review of #255: the axis preview checks the bound fingerprint after the read too, so a + # file rewritten while its plane was read is a failed preview, never the image checked. + import os + + from channel_images import write_channels + from morphobricks_preprocessing import notebook_axis_preview + from test_notebook_check_save import KINDS + + nb = stepper(*exp) + rel = nb.preview_file.value + path = exp[0] / rel + real = notebook_axis_preview.read_plane + + def read_then_rewrite(*args, **kwargs): + plane = real(*args, **kwargs) + write_channels(path, ["DAPI", "Actin", "Ch3"], KINDS, seed=9) + stat = path.stat() + os.utime(path, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**10)) + return plane + + monkeypatch.setattr(notebook_axis_preview, "read_plane", read_then_rewrite) + nb._refresh_quick_axes() + assert nb.preview_image.value == b"" and nb._preview_drawn is None + assert "changed since the folder check read it" in nb.quick_caption.value + assert nb.quick_confirm.disabled and not nb._confirm_quick_axes() + assert nb.confirmed_mapping is None and not nb.preview_checked.value diff --git a/tests/test_notebook_row_fold.py b/tests/test_notebook_row_fold.py index 5cd7a15d..79760e40 100644 --- a/tests/test_notebook_row_fold.py +++ b/tests/test_notebook_row_fold.py @@ -172,3 +172,52 @@ def lacking(applied, answers): nb._refresh_check_save() assert nb._replicate_fold.count == 0 assert "A_SCOPE_INCOMPLETE" in _text(nb.replicates_box) + + +def test_a_header_rewritten_under_the_same_files_keeps_its_row_open(tmp_path): + # Regression (batch 34b BLOCKER 4): the reopen reconciled image counts only, so headers + # rewritten with swapped channel names (same files, shape and count) folded as "nothing + # changed" while the run's checks refused R_HEADER_CONTRADICTION. Real workflow, no fake. + from channel_images import write_channels + from test_notebook_check_save import FOLDERS, KINDS + + root, masks, out = tmp_path / "exp", tmp_path / "masks", tmp_path / "out" + for i, folder in enumerate(FOLDERS): + add_folder(root, masks, folder, i + 1) + first = stepper(root, masks, out) + answer_all(first) + assert first.save_experiment() + write_channels(root / "ctrl/rep1/a.tif", ["Actin", "DAPI", "Ch3"], KINDS, seed=1) + nb = stepper(root, masks, out) + reasons = {u.folders: u.reasons for u in nb._published().reconciliation.units} + assert reasons.pop(UnitFolders("ctrl", "rep1", "single")) == ("channels_changed",) + assert set(reasons.values()) == {()} # produced by the real path, for that unit only + assert nb._reopen_units()[("ctrl", "rep1", "single")] == CHANGED + assert not nb._folded and not ui.is_visible(nb.fold_bar) + assert ui.is_visible(nb.check_card) + assert "headers changed: ctrl/rep1" in nb.folder_changes.value # the unit is named + assert "R_HEADER_CONTRADICTION" in nb.checks.value # the refusal is visible + assert "changed since saved" in _text(nb.replicates_box) # rep1's block stays open + assert "can start" not in nb.save_status.value and "nothing changed" not in nb.saved_line.value + assert nb.run_button.disabled + + +def test_a_refusal_of_the_run_s_checks_unfolds_an_unchanged_reopen(tmp_path): + # Regression (batch 34b BLOCKER 4, second layer): a refusal the reconciliation cannot see + # (a mask type added beside the saved folders) was folded behind "nothing changed". + from channel_images import write_masks + from test_notebook_check_save import FOLDERS + + root, masks, out = tmp_path / "exp", tmp_path / "masks", tmp_path / "out" + for i, folder in enumerate(FOLDERS): + add_folder(root, masks, folder, i + 1) + first = stepper(root, masks, out) + answer_all(first) + assert first.save_experiment() + write_masks(masks, {"ctrl/rep1/a.tif": 1}, {"Cells": "labels"}) + nb = stepper(root, masks, out) + assert not nb._published().changed # the folders are as saved + assert not nb._published_check().can_run + assert not nb._folded and not ui.is_visible(nb.fold_bar) and ui.is_visible(nb.check_card) + assert "nothing changed" not in nb.saved_line.value and "can start" not in nb.save_status.value + assert nb.run_button.disabled diff --git a/tests/test_notebook_thumbnails.py b/tests/test_notebook_thumbnails.py index f66ed60c..ec11901a 100644 --- a/tests/test_notebook_thumbnails.py +++ b/tests/test_notebook_thumbnails.py @@ -15,7 +15,7 @@ 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 +from morphobricks_preprocessing.preview_render import MAGENTA, display_panel from test_notebook_check_save import decode # --- the crop (pure) ------------------------------------------------------------------------------ @@ -234,3 +234,27 @@ def test_the_crop_says_why_it_is_missing_or_where_it_is_when_nothing_varies(): found = tile_pictures([flat], False, 256) assert found.crop_box == (0, 128, 0, 128) and "does not vary" in found.crop_text assert "varies most" not in found.crop_text + + +def test_non_finite_pixels_inside_and_outside_the_crop_are_flagged_and_do_not_move_it(): + # Regression (batch 34b FILE 1): magenta non-finite pixels counted as detail, so a block of + # NaN outside the busy square took the crop; one inside the crop was not named, and the + # thumbnail's note named its non-finite pixels twice. + plane = _busy_at(256, 640).astype(np.float32) + plane[0:128, 0:128:8] = np.nan # outside the busy square, on the thumbnail's grid + plane[301, 701] = np.inf # inside it, off the thumbnail's grid: in the detail only + plane[300, 700] = -np.inf # inside it, on the thumbnail's grid + before = plane.copy() + runs = [tile_pictures([plane], False, 256) for _ in range(3)] + assert np.array_equal(plane, before, equal_nan=True) # the source array is never changed + found = runs[0] + assert all(r == found for r in runs) # deterministic: the same pictures every time + assert found.crop_box == (256, 384, 640, 768) # the busy square, not the NaN block + assert found.note == "513 non-finite pixels shown in magenta" # named once + assert "; 2 non-finite pixels in the detail, shown in magenta" in found.crop_text + thumb, crop = decode(found.png), decode(found.crop_png) + assert (thumb[0, 0] == MAGENTA).all() and (thumb[75, 175] == MAGENTA).all() + native = crop[::2, ::2] + assert (native[301 - 256, 701 - 640] == MAGENTA).all() + assert (native[300 - 256, 700 - 640] == MAGENTA).all() + assert int((native == MAGENTA).all(axis=2).sum()) == 2 diff --git a/tests/test_reconcile_observe.py b/tests/test_reconcile_observe.py index 9200152e..7ef87d73 100644 --- a/tests/test_reconcile_observe.py +++ b/tests/test_reconcile_observe.py @@ -26,3 +26,64 @@ def test_excluded_images_are_not_counted_and_an_emptied_folder_is_not_a_unit(tmp discover_experiment(tmp_path), excluded={"CTRL/rep1/a.tif", "DrugA/rep1/a.tif"} ) assert seen == {UnitFolders("CTRL", "rep1", "single"): ObservedUnit(1)} + + +def test_the_headers_read_give_each_unit_its_channel_evidence(tmp_path): + # Regression (batch 34b BLOCKER 4): only image counts were observed, so a header rewritten + # under the same files was never a channel change. + from channel_images import write_channels + from morphobricks_preprocessing.metadata import inspect_image + + kinds = ["blobs", "filaments"] + write_channels(tmp_path / "CTRL" / "rep1" / "a.tif", ["DAPI", "Actin"], kinds) + write_channels(tmp_path / "CTRL" / "rep1" / "b.tif", ["DAPI", "Actin"], kinds, seed=2) + write_channels(tmp_path / "DrugA" / "rep1" / "a.tif", ["DAPI", "Actin"], kinds) + write_channels(tmp_path / "DrugA" / "rep1" / "b.tif", ["Actin", "DAPI"], kinds, seed=2) + write_tiff(tmp_path / "Plain" / "rep1" / "a.tif") + discovery = discover_experiment(tmp_path) + metas = [inspect_image(tmp_path / rel, rel) for rel in discovery.images] + seen = observe_folders(discovery, metadata=metas) + assert seen[UnitFolders("CTRL", "rep1", "single")] == ObservedUnit( + 2, 2, ("DAPI", "Actin"), rgb=False + ) + assert not seen[UnitFolders("DrugA", "rep1", "single")].uniform # its headers disagree + plain = seen[UnitFolders("Plain", "rep1", "single")] + assert plain.header_names is None and plain.uniform + assert observe_folders(discovery) == {k: ObservedUnit(v.image_count) for k, v in seen.items()} + + +def test_a_colour_header_flag_changed_under_the_same_files_is_a_channel_change(tmp_path): + # Regression (review of #255): the RGB flag was not observed, so an RGB unit rewritten as a + # plain three-channel image (same files, count and channel count) reconciled as unchanged + # while the run's checks refuse IMAGE_TYPE_CONTRADICTS_HEADER. + import numpy as np + import tifffile + + from morphobricks_preprocessing.description import description_from_bytes + from morphobricks_preprocessing.description_reconcile import CHANNELS_CHANGED, reconcile + from morphobricks_preprocessing.metadata import inspect_image + from spec_pin import fixtures + + rgb = next(f for f in fixtures("valid") if f.name.startswith("v28_")) + published = description_from_bytes(rgb.data) + for condition in ("CTRL", "DrugA"): + for repeat in ("rep1", "rep2"): + for i in range(8): + path = tmp_path / condition / repeat / f"{i}.tif" + path.parent.mkdir(parents=True, exist_ok=True) + tifffile.imwrite(path, np.zeros((8, 8, 3), np.uint8), photometric="rgb") + + def seen(): + discovery = discover_experiment(tmp_path) + metas = [inspect_image(tmp_path / rel, rel) for rel in discovery.images] + return observe_folders(discovery, metadata=metas) + + assert reconcile(published, seen()).reopened == {} + for i in range(8): + plain = np.zeros((3, 8, 8), np.uint8) + path = tmp_path / "CTRL" / "rep1" / f"{i}.tif" + tifffile.imwrite(path, plain, photometric="minisblack", metadata={"axes": "CYX"}) + observed = seen() + assert observed[UnitFolders("CTRL", "rep1", "single")].rgb is False + assert observed[UnitFolders("CTRL", "rep2", "single")].rgb is True + assert reconcile(published, observed).reopened == {"rep1": (CHANNELS_CHANGED,)}