Fixes #279

CHANGELOG.md

@@ -10,6 +10,14 @@ v0.5.1: no run before v0.5.1 is read.)
 
 **Fixed**
 
+- **Notebook: a removal or rename answer confirms only the change it answers (#279).** A
+  re-confirmation names a channel ID, not a folder (experiment-v1 §4.6), so **Yes, remove** of
+  one folder accepted any change of its channel IDs, including a saved header name lost in
+  another folder that is still there. Such an ID now stays refused (`R_CONFIRMED_ID_CHANGED`) and
+  the line beside Save names the channel and the folder (undo that tag on Channels). A header
+  name held only by the removed folder is still the removal's, and a moved position still needs
+  its own click (#258). A header that now states no channel names at all counts as a lost name
+  too, and a folder rename answer does not accept channels re-ordered inside the renamed folder.
 - **Notebook: a channel re-confirmation after re-ordering a unit's channels (#258).** A saved
   unit re-tagged in another order (its images re-exported with the channels re-ordered) was
   refused with `R_CONFIRMED_ID_CHANGED` with no way forward. The Channels step now lists each

src/morphobricks_preprocessing/description_reconcile.py

@@ -119,7 +119,10 @@ class UnitChange:
 
     ``repeat_id``, ``condition_id`` and ``technical_id`` are the published IDs of an unchanged or
     removed unit, None for a new one, which is pending. ``proposed_repeat`` is the only proposal
-    for a new unit: the published repeat ID equal to its source repeat label.
+    for a new unit: the published repeat ID equal to its source repeat label. ``headers_changed``
+    are the saved channels of a unit still in the folder whose header, at their saved position,
+    no longer gives one of their saved header names (another name, or none): a change in this
+    folder that no removal or rename answer elsewhere confirms (issue #279).
     """
 
     folders: UnitFolders
@@ -130,6 +133,7 @@ class UnitChange:
     proposed_repeat: str | None = None
     reasons: tuple[str, ...] = ()
     image_count: int | None = None
+    headers_changed: tuple[str, ...] = ()
 
 
 @dataclass(frozen=True)
@@ -245,6 +249,29 @@ def _channels_differ(
     return False
 
 
+def _headers_changed(
+    published: PublishedUnit, seen: ObservedUnit, description: Description
+) -> tuple[str, ...]:
+    """Return the unit's saved channels whose header name at their position is not a saved one.
+
+    A position whose header now gives no name counts too when the channel has saved names: the
+    name it may have given is lost. Headers that state no names at all (or were not read) give
+    no saved name either, so they count the same: no name is known to be still given there.
+    A colour unit has no header names.
+    """
+    unit = published.unit
+    if unit.image_type == "rgb":
+        return ()
+    names = seen.header_names or ()
+    out = set()
+    for index, channel in enumerate(unit.channels):
+        known = {_nfc(entry) for entry in description.channels[channel].header_names or ()}
+        name = names[index] if index < len(names) else None
+        if known and (not name or _nfc(name) not in known):
+            out.add(channel)
+    return tuple(sorted(out))
+
+
 def _nfc(text: str) -> str:
     return unicodedata.normalize("NFC", text)
 
@@ -319,7 +346,7 @@ def reconcile(
         found = [RENAMED_FOLDER] * (key in renamed)
         found += [CHANNELS_CHANGED] * _channels_differ(unit, seen, published)
         reasons[key].update(found)
-        changes.append(_change(unit, Status.UNCHANGED, tuple(found), current, seen))
+        changes.append(_change(unit, Status.UNCHANGED, tuple(found), current, seen, published))
     known = set(now.values())
     pending = [key for key in observed if key not in known]
     changes += [_new(key, observed[key], published, now, reasons, units) for key in sorted(pending)]
@@ -336,7 +363,9 @@ def _change(
     reasons: tuple[str, ...],
     folders: UnitFolders | None = None,
     seen: ObservedUnit | None = None,
+    description: Description | None = None,
 ) -> UnitChange:
+    """Return one published unit's change (``seen`` and ``description`` for a unit still there)."""
     return UnitChange(
         folders or unit.folders,
         status,
@@ -346,6 +375,7 @@ def _change(
         None,
         tuple(sorted(reasons)),
         seen.image_count if seen else None,
+        _headers_changed(unit, seen, description) if seen and description else (),
     )
 
 

src/morphobricks_preprocessing/description_removal.py

@@ -43,6 +43,9 @@
 Change = tuple[str, str, str | None]
 """A changed confirmed ID as ``Reconfirmation`` names it: (kind, old ID, new ID or None)."""
 
+POSITION, HEADER_NAME = "position", "header name"
+"""What a channel lost in a folder still there (:attr:`Reconfirmations.unseen`)."""
+
 
 @dataclass(frozen=True)
 class RemovalQuestion:
@@ -81,10 +84,16 @@ def __post_init__(self) -> None:
 
 @dataclass(frozen=True)
 class Reconfirmations:
-    """The records the candidate needs: those the user's answers cover, and those they do not."""
+    """The records the candidate needs: those the user's answers cover, and those they do not.
+
+    ``unseen`` are the channel changes in folders still there that keep an ID open although an
+    answer touches it: each the channel ID, the folder and what it lost there (``POSITION`` or
+    ``HEADER_NAME``; issue #279).
+    """
 
     accepted: tuple[Reconfirmation, ...]
     open: tuple[TransitionIssue, ...]
+    unseen: tuple[tuple[str, UnitFolders, str], ...] = ()
 
 
 def _moves(
@@ -236,6 +245,52 @@ def candidate(
     return built
 
 
+def _unanswered(
+    issue: TransitionIssue,
+    published: Description,
+    built: Description,
+    rec: Reconciliation,
+    removed: set[UnitFolders],
+    now: dict[UnitFolders, UnitFolders],
+) -> set[tuple[UnitFolders, str]]:
+    """Return the folders still there where a channel changed in a way no answer covers.
+
+    Section 4.6 keys a re-confirmation by ID only: a removal or rename answer that touches a
+    channel would also accept its other changes. Those are a position lost in a unit no answer
+    names (a move, confirmed on its own, #258) and a lost header name while a folder still there
+    no longer gives a saved name at the channel's position (#279). A position is compared in
+    the unit's folders after the renames (``now``), so a move inside a renamed folder is lost
+    too; only a unit the user confirmed removed (``removed``) loses its positions by the answer.
+    """
+    if issue.kind != CHANNELS or issue.new_id is not None:
+        return set()
+    channel, units = issue.old_id, published_units(published)
+    by_ids = {(u.condition_id, k.source_repeat, u.technical_id): k for k, u in units.items()}
+    after = _positions_of(built, channel)
+    lost = {
+        key
+        for cond, repeat, tid, index in _positions_of(published, channel)
+        if (key := by_ids[cond, repeat, tid]) not in removed
+        and (cond, now[key].source_repeat, tid, index) not in after
+    }
+    out = {(now[key], POSITION) for key in lost}
+    names = set(published.channels[channel].header_names or ())
+    if not names <= set(built.channels[channel].header_names or ()):
+        out |= {(u.folders, HEADER_NAME) for u in rec.unchanged if channel in u.headers_changed}
+    return out
+
+
+def _positions_of(description: Description, channel: str) -> set[tuple[str, str, str, int]]:
+    """Return the positions ``(condition, source repeat, technical, index)`` of one channel."""
+    return {
+        (cond, member.source_repeat, tid, unit.channels.index(channel))
+        for repeat in description.repeats.values()
+        for cond, member in repeat.members.items()
+        for tid, unit in member.technical_repeats.items()
+        if channel in unit.channels
+    }
+
+
 def reconfirmations(
     published: Description,
     built: Description,
@@ -245,21 +300,26 @@ def reconfirmations(
     """Return the re-confirmations of ``built`` that the user's answers cover, and the rest.
 
     Each changed confirmed ID takes the date of the first answer that touches one of its units;
-    an ID no answer touches is in ``open`` (the candidate would be refused for it).
+    an ID no answer touches is in ``open`` (the candidate would be refused for it). A channel
+    that also changed in a folder still there in a way no answer covers is open as well, with
+    those folders in ``unseen``: the record cannot say "this ID, only in that folder" (4.6).
     """
     units = published_units(published)
     answers = [(set(r.units), r.confirmed_on) for r in removals]
+    removed = {key for held, _ in answers for key in held}
     now = {key: key for key in units}
     for rename in rec.renames:
         after = {key: renamed_key(current, rename) for key, current in now.items()}
         answers.append(({key for key in units if after[key] != now[key]}, rename.confirmed_on))
         now = after
-    accepted, left = set(), []
+    accepted, left, unseen = set(), [], set()
     for issue in _changes(published, built):
         reached = _affected(issue, units)
         dates = [day for held, day in answers if held & reached]
-        if dates:
+        others = _unanswered(issue, published, built, rec, removed, now) if dates else set()
+        if dates and not others:
             accepted.add(Reconfirmation(issue.kind, issue.old_id, issue.new_id, dates[0]))
         else:
             left.append(issue)
-    return Reconfirmations(tuple(sorted(accepted)), tuple(left))
+            unseen |= {(issue.old_id, *other) for other in others}
+    return Reconfirmations(tuple(sorted(accepted)), tuple(left), tuple(sorted(unseen)))

src/morphobricks_preprocessing/notebook_channel_moves.py

@@ -11,14 +11,15 @@
 from __future__ import annotations
 
 import html
-from dataclasses import replace
+from dataclasses import astuple, replace
 from datetime import date
 
 import ipywidgets as widgets
 
 from . import notebook_style as ui
 from .channel_moves import ChannelMove, channel_moves, move_confirmations
 from .description import Description
+from .description_removal import reconfirmations
 from .description_transition import Reconfirmation
 from .draft import DraftError
 from .draft_answers import description_from_draft
@@ -91,6 +92,30 @@ def _move_terms(self, state: object, candidate: Description) -> tuple[Reconfirma
         clicked = move_confirmations(self._moves_of(state, candidate), self._moves_clicked)
         return tuple(c for c in clicked if (c.kind, c.old_id) not in taken)
 
+    def _unseen_lines(self) -> list[str]:
+        """Say which channel changed in a folder still there that no answer confirms (#279).
+
+        A removal or rename answer re-confirms a channel ID, never one folder of it, so that
+        change keeps the ID refused until it is undone; nothing is confirmed for the user.
+        """
+        state, check = self._published(), self._check
+        if state is None or check is None or check.description is None:
+            return []
+        candidate = check.description
+        found = reconfirmations(
+            state.description,
+            candidate,
+            state.reconciliation,
+            self._removal_terms(state, candidate)[0],
+        )
+        kept = {c.old_id for c in self._publication_terms(state, candidate)[1]}
+        return [
+            f"{channel} lost a saved {what} in {_where(astuple(folders))}, which is still there:"
+            " a removal or rename does not confirm it (undo that tag on Channels)"
+            for channel, folders, what in found.unseen
+            if channel not in kept
+        ]
+
     def _refresh_channel_moves(self) -> None:
         """Show the moved channels of the answers now and whether the user confirmed them."""
         moves: tuple[ChannelMove, ...] = ()

src/morphobricks_preprocessing/notebook_check_save.py

@@ -250,7 +250,9 @@ def _save_blockers(self, todo: tuple[list, list[str]]) -> list[str]:
             return [f"{plural(missing, 'answer')} still missing (listed above the button)"]
         if self._moves_open():
             return ["the moved channels are not confirmed: see Channels"]
-        return ["these answers are refused: see the red message above the button"]
+        return self._unseen_lines() or [
+            "these answers are refused: see the red message above the button"
+        ]
 
     def _blocked(self, state: object) -> str:
         """Say what already stops the run while the candidate cannot be checked (answers missing).

tests/test_reconfirm_scope.py

@@ -0,0 +1,107 @@
+"""A removal or rename answer re-confirms only the change it answers (issue #279).
+
+Experiment-v1 4.6 keys a re-confirmation by ID (old ID, new ID, date), never by folder: the
+"Yes, remove" of one folder records every channel ID that folder held, and that record would
+accept any other change of the same ID, such as a saved header name lost in another folder that
+is still there and that the user never saw. Such an ID stays refused (``R_CONFIRMED_ID_CHANGED``)
+and the reason is said beside the Save button; a header name held only by the removed folder is
+still the removal's.
+"""
+
+import json
+import shutil
+
+import pytest
+
+from channel_images import write_channels
+from morphobricks_preprocessing.description_file import RECORDS_FILENAME
+from test_notebook_check_save import KINDS, answer_all, exp  # noqa: F401
+
+pytest.importorskip("ipywidgets")
+pytest.importorskip("matplotlib")
+
+from test_notebook_axes_quick import stepper  # noqa: E402
+from test_notebook_prerelease import _tag, _text  # noqa: E402
+
+GONE = "drug/rep2"
+NO_NAMES = object()
+"""The rewritten header states no channel name at all (not one empty name)."""
+
+
+def _reopened(exp, unique, now):  # noqa: F811
+    """Save with ``unique`` as ctrl/rep1's DNA header name, then rewrite it to ``now``; drop B."""
+    write_channels(exp[0] / "ctrl/rep1/a.tif", [unique, "Actin", "Ch3"], KINDS, seed=1)
+    first = stepper(*exp)
+    first.quick_confirm.click()
+    answer_all(first)
+    assert first.save_experiment()
+    assert first.draft_answers.channels["DNA"].header_names == ("DAPI", unique)
+    names = None if now is NO_NAMES else [now, "Actin", "Ch3"]
+    write_channels(exp[0] / "ctrl/rep1/a.tif", names, KINDS, seed=1)
+    shutil.rmtree(exp[0] / GONE)
+    shutil.rmtree(exp[1] / "Nuclei" / GONE)
+    nb = stepper(*exp)
+    nb.quick_confirm.click()
+    return nb
+
+
+def _remove(nb):
+    answer_all(nb)
+    (row,) = nb.removals_box.children
+    row.children[1].click()  # Yes, remove (drug/rep2 holds DNA too)
+    nb._refresh_check_save()
+
+
+def _records(exp):  # noqa: F811
+    path = exp[2] / RECORDS_FILENAME
+    return [json.loads(x) for x in path.read_text().splitlines()] if path.exists() else []
+
+
+@pytest.mark.parametrize(
+    "now", ["Nuc", None, NO_NAMES], ids=["other-name", "no-name", "no-header-names"]
+)
+def test_a_removal_does_not_accept_a_header_name_lost_in_a_folder_still_there(exp, now):  # noqa: F811
+    nb = _reopened(exp, "Hoechst", now)
+    before = len(_records(exp))
+    _tag(nb, 0, "DNA")  # re-tag ctrl/rep1's channel 1: Hoechst is no longer a name of DNA
+    assert "Hoechst" not in (nb.draft_answers.channels["DNA"].header_names or ())
+    _remove(nb)
+    assert nb.save_button.disabled and not nb.save_experiment()
+    assert "R_CONFIRMED_ID_CHANGED at channels.DNA" in _text(nb.checks.value)
+    assert (
+        "DNA lost a saved header name in ctrl/rep1, which is still there: a removal or rename "
+        "does not confirm it (undo that tag on Channels)"
+    ) in _text(nb.save_blocker.value)
+    assert len(_records(exp)) == before  # nothing published, nothing recorded
+
+
+def test_without_the_tag_change_the_removal_saves(exp):  # noqa: F811
+    # The way out the reason names: DNA keeps its saved names, so only the removal changes it
+    # (the run then stops on the header itself, R_HEADER_CONTRADICTION, as without a removal).
+    nb = _reopened(exp, "Hoechst", "Nuc")
+    _remove(nb)
+    assert not nb.save_button.disabled
+    assert nb.save_experiment(), _text(nb.save_status.value)
+    kept = {r["old_id"] for r in _records(exp) if r["type"] == "reconfirmation"}
+    assert {"ACTIN", "DNA", "PHASE"} <= kept
+
+
+def test_a_header_name_held_only_by_the_removed_folder_is_the_removals(exp):  # noqa: F811
+    # drug/rep2 alone named DNA "Hoechst"; with it removed, a re-tag drops that name. Every
+    # folder still there gives a saved name, so the removal answer covers the lost name.
+    write_channels(exp[0] / GONE / "a.tif", ["Hoechst", "Actin", "Ch3"], KINDS, seed=4)
+    first = stepper(*exp)
+    first.quick_confirm.click()
+    answer_all(first)
+    assert first.save_experiment()
+    shutil.rmtree(exp[0] / GONE)
+    shutil.rmtree(exp[1] / "Nuclei" / GONE)
+    nb = stepper(*exp)
+    nb.quick_confirm.click()
+    _tag(nb, 0, "DNA")
+    assert nb.draft_answers.channels["DNA"].header_names == ("DAPI",)
+    _remove(nb)
+    assert not nb.save_button.disabled, _text(nb.save_blocker.value)
+    assert nb.save_experiment(), _text(nb.save_status.value)
+    kept = [r["old_id"] for r in _records(exp) if r["type"] == "reconfirmation"]
+    assert kept.count("DNA") == 1