From 1ff73437b1242e8c718fa193adf48e3df9cb3b32 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 20:25:23 +0000 Subject: [PATCH 1/2] Design: confirming the design is one answer, one refresh, with a busy state Owner bug on the v0.5.2 notebook (12 November 2026): *Looks right: confirm the design* seemed to do nothing, or was very slow. Cause: the click saved the answers twice and refreshed every page three times; each refresh rebuilt one image-type row per folder (the Channels page) and every replicate box of Check and save, twice. The cost grew with the number of folders (headless: 1.0 s for 36 images, 5.3 s for 384, 7.7 s for 900; about 2,400 to 14,500 widgets sent to the page per click). - The IDs and the "no comparison" answers are saved as one answer (each ID is still tried on its own first, so every refusal is listed; all or none). - ImageTypeRows and the replicate boxes are rebuilt only when what they show changed (folders, saved types and notes, pictures, reopen statuses, views). - The button is disabled and says "Confirming the design..." with a working line before any work; the click ends with the result or the reason; an unexpected failure is a red card naming the error, with the traceback in Technical details, and the button is given back. - The replicate boxes move to notebook_replicates.py (600-line module rule). After: 0.14 s, 0.29 s, 0.46 s; about 250 to 450 widgets. Nothing recorded changed: the same answers are saved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a --- AGENTS.md | 9 +- CHANGELOG.md | 15 ++ pyproject.toml | 1 + .../notebook_channels_type.py | 48 ++++- .../notebook_check_save.py | 109 +----------- .../notebook_description.py | 10 +- .../notebook_design_step.py | 67 +++++-- .../notebook_replicates.py | 166 ++++++++++++++++++ tests/test_notebook_design_confirm_speed.py | 143 +++++++++++++++ tests/test_notebook_row_fold.py | 6 +- 10 files changed, 445 insertions(+), 129 deletions(-) create mode 100644 src/morphobricks_preprocessing/notebook_replicates.py create mode 100644 tests/test_notebook_design_confirm_speed.py diff --git a/AGENTS.md b/AGENTS.md index 014b700..782cd75 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -305,8 +305,10 @@ src/morphobricks_preprocessing/ comparable questions and CSV writes, the IDs toggle and the Image type card, and reopen from the published file notebook_check_save.py the Check and save screen (sentences, one "same way?" per replicate, - Yes, remove, the save click, the folded reopen); notebook_check_view.py its - HTML; description_review.py (pure) the sentences and the CSV row diff; + Yes, remove, the save click, the folded reopen); notebook_replicates.py its + replicate boxes (rebuilt only when what they show changed); + notebook_check_view.py its HTML; description_review.py (pure) the + sentences and the CSV row diff; description_candidate.py the checks on the exact bytes; description_reopen.py the published file against the folders; published_answers.py (pure) the published file as draft answers, retired IDs, reopened replicates @@ -1099,7 +1101,8 @@ jupyter lab notebooks/MorphoBricks_Preprocessing.ipynb [docs/archive/plan-milestones-v0.1-v0.4.md](docs/archive/plan-milestones-v0.1-v0.4.md); the older status log of this file is in [docs/archive/agents-status-history-v0.1-v0.3.md](docs/archive/agents-status-history-v0.1-v0.3.md). -- **Unreleased:** none yet; later changes go in a new "Unreleased" section of the changelog. +- **Unreleased:** the faster *Looks right: confirm the design* click (see the changelog's + "Unreleased" section). - **Scope:** 2-D per plane (a Z-stack is refused as `z_stack_not_supported`; since v0.5.2 the notebook saves Z-stacks as recorded projections); acquisitions may have different channel lists (v0.5.2, Stage 3 S3.3: manifest version 5, histogram table schema version 5; units of diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cd532a..6268cee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ The design is `docs/restart/preprocessing-design-v0.4.md` in MorphoFeature, on b are in [docs/plan.md](docs/plan.md) §12. (The v0.3 to v0.4 migration guide was removed in v0.5.1: no run before v0.5.1 is read.) +## Unreleased + +### Fixed + +- **Looks right: confirm the design** seemed to do nothing, or was very slow (owner, v0.5.2 + Colab test). The click saved the answers twice and refreshed every page three times, each + time rebuilding one image-type row per folder and every replicate box of *Check and save*, + so it grew with the number of folders (headless: 1.0 s for 36 images, 5.3 s for 384, 7.7 s + for 900; about 2,400 to 14,500 widgets sent to the page). It now saves one answer, refreshes + once, and rebuilds the image-type rows and the replicate boxes only when what they show + changed (0.14 s, 0.29 s and 0.46 s; about 250 to 450 widgets). The button is disabled and + says *Confirming the design…* with a working line before any work, and the click ends with + the result, or with the reason it was not confirmed; an unexpected failure is said there, + the full report in Technical details. Nothing recorded changed. + ## v0.5.2 (12 November 2026) Acquisitions may now have different channels (Stage 3 S3.3, owner decision of 11 November diff --git a/pyproject.toml b/pyproject.toml index 3799c07..9bf1884 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -179,6 +179,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.notebook_masks_screen" = "ui" "morphobricks_preprocessing.notebook_check_save" = "ui" "morphobricks_preprocessing.notebook_check_view" = "ui" +"morphobricks_preprocessing.notebook_replicates" = "ui" "morphobricks_preprocessing.notebook_described" = "ui" "morphobricks_preprocessing.notebook_mask_question" = "ui" "morphobricks_preprocessing.notebook_mask_step" = "ui" diff --git a/src/morphobricks_preprocessing/notebook_channels_type.py b/src/morphobricks_preprocessing/notebook_channels_type.py index f6e3561..37baa77 100644 --- a/src/morphobricks_preprocessing/notebook_channels_type.py +++ b/src/morphobricks_preprocessing/notebook_channels_type.py @@ -189,10 +189,21 @@ def answered(self) -> int: return sum(1 for u in self.types.layouts if u.usable and self.types.saved(u.key)) def refresh(self) -> None: - """Rebuild the rows from the saved answers (without pictures while they are folded).""" - self.types.open(self.types.layouts) # a fresh set of cells, built from the answers - self.pictured = not self.folded() - self._build_rows() + """Rebuild the rows from the saved answers (without pictures while they are folded). + + The rows are rebuilt only when something they show changed: the folders, a saved + type, the pictures or the saved experiment's statuses. Every answer elsewhere (a + name, a pairing, the design's confirmation) refreshes this page too, and rebuilding + one row per folder each time made those clicks slow on large experiments (owner, + 12 November 2026: *Looks right: confirm the design* seemed to do nothing). + """ + pictured = not self.folded() + shown = self._shown(pictured) + if not self._same_rows(shown): + self.types.open(self.types.layouts) # a fresh set of cells, built from the answers + self.pictured = pictured + self._build_rows() + self._built = (self.types.radios, shown) self.problem.value = self.types.message usable = sum(1 for u in self.types.layouts if u.usable) done = self.answered @@ -202,6 +213,35 @@ def refresh(self) -> None: else ui.status("warn", f"Not finished: {usable - done} image type(s) to choose.") ) + def _shown(self, pictured: bool) -> tuple: + """Return what the rows show, to tell whether they must be rebuilt.""" + layouts = self.types.layouts + units = self.reopen() + return ( + layouts, + pictured, + self._thumbs() if pictured else None, + tuple((self.types.saved(u.key), self.types.note(u)) for u in layouts), + None if units is None else tuple(sorted(units.items())), + ) + + def _same_rows(self, shown: tuple) -> bool: + """Return True when the rows on the page were built for exactly ``shown``. + + Objects (the layouts, the pictures) are compared by identity, the answers by value; + cells replaced since (:meth:`ImageTypeColumn.open`) are never kept. + """ + built = getattr(self, "_built", None) + if built is None or built[0] is not self.types.radios: + return False + before = built[1] + return ( + before[0] is shown[0] + and before[1] == shown[1] + and before[2] is shown[2] + and before[3:] == shown[3:] + ) + def unfolded(self) -> None: """Rebuild the rows with their pictures now that they are shown, unless they have them.""" if not self.pictured and not self.folded() and ui.is_visible(self.widget): diff --git a/src/morphobricks_preprocessing/notebook_check_save.py b/src/morphobricks_preprocessing/notebook_check_save.py index 735429a..1eb84c5 100644 --- a/src/morphobricks_preprocessing/notebook_check_save.py +++ b/src/morphobricks_preprocessing/notebook_check_save.py @@ -18,7 +18,6 @@ from __future__ import annotations -import html from dataclasses import replace from datetime import date from pathlib import Path @@ -31,7 +30,7 @@ from .description import DescriptionError from .description_candidate import CandidateCheck, check_candidate from .description_removal import Removal, reconfirmations -from .description_review import comparisons, replicates, short_names +from .description_review import comparisons, short_names from .draft import DraftError from .draft_answers import ( RepeatAnswer, @@ -43,28 +42,20 @@ from .fingerprint import changed_since_check from .mask_answers import masks_unanswered from .name_issues import NAME_CODES -from .notebook_channel_thumbs import Thumb from .notebook_check_view import ( - SAME_WAY, - SAME_WAY_HELP, blocked_html, changes_html, checks_html, comparisons_html, - member_html, - missing_html, names_html, removal_text, - replicate_title, run_stop_html, saved_html, todo_html, ) from .notebook_description import _with_ids from .notebook_render import blocked_line -from .notebook_row_fold import RowFold, row_status -from .notebook_thumb_tile import ThumbTile, show_detail -from .preview_crop import TilePictures +from .notebook_replicates import _ReplicateBoxes from .published_answers import kept_date from .run import OutputLockedError from .run.description_file import PublicationRefused @@ -79,15 +70,7 @@ FOLDED = ("design_card", "names_card", "references_card", "channels_box", "masks_box") -def _enlarge(detail: widgets.VBox, title: str, found: TilePictures | Thumb) -> None: - """Show a folder's enlarged picture in ``detail``, or why there is none.""" - if isinstance(found, Thumb): - show_detail(detail, title, None, found.note) - else: - show_detail(detail, title, found, "") - - -class _CheckSaveScreen: +class _CheckSaveScreen(_ReplicateBoxes): """The *Check and save* screen, its save click and the folded reopening.""" def _build_check_save(self) -> None: @@ -319,92 +302,6 @@ def _candidate_check(self, answers: object) -> CandidateCheck | None: _, confirmations = self._publication_terms(state, candidate) return check_candidate(candidate, self.preflight, roles, published, confirmations) - 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, counts = self._reopen_units(), self._channel_counts() - state = self._published() - saved_dates = ( - {} - if state is None - else {key: r.confirmed_on for key, r in state.description.repeats.items()} - ) - views = replicates(applied, answers) - for view in views: - toggle = ui.ConfirmToggle( - SAME_WAY, - SAME_WAY_HELP, - "Yes, the same way", - f"Confirmed on {view.confirmed_on}: one replicate." if view.confirmed_on else "", - "Undo", - ) - toggle.set(view.confirmed_on is not None, notify=False) - toggle.observe(lambda value, label=view.label: self._same_way(label, value)) - toggles[view.label] = toggle - members = [ - widgets.HBox( - [ - self._member_picture( - (m.condition, m.source_repeat), self.replicates_detail - ), - widgets.HTML(member_html(m, labels, counts)), - ] - ) - for m in view.members - ] - body = [widgets.HTML(replicate_title(view)), *members, widgets.HTML(missing_html(view))] - box = widgets.VBox( - [*body, toggle.widget], - layout=widgets.Layout( - border="1px solid #E2E8F0", padding="8px 12px", margin="0 0 10px 0" - ), - ) - keys = [u.key for m in view.members for u in m.units] - # A box with a warning (an acquisition lacking a label) never folds, nor one whose - # date is not the published one (publication's R_CONFIRMED_ON_CHANGED names it). - settled = ( - view.confirmed_on is not None - and not view.missing - and view.confirmed_on == saved_dates.get(view.label, view.confirmed_on) - ) - boxes.append((box, row_status(units, keys, settled))) - self.same_way = toggles - shown = ( - self._channel_thumbs, - [[(m.condition, m.source_repeat) for m in v.members] for v in views], - ) - if shown != getattr(self, "_replicates_shown", None): # other folders or pictures (#252) - self.replicates_detail.children = [] # an Enlarge of the boxes before is closed - self._replicates_shown = shown - if not hasattr(self, "_replicate_fold"): - self._replicate_fold = RowFold("replicate") - self.replicates_box.children = self._replicate_fold.arrange(boxes) - - def _member_picture(self, key: tuple[str, str], detail: widgets.VBox) -> widgets.Widget: - """Return the picture of a folder's first image (channel 1, or in colour when RGB). - - A large image gets its crop at native pixels beside it; **Enlarge** shows both in - ``detail``. - """ - thumbs, layouts = self._channel_thumbs, self._channel_layouts - layout = next((u for u in layouts if u.key[:2] == key), None) - if thumbs is None or layout is None: - return widgets.HTML( - "
picture once the axis roles are " - "confirmed
" - ) - colour = self.channel_types.shown(layout) == "rgb" - found = thumbs.tile(layout, colour) - if isinstance(found, Thumb): - return ThumbTile(None, problem=found.note, width="120px").widget - title = f"{html.escape('/'.join(key))} · {html.escape(layout.image or '')}" - return ThumbTile( - found, - colour=colour, - enlarge=lambda: _enlarge(detail, title, thumbs.enlarged(layout, colour)), - width="120px", - ).widget - def _folder_cell(self, key: tuple[str, str], text: str) -> widgets.Widget: """Return a folder of the pairing card, with its picture.""" return widgets.VBox([widgets.HTML(text), self._member_picture(key, self.pairing_detail)]) diff --git a/src/morphobricks_preprocessing/notebook_description.py b/src/morphobricks_preprocessing/notebook_description.py index 26b0ef3..51849e9 100644 --- a/src/morphobricks_preprocessing/notebook_description.py +++ b/src/morphobricks_preprocessing/notebook_description.py @@ -313,6 +313,14 @@ def _wanted_ids(self) -> dict[tuple[str, str], str]: def _confirm_ids(self) -> bool: """Adopt every ID as shown, or none: each refusal is listed with the reader's reason.""" wanted = self._wanted_ids() + return self._adopt_ids(wanted, _with_ids(self.draft_answers, wanted)) + + def _adopt_ids(self, wanted: dict[tuple[str, str], str], answers: DraftAnswers) -> bool: + """Save ``answers`` (which adopt ``wanted``) in one answer, unless an ID is refused. + + Each ID is first tried on its own, so every refusal is listed with the reader's + reason; then ``answers`` are saved once. Nothing is adopted when anything is refused. + """ problems = [] for (kind, key), text in wanted.items(): try: @@ -320,7 +328,7 @@ def _confirm_ids(self) -> bool: except DraftError as exc: problems.append((f"{KIND_NAMES[kind].capitalize()} {key}", exc)) if not problems: - if self._answer(_with_ids(self.draft_answers, wanted)): + if self._answer(answers): return True problems.append(("These IDs together", self._refusal)) self._names_refusal = refusal_card( diff --git a/src/morphobricks_preprocessing/notebook_design_step.py b/src/morphobricks_preprocessing/notebook_design_step.py index 7f0234b..38d008b 100644 --- a/src/morphobricks_preprocessing/notebook_design_step.py +++ b/src/morphobricks_preprocessing/notebook_design_step.py @@ -24,17 +24,19 @@ from __future__ import annotations import html +import traceback import ipywidgets as widgets from . import notebook_style as ui -from .notebook_description import _updated +from .notebook_description import _updated, _with_ids from .notebook_description_view import pill from .notebook_render import blocked_line from .notebook_values import natural_key from .validation import plural CONFIRM_LABEL = " Looks right: confirm the design" +CONFIRMING = "Confirming the design…" HELP = ( "Rows are conditions, columns are biological repeats, and a cell is the folder of that " "repeat. The names are the folder names and nothing is compared, until you say otherwise." @@ -81,7 +83,7 @@ def _build_design_step(self) -> None: ], visible=False, ) - self.design_confirm.on_click(lambda _: self._confirm_design()) + self.design_confirm.on_click(lambda _: self._design_clicked()) self.design_change.on_click(lambda _: self._change_design()) # State ------------------------------------------------------------------------------- @@ -110,9 +112,46 @@ def _in_steps(self) -> bool: # The click --------------------------------------------------------------------------- + def _design_clicked(self) -> None: + """Run the confirmation behind a busy button, then say what came of it. + + Owner (12 November 2026): the click must never look as if nothing happened. The button + is disabled and says *Confirming…* and a working line shows before any of the work + (the widgets' state is sent to the page as it is set); the result line then says that + the design is confirmed, or why not. An unexpected failure is said there too, the full + report in Technical details, and the button is given back. + """ + button = self.design_confirm + if button.disabled: + return + saved = (button.description, button.icon) + self._busy(button, CONFIRMING) + self.design_result.value = ui.working(CONFIRMING) + try: + self._confirm_design() + except Exception as exc: # unexpected failures still get a readable message + self.log.lines.append(traceback.format_exc()) + self._show_log() + self._design_problem = ui.status( + "error", + "The design was not confirmed: an unexpected problem stopped it.", + [ + f"{type(exc).__name__}: {exc}", + "Open Technical details for the full report, then click the button again.", + ], + ) + finally: + button.description, button.icon = saved + button.disabled = False + self._refresh_design_step() + def _confirm_design(self) -> bool: """Adopt the design as shown: every ID, and "no comparison" where nothing was chosen. + One answer, saved once: the IDs and the comparisons together, so the pages that follow + the answers are refreshed once (they were refreshed three times, which made the click + slow on large experiments). + Returns ------- bool @@ -122,23 +161,27 @@ def _confirm_design(self) -> bool: if self.draft is None or not self._in_steps(): return False self._names_refusal = self._draft_note = self._design_problem = "" - if not self._confirm_ids(): - self._design_problem = self._names_refusal - self.design_edit.selected_index = 0 - self._refresh_design_step() - return False - answers = self.draft_answers + wanted = self._wanted_ids() + answers = _with_ids(self.draft_answers, wanted) if len(self.draft.conditions) > 1: for condition in sorted(self.draft.conditions): if self._chosen_reference(condition) is None: answers = _updated(answers, "conditions", condition, reference="") - if answers != self.draft_answers and not self._answer(answers): - self._design_problem = ui.status("error", "Not taken.", [str(self._refusal)]) + if not self._adopt_ids(wanted, answers): + self._design_problem = self._names_refusal + self.design_edit.selected_index = 0 self._refresh_design_step() return False - self._refresh_description() self._refresh_design_status() - return self._design_confirmed() + if not self._design_confirmed(): + self._design_problem = ui.status( + "warn", + "The design is not confirmed yet.", + ["Read the names and comparisons under Edit names or pairing, then click again."], + ) + self._refresh_design_step() + return False + return True def _change_design(self) -> None: """Take the IDs back so the names can be edited (as *Change the IDs* does).""" diff --git a/src/morphobricks_preprocessing/notebook_replicates.py b/src/morphobricks_preprocessing/notebook_replicates.py new file mode 100644 index 0000000..a868007 --- /dev/null +++ b/src/morphobricks_preprocessing/notebook_replicates.py @@ -0,0 +1,166 @@ +"""The replicate boxes of *Check and save*: folders, pictures, channels and "same way?". + +One box per replicate (biological repeat): its folders with a picture of each folder's first +image, the channels, what an acquisition lacks, and the replicate's own comparability question +(the N35 answer, :data:`.notebook_check_view.SAME_WAY`). Moved out of +:mod:`.notebook_check_save` (code rules: 600 lines a module). + +Every answer anywhere in the notebook refreshes *Check and save*, so the boxes are rebuilt only +when something they show changed (owner, 12 November 2026: *Looks right: confirm the design* +rebuilt every box twice and seemed to do nothing on large experiments). +""" + +from __future__ import annotations + +import html + +import ipywidgets as widgets + +from . import notebook_style as ui +from .description_review import replicates +from .notebook_channel_thumbs import Thumb +from .notebook_check_view import ( + SAME_WAY, + SAME_WAY_HELP, + member_html, + missing_html, + replicate_title, +) +from .notebook_row_fold import RowFold, row_status +from .notebook_thumb_tile import ThumbTile, show_detail +from .preview_crop import TilePictures + + +def _enlarge(detail: widgets.VBox, title: str, found: TilePictures | Thumb) -> None: + """Show a folder's enlarged picture in ``detail``, or why there is none.""" + if isinstance(found, Thumb): + show_detail(detail, title, None, found.note) + else: + show_detail(detail, title, found, "") + + +def _same(before: tuple, now: tuple) -> bool: + """Return True when two states of the replicate boxes are the same. + + The pictures and the folders' layouts are compared by identity, the rest by value. + """ + return before[0] is now[0] and before[1] is now[1] and before[2:] == now[2:] + + +class _ReplicateBoxes: + """One box per replicate on *Check and save*, rebuilt only when what it shows changed.""" + + 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, counts = self._reopen_units(), self._channel_counts() + state = self._published() + saved_dates = ( + {} + if state is None + else {key: r.confirmed_on for key, r in state.description.repeats.items()} + ) + views = replicates(applied, answers) + shown_as = self._replicates_state(views, labels, counts, units, saved_dates) + built = getattr(self, "_replicates_built", None) + if built is not None and built[0] is self.same_way and _same(built[1], shown_as): + return # the boxes on the page show exactly this (each answer refreshes the screen) + for view in views: + toggles[view.label], box = self._replicate_box(view, labels, counts) + keys = [u.key for m in view.members for u in m.units] + # A box with a warning (an acquisition lacking a label) never folds, nor one whose + # date is not the published one (publication's R_CONFIRMED_ON_CHANGED names it). + settled = ( + view.confirmed_on is not None + and not view.missing + and view.confirmed_on == saved_dates.get(view.label, view.confirmed_on) + ) + boxes.append((box, row_status(units, keys, settled))) + self.same_way = toggles + shown = ( + self._channel_thumbs, + [[(m.condition, m.source_repeat) for m in v.members] for v in views], + ) + if shown != getattr(self, "_replicates_shown", None): # other folders or pictures (#252) + self.replicates_detail.children = [] # an Enlarge of the boxes before is closed + self._replicates_shown = shown + if not hasattr(self, "_replicate_fold"): + self._replicate_fold = RowFold("replicate") + self.replicates_box.children = self._replicate_fold.arrange(boxes) + self._replicates_built = (toggles, shown_as) + + def _member_picture(self, key: tuple[str, str], detail: widgets.VBox) -> widgets.Widget: + """Return the picture of a folder's first image (channel 1, or in colour when RGB). + + A large image gets its crop at native pixels beside it; **Enlarge** shows both in + ``detail``. + """ + thumbs, layouts = self._channel_thumbs, self._channel_layouts + layout = next((u for u in layouts if u.key[:2] == key), None) + if thumbs is None or layout is None: + return widgets.HTML( + "
picture once the axis roles are " + "confirmed
" + ) + colour = self.channel_types.shown(layout) == "rgb" + found = thumbs.tile(layout, colour) + if isinstance(found, Thumb): + return ThumbTile(None, problem=found.note, width="120px").widget + title = f"{html.escape('/'.join(key))} · {html.escape(layout.image or '')}" + return ThumbTile( + found, + colour=colour, + enlarge=lambda: _enlarge(detail, title, thumbs.enlarged(layout, colour)), + width="120px", + ).widget + + def _replicate_box( + self, view: object, labels: list[str], counts: object + ) -> tuple[ui.ConfirmToggle, widgets.VBox]: + """Return one replicate's "same way?" question and its box (folders, pictures).""" + toggle = ui.ConfirmToggle( + SAME_WAY, + SAME_WAY_HELP, + "Yes, the same way", + f"Confirmed on {view.confirmed_on}: one replicate." if view.confirmed_on else "", + "Undo", + ) + toggle.set(view.confirmed_on is not None, notify=False) + toggle.observe(lambda value, label=view.label: self._same_way(label, value)) + members = [ + widgets.HBox( + [ + self._member_picture((m.condition, m.source_repeat), self.replicates_detail), + widgets.HTML(member_html(m, labels, counts)), + ] + ) + for m in view.members + ] + body = [widgets.HTML(replicate_title(view)), *members, widgets.HTML(missing_html(view))] + box = widgets.VBox( + [*body, toggle.widget], + layout=widgets.Layout( + border="1px solid #E2E8F0", padding="8px 12px", margin="0 0 10px 0" + ), + ) + return toggle, box + + def _replicates_state( + self, views: tuple, labels: list[str], counts: object, units: object, dates: dict + ) -> tuple: + """Return everything the replicate boxes show, to tell whether they must be rebuilt.""" + layouts = self._channel_layouts + return ( + self._channel_thumbs, + layouts, + self._cards_show_pictures(), + tuple(self.channel_types.shown(u) for u in layouts or ()), + views, + tuple(labels), + None if counts is None else tuple(sorted(counts.items())), + None if units is None else tuple(sorted(units.items())), + tuple(sorted(dates.items())), + ) + + +__all__ = ["_ReplicateBoxes"] diff --git a/tests/test_notebook_design_confirm_speed.py b/tests/test_notebook_design_confirm_speed.py new file mode 100644 index 0000000..723d789 --- /dev/null +++ b/tests/test_notebook_design_confirm_speed.py @@ -0,0 +1,143 @@ +"""*Looks right: confirm the design* on a sized experiment (owner bug, 12 November 2026). + +On the v0.5.2 notebook the click seemed to do nothing, or was very slow. It saved the answers +twice and refreshed every page three times, each time rebuilding one image-type row per folder +and every replicate box of *Check and save*; the time grew with the number of folders. Now the +click saves one answer, refreshes once, rebuilds only rows whose content changed, shows a busy +button and a working line before any work, and ends with a result line. +""" + +import shutil +import time + +import pytest + +from channel_images import write_channels + +pytest.importorskip("ipywidgets") +pytest.importorskip("matplotlib") + +from morphobricks_preprocessing import notebook_style as ui # noqa: E402 +from morphobricks_preprocessing.notebook_channels_type import ImageTypeRows # noqa: E402 +from morphobricks_preprocessing.notebook_design_step import ( # noqa: E402 + CONFIRM_LABEL, + CONFIRMING, +) +from morphobricks_preprocessing.notebook_ui import PreprocessingNotebook # noqa: E402 + +CONDITIONS, REPEATS, TECHNICAL, FOVS = 4, 4, 2, 2 +BUDGET = 2.0 +"""Seconds for the click on 64 images; it took about 1.5 s here before the fix, 0.15 s after. + +Generous for slow CI runners: the call counts below are the exact check. +""" + + +@pytest.fixture +def sized(tmp_path): + """Return a stepper with its axes confirmed on 4 conditions x 4 repeats x 2 x 2 images.""" + root = tmp_path / "exp" + one = write_channels(tmp_path / "one.tif", ["DAPI", "Actin", "Ch3"], ["blobs"] * 3, size=32) + for c in range(CONDITIONS): + for r in range(REPEATS): + for t in range(TECHNICAL): + for f in range(FOVS): + path = root / f"cond{c}" / f"rep{r + 1}" / f"tech{t + 1}" / f"fov{f + 1}.tif" + path.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(one, path) + nb = PreprocessingNotebook(str(root), output_folder=str(tmp_path / "out")) + nb.welcome() + nb.steps() + nb.check_button.click() + nb.quick_confirm.click() + assert ui.is_visible(nb.design_confirm) and not nb.design_confirm.disabled + return nb + + +def _count(monkeypatch, owner, name): + calls = [] + original = getattr(owner, name) + + def spy(self, *args, **kwargs): + calls.append(name) + return original(self, *args, **kwargs) + + monkeypatch.setattr(owner, name, spy) + return calls + + +def test_the_click_saves_once_refreshes_once_and_keeps_the_unchanged_rows(sized, monkeypatch): + nb = sized + cls = type(nb) + answers = _count(monkeypatch, cls, "_answer") + refreshes = _count(monkeypatch, cls, "_refresh_description") + type_rows = _count(monkeypatch, ImageTypeRows, "_build_rows") + started = time.perf_counter() + nb.design_confirm.click() + took = time.perf_counter() - started + assert nb._design_confirmed() + assert len(answers) == 1 and len(refreshes) == 1 + assert type_rows == [] # the image types did not change: their rows are kept + assert took < BUDGET, f"the click took {took:.2f} s on 64 images" + assert "Design confirmed: 4 conditions × 4 biological repeats" in nb.design_result.value + + +def test_the_click_shows_a_busy_button_and_a_working_line_before_any_work(sized, monkeypatch): + nb = sized + seen = {} + original = type(nb)._confirm_design + + def watched(self): + button = self.design_confirm + seen.update( + disabled=button.disabled, + label=button.description, + icon=button.icon, + line=self.design_result.value, + ) + return original(self) + + monkeypatch.setattr(type(nb), "_confirm_design", watched) + nb.design_confirm.click() + assert seen["disabled"] and seen["icon"] == "spinner" + assert seen["label"].strip() == CONFIRMING and "Confirming the design" in seen["line"] + # Afterwards: the result, and the button given back (hidden once confirmed). + assert nb.design_confirm.description == CONFIRM_LABEL and nb.design_confirm.icon == "check" + assert "Design confirmed" in nb.design_result.value + assert not ui.is_visible(nb.design_confirm) and ui.is_visible(nb.design_change) + + +def test_an_unexpected_failure_is_said_and_the_button_is_given_back(sized, monkeypatch): + nb = sized + + def broken(self): + raise RuntimeError("disk on fire") + + monkeypatch.setattr(type(nb), "_confirm_design", broken) + nb.design_confirm.click() + said = nb.design_result.value + assert "The design was not confirmed" in said and "RuntimeError: disk on fire" in said + assert "Technical details" in said + assert nb.design_confirm.description == CONFIRM_LABEL and not nb.design_confirm.disabled + assert ui.is_visible(nb.design_confirm) and not nb._design_confirmed() + assert any("disk on fire" in line for line in nb.log.lines) + + +def test_a_disabled_button_does_nothing(sized, monkeypatch): + nb = sized + calls = _count(monkeypatch, type(nb), "_confirm_design") + nb.design_confirm.disabled = True + nb._design_clicked() + assert calls == [] and not nb._design_confirmed() + + +def test_a_changed_image_type_still_rebuilds_its_rows(sized): + nb = sized + rows = nb.type_rows + before = rows.rows.children + nb.type_line.apply.click() # separate channels for all folders: what the rows show changed + assert rows.rows.children is not before + assert all(radio.value == "channels" for radio in nb.channel_types.radios.values()) + kept = rows.rows.children + nb.design_confirm.click() + assert rows.rows.children is kept # the design does not change them diff --git a/tests/test_notebook_row_fold.py b/tests/test_notebook_row_fold.py index ae189a1..fa7dfd1 100644 --- a/tests/test_notebook_row_fold.py +++ b/tests/test_notebook_row_fold.py @@ -160,15 +160,15 @@ def test_a_row_with_a_warning_never_folds(reopened, monkeypatch): assert nb.type_rows.fold.count == 2 # ctrl/rep1 stays open, with its warning assert "The header says colour (RGB)" not in _text(nb.type_rows.fold.folded) - from morphobricks_preprocessing import notebook_check_save + from morphobricks_preprocessing import notebook_replicates - real = notebook_check_save.replicates + real = notebook_replicates.replicates def lacking(applied, answers): views = real(applied, answers) return [replace(v, missing=((key, "DNA"),)) if v.repeat_id == "rep1" else v for v in views] - monkeypatch.setattr(notebook_check_save, "replicates", lacking) + monkeypatch.setattr(notebook_replicates, "replicates", lacking) nb._refresh_check_save() assert nb._replicate_fold.count == 0 assert "A_SCOPE_INCOMPLETE" in _text(nb.replicates_box) From 7cdf67c3ff96c6e12e62fb9bdbcb86282ce7dd42 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 21:39:30 +0000 Subject: [PATCH 2/2] Check and save: a refused "same way" answer no longer stays shown as confirmed The replicate boxes are now kept when nothing they show changed. A "same way" click whose answer was refused changes nothing in the answers, so the clicked toggle stayed on "confirmed" although nothing was saved (on main the rebuild reset it). Drop the kept boxes when the answer is not taken. Regression test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a --- .../notebook_check_save.py | 1 + tests/test_notebook_check_save.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/morphobricks_preprocessing/notebook_check_save.py b/src/morphobricks_preprocessing/notebook_check_save.py index 1eb84c5..bd27097 100644 --- a/src/morphobricks_preprocessing/notebook_check_save.py +++ b/src/morphobricks_preprocessing/notebook_check_save.py @@ -361,6 +361,7 @@ def _same_way(self, label: str, value: bool) -> None: repeats = {**answers.repeats, label: replace(entry, confirmed_on=day)} if not self._answer(replace(answers, repeats=repeats)): self.save_status.value = ui.status("error", "Not taken.", [str(self._refusal)]) + self._replicates_built = None # the clicked box shows an answer that was not taken self._refresh_check_save() def _remove(self, condition_id: str) -> None: diff --git a/tests/test_notebook_check_save.py b/tests/test_notebook_check_save.py index 6d31ab1..22493b6 100644 --- a/tests/test_notebook_check_save.py +++ b/tests/test_notebook_check_save.py @@ -341,3 +341,19 @@ def test_replicates_are_listed_in_natural_order_rep2_before_rep10(tmp_path): assert "Replicates: rep2, rep10" in nb.short_names.value rows = [key for key in nb.id_inputs if key[0] == "repeats"] assert rows == [("repeats", "rep2"), ("repeats", "rep10")] + + +def test_a_refused_same_way_answer_is_not_left_shown_as_confirmed(exp, monkeypatch): + nb = opened(*exp) + tag(nb) + nb.mask_card.questions["Nuclei"].choice.value = "instances" + nb.reference_boxes["drug"].value = "ctrl" + nb.reference_boxes["ctrl"].value = "" + monkeypatch.setattr(type(nb), "_save_description", lambda self: False) + nb._save_problem = "disk full" + nb.same_way["rep1"].button.click() # the answer cannot be saved + assert ( + "rep1" not in nb.draft_answers.repeats or not nb.draft_answers.repeats["rep1"].confirmed_on + ) + assert not nb.same_way["rep1"].value # the boxes are rebuilt from the answers, not kept + assert "Not taken" in nb.save_status.value