From 4498dec7da08fa1a4f8ab1a0d8bac7f6eaefbf29 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:03:43 +0200 Subject: [PATCH 01/41] Refactor e2e project fixture helpers Extract shared test utilities to write DLC `config.yaml` files and labeled frames, then reuse them across existing fixture builders to remove duplicated setup code. Also add a new helper that creates a project with two `labeled-data` folders (including optional basename-collision frame names) for multi-folder e2e scenarios. --- src/napari_deeplabcut/_tests/e2e/utils.py | 114 +++++++++++++--------- 1 file changed, 70 insertions(+), 44 deletions(-) diff --git a/src/napari_deeplabcut/_tests/e2e/utils.py b/src/napari_deeplabcut/_tests/e2e/utils.py index 9a9aa9e7..a874800e 100644 --- a/src/napari_deeplabcut/_tests/e2e/utils.py +++ b/src/napari_deeplabcut/_tests/e2e/utils.py @@ -38,6 +38,37 @@ def _write_minimal_png(path: Path, *, shape=(64, 64, 3)) -> None: imsave(str(path), img, check_contrast=False) +def _write_dlc_config( + project: Path, + *, + scorer: str = "John", + bodyparts=("bodypart1", "bodypart2"), + colormap: str = "viridis", +) -> Path: + """Write a minimal DLC config.yaml at the project root.""" + import yaml + + project.mkdir(parents=True, exist_ok=True) + cfg = { + "scorer": scorer, + "bodyparts": list(bodyparts), + "dotsize": 8, + "pcutoff": 0.6, + "colormap": colormap, + } + config_path = project / "config.yaml" + config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") + return config_path + + +def _write_frames(folder: Path, names: tuple[str, ...]) -> Path: + """Populate a labeled-data folder with tiny frames and return it.""" + folder.mkdir(parents=True, exist_ok=True) + for name in names: + _write_minimal_png(folder / name) + return folder + + def _make_minimal_dlc_project(tmp_path: Path, *, bodyparts=("bodypart1", "bodypart2")): """ Build a minimal DLC-like folder: @@ -49,36 +80,16 @@ def _make_minimal_dlc_project(tmp_path: Path, *, bodyparts=("bodypart1", "bodypa ``bodyparts`` accepts non-string values so callers can build a project whose keypoint names are numeric in both config.yaml and the H5 column level. """ - import yaml - project = tmp_path / "project" - labeled = project / "labeled-data" / "test" - labeled.mkdir(parents=True, exist_ok=True) - - img_rel = ("labeled-data", "test", "img000.png") - img_path = project / Path(*img_rel) - _write_minimal_png(img_path) - - cfg = { - "scorer": "John", - "bodyparts": list(bodyparts), - "dotsize": 8, - "pcutoff": 0.6, - "colormap": "viridis", - } - config_path = project / "config.yaml" - config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") - - cols = pd.MultiIndex.from_product( - [["John"], list(bodyparts), ["x", "y"]], - names=["scorer", "bodyparts", "coords"], + labeled = _write_frames(project / "labeled-data" / "test", ("img000.png",)) + config_path = _write_dlc_config(project, bodyparts=bodyparts) + + h5_path = _write_keypoints_h5( + labeled / "CollectedData_John.h5", + scorer="John", + img_rel=("labeled-data", "test", "img000.png"), + bodyparts=bodyparts, ) - idx = pd.MultiIndex.from_tuples([img_rel]) - df0 = pd.DataFrame([[10.0, 20.0, np.nan, np.nan]], index=idx, columns=cols) - - h5_path = labeled / "CollectedData_John.h5" - df0.to_hdf(h5_path, key="df_with_missing", mode="w") - df0.to_csv(str(h5_path).replace(".h5", ".csv")) return project, config_path, labeled, h5_path @@ -191,26 +202,41 @@ def _make_project_config_and_frames_no_gt(tmp_path: Path): project/labeled-data/test/img000.png No CollectedData*.h5 initially. """ - import yaml - project = tmp_path / "project" - labeled = project / "labeled-data" / "test" - labeled.mkdir(parents=True, exist_ok=True) + labeled = _write_frames(project / "labeled-data" / "test", ("img000.png",)) + config_path = _write_dlc_config(project, colormap="magma") - img_rel = ("labeled-data", "test", "img000.png") - _write_minimal_png(project / Path(*img_rel)) + return project, config_path, labeled - cfg = { - "scorer": "John", - "bodyparts": ["bodypart1", "bodypart2"], - "dotsize": 8, - "pcutoff": 0.6, - "colormap": "magma", - } - config_path = project / "config.yaml" - config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") - return project, config_path, labeled +def _make_project_with_two_labeled_folders( + tmp_path: Path, + *, + b_frames: tuple[str, ...] = ("imgB000.png", "imgB001.png", "imgB002.png"), +): + """ + Project with two labeled-data folders: + + project/config.yaml + project/labeled-data/videoA/imgA000.png + project/labeled-data/videoA/CollectedData_John.h5 (bodypart1 labeled) + project/labeled-data/videoB/ (no annotations) + + By default videoB's frame names do not overlap videoA's. Pass ``b_frames`` matching + videoA's names to build the basename-collision case instead. + """ + project = tmp_path / "project" + folder_a = _write_frames(project / "labeled-data" / "videoA", ("imgA000.png",)) + folder_b = _write_frames(project / "labeled-data" / "videoB", b_frames) + config_path = _write_dlc_config(project, colormap="magma") + + gt_path = _write_keypoints_h5( + folder_a / "CollectedData_John.h5", + scorer="John", + img_rel=("labeled-data", "videoA", "imgA000.png"), + ) + + return project, config_path, folder_a, folder_b, gt_path def _read_h5_keypoints(path: Path) -> pd.DataFrame: From 8591cb95a4c0141ea41a44db274565d4dc1783ce Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:05:39 +0200 Subject: [PATCH 02/41] Add folder switch integrity E2E tests Add end-to-end coverage for reopening a different labeled folder while a Points layer survives. The new tests guard against partially rebinding layer metadata on unmappable switches, verify saves do not write foreign-indexed annotations into the new dataset, and include a positive control where matching frames correctly move root and paths together. --- .../e2e/test_folder_switch_integrity.py | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py diff --git a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py new file mode 100644 index 00000000..3d4d4077 --- /dev/null +++ b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py @@ -0,0 +1,129 @@ +# src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py +"""Switching the image folder underneath a surviving Points layer. + +Opening a labeled folder adopts its image context onto every non-Image layer. When the +new folder's frames cannot be mapped onto a layer's existing ones, that layer must not be +half-adopted: rebinding ``root`` while ``paths`` still names the previous folder yields an +annotation file written into one dataset but indexed against another. +""" + +from __future__ import annotations + +from pathlib import Path + +import pandas as pd +import pytest +from napari.layers import Image, Points + +from .utils import _make_project_with_two_labeled_folders, _read_h5_keypoints + + +def _points_layers(viewer): + return [ly for ly in viewer.layers if isinstance(ly, Points)] + + +def _open_folder(viewer, qtbot, folder: Path, *, expect_points: bool) -> None: + viewer.open(str(folder), plugin="napari-deeplabcut") + if expect_points: + qtbot.waitUntil(lambda: bool(_points_layers(viewer)), timeout=10_000) + else: + qtbot.waitUntil( + lambda: any(isinstance(ly, Image) for ly in viewer.layers), + timeout=10_000, + ) + qtbot.wait(100) + + +def _remove_image_layers(viewer, qtbot) -> None: + for layer in [ly for ly in viewer.layers if isinstance(ly, Image)]: + viewer.layers.remove(layer) + qtbot.wait(100) + + +def _dataset_names_in_index(df: pd.DataFrame) -> set[str]: + """Dataset-folder component of each row key.""" + if isinstance(df.index, pd.MultiIndex): + return {str(parts[-2]) for parts in df.index} + return {Path(str(v)).parent.name for v in df.index} + + +@pytest.mark.usefixtures("qtbot") +def test_unmappable_folder_switch_leaves_points_layer_bound_to_its_own_dataset( + viewer, + keypoint_controls, + qtbot, + tmp_path, +) -> None: + """videoB's frames cannot be mapped onto videoA's, so the layer must not be rebound.""" + _project, _config_path, folder_a, folder_b, _gt_path = _make_project_with_two_labeled_folders(tmp_path) + + _open_folder(viewer, qtbot, folder_a, expect_points=True) + + layer = _points_layers(viewer)[0] + paths_before = list(layer.metadata.get("paths") or []) + root_before = layer.metadata.get("root") + assert paths_before, "Expected the folder reader to bind frame paths to the Points layer" + + _remove_image_layers(viewer, qtbot) + _open_folder(viewer, qtbot, folder_b, expect_points=False) + + assert list(layer.metadata.get("paths") or []) == paths_before, ( + "Points layer frame paths changed after opening an unrelated folder" + ) + assert layer.metadata.get("root") == root_before, ( + "Points layer root was rebound to a folder whose frames it does not contain" + ) + + +@pytest.mark.usefixtures("qtbot") +def test_unmappable_folder_switch_does_not_write_annotations_into_new_folder( + viewer, + keypoint_controls, + qtbot, + tmp_path, + overwrite_confirm, +) -> None: + """Saving after an unmappable switch must not drop a foreign-indexed file into videoB.""" + overwrite_confirm.capture() + + _project, _config_path, folder_a, folder_b, gt_path = _make_project_with_two_labeled_folders(tmp_path) + + _open_folder(viewer, qtbot, folder_a, expect_points=True) + layer = _points_layers(viewer)[0] + + _remove_image_layers(viewer, qtbot) + _open_folder(viewer, qtbot, folder_b, expect_points=False) + + viewer.layers.selection.select_only(layer) + keypoint_controls._save_layers_dialog(selected=True) + qtbot.wait(200) + + stray = sorted(p.name for p in folder_b.glob("CollectedData*")) + assert not stray, f"Annotations were written into {folder_b.name}: {stray}" + + assert _dataset_names_in_index(_read_h5_keypoints(gt_path)) == {"videoA"} + + +@pytest.mark.usefixtures("qtbot") +def test_mappable_folder_switch_rebinds_points_layer( + viewer, + keypoint_controls, + qtbot, + tmp_path, +) -> None: + """Positive control: when the frames do map, root and paths move together.""" + _project, _config_path, folder_a, folder_b, _gt_path = _make_project_with_two_labeled_folders( + tmp_path, + b_frames=("imgA000.png",), + ) + + _open_folder(viewer, qtbot, folder_a, expect_points=True) + layer = _points_layers(viewer)[0] + + _remove_image_layers(viewer, qtbot) + _open_folder(viewer, qtbot, folder_b, expect_points=False) + + assert Path(str(layer.metadata.get("root"))).name == "videoB" + assert all("videoB" in str(p) for p in layer.metadata.get("paths") or []), ( + f"Expected frame paths to follow the layer's new root, got {layer.metadata.get('paths')}" + ) From 1b920cf496ad0484071d744ae5f0c86ca6bf6e56 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:18:16 +0200 Subject: [PATCH 03/41] Warn on layers left on previous dataset Only adopt the current image context for a layer when it has no existing paths or when a path remap is explicitly accepted. If remapping is rejected because the new folder does not match cleanly, show a deduplicated warning so users know the layer still saves back to its original dataset instead of silently following the newly opened folder. --- .../core/layer_lifecycle/manager.py | 68 ++++++++++++++++--- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 5fafa4a8..b40460b1 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -4,13 +4,16 @@ import logging from collections.abc import Callable, Iterator from enum import Enum +from pathlib import Path from types import MethodType from typing import TYPE_CHECKING, Any +from weakref import WeakKeyDictionary import numpy as np from napari.layers import Image, Layer, Points, Tracks from napari.utils.events import Event from napari.utils.history import update_save_history +from napari.utils.notifications import show_warning from qtpy.QtCore import QObject, Signal from ...config.keybinds import install_points_layer_keybindings, install_viewer_keybindings @@ -111,6 +114,9 @@ def __init__(self, viewer: napari.Viewer, *, parent: QObject | None = None) -> N self._image_meta = ImageMetadata() self._project_path: str | None = None + # Last folder each layer was warned about failing to follow + self._dataset_mismatch_warned: WeakKeyDictionary[Layer, str] = WeakKeyDictionary() + self._attached = False self.viewer_keybinds_installed = False @@ -361,6 +367,27 @@ def can_accept_dlc_session_image(self, layer: Image) -> tuple[bool, str | None]: "please save and clear the current layers before loading the new labeled data folder.", ) + def _warn_layer_left_on_previous_dataset(self, layer: Any) -> None: + """Tell the user a layer did not follow the newly opened folder.""" + root = (layer.metadata or {}).get("root") + dataset = Path(str(root)).name if root else "its original folder" + new_root = str(self._image_meta.root or "") + + if self._dataset_mismatch_warned.get(layer) == new_root: + logger.debug( + "Extra dataset-mismatch notification for layer=%r folder=%r", + getattr(layer, "name", layer), + new_root, + ) + return + + self._dataset_mismatch_warned[layer] = new_root + show_warning( + f"'{getattr(layer, 'name', layer)}' does not contain any of the frames in the folder " + f"you just opened, so it still belongs to '{dataset}'.\n" + "Saving it will write back there. Clear it before labelling the new folder." + ) + def _reject_conflicting_dlc_image_layer(self, layer: Image, reason: str) -> None: """Reject a conflicting DLC session image safely. @@ -878,18 +905,27 @@ def _remap_frame_indices(self, layer: Any) -> None: md = layer.metadata old_paths = md.get("paths") or [] - try: - safe_image_meta = self._image_meta.model_dump(exclude_none=True) - safe_image_meta.pop("paths", None) - layer.metadata.update(safe_image_meta) - except Exception: - logger.debug( - "Failed to sync non-path image metadata for layer=%r", - getattr(layer, "name", str(layer)), - exc_info=True, - ) + def _adopt_image_context() -> None: + """Take root/shape/name from the image context. + + Only safe alongside a `paths` update: a layer whose root names one dataset + while its paths name another saves into the first and is indexed against + the second. + """ + try: + safe_image_meta = self._image_meta.model_dump(exclude_none=True) + safe_image_meta.pop("paths", None) + layer.metadata.update(safe_image_meta) + except Exception: + logger.debug( + "Failed to sync non-path image metadata for layer=%r", + getattr(layer, "name", str(layer)), + exc_info=True, + ) if not old_paths: + # Not yet bound to any dataset, so there is nothing to contradict. + _adopt_image_context() logger.debug( "Skipping remap for layer=%r: no existing layer metadata paths.", getattr(layer, "name", str(layer)), @@ -932,10 +968,22 @@ def _remap_frame_indices(self, layer: Any) -> None: layer.data = res.data if res.accept_paths_update: + _adopt_image_context() layer.metadata["paths"] = list(new_paths) if isinstance(layer, Points): mark_layer_presentation_changed(layer) + else: + # Either no overlap at all, or a match too ambiguous to trust. Both leave + # the layer on its own dataset, so both are worth telling the user about. + logger.warning( + "Remap rejected for %s: %s Leaving it bound to %s.", + getattr(layer, "name", str(layer)), + res.message, + md.get("root"), + ) + self._warn_layer_left_on_previous_dataset(layer) + if res.depth_used is None: logger.debug("Remap skipped for %s: %s", getattr(layer, "name", str(layer)), res.message) else: From 1de2098e22a88a0dc72c72bf4fbded90ef9bae89 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:26:44 +0200 Subject: [PATCH 04/41] Test remap rejection metadata handling Add coverage for remap cases that must leave layer metadata unchanged when frame matching fails or is ambiguous, and for successful remaps that must update `root` and `paths` together. The new tests also lock in suppression of duplicate dataset-mismatch warnings and cover remap results that reject path updates on ambiguous depth-1 or no-overlap matches. --- .../_tests/core/layer_manager/test_manager.py | 95 ++++++++++++++++++- .../_tests/core/test_remap.py | 42 ++++++++ .../core/layer_lifecycle/manager.py | 3 +- 3 files changed, 137 insertions(+), 3 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 5bc07b6d..585e6bc8 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -8,7 +8,8 @@ import pytest from napari.layers import Image, Points -from napari_deeplabcut.config.models import AnnotationKind +import napari_deeplabcut.core.layer_lifecycle.manager as manager_mod +from napari_deeplabcut.config.models import AnnotationKind, ImageMetadata from napari_deeplabcut.core.layer_lifecycle import LayerLifecycleManager from napari_deeplabcut.core.layer_lifecycle.display_settings import ( MACHINE_LABELS_POINTS_DISPLAY, @@ -702,3 +703,95 @@ def attach(store, controls, resources): for key in ("M", "F"): assert keymap[key].__self__ is second_controls, f"{key} still bound to the previous controls" + + +# --------------------------------------------------------------------------- +# _remap_frame_indices: root and paths must move together +# --------------------------------------------------------------------------- +def _points_bound_to(paths, *, root): + layer = make_nonempty_points("bound") + layer.metadata = {"paths": list(paths), "root": root} + return layer + + +def test_remap_frame_indices_leaves_metadata_alone_when_nothing_maps(monkeypatch): + old_paths = ["labeled-data/videoA/imgA000.png"] + layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoB/imgB000.png"], + root="C:/project/labeled-data/videoB", + ) + + warned = [] + monkeypatch.setattr(manager, "_warn_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._remap_frame_indices(layer) + + assert layer.metadata["paths"] == old_paths + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] + + +def test_remap_frame_indices_leaves_metadata_alone_when_match_is_ambiguous(monkeypatch): + # Both old paths collapse onto the same basename, so the only available match is a + # depth-1 one that remap refuses. + old_paths = ["labeled-data/videoA/img0.png", "labeled-data/videoB/img0.png"] + layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata( + paths=["other/videoC/img0.png", "other/videoC/img1.png"], + root="C:/project/labeled-data/videoC", + ) + + warned = [] + monkeypatch.setattr(manager, "_warn_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._remap_frame_indices(layer) + + assert layer.metadata["paths"] == old_paths + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] + + +def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): + new_paths = ["labeled-data/videoB/imgA000.png"] + layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root="C:/project/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata(paths=new_paths, root="C:/project/labeled-data/videoB") + + manager._remap_frame_indices(layer) + + assert layer.metadata["paths"] == new_paths + assert layer.metadata["root"] == "C:/project/labeled-data/videoB" + + +def test_dataset_mismatch_warning_is_not_repeated_for_the_same_folder(monkeypatch): + """The remap sweep revisits every layer per insert, so repeats must be suppressed.""" + layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root="C:/project/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoB/imgB000.png"], + root="C:/project/labeled-data/videoB", + ) + + shown = [] + monkeypatch.setattr(manager_mod, "show_warning", lambda msg: shown.append(msg)) + + manager._remap_frame_indices(layer) + manager._remap_frame_indices(layer) + + assert len(shown) == 1 + + # A different folder is a new fact, so it is reported again. + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoC/imgC000.png"], + root="C:/project/labeled-data/videoC", + ) + manager._remap_frame_indices(layer) + + assert len(shown) == 2 diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 869864ad..69cf0eb9 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -281,6 +281,48 @@ def test_remap_warns_on_duplicate_canonical_keys(caplog): assert any("Duplicate canonical keys" in w for w in res.warnings) +def test_ambiguous_depth1_remap_is_rejected_and_refuses_paths_update(): + """A basename-only match that is not bijective must not be trusted. + + Callers key further metadata updates off ``accept_paths_update``, so it has to stay + False here even though a depth was found. + """ + # No shared 3- or 2-level suffix, so matching falls back to bare basenames, where + # both old paths collapse onto the same key. + old_paths = ["A/a/img0.png", "B/b/img0.png"] + new_paths = ["X/x/img0.png", "Y/y/img1.png"] + + data = np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float) + + res = remap_layer_data_by_paths( + data=data, + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + policy=PathMatchPolicy.ORDERED_DEPTHS, + ) + + assert res.depth_used == 1 + assert res.is_ambiguous is True + assert res.accept_paths_update is False + assert res.applied is False + assert res.changed is False + + +def test_no_overlap_remap_refuses_paths_update(): + """The other rejection path callers depend on: nothing matched at any depth.""" + res = remap_layer_data_by_paths( + data=np.array([[0.0, 1.0, 2.0]], dtype=float), + old_paths=["A/a/img0.png"], + new_paths=["B/b/other.png"], + time_col=0, + policy=PathMatchPolicy.ORDERED_DEPTHS, + ) + + assert res.depth_used is None + assert res.accept_paths_update is False + + def test_remap_warns_on_low_overlap_ratio(caplog): caplog.set_level(logging.WARNING) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index b40460b1..f52e3b61 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -974,8 +974,7 @@ def _adopt_image_context() -> None: mark_layer_presentation_changed(layer) else: - # Either no overlap at all, or a match too ambiguous to trust. Both leave - # the layer on its own dataset, so both are worth telling the user about. + # Either no overlap at all, or a match too ambiguous to trust logger.warning( "Remap rejected for %s: %s Leaving it bound to %s.", getattr(layer, "name", str(layer)), From bec2496a69d151f2bf8a66e331443eaa8032529e Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:28:46 +0200 Subject: [PATCH 05/41] Route dataset mismatch warnings via signal Connect `LayerLifecycleManager.layer_dataset_mismatch` to `KeypointControls` so dataset-folder mismatch warnings are shown from the widget layer. Tests were updated to assert the signal-based behavior (including deduping once per target folder) instead of monkeypatching the old direct warning path. Route dataset mismatch warnings via signal Replace direct `show_warning` calls when a layer stays on its previous dataset with `_report_layer_left_on_previous_dataset`, which records mismatches per layer/new-root and reports only the first occurrence per folder switch. The message is now sent through `viewer.status` and a new `layer_dataset_mismatch` signal, while logging still captures every remap pass. --- .../_tests/core/layer_manager/test_manager.py | 18 +++++++++--------- src/napari_deeplabcut/_widgets.py | 9 +++++++++ .../core/layer_lifecycle/manager.py | 18 +++++++++++++----- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 585e6bc8..1fed133e 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -8,7 +8,6 @@ import pytest from napari.layers import Image, Points -import napari_deeplabcut.core.layer_lifecycle.manager as manager_mod from napari_deeplabcut.config.models import AnnotationKind, ImageMetadata from napari_deeplabcut.core.layer_lifecycle import LayerLifecycleManager from napari_deeplabcut.core.layer_lifecycle.display_settings import ( @@ -106,6 +105,7 @@ def connect_signal_recorders(manager): inserted=SignalRecorder(), removed=SignalRecorder(), conflicts=SignalRecorder(), + dataset_mismatch=SignalRecorder(), ) manager.refresh_video_panel_requested.connect(rec.refresh_video) @@ -120,6 +120,7 @@ def connect_signal_recorders(manager): manager.layer_insert_processed.connect(rec.inserted) manager.layer_remove_processed.connect(rec.removed) manager.session_conflict_rejected.connect(rec.conflicts) + manager.layer_dataset_mismatch.connect(rec.dataset_mismatch) return rec @@ -725,7 +726,7 @@ def test_remap_frame_indices_leaves_metadata_alone_when_nothing_maps(monkeypatch ) warned = [] - monkeypatch.setattr(manager, "_warn_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) manager._remap_frame_indices(layer) @@ -747,7 +748,7 @@ def test_remap_frame_indices_leaves_metadata_alone_when_match_is_ambiguous(monke ) warned = [] - monkeypatch.setattr(manager, "_warn_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) manager._remap_frame_indices(layer) @@ -769,23 +770,22 @@ def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): assert layer.metadata["root"] == "C:/project/labeled-data/videoB" -def test_dataset_mismatch_warning_is_not_repeated_for_the_same_folder(monkeypatch): +def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): """The remap sweep revisits every layer per insert, so repeats must be suppressed.""" layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root="C:/project/labeled-data/videoA") manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + rec = connect_signal_recorders(manager) manager._image_meta = ImageMetadata( paths=["labeled-data/videoB/imgB000.png"], root="C:/project/labeled-data/videoB", ) - shown = [] - monkeypatch.setattr(manager_mod, "show_warning", lambda msg: shown.append(msg)) - manager._remap_frame_indices(layer) manager._remap_frame_indices(layer) - assert len(shown) == 1 + assert rec.dataset_mismatch.count == 1 + assert "videoA" in rec.dataset_mismatch.calls[0][0] # A different folder is a new fact, so it is reported again. manager._image_meta = ImageMetadata( @@ -794,4 +794,4 @@ def test_dataset_mismatch_warning_is_not_repeated_for_the_same_folder(monkeypatc ) manager._remap_frame_indices(layer) - assert len(shown) == 2 + assert rec.dataset_mismatch.count == 2 diff --git a/src/napari_deeplabcut/_widgets.py b/src/napari_deeplabcut/_widgets.py index ca7ab912..a3124afa 100644 --- a/src/napari_deeplabcut/_widgets.py +++ b/src/napari_deeplabcut/_widgets.py @@ -129,6 +129,7 @@ def __init__(self, napari_viewer): self.layer_manager.set_placeholder_config_decision_provider(self) ## Hook up signals for layer lifecycle events as needed, e.g.: self.layer_manager.session_conflict_rejected.connect(self._on_session_conflict_detected) + self.layer_manager.layer_dataset_mismatch.connect(self._on_layer_dataset_mismatch) self.layer_manager.refresh_video_panel_requested.connect(self._refresh_video_panel_context) self.layer_manager.refresh_layer_status_requested.connect(self._refresh_layer_status_panel) self.layer_manager.video_widget_visibility_requested.connect(self._on_video_widget_visibility_requested) @@ -422,6 +423,14 @@ def _on_session_conflict_detected(self, reason: str) -> None: QMessageBox.Ok, ) + def _on_layer_dataset_mismatch(self, reason: str) -> None: + QMessageBox.warning( + self, + "A layer does not match the folder you opened:", + f"{reason}\n\n", + QMessageBox.Ok, + ) + def _show_debug_window(self) -> None: try: if self._debug_window is None: diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index f52e3b61..7a1b25cf 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -13,7 +13,6 @@ from napari.layers import Image, Layer, Points, Tracks from napari.utils.events import Event from napari.utils.history import update_save_history -from napari.utils.notifications import show_warning from qtpy.QtCore import QObject, Signal from ...config.keybinds import install_points_layer_keybindings, install_viewer_keybindings @@ -99,6 +98,7 @@ class LayerLifecycleManager(QObject, OwnedTimersMixin): # Session management session_conflict_rejected = Signal(str) # if a new DLC folder is loaded on top of the current one + layer_dataset_mismatch = Signal(str) # if a layer could not follow the newly opened folder def __init__(self, viewer: napari.Viewer, *, parent: QObject | None = None) -> None: super().__init__(parent=parent) @@ -367,8 +367,13 @@ def can_accept_dlc_session_image(self, layer: Image) -> tuple[bool, str | None]: "please save and clear the current layers before loading the new labeled data folder.", ) - def _warn_layer_left_on_previous_dataset(self, layer: Any) -> None: - """Tell the user a layer did not follow the newly opened folder.""" + def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: + """Report that a layer did not follow the newly opened folder. + + The remap sweep visits every non-Image layer on every qualifying insert, so one + folder open reaches this more than once for the same layer. Report only the first + time a given layer fails to follow a given folder; the log records every pass. + """ root = (layer.metadata or {}).get("root") dataset = Path(str(root)).name if root else "its original folder" new_root = str(self._image_meta.root or "") @@ -382,11 +387,14 @@ def _warn_layer_left_on_previous_dataset(self, layer: Any) -> None: return self._dataset_mismatch_warned[layer] = new_root - show_warning( + reason = ( f"'{getattr(layer, 'name', layer)}' does not contain any of the frames in the folder " f"you just opened, so it still belongs to '{dataset}'.\n" "Saving it will write back there. Clear it before labelling the new folder." ) + self.viewer.status = reason + + self._single_shot_owned(0, lambda: self.layer_dataset_mismatch.emit(reason)) def _reject_conflicting_dlc_image_layer(self, layer: Image, reason: str) -> None: """Reject a conflicting DLC session image safely. @@ -981,7 +989,7 @@ def _adopt_image_context() -> None: res.message, md.get("root"), ) - self._warn_layer_left_on_previous_dataset(layer) + self._report_layer_left_on_previous_dataset(layer) if res.depth_used is None: logger.debug("Remap skipped for %s: %s", getattr(layer, "name", str(layer)), res.message) From f4ba1f88d58edf637009c58c6ea071544a480247 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:48:59 +0200 Subject: [PATCH 06/41] Adjust warning phrasing --- src/napari_deeplabcut/core/layer_lifecycle/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 7a1b25cf..2603e0fd 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -389,8 +389,8 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: self._dataset_mismatch_warned[layer] = new_root reason = ( f"'{getattr(layer, 'name', layer)}' does not contain any of the frames in the folder " - f"you just opened, so it still belongs to '{dataset}'.\n" - "Saving it will write back there. Clear it before labelling the new folder." + f"you just opened, so it still will save to '{dataset}'.\n" + "Clear it before labelling the new folder." ) self.viewer.status = reason From c601e461959f527aa438a4e83892625f8dc4a069 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:52:48 +0200 Subject: [PATCH 07/41] Scope path remap by dataset across folders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevented layer remapping from matching unrelated files when switching labeled-data folders. The lifecycle manager now checks whether a layer root and current image root refer to the same dataset folder, using full ordered-depth matching only in that case. For cross-dataset cases, remapping uses a new `DATASET_SCOPED` policy (depths 3 and 2 only) to avoid bare-filename collisions from DLC’s fixed frame names. --- .../core/layer_lifecycle/manager.py | 27 ++++++++++++++++++- src/napari_deeplabcut/core/project_paths.py | 3 +++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 2603e0fd..5dc43b3f 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -367,6 +367,23 @@ def can_accept_dlc_session_image(self, layer: Image) -> tuple[bool, str | None]: "please save and clear the current layers before loading the new labeled data folder.", ) + def _same_dataset_folder(self, layer_root: str | None) -> bool: + """Return True if a layer's root and the image context name the same dataset folder. + + Compares the folder name rather than the whole path. + Only case where we must match frames on filename alone is a rewritten prefix + (project moved between machines, labeled-data renamed). + """ + image_root = self._image_meta.root + if not layer_root or not image_root: + return False + + try: + return Path(str(layer_root)).name.casefold() == Path(str(image_root)).name.casefold() + except Exception: + logger.debug("Could not compare dataset folders %r and %r", layer_root, image_root, exc_info=True) + return False + def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: """Report that a layer did not follow the newly opened folder. @@ -954,12 +971,20 @@ def _adopt_image_context() -> None: int(np.nanmax(arr_before[:, time_col])) if arr_before.size else None, ) + # Matching on bare filenames is only meaningful once we know both sides are the + # same dataset; otherwise DLC's fixed frame naming makes unrelated folders match. + policy = ( + PathMatchPolicy.ORDERED_DEPTHS + if self._same_dataset_folder(md.get("root")) + else PathMatchPolicy.DATASET_SCOPED + ) + res = remap_layer_data_by_paths( data=layer.data, old_paths=old_paths, new_paths=new_paths, time_col=time_col, - policy=PathMatchPolicy.ORDERED_DEPTHS, + policy=policy, ) logger.debug( diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 4d3f97d6..65a6ef17 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -94,11 +94,14 @@ class PathMatchPolicy(Enum): """ ORDERED_DEPTHS = "ordered_depths" + DATASET_SCOPED = "dataset_scoped" @property def depths(self) -> tuple[int, ...]: if self is PathMatchPolicy.ORDERED_DEPTHS: return (3, 2, 1) + if self is PathMatchPolicy.DATASET_SCOPED: + return (3, 2) raise NotImplementedError(f"Unhandled PathMatchPolicy: {self}") From f1fb49ce954a5a460814d8fae8040ff24435e932 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 11:56:58 +0200 Subject: [PATCH 08/41] Reject basename-only dataset remaps Add a dataset-scoped path matching policy that refuses ambiguous basename-only overlaps when frame names repeat across folders. This prevents layers from silently migrating annotations onto another dataset when the only commonality is matching filenames after a project or folder move. Updated remap and folder-switch tests to cover the safer behavior and guard against false-positive rebinds. --- .../_tests/core/layer_manager/test_manager.py | 43 +++++++++++++++++-- .../_tests/core/test_project_paths.py | 35 +++++++++++++++ .../_tests/core/test_remap.py | 31 +++++++++++++ .../e2e/test_folder_switch_integrity.py | 28 +++++++++--- 4 files changed, 126 insertions(+), 11 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 1fed133e..ffc08d7a 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -758,16 +758,51 @@ def test_remap_frame_indices_leaves_metadata_alone_when_match_is_ambiguous(monke def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): - new_paths = ["labeled-data/videoB/imgA000.png"] - layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root="C:/project/labeled-data/videoA") + new_paths = ["labeled-data/videoA/imgA000.png"] + layer = _points_bound_to(["old/labeled-data/videoA/imgA000.png"], root="D:/moved/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata(paths=new_paths, root="C:/project/labeled-data/videoA") + + manager._remap_frame_indices(layer) + + assert layer.metadata["paths"] == new_paths + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + + +def test_remap_frame_indices_refuses_a_different_folder_with_the_same_frame_names(monkeypatch): + """DLC frame names repeat across datasets, so a full basename match is not identity.""" + old_paths = ["labeled-data/videoA/img000.png", "labeled-data/videoA/img001.png"] + layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoB/img000.png", "labeled-data/videoB/img001.png"], + root="C:/project/labeled-data/videoB", + ) + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._remap_frame_indices(layer) + + assert layer.metadata["paths"] == old_paths + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] + + +def test_remap_frame_indices_allows_basename_match_within_the_same_dataset_folder(): + """A rewritten prefix is the case depth-1 matching exists to repair.""" + new_paths = ["/mnt/moved/labeled-data/videoA/img000.png"] + layer = _points_bound_to(["img000.png"], root="C:/project/labeled-data/videoA") manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata(paths=new_paths, root="C:/project/labeled-data/videoB") + manager._image_meta = ImageMetadata(paths=new_paths, root="/mnt/moved/labeled-data/videoA") manager._remap_frame_indices(layer) assert layer.metadata["paths"] == new_paths - assert layer.metadata["root"] == "C:/project/labeled-data/videoB" + assert layer.metadata["root"] == "/mnt/moved/labeled-data/videoA" def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): diff --git a/src/napari_deeplabcut/_tests/core/test_project_paths.py b/src/napari_deeplabcut/_tests/core/test_project_paths.py index 92c19245..6e619e15 100644 --- a/src/napari_deeplabcut/_tests/core/test_project_paths.py +++ b/src/napari_deeplabcut/_tests/core/test_project_paths.py @@ -87,6 +87,41 @@ def test_path_match_policy_ordered_depths(): assert paths_mod.PathMatchPolicy.ORDERED_DEPTHS.depths == (3, 2, 1) +def test_path_match_policy_dataset_scoped_stops_above_basenames(): + assert paths_mod.PathMatchPolicy.DATASET_SCOPED.depths == (3, 2) + + +def test_find_matching_depth_dataset_scoped_refuses_basename_only_overlap(): + """DLC frame names collide across datasets, so a basename match proves nothing.""" + old_paths = ["/project/labeled-data/videoA/img001.png"] + new_paths = ["/project/labeled-data/videoB/img001.png"] + + assert paths_mod.find_matching_depth(old_paths, new_paths) == 1 + assert ( + paths_mod.find_matching_depth( + old_paths, + new_paths, + policy=paths_mod.PathMatchPolicy.DATASET_SCOPED, + ) + is None + ) + + +def test_find_matching_depth_dataset_scoped_still_matches_a_moved_project(): + """Only the prefix changed, so depth=2 still identifies the dataset.""" + old_paths = ["C:/old/labeled-data/videoA/img001.png"] + new_paths = ["/mnt/new/place/labeled-data/videoA/img001.png"] + + assert ( + paths_mod.find_matching_depth( + old_paths, + new_paths, + policy=paths_mod.PathMatchPolicy.DATASET_SCOPED, + ) + == 3 + ) + + def test_find_matching_depth_prefers_deepest_first_match(): old_paths = [ "/project/labeled-data/mouse1/img001.png", diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 69cf0eb9..d364ac94 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -309,6 +309,37 @@ def test_ambiguous_depth1_remap_is_rejected_and_refuses_paths_update(): assert res.changed is False +def test_dataset_scoped_policy_refuses_a_clean_basename_only_match(): + """The dangerous case: a perfect 1:1 match that proves nothing about identity. + + ORDERED_DEPTHS accepts this and would migrate the data onto another dataset's frames. + """ + old_paths = ["p/labeled-data/videoA/img000.png", "p/labeled-data/videoA/img001.png"] + new_paths = ["p/labeled-data/videoB/img000.png", "p/labeled-data/videoB/img001.png"] + data = np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float) + + permissive = remap_layer_data_by_paths( + data=data, + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + policy=PathMatchPolicy.ORDERED_DEPTHS, + ) + assert permissive.depth_used == 1 + assert permissive.accept_paths_update is True + + scoped = remap_layer_data_by_paths( + data=data, + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + policy=PathMatchPolicy.DATASET_SCOPED, + ) + assert scoped.depth_used is None + assert scoped.accept_paths_update is False + assert scoped.applied is False + + def test_no_overlap_remap_refuses_paths_update(): """The other rejection path callers depend on: nothing matched at any depth.""" res = remap_layer_data_by_paths( diff --git a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py index 3d4d4077..91c80683 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py +++ b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py @@ -105,25 +105,39 @@ def test_unmappable_folder_switch_does_not_write_annotations_into_new_folder( @pytest.mark.usefixtures("qtbot") -def test_mappable_folder_switch_rebinds_points_layer( +def test_identical_frame_names_in_another_folder_do_not_migrate_the_layer( viewer, keypoint_controls, qtbot, tmp_path, + overwrite_confirm, ) -> None: - """Positive control: when the frames do map, root and paths move together.""" - _project, _config_path, folder_a, folder_b, _gt_path = _make_project_with_two_labeled_folders( + """videoB reuses videoA's frame names, which is the DLC norm, not evidence of identity. + + Matching on basenames alone would give a perfect 1:1 map here and silently carry the + annotation onto a different dataset's frames. + """ + overwrite_confirm.capture() + + _project, _config_path, folder_a, folder_b, gt_path = _make_project_with_two_labeled_folders( tmp_path, b_frames=("imgA000.png",), ) _open_folder(viewer, qtbot, folder_a, expect_points=True) layer = _points_layers(viewer)[0] + paths_before = list(layer.metadata.get("paths") or []) _remove_image_layers(viewer, qtbot) _open_folder(viewer, qtbot, folder_b, expect_points=False) - assert Path(str(layer.metadata.get("root"))).name == "videoB" - assert all("videoB" in str(p) for p in layer.metadata.get("paths") or []), ( - f"Expected frame paths to follow the layer's new root, got {layer.metadata.get('paths')}" - ) + assert Path(str(layer.metadata.get("root"))).name == "videoA" + assert list(layer.metadata.get("paths") or []) == paths_before + + viewer.layers.selection.select_only(layer) + keypoint_controls._save_layers_dialog(selected=True) + qtbot.wait(200) + + stray = sorted(p.name for p in folder_b.glob("CollectedData*")) + assert not stray, f"Annotations migrated into {folder_b.name}: {stray}" + assert _dataset_names_in_index(_read_h5_keypoints(gt_path)) == {"videoA"} From 61b4d39ac979648c943859b6ea1d7de50217071f Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 12:07:59 +0200 Subject: [PATCH 09/41] Add e2e test for cross-project layer rebinding Add a regression test covering two DeepLabCut projects that share the same video-derived dataset path (`labeled-data/mouse1/...`). The new helper builds that fixture setup, and the test asserts an existing points layer keeps its original project/root metadata and does not save annotations into the second project when folders are switched. --- .../e2e/test_folder_switch_integrity.py | 44 ++++++++++++++++++- src/napari_deeplabcut/_tests/e2e/utils.py | 38 ++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py index 91c80683..aeceb18b 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py +++ b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py @@ -15,7 +15,11 @@ import pytest from napari.layers import Image, Points -from .utils import _make_project_with_two_labeled_folders, _read_h5_keypoints +from .utils import ( + _make_project_with_two_labeled_folders, + _make_two_projects_sharing_a_video_name, + _read_h5_keypoints, +) def _points_layers(viewer): @@ -141,3 +145,41 @@ def test_identical_frame_names_in_another_folder_do_not_migrate_the_layer( stray = sorted(p.name for p in folder_b.glob("CollectedData*")) assert not stray, f"Annotations migrated into {folder_b.name}: {stray}" assert _dataset_names_in_index(_read_h5_keypoints(gt_path)) == {"videoA"} + + +@pytest.mark.usefixtures("qtbot") +def test_layer_does_not_follow_a_different_project_using_the_same_video_name( + viewer, + keypoint_controls, + qtbot, + tmp_path, + overwrite_confirm, +) -> None: + """Re-labelling the same footage in a fresh project is an ordinary DLC workflow. + + Both projects then hold `labeled-data/mouse1/img000.png`, which is identical at every + canonicalization depth: the project root is not part of the key. A layer from the + first project must not silently rebind to, or be saved into, the second. + """ + overwrite_confirm.capture() + + proj = _make_two_projects_sharing_a_video_name(tmp_path) + + _open_folder(viewer, qtbot, proj.folder_a, expect_points=True) + layer = _points_layers(viewer)[0] + root_before = Path(str(layer.metadata.get("root"))) + project_before = layer.metadata.get("project") + + _remove_image_layers(viewer, qtbot) + _open_folder(viewer, qtbot, proj.folder_b, expect_points=False) + + assert Path(str(layer.metadata.get("root"))) == root_before, ( + f"Layer rebound from {project_before} to another project's dataset of the same name" + ) + + viewer.layers.selection.select_only(layer) + keypoint_controls._save_layers_dialog(selected=True) + qtbot.wait(200) + + stray = sorted(p.name for p in proj.folder_b.glob("CollectedData*")) + assert not stray, f"Annotations from {project_before} were written into project-B: {stray}" diff --git a/src/napari_deeplabcut/_tests/e2e/utils.py b/src/napari_deeplabcut/_tests/e2e/utils.py index a874800e..da181e0a 100644 --- a/src/napari_deeplabcut/_tests/e2e/utils.py +++ b/src/napari_deeplabcut/_tests/e2e/utils.py @@ -5,6 +5,7 @@ import math import os from pathlib import Path +from types import SimpleNamespace import numpy as np import pandas as pd @@ -239,6 +240,43 @@ def _make_project_with_two_labeled_folders( return project, config_path, folder_a, folder_b, gt_path +def _make_two_projects_sharing_a_video_name(tmp_path: Path): + """Two DLC projects whose dataset folders are named after the same video. + + project-A/config.yaml, project-A/labeled-data/mouse1/{img000,img001}.png + GT + project-B/config.yaml, project-B/labeled-data/mouse1/{img000,img001}.png + + DLC names dataset folders after the video stem and frames by fixed convention, so + this is what re-labelling the same footage in a fresh project looks like on disk: + every path component below the project root is identical between the two. + """ + project_a = tmp_path / "project-A" + project_b = tmp_path / "project-B" + frames = ("img000.png", "img001.png") + + folder_a = _write_frames(project_a / "labeled-data" / "mouse1", frames) + folder_b = _write_frames(project_b / "labeled-data" / "mouse1", frames) + + config_a = _write_dlc_config(project_a, scorer="John") + config_b = _write_dlc_config(project_b, scorer="Jane") + + gt_path = _write_keypoints_h5( + folder_a / "CollectedData_John.h5", + scorer="John", + img_rel=("labeled-data", "mouse1", "img000.png"), + ) + + return SimpleNamespace( + project_a=project_a, + project_b=project_b, + folder_a=folder_a, + folder_b=folder_b, + config_a=config_a, + config_b=config_b, + gt_path=gt_path, + ) + + def _read_h5_keypoints(path: Path) -> pd.DataFrame: return pd.read_hdf(path, key="df_with_missing") From 6ac933f154b586b3598b4fbde2dc10dd6be8a7ec Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 13:12:30 +0200 Subject: [PATCH 10/41] Track dataset identity for layer remapping Add an immutable `dataset_key` to image and points metadata based on the source folder. The lifecycle manager now uses that key to decide whether a layer should adopt the current image context, leaving layers from other datasets untouched and warning instead of remapping them. Since dataset identity is checked up front, path remapping can always use the ordered depth fallback safely, and the old dataset-scoped policy is removed. --- src/napari_deeplabcut/config/models.py | 9 ++++ src/napari_deeplabcut/core/io.py | 3 ++ .../core/layer_lifecycle/manager.py | 43 ++++++++++--------- src/napari_deeplabcut/core/project_paths.py | 28 ++++++++++-- 4 files changed, 59 insertions(+), 24 deletions(-) diff --git a/src/napari_deeplabcut/config/models.py b/src/napari_deeplabcut/config/models.py index 258fde10..cc495c36 100644 --- a/src/napari_deeplabcut/config/models.py +++ b/src/napari_deeplabcut/config/models.py @@ -559,6 +559,15 @@ class PointsMetadata(BaseModel): shape: tuple[int, ...] | None = None name: str | None = None + dataset_key: str | None = Field( + default=None, + description=( + "Absolute folder this layer was read from, assigned once at read time and never " + "rewritten. Unlike root and paths, which are overwritten when a layer adopts a new " + "image context, this can be used to decide whether that adoption should happen. " + ), + ) + project: str | None = None header: DLCHeaderModel | None = None io: IOProvenance | None = None diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index 699e8234..18179e2c 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -67,6 +67,7 @@ from napari_deeplabcut.core.metadata import attach_source_and_io_to_layer_kwargs, parse_points_metadata from napari_deeplabcut.core.project_paths import ( canonicalize_path, + dataset_key_for_folder, find_nearest_config, infer_dlc_project_from_points_meta, ) @@ -247,6 +248,7 @@ def read_hdf_single(file: Path, *, kind: AnnotationKind | None = None) -> list[L ) layer_props["name"] = file.stem layer_props["metadata"]["root"] = str(file.parent) + layer_props["metadata"]["dataset_key"] = dataset_key_for_folder(file.parent) layer_props["metadata"]["name"] = layer_props["name"] layer_props["metadata"]["config_colormap"] = config_colormap @@ -852,6 +854,7 @@ def _build_image_layer_kwargs( metadata = { "paths": [canonicalize_path(fp, 3) for fp in filepaths], "root": str(filepaths[0].parent), + "dataset_key": dataset_key_for_folder(filepaths[0].parent), } if dlc_meta is not None: metadata["dlc"] = dlc_meta diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 5dc43b3f..8454b748 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -112,6 +112,7 @@ def __init__(self, viewer: napari.Viewer, *, parent: QObject | None = None) -> N self._label_mode = keypoints.LabelMode.default() self._active_dlc_image_layer_id: int | None = None self._image_meta = ImageMetadata() + self._image_dataset_key: str | None = None self._project_path: str | None = None # Last folder each layer was warned about failing to follow @@ -367,22 +368,16 @@ def can_accept_dlc_session_image(self, layer: Image) -> tuple[bool, str | None]: "please save and clear the current layers before loading the new labeled data folder.", ) - def _same_dataset_folder(self, layer_root: str | None) -> bool: - """Return True if a layer's root and the image context name the same dataset folder. + def _belongs_to_current_dataset(self, layer_dataset_key: str | None) -> bool: + """Return True if a layer may follow the image context now loaded. - Compares the folder name rather than the whole path. - Only case where we must match frames on filename alone is a rewritten prefix - (project moved between machines, labeled-data renamed). + A layer with no key is unbound (a config placeholder, say) and adopts what is + open. """ - image_root = self._image_meta.root - if not layer_root or not image_root: - return False + if layer_dataset_key is None: + return True - try: - return Path(str(layer_root)).name.casefold() == Path(str(image_root)).name.casefold() - except Exception: - logger.debug("Could not compare dataset folders %r and %r", layer_root, image_root, exc_info=True) - return False + return layer_dataset_key == self._image_dataset_key def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: """Report that a layer did not follow the newly opened folder. @@ -698,6 +693,7 @@ def _setup_image_layer(self, layer: Image, index: int | None = None, *, reorder: pass self._active_dlc_image_layer_id = layer_key(layer) + self._image_dataset_key = (layer.metadata or {}).get("dataset_key") context_changed = self._update_image_meta_from_layer(layer) if not self._project_path: @@ -901,6 +897,7 @@ def _handle_removed_layer(self, layer: Any) -> None: if self._active_dlc_image_layer_id == layer_key(layer): self._active_dlc_image_layer_id = None self._image_meta = ImageMetadata() + self._image_dataset_key = None self._project_path = None paths = layer.metadata.get("paths") @@ -930,6 +927,16 @@ def _remap_frame_indices(self, layer: Any) -> None: md = layer.metadata old_paths = md.get("paths") or [] + if not self._belongs_to_current_dataset(md.get("dataset_key")): + logger.warning( + "Layer %r belongs to %s, not to the folder just opened (%s); leaving it alone.", + getattr(layer, "name", str(layer)), + md.get("dataset_key"), + self._image_dataset_key, + ) + self._report_layer_left_on_previous_dataset(layer) + return + def _adopt_image_context() -> None: """Take root/shape/name from the image context. @@ -972,19 +979,13 @@ def _adopt_image_context() -> None: ) # Matching on bare filenames is only meaningful once we know both sides are the - # same dataset; otherwise DLC's fixed frame naming makes unrelated folders match. - policy = ( - PathMatchPolicy.ORDERED_DEPTHS - if self._same_dataset_folder(md.get("root")) - else PathMatchPolicy.DATASET_SCOPED - ) - + # same dataset; to avoid fixed frame naming making unrelated folders match. res = remap_layer_data_by_paths( data=layer.data, old_paths=old_paths, new_paths=new_paths, time_col=time_col, - policy=policy, + policy=PathMatchPolicy.ORDERED_DEPTHS, ) logger.debug( diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 65a6ef17..608b68e7 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -91,17 +91,19 @@ class PathMatchPolicy(Enum): - Try matching with depth=3 - If no overlap, try depth=2 - If still no overlap, try depth=1 + + Depth=1 compares bare filenames, and DLC names are standard for extracted frames + (img000.png, img001.png, ...) in every dataset folder of every project, so a match + there does not inform about actual data provenance. + Use this only to realign frames within a dataset already known to be the same. """ ORDERED_DEPTHS = "ordered_depths" - DATASET_SCOPED = "dataset_scoped" @property def depths(self) -> tuple[int, ...]: if self is PathMatchPolicy.ORDERED_DEPTHS: return (3, 2, 1) - if self is PathMatchPolicy.DATASET_SCOPED: - return (3, 2) raise NotImplementedError(f"Unhandled PathMatchPolicy: {self}") @@ -735,6 +737,26 @@ def infer_dlc_project_from_video_path( # ----------------------------------------------------------------------------- # Lifecycle/session helpers # ----------------------------------------------------------------------------- +def dataset_key_for_folder(folder: str | Path | None) -> str | None: + """Stable identity of the dataset folder a layer was read from. + + Assigned once at read time and never rewritten. ``root`` and ``paths`` cannot serve + this purpose: adopting a new image context overwrites them, so using them as evidence + for whether that adoption should happen is circular. + + Note this is dataset-level, not project-level: `session_key_from_project_context` + resolves to the project root and so cannot tell two videos in one project apart. + """ + if not folder: + return None + + try: + return str(Path(folder).expanduser().resolve()) + except Exception: + logger.debug("Could not resolve dataset folder %r", folder, exc_info=True) + return str(folder) + + def session_key_from_project_context(ctx: DLCProjectContext | None) -> str | None: """ Build a stable session key from the strongest available project context hint. From c4ea1659f1f723c7577d31ff20b90af4f7552370 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 13:17:45 +0200 Subject: [PATCH 11/41] Refocus remap tests on dataset_key identity Updates remap-related tests to reflect the new identity model: dataset ownership is determined by `dataset_key` (absolute dataset folder), not by basename/path-match policy alone. The layer manager tests now cover cross-dataset rejection, cross-project same-folder-name rejection, same-dataset remap adoption, and adoption of unbound placeholder layers. Related path/remap tests were simplified by removing `DATASET_SCOPED` policy assertions and adding checks that dataset keys round-trip and remain distinct across projects. --- .../_tests/core/layer_manager/test_manager.py | 111 ++++++++++-------- .../_tests/core/test_project_paths.py | 43 +++---- .../_tests/core/test_remap.py | 33 ++---- 3 files changed, 89 insertions(+), 98 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index ffc08d7a..56c25517 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -707,21 +707,39 @@ def attach(store, controls, resources): # --------------------------------------------------------------------------- -# _remap_frame_indices: root and paths must move together +# _remap_frame_indices # --------------------------------------------------------------------------- -def _points_bound_to(paths, *, root): +def _points_bound_to(paths, *, root, dataset_key=None): + """A Points layer as the readers build one: paths, root, and an immutable identity.""" layer = make_nonempty_points("bound") - layer.metadata = {"paths": list(paths), "root": root} + layer.metadata = { + "paths": list(paths), + "root": root, + "dataset_key": root if dataset_key is None else dataset_key, + } return layer -def test_remap_frame_indices_leaves_metadata_alone_when_nothing_maps(monkeypatch): - old_paths = ["labeled-data/videoA/imgA000.png"] +def _manager_showing(layer, *, paths, root, dataset_key=None): + """A manager whose image context is the given folder.""" + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + manager._image_meta = ImageMetadata(paths=list(paths), root=root) + manager._image_dataset_key = root if dataset_key is None else dataset_key + return manager + + +def test_remap_frame_indices_refuses_a_layer_from_another_dataset(monkeypatch): + """Identity is decided by dataset_key, whatever the frame names happen to be. + + These two folders share every frame name, which is the DLC norm rather than evidence + that they hold the same footage. + """ + old_paths = ["labeled-data/videoA/img000.png", "labeled-data/videoA/img001.png"] layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") - manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata( - paths=["labeled-data/videoB/imgB000.png"], + manager = _manager_showing( + layer, + paths=["labeled-data/videoB/img000.png", "labeled-data/videoB/img001.png"], root="C:/project/labeled-data/videoB", ) @@ -735,16 +753,15 @@ def test_remap_frame_indices_leaves_metadata_alone_when_nothing_maps(monkeypatch assert warned == [layer] -def test_remap_frame_indices_leaves_metadata_alone_when_match_is_ambiguous(monkeypatch): - # Both old paths collapse onto the same basename, so the only available match is a - # depth-1 one that remap refuses. - old_paths = ["labeled-data/videoA/img0.png", "labeled-data/videoB/img0.png"] - layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") +def test_remap_frame_indices_refuses_another_project_with_the_same_video_name(monkeypatch): + """The keys are absolute, so two projects holding `labeled-data/mouse1` stay distinct.""" + old_paths = ["labeled-data/mouse1/img000.png"] + layer = _points_bound_to(old_paths, root="C:/project-A/labeled-data/mouse1") - manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata( - paths=["other/videoC/img0.png", "other/videoC/img1.png"], - root="C:/project/labeled-data/videoC", + manager = _manager_showing( + layer, + paths=["labeled-data/mouse1/img000.png"], + root="C:/project-B/labeled-data/mouse1", ) warned = [] @@ -752,33 +769,19 @@ def test_remap_frame_indices_leaves_metadata_alone_when_match_is_ambiguous(monke manager._remap_frame_indices(layer) - assert layer.metadata["paths"] == old_paths - assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert layer.metadata["root"] == "C:/project-A/labeled-data/mouse1" assert warned == [layer] -def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): - new_paths = ["labeled-data/videoA/imgA000.png"] - layer = _points_bound_to(["old/labeled-data/videoA/imgA000.png"], root="D:/moved/labeled-data/videoA") - - manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata(paths=new_paths, root="C:/project/labeled-data/videoA") - - manager._remap_frame_indices(layer) - - assert layer.metadata["paths"] == new_paths - assert layer.metadata["root"] == "C:/project/labeled-data/videoA" - - -def test_remap_frame_indices_refuses_a_different_folder_with_the_same_frame_names(monkeypatch): - """DLC frame names repeat across datasets, so a full basename match is not identity.""" - old_paths = ["labeled-data/videoA/img000.png", "labeled-data/videoA/img001.png"] +def test_remap_frame_indices_leaves_metadata_alone_when_nothing_maps(monkeypatch): + """Same dataset, but no frame overlap: adopt neither root nor paths.""" + old_paths = ["labeled-data/videoA/imgA000.png"] layer = _points_bound_to(old_paths, root="C:/project/labeled-data/videoA") - manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata( - paths=["labeled-data/videoB/img000.png", "labeled-data/videoB/img001.png"], - root="C:/project/labeled-data/videoB", + manager = _manager_showing( + layer, + paths=["labeled-data/videoA/renamed000.png"], + root="C:/project/labeled-data/videoA", ) warned = [] @@ -791,18 +794,34 @@ def test_remap_frame_indices_refuses_a_different_folder_with_the_same_frame_name assert warned == [layer] -def test_remap_frame_indices_allows_basename_match_within_the_same_dataset_folder(): - """A rewritten prefix is the case depth-1 matching exists to repair.""" - new_paths = ["/mnt/moved/labeled-data/videoA/img000.png"] - layer = _points_bound_to(["img000.png"], root="C:/project/labeled-data/videoA") +def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): + """The project moved: same dataset, new prefix, so both fields follow.""" + new_paths = ["labeled-data/videoA/imgA000.png"] + layer = _points_bound_to( + ["old/labeled-data/videoA/imgA000.png"], + root="D:/moved/labeled-data/videoA", + dataset_key="C:/project/labeled-data/videoA", + ) - manager = LayerLifecycleManager(viewer=DummyViewer([layer])) - manager._image_meta = ImageMetadata(paths=new_paths, root="/mnt/moved/labeled-data/videoA") + manager = _manager_showing(layer, paths=new_paths, root="C:/project/labeled-data/videoA") manager._remap_frame_indices(layer) assert layer.metadata["paths"] == new_paths - assert layer.metadata["root"] == "/mnt/moved/labeled-data/videoA" + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + + +def test_remap_frame_indices_adopts_an_unbound_layer(): + """A config placeholder has no dataset of its own, so it takes whatever is open.""" + new_paths = ["labeled-data/videoA/img000.png"] + layer = make_points("placeholder") + layer.metadata = {"project": "C:/project"} + + manager = _manager_showing(layer, paths=new_paths, root="C:/project/labeled-data/videoA") + + manager._remap_frame_indices(layer) + + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): diff --git a/src/napari_deeplabcut/_tests/core/test_project_paths.py b/src/napari_deeplabcut/_tests/core/test_project_paths.py index 6e619e15..57b290bb 100644 --- a/src/napari_deeplabcut/_tests/core/test_project_paths.py +++ b/src/napari_deeplabcut/_tests/core/test_project_paths.py @@ -87,39 +87,28 @@ def test_path_match_policy_ordered_depths(): assert paths_mod.PathMatchPolicy.ORDERED_DEPTHS.depths == (3, 2, 1) -def test_path_match_policy_dataset_scoped_stops_above_basenames(): - assert paths_mod.PathMatchPolicy.DATASET_SCOPED.depths == (3, 2) +def test_dataset_key_is_absolute_two_projects_stay_distinct(tmp_path: Path): + """The dataset folder name alone repeats across projects; the resolved path does not.""" + a = tmp_path / "project-A" / "labeled-data" / "mouse1" + b = tmp_path / "project-B" / "labeled-data" / "mouse1" + a.mkdir(parents=True) + b.mkdir(parents=True) + key_a = paths_mod.dataset_key_for_folder(a) + key_b = paths_mod.dataset_key_for_folder(b) -def test_find_matching_depth_dataset_scoped_refuses_basename_only_overlap(): - """DLC frame names collide across datasets, so a basename match proves nothing.""" - old_paths = ["/project/labeled-data/videoA/img001.png"] - new_paths = ["/project/labeled-data/videoB/img001.png"] + assert key_a != key_b + assert key_a == paths_mod.dataset_key_for_folder(str(a)) + assert paths_mod.dataset_key_for_folder(None) is None - assert paths_mod.find_matching_depth(old_paths, new_paths) == 1 - assert ( - paths_mod.find_matching_depth( - old_paths, - new_paths, - policy=paths_mod.PathMatchPolicy.DATASET_SCOPED, - ) - is None - ) +def test_points_metadata_round_trip_preserves_dataset_key(): + """Identity must survive the metadata sync that runs on every image insert.""" + from napari_deeplabcut.config.models import PointsMetadata -def test_find_matching_depth_dataset_scoped_still_matches_a_moved_project(): - """Only the prefix changed, so depth=2 still identifies the dataset.""" - old_paths = ["C:/old/labeled-data/videoA/img001.png"] - new_paths = ["/mnt/new/place/labeled-data/videoA/img001.png"] + meta = PointsMetadata(root="C:/p/labeled-data/videoA", dataset_key="C:/p/labeled-data/videoA") - assert ( - paths_mod.find_matching_depth( - old_paths, - new_paths, - policy=paths_mod.PathMatchPolicy.DATASET_SCOPED, - ) - == 3 - ) + assert PointsMetadata(**meta.model_dump()).dataset_key == "C:/p/labeled-data/videoA" def test_find_matching_depth_prefers_deepest_first_match(): diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index d364ac94..4b3d1f1d 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -309,35 +309,18 @@ def test_ambiguous_depth1_remap_is_rejected_and_refuses_paths_update(): assert res.changed is False -def test_dataset_scoped_policy_refuses_a_clean_basename_only_match(): - """The dangerous case: a perfect 1:1 match that proves nothing about identity. - - ORDERED_DEPTHS accepts this and would migrate the data onto another dataset's frames. - """ - old_paths = ["p/labeled-data/videoA/img000.png", "p/labeled-data/videoA/img001.png"] - new_paths = ["p/labeled-data/videoB/img000.png", "p/labeled-data/videoB/img001.png"] - data = np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float) - - permissive = remap_layer_data_by_paths( - data=data, - old_paths=old_paths, - new_paths=new_paths, +def test_basename_only_match_is_accepted_and_cannot_prove_identity(): + """`LayerLifecycleManager` gates on `dataset_key` for that reason.""" + res = remap_layer_data_by_paths( + data=np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float), + old_paths=["p/labeled-data/videoA/img000.png", "p/labeled-data/videoA/img001.png"], + new_paths=["p/labeled-data/videoB/img000.png", "p/labeled-data/videoB/img001.png"], time_col=0, policy=PathMatchPolicy.ORDERED_DEPTHS, ) - assert permissive.depth_used == 1 - assert permissive.accept_paths_update is True - scoped = remap_layer_data_by_paths( - data=data, - old_paths=old_paths, - new_paths=new_paths, - time_col=0, - policy=PathMatchPolicy.DATASET_SCOPED, - ) - assert scoped.depth_used is None - assert scoped.accept_paths_update is False - assert scoped.applied is False + assert res.depth_used == 1 + assert res.accept_paths_update is True def test_no_overlap_remap_refuses_paths_update(): From 1dd8292ba0443bc99de3b3db80ee6d88cedcace5 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 13:28:51 +0200 Subject: [PATCH 12/41] Refine dataset mismatch layer warnings Use the image dataset key when tracking repeated dataset-mismatch warnings so duplicate notifications are suppressed consistently across folder changes. The commit also tightens the status message shown to users and clarifies the filename-matching comment around remapping frame paths. --- .../core/layer_lifecycle/manager.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 8454b748..9679e3f4 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -388,20 +388,20 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: """ root = (layer.metadata or {}).get("root") dataset = Path(str(root)).name if root else "its original folder" - new_root = str(self._image_meta.root or "") + target = self._image_dataset_key or str(self._image_meta.root or "") - if self._dataset_mismatch_warned.get(layer) == new_root: + if self._dataset_mismatch_warned.get(layer) == target: logger.debug( "Extra dataset-mismatch notification for layer=%r folder=%r", getattr(layer, "name", layer), - new_root, + target, ) return - self._dataset_mismatch_warned[layer] = new_root + self._dataset_mismatch_warned[layer] = target reason = ( f"'{getattr(layer, 'name', layer)}' does not contain any of the frames in the folder " - f"you just opened, so it still will save to '{dataset}'.\n" + f"you just opened; it will still save as '{dataset}'.\n" "Clear it before labelling the new folder." ) self.viewer.status = reason @@ -979,7 +979,7 @@ def _adopt_image_context() -> None: ) # Matching on bare filenames is only meaningful once we know both sides are the - # same dataset; to avoid fixed frame naming making unrelated folders match. + # same dataset; so extract_frames's fixed frame naming cannot make unrelated folders match. res = remap_layer_data_by_paths( data=layer.data, old_paths=old_paths, From f5549844e1acef5783f3d4e2502a54dfcb94a9a1 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 13:50:16 +0200 Subject: [PATCH 13/41] Resolve dataset key aliases across paths Add `is_same_dataset` in `project_paths` to compare dataset folders robustly using normalized case and `os.path.samefile`, with fail-closed behavior when on-disk comparison is unavailable. Update layer lifecycle matching to use this helper instead of raw string equality, so equivalent paths (e.g., symlink/UNC aliases and Windows case variants) are treated as the same dataset. Expand tests to cover identical/distinct paths, missing folders, alternate routes to one folder, and Windows case-insensitive matching. --- .../_tests/core/test_project_paths.py | 41 +++++++++++++++++++ .../core/layer_lifecycle/manager.py | 4 +- src/napari_deeplabcut/core/project_paths.py | 20 +++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/test_project_paths.py b/src/napari_deeplabcut/_tests/core/test_project_paths.py index 57b290bb..bf9ac230 100644 --- a/src/napari_deeplabcut/_tests/core/test_project_paths.py +++ b/src/napari_deeplabcut/_tests/core/test_project_paths.py @@ -1,6 +1,7 @@ from __future__ import annotations import inspect +import os from pathlib import Path from types import SimpleNamespace @@ -111,6 +112,46 @@ def test_points_metadata_round_trip_preserves_dataset_key(): assert PointsMetadata(**meta.model_dump()).dataset_key == "C:/p/labeled-data/videoA" +def test_is_same_dataset_matches_identical_and_rejects_distinct(tmp_path: Path): + a = tmp_path / "labeled-data" / "videoA" + b = tmp_path / "labeled-data" / "videoB" + a.mkdir(parents=True) + b.mkdir(parents=True) + + assert paths_mod.is_same_dataset(str(a), str(a)) is True + assert paths_mod.is_same_dataset(str(a), str(b)) is False + assert paths_mod.is_same_dataset(None, str(a)) is False + assert paths_mod.is_same_dataset(str(a), None) is False + + +def test_is_same_dataset_fails_closed_for_missing_folders(tmp_path: Path): + """Two spellings that cannot be compared on disk are not assumed to be the same.""" + assert paths_mod.is_same_dataset(str(tmp_path / "gone-a"), str(tmp_path / "gone-b")) is False + + +def test_is_same_dataset_sees_through_a_second_route_to_one_folder(tmp_path: Path): + """A mapped drive against its UNC path is the real case; a symlink stands in for it.""" + real = tmp_path / "labeled-data" / "videoA" + real.mkdir(parents=True) + link = tmp_path / "via-link" + try: + link.symlink_to(real, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symlink creation not permitted here") + + # Deliberately unresolved, so the strings differ and the on-disk check is what decides. + assert str(link) != str(real) + assert paths_mod.is_same_dataset(str(link), str(real)) is True + + +@pytest.mark.skipif(os.name != "nt", reason="path case is only insensitive on Windows") +def test_is_same_dataset_ignores_case_on_windows(tmp_path: Path): + a = tmp_path / "labeled-data" / "videoA" + a.mkdir(parents=True) + + assert paths_mod.is_same_dataset(str(a), str(a).upper()) is True + + def test_find_matching_depth_prefers_deepest_first_match(): old_paths = [ "/project/labeled-data/mouse1/img001.png", diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 9679e3f4..f0806ba5 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -28,7 +28,7 @@ sync_points_from_image, write_points_meta, ) -from ...core.project_paths import PathMatchPolicy +from ...core.project_paths import PathMatchPolicy, is_same_dataset from ...core.remap import remap_layer_data_by_paths from ...napari_compat import install_add_wrapper, install_paste_patch, layer_key, unwrap from ...napari_compat.points_layer import make_paste_data @@ -377,7 +377,7 @@ def _belongs_to_current_dataset(self, layer_dataset_key: str | None) -> bool: if layer_dataset_key is None: return True - return layer_dataset_key == self._image_dataset_key + return is_same_dataset(layer_dataset_key, self._image_dataset_key) def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: """Report that a layer did not follow the newly opened folder. diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 608b68e7..3bc32ee0 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -19,6 +19,7 @@ from __future__ import annotations import logging +import os from collections.abc import Iterable from enum import Enum from pathlib import Path, PureWindowsPath @@ -757,6 +758,25 @@ def dataset_key_for_folder(folder: str | Path | None) -> str | None: return str(folder) +def is_same_dataset(a: str | None, b: str | None) -> bool: + """Return True if two dataset keys name the same folder on disk. + + The same folder reaches us under more than one spelling: differing case or separators + on Windows, and a mapped drive against the UNC path behind it. + """ + if a is None or b is None: + return False + + if os.path.normcase(a) == os.path.normcase(b): + return True + + try: + return os.path.samefile(a, b) + except OSError: + logger.debug("Could not compare dataset folders %r and %r on disk", a, b, exc_info=True) + return False + + def session_key_from_project_context(ctx: DLCProjectContext | None) -> str | None: """ Build a stable session key from the strongest available project context hint. From 6a19779d3acdcec2b645b19d512d03addc80e42b Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 14:04:38 +0200 Subject: [PATCH 14/41] Bind adopted datasets for unbound layers Treat config placeholders as dataset-bound when they adopt image context, so layers that start without a `dataset_key` keep the dataset they first take over instead of being remapped to a later folder with matching frame names. --- .../_tests/core/layer_manager/test_manager.py | 30 +++++++++++++++++++ .../core/layer_lifecycle/manager.py | 7 ++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 56c25517..3e1f0841 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -849,3 +849,33 @@ def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): manager._remap_frame_indices(layer) assert rec.dataset_mismatch.count == 2 + + +def test_an_unbound_layer_binds_to_the_dataset_it_adopts(monkeypatch): + """A config placeholder must stop being unbound once it takes a folder.""" + layer = make_points("placeholder") + layer.metadata = {"project": "C:/project"} + + manager = _manager_showing( + layer, + paths=["labeled-data/videoA/img000.png"], + root="C:/project/labeled-data/videoA", + ) + manager._remap_frame_indices(layer) + + assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + + # Now a different folder reusing the same frame names must be refused. + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoB/img000.png"], + root="C:/project/labeled-data/videoB", + ) + manager._image_dataset_key = "C:/project/labeled-data/videoB" + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._remap_frame_indices(layer) + + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index f0806ba5..fa9c5b05 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -938,16 +938,21 @@ def _remap_frame_indices(self, layer: Any) -> None: return def _adopt_image_context() -> None: - """Take root/shape/name from the image context. + """Take root/shape/name from the image context, and bind the dataset. Only safe alongside a `paths` update: a layer whose root names one dataset while its paths name another saves into the first and is indexed against the second. + + Binding matters for layers that start unbound, such as the config.yaml + placeholder. """ try: safe_image_meta = self._image_meta.model_dump(exclude_none=True) safe_image_meta.pop("paths", None) layer.metadata.update(safe_image_meta) + if layer.metadata.get("dataset_key") is None and self._image_dataset_key is not None: + layer.metadata["dataset_key"] = self._image_dataset_key except Exception: logger.debug( "Failed to sync non-path image metadata for layer=%r", From c83ee8d33d10a07a7c9eb0420e17dde30e85dbfd Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 14:21:51 +0200 Subject: [PATCH 15/41] Stop rewriting points roots from image metadata The override rewrote `root` when the points root equalled the project root or was not a dataset root, so `sync_points_from_image` had two jobs: seeding missing image-derived fields, and second-guessing a root that was already set. Only the first was reachable. --- .../_tests/core/test_metadata.py | 60 +------------ src/napari_deeplabcut/core/metadata.py | 87 +------------------ 2 files changed, 6 insertions(+), 141 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/test_metadata.py b/src/napari_deeplabcut/_tests/core/test_metadata.py index d6cbad65..dae31b6a 100644 --- a/src/napari_deeplabcut/_tests/core/test_metadata.py +++ b/src/napari_deeplabcut/_tests/core/test_metadata.py @@ -39,42 +39,6 @@ def __init__(self, metadata=None, name="dummy-layer"): # ----------------------------------------------------------------------------- -@pytest.mark.parametrize( - ("path_str", "expected"), - [ - ("project/labeled-data/mouse1", True), - ("project/LABELED-DATA/mouse1", True), - ("project/labeled-data", False), - ("project/images/mouse1", False), - ], -) -def test_is_dlc_dataset_root(path_str: str, expected: bool): - assert metadata_mod._is_dlc_dataset_root(Path(path_str)) is expected - - -@pytest.mark.parametrize( - ("paths", "expected"), - [ - (None, False), - ([], False), - (["images/img001.png"], False), - (["labeled-data/test/img001.png"], True), - ([r"labeled-data\test\img001.png"], True), - ], -) -def test_paths_look_like_labeled_data(paths, expected): - assert metadata_mod._paths_look_like_labeled_data(paths) is expected - - -def test_looks_like_project_root_true_when_same_path(tmp_path: Path): - assert metadata_mod._looks_like_project_root(str(tmp_path), str(tmp_path)) is True - - -def test_looks_like_project_root_false_when_different(tmp_path: Path): - other = tmp_path / "other" - assert metadata_mod._looks_like_project_root(str(tmp_path), str(other)) is False - - def test_infer_image_root_prefers_explicit_root(tmp_path: Path): p = tmp_path / "images" / "img001.png" p.parent.mkdir(parents=True) @@ -188,27 +152,7 @@ def test_sync_points_from_image_fills_missing_fields(): assert synced.name == "images" -def test_sync_points_from_image_overrides_project_root_with_dataset_root(tmp_path: Path): - project_root = tmp_path / "project" - dataset_root = project_root / "labeled-data" / "mouse1" - dataset_root.mkdir(parents=True) - - image_meta = ImageMetadata( - root=str(dataset_root), - paths=[str(dataset_root / "img001.png")], - name="images", - ) - points_meta = PointsMetadata( - root=str(project_root), # stale / wrong - project=str(project_root), - ) - - synced = metadata_mod.sync_points_from_image(image_meta, points_meta) - - assert synced.root == str(dataset_root) - - -def test_sync_points_from_image_keeps_existing_dataset_root_when_already_good(tmp_path: Path): +def test_sync_points_from_image_never_rewrites_a_root_that_is_already_set(tmp_path: Path): project_root = tmp_path / "project" good_points_root = project_root / "labeled-data" / "mouse1" other_dataset_root = project_root / "labeled-data" / "mouse2" @@ -223,7 +167,7 @@ def test_sync_points_from_image_keeps_existing_dataset_root_when_already_good(tm synced = metadata_mod.sync_points_from_image(image_meta, points_meta) - # already a valid dataset root -> do not overwrite + # Which dataset a layer belongs to is settled by dataset_key, not re-derived here. assert synced.root == str(good_points_root) diff --git a/src/napari_deeplabcut/core/metadata.py b/src/napari_deeplabcut/core/metadata.py index 916e3802..1394ebac 100644 --- a/src/napari_deeplabcut/core/metadata.py +++ b/src/napari_deeplabcut/core/metadata.py @@ -20,53 +20,6 @@ # ----------------------------------------------------------------------------- # Inference # ----------------------------------------------------------------------------- -def _coerce_path(p: str | None) -> Path | None: - if not p: - return None - try: - return Path(p).expanduser().resolve() - except Exception: - return Path(p) - - -def _is_dlc_dataset_root(p: Path) -> bool: - """ - Heuristic: DLC dataset folder usually looks like: - /labeled-data/ - - True if path contains a 'labeled-data' segment AND is deeper than that folder. - """ - parts = [s.lower() for s in p.parts] - if "labeled-data" not in parts: - return False - return parts[-1] != "labeled-data" - - -def _paths_look_like_labeled_data(paths: list[str] | None) -> bool: - """ - Check if any path strings contain 'labeled-data//'. - Works with canonicalized paths like 'labeled-data/test/img000.png'. - """ - if not paths: - return False - for s in paths: - if isinstance(s, str) and "labeled-data" in s.replace("\\", "/").lower(): - return True - return False - - -def _looks_like_project_root(points_root: str | None, project: str | None) -> bool: - """ - Root equals project root (config parent) => this is WRONG for saving GT. - """ - if not points_root or not project: - return False - try: - return Path(points_root).expanduser().resolve() == Path(project).expanduser().resolve() - except Exception: - return points_root == project - - def build_io_provenance_dict( *, project_root: str | Path, @@ -152,52 +105,20 @@ def merge_points_metadata(base: PointsMetadata, incoming: PointsMetadata) -> Poi # ----------------------------------------------------------------------------- def sync_points_from_image(image_meta: ImageMetadata, points_meta: PointsMetadata) -> PointsMetadata: """ - Ensure PointsMetadata contains required image-derived fields. + Fill image-derived fields that the Points layer does not have yet. - Robust DLC policy: - - If image root looks like a DLC dataset folder (…/labeled-data/), - prefer it for points_meta.root even if points_meta.root is already set - but equals project root (config parent) or is not a dataset root. + Only seeds what is missing. A field already set on the layer is never rewritten here: + which dataset a layer belongs to is decided by its `dataset_key`, and rewriting `root` + behind that decision is what let annotations follow the wrong folder. """ updated = points_meta.model_dump(mode="python") - # --- First: fill missing fields (existing behavior) --- for key in ("root", "paths", "shape", "name"): if updated.get(key) in (None, "", []): value = getattr(image_meta, key, None) if value not in (None, "", []): updated[key] = value - # --- Second: if we have dataset context, correct stale root --- - img_root_p = _coerce_path(getattr(image_meta, "root", None)) - pts_root_p = _coerce_path(updated.get("root")) - project_p = _coerce_path(updated.get("project")) - - # Determine if the image root is a DLC dataset directory - image_is_dataset = bool(img_root_p is not None and _is_dlc_dataset_root(img_root_p)) - - # Additional hint: sometimes image_meta.root might be missing, but paths show labeled-data - # (depends on readers / napari versions). Use that as secondary signal. - if not image_is_dataset: - if _paths_look_like_labeled_data(getattr(image_meta, "paths", None)): - # If image paths look like labeled-data/... and we have a root-like string, - # try to interpret image_meta.root anyway. - image_is_dataset = bool(img_root_p is not None and _is_dlc_dataset_root(img_root_p)) - - if image_is_dataset and img_root_p is not None: - # Override root if: - # - points root equals project root (typical config-first bug), OR - # - points root exists but isn't a dataset root. - should_override_root = False - - if _looks_like_project_root(str(pts_root_p) if pts_root_p else None, str(project_p) if project_p else None): - should_override_root = True - elif pts_root_p is not None and not _is_dlc_dataset_root(pts_root_p): - should_override_root = True - - if should_override_root: - updated["root"] = str(img_root_p) - return PointsMetadata(**updated) From 26d0c344a449e08d100834a53b743e2ba65cfb92 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 15 Sep 2026 14:23:33 +0200 Subject: [PATCH 16/41] Clarify warning message --- src/napari_deeplabcut/core/layer_lifecycle/manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index fa9c5b05..c85b9f1b 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -400,9 +400,9 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: self._dataset_mismatch_warned[layer] = target reason = ( - f"'{getattr(layer, 'name', layer)}' does not contain any of the frames in the folder " - f"you just opened; it will still save as '{dataset}'.\n" - "Clear it before labelling the new folder." + f"'{getattr(layer, 'name', layer)}' could not be matched to the frames in the folder " + f"that was opened; it will still save as '{dataset}'.\n" + "Please clear it before labelling the new folder." ) self.viewer.status = reason From 593233fa474868af401865e675fef938e3cb9739 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Thu, 17 Sep 2026 15:49:13 +0200 Subject: [PATCH 17/41] Guard layer dataset adoption Centralize dataset-follow checks for layer metadata seeding and bind layers to the dataset they actually adopt. This prevents points and image layers from inheriting paths or context from the wrong folder, especially when a layer is already tied to another dataset or starts unbound. --- .../core/layer_lifecycle/manager.py | 83 +++++++++++++++---- 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index c85b9f1b..4246b9bc 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -408,6 +408,45 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: self._single_shot_owned(0, lambda: self.layer_dataset_mismatch.emit(reason)) + def _may_follow_current_dataset(self, layer: Any) -> bool: + """Whether `layer` may take metadata from the folder now open. + + Every path that seeds image context onto a layer should use this. + A layer bound to another dataset keeps its own `root` and `paths`, and is reported once per + folder; seeding it would leave the two naming different datasets, so it would + save into one and be indexed against the other. + + With no image context loaded, a layer that will not be written to must not be reported either. + """ + metadata = layer.metadata or {} + + if self._belongs_to_current_dataset(metadata.get("dataset_key")): + return True + + logger.warning( + "Layer %r belongs to %s, not to the folder just opened (%s); leaving it alone.", + getattr(layer, "name", str(layer)), + metadata.get("dataset_key"), + self._image_dataset_key, + ) + self._report_layer_left_on_previous_dataset(layer) + return False + + def _bind_to_current_dataset(self, layer: Any) -> None: + """Bind a layer to the dataset whose frames it registered. + + A layer can arrive unbound e.g. config placeholder and + `_belongs_to_current_dataset` lets it adopt from already open context. + Recording the key it adopted prevents a later folder with the same frame names claiming it. + Call only once the layer's `paths` come from the open folder. + """ + metadata = layer.metadata + if metadata is None: + return + + if metadata.get("dataset_key") is None and self._image_dataset_key is not None: + metadata["dataset_key"] = self._image_dataset_key + def _reject_conflicting_dlc_image_layer(self, layer: Image, reason: str) -> None: """Reject a conflicting DLC session image safely. @@ -629,6 +668,8 @@ def _sync_points_layers_from_image_meta(self) -> None: if self._image_meta is None: return + has_context = bool(self._image_meta.root or self._image_meta.paths) + for ly in list(self.viewer.layers): if not isinstance(ly, Points): continue @@ -636,6 +677,14 @@ def _sync_points_layers_from_image_meta(self) -> None: if ly.metadata is None: ly.metadata = {} + # This seeds whatever the layer is missing, so it runs before the remap sweep + # A layer from another dataset must be skipped here too, or + # it is handed this folder's paths. + if has_context and not self._may_follow_current_dataset(ly): + continue + + adopts_paths = not ly.metadata.get("paths") and bool(self._image_meta.paths) + res = read_points_meta(ly, migrate_legacy=True, drop_controls=False, drop_header=False) if hasattr(res, "errors"): logger.warning( @@ -661,6 +710,10 @@ def _sync_points_layers_from_image_meta(self) -> None: getattr(ly, "name", ly), out, ) + continue + + if adopts_paths: + self._bind_to_current_dataset(ly) def _cache_project_path_from_image_layer(self, layer: Image) -> None: """Best-effort lifecycle-owned cache of project path from an image/video layer.""" @@ -771,10 +824,17 @@ def _wire_points_layer(self, layer: Points) -> KeypointStore | None: if proj: self._project_path = proj - if not layer.metadata.get("root") and self._image_meta.root: - layer.metadata["root"] = self._image_meta.root - if not layer.metadata.get("paths") and self._image_meta.paths: - layer.metadata["paths"] = self._image_meta.paths + # Seed only what the layer is missing, and only from a folder it is allowed to follow. + # Loading annotations before any image should not trigger any seeding. + seeds_root = not layer.metadata.get("root") and self._image_meta.root + seeds_paths = not layer.metadata.get("paths") and self._image_meta.paths + + if (seeds_root or seeds_paths) and self._may_follow_current_dataset(layer): + if seeds_root: + layer.metadata["root"] = self._image_meta.root + if seeds_paths: + layer.metadata["paths"] = self._image_meta.paths + self._bind_to_current_dataset(layer) if root := layer.metadata.get("root"): update_save_history(root) @@ -927,14 +987,7 @@ def _remap_frame_indices(self, layer: Any) -> None: md = layer.metadata old_paths = md.get("paths") or [] - if not self._belongs_to_current_dataset(md.get("dataset_key")): - logger.warning( - "Layer %r belongs to %s, not to the folder just opened (%s); leaving it alone.", - getattr(layer, "name", str(layer)), - md.get("dataset_key"), - self._image_dataset_key, - ) - self._report_layer_left_on_previous_dataset(layer) + if not self._may_follow_current_dataset(layer): return def _adopt_image_context() -> None: @@ -943,16 +996,12 @@ def _adopt_image_context() -> None: Only safe alongside a `paths` update: a layer whose root names one dataset while its paths name another saves into the first and is indexed against the second. - - Binding matters for layers that start unbound, such as the config.yaml - placeholder. """ try: safe_image_meta = self._image_meta.model_dump(exclude_none=True) safe_image_meta.pop("paths", None) layer.metadata.update(safe_image_meta) - if layer.metadata.get("dataset_key") is None and self._image_dataset_key is not None: - layer.metadata["dataset_key"] = self._image_dataset_key + self._bind_to_current_dataset(layer) except Exception: logger.debug( "Failed to sync non-path image metadata for layer=%r", From bf3edf86fb1a06b5e80669bb082cb48115dc2659 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Thu, 17 Sep 2026 15:56:08 +0200 Subject: [PATCH 18/41] Tighten dataset context inheritance Clarify and tighten how points layers inherit image context from the currently open folder. Layers now consistently check whether they may follow the current dataset before inheriting `root` or `paths`, and the helper that records the adopted dataset key is renamed to better reflect that it only persists the folder identity after paths are inherited. This helps prevent layers from being accidentally associated with the wrong dataset when folders share frame names. --- .../core/layer_lifecycle/manager.py | 55 +++++++++---------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 4246b9bc..f514065c 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -409,15 +409,18 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: self._single_shot_owned(0, lambda: self.layer_dataset_mismatch.emit(reason)) def _may_follow_current_dataset(self, layer: Any) -> bool: - """Whether `layer` may take metadata from the folder now open. + """Whether `layer` may take `root` or `paths` from the folder now open. - Every path that seeds image context onto a layer should use this. - A layer bound to another dataset keeps its own `root` and `paths`, and is reported once per - folder; seeding it would leave the two naming different datasets, so it would + Every path that hands a layer image context should ask this. A layer from another + dataset keeps its own `root` and `paths` and is reported once per folder: giving + it this folder's paths would leave the two naming different datasets, so it would save into one and be indexed against the other. - With no image context loaded, a layer that will not be written to must not be reported either. + With no folder open there is nothing to follow. """ + if not (self._image_meta.root or self._image_meta.paths): + return True + metadata = layer.metadata or {} if self._belongs_to_current_dataset(metadata.get("dataset_key")): @@ -432,13 +435,12 @@ def _may_follow_current_dataset(self, layer: Any) -> bool: self._report_layer_left_on_previous_dataset(layer) return False - def _bind_to_current_dataset(self, layer: Any) -> None: - """Bind a layer to the dataset whose frames it registered. + def _record_dataset_key(self, layer: Any) -> None: + """Name the folder a layer just took its paths from, if it did not name one. - A layer can arrive unbound e.g. config placeholder and - `_belongs_to_current_dataset` lets it adopt from already open context. - Recording the key it adopted prevents a later folder with the same frame names claiming it. - Call only once the layer's `paths` come from the open folder. + A config placeholder arrives with no `dataset_key`, so `_belongs_to_current_dataset` + lets it follow what is currently open. Persisting the dataset key stops the next folder + with the same frame names from following as well. """ metadata = layer.metadata if metadata is None: @@ -668,8 +670,6 @@ def _sync_points_layers_from_image_meta(self) -> None: if self._image_meta is None: return - has_context = bool(self._image_meta.root or self._image_meta.paths) - for ly in list(self.viewer.layers): if not isinstance(ly, Points): continue @@ -677,13 +677,12 @@ def _sync_points_layers_from_image_meta(self) -> None: if ly.metadata is None: ly.metadata = {} - # This seeds whatever the layer is missing, so it runs before the remap sweep - # A layer from another dataset must be skipped here too, or - # it is handed this folder's paths. - if has_context and not self._may_follow_current_dataset(ly): + # This runs before the remap sweep, so it has to make the same call: a layer + # from another dataset must not be handed this folder's paths here either. + if not self._may_follow_current_dataset(ly): continue - adopts_paths = not ly.metadata.get("paths") and bool(self._image_meta.paths) + inherits_paths = not ly.metadata.get("paths") and bool(self._image_meta.paths) res = read_points_meta(ly, migrate_legacy=True, drop_controls=False, drop_header=False) if hasattr(res, "errors"): @@ -712,8 +711,8 @@ def _sync_points_layers_from_image_meta(self) -> None: ) continue - if adopts_paths: - self._bind_to_current_dataset(ly) + if inherits_paths: + self._record_dataset_key(ly) def _cache_project_path_from_image_layer(self, layer: Image) -> None: """Best-effort lifecycle-owned cache of project path from an image/video layer.""" @@ -824,17 +823,13 @@ def _wire_points_layer(self, layer: Points) -> KeypointStore | None: if proj: self._project_path = proj - # Seed only what the layer is missing, and only from a folder it is allowed to follow. - # Loading annotations before any image should not trigger any seeding. - seeds_root = not layer.metadata.get("root") and self._image_meta.root - seeds_paths = not layer.metadata.get("paths") and self._image_meta.paths - - if (seeds_root or seeds_paths) and self._may_follow_current_dataset(layer): - if seeds_root: + # Inherit only what the layer never had, and only from a folder it may follow. + if self._may_follow_current_dataset(layer): + if not layer.metadata.get("root") and self._image_meta.root: layer.metadata["root"] = self._image_meta.root - if seeds_paths: + if not layer.metadata.get("paths") and self._image_meta.paths: layer.metadata["paths"] = self._image_meta.paths - self._bind_to_current_dataset(layer) + self._record_dataset_key(layer) if root := layer.metadata.get("root"): update_save_history(root) @@ -1001,7 +996,7 @@ def _adopt_image_context() -> None: safe_image_meta = self._image_meta.model_dump(exclude_none=True) safe_image_meta.pop("paths", None) layer.metadata.update(safe_image_meta) - self._bind_to_current_dataset(layer) + self._record_dataset_key(layer) except Exception: logger.debug( "Failed to sync non-path image metadata for layer=%r", From 37968986a2490b37bdcb6aa5cb18146c4c2acacc Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Thu, 17 Sep 2026 15:57:28 +0200 Subject: [PATCH 19/41] Add dataset inheritance layer tests Cover points-layer syncing and wiring when metadata inherits paths and dataset keys from the open folder. The new tests ensure layers keep their original dataset root, adopt matching paths, and reject paths from a different dataset without spuriously warning when no image is open. --- .../_tests/core/layer_manager/test_manager.py | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 3e1f0841..9cf43dbe 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -851,6 +851,119 @@ def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): assert rec.dataset_mismatch.count == 2 +# --------------------------------------------------------------------------- +# Inheriting root and paths from the open folder +# --------------------------------------------------------------------------- +def _points_keyed_without_paths(*, root): + """A keyed layer with no paths, as a numeric-index h5 produces. + + `read_hdf` leaves `paths` empty when the frame index is numeric, but still records + `root` and `dataset_key`, so a layer can name its dataset while listing no frames. + """ + layer = make_nonempty_points("keyed") + layer.metadata = {"paths": [], "root": root, "dataset_key": root} + return layer + + +def test_sync_from_image_meta_refuses_paths_for_a_layer_from_another_dataset(monkeypatch): + layer = _points_keyed_without_paths(root="C:/project/labeled-data/videoA") + + manager = _manager_showing( + layer, + paths=["labeled-data/videoB/img000.png"], + root="C:/project/labeled-data/videoB", + ) + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._sync_points_layers_from_image_meta() + + assert layer.metadata["paths"] == [] + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] + + +def test_sync_from_image_meta_still_inherits_paths_for_its_own_dataset(): + new_paths = ["labeled-data/videoA/img000.png"] + layer = _points_keyed_without_paths(root="C:/project/labeled-data/videoA") + + manager = _manager_showing(layer, paths=new_paths, root="C:/project/labeled-data/videoA") + + manager._sync_points_layers_from_image_meta() + + assert layer.metadata["paths"] == new_paths + + +def test_sync_from_image_meta_records_the_dataset_key_it_inherits_from(): + layer = make_points("placeholder") + layer.metadata = {"project": "C:/project"} + + manager = _manager_showing( + layer, + paths=["labeled-data/videoA/img000.png"], + root="C:/project/labeled-data/videoA", + ) + + manager._sync_points_layers_from_image_meta() + + assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + + +def test_wire_points_layer_refuses_paths_from_another_dataset(monkeypatch, fake_store): + """Wiring inherits missing paths too, so it asks the same question.""" + layer = _points_keyed_without_paths(root="C:/project/labeled-data/videoA") + + manager = _manager_showing( + layer, + paths=["labeled-data/videoB/img000.png"], + root="C:/project/labeled-data/videoB", + ) + monkeypatch.setattr(manager, "validate_header", lambda _layer: True) + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._wire_points_layer(layer) + + assert layer.metadata["paths"] == [] + assert layer.metadata["root"] == "C:/project/labeled-data/videoA" + assert warned == [layer] + + +def test_wire_points_layer_records_the_dataset_key_it_inherits_from(monkeypatch, fake_store): + """A layer with no dataset of its own keeps the folder it took its paths from.""" + new_paths = ["labeled-data/videoA/img000.png"] + layer = make_nonempty_points("placeholder") + layer.metadata = {"project": "C:/project"} + + manager = _manager_showing(layer, paths=new_paths, root="C:/project/labeled-data/videoA") + monkeypatch.setattr(manager, "validate_header", lambda _layer: True) + + manager._wire_points_layer(layer) + + assert layer.metadata["paths"] == new_paths + assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + + +def test_wire_points_layer_says_nothing_when_no_image_is_open(monkeypatch, fake_store): + """Loading annotations first is the normal way in, not a mismatch.""" + layer = _points_bound_to( + ["labeled-data/videoA/img000.png"], + root="C:/project/labeled-data/videoA", + ) + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + monkeypatch.setattr(manager, "validate_header", lambda _layer: True) + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._wire_points_layer(layer) + + assert warned == [] + + def test_an_unbound_layer_binds_to_the_dataset_it_adopts(monkeypatch): """A config placeholder must stop being unbound once it takes a folder.""" layer = make_points("placeholder") From 0dd33043f81a54dc3575aee0c497075b736148e5 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Thu, 17 Sep 2026 17:14:04 +0200 Subject: [PATCH 20/41] Track dataset keys across image folders When a video/image is opened, store its dataset key in metadata so layer lifecycle logic can compare the correct dataset identity against the active folder. This fixes dataset-mismatch detection for layers that have been reopened or moved across folders. The warning logic now tracks mismatches per layer more accurately and shows both the original save location and the active folder, making it clear where data will still be written. --- src/napari_deeplabcut/core/io.py | 1 + .../core/layer_lifecycle/manager.py | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index 18179e2c..92fa6323 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -1069,6 +1069,7 @@ def _read_block(start: int, stop: int): "name": filename, "metadata": { "root": root, + "dataset_key": dataset_key_for_folder(root), }, } if dlc_meta is not None: diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index f514065c..fe8de377 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -4,7 +4,6 @@ import logging from collections.abc import Callable, Iterator from enum import Enum -from pathlib import Path from types import MethodType from typing import TYPE_CHECKING, Any from weakref import WeakKeyDictionary @@ -28,7 +27,7 @@ sync_points_from_image, write_points_meta, ) -from ...core.project_paths import PathMatchPolicy, is_same_dataset +from ...core.project_paths import PathMatchPolicy, dataset_key_for_folder, is_same_dataset from ...core.remap import remap_layer_data_by_paths from ...napari_compat import install_add_wrapper, install_paste_patch, layer_key, unwrap from ...napari_compat.points_layer import make_paste_data @@ -115,8 +114,7 @@ def __init__(self, viewer: napari.Viewer, *, parent: QObject | None = None) -> N self._image_dataset_key: str | None = None self._project_path: str | None = None - # Last folder each layer was warned about failing to follow - self._dataset_mismatch_warned: WeakKeyDictionary[Layer, str] = WeakKeyDictionary() + self._dataset_mismatch_warned: WeakKeyDictionary[Layer, set[str]] = WeakKeyDictionary() self._attached = False self.viewer_keybinds_installed = False @@ -387,10 +385,11 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: time a given layer fails to follow a given folder; the log records every pass. """ root = (layer.metadata or {}).get("root") - dataset = Path(str(root)).name if root else "its original folder" + dataset = str(root) if root else "its original folder" target = self._image_dataset_key or str(self._image_meta.root or "") - if self._dataset_mismatch_warned.get(layer) == target: + warned = self._dataset_mismatch_warned.setdefault(layer, set()) + if target in warned: logger.debug( "Extra dataset-mismatch notification for layer=%r folder=%r", getattr(layer, "name", layer), @@ -398,10 +397,12 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: ) return - self._dataset_mismatch_warned[layer] = target + warned.add(target) reason = ( f"'{getattr(layer, 'name', layer)}' could not be matched to the frames in the folder " - f"that was opened; it will still save as '{dataset}'.\n" + f"that was opened.\n\n" + f"It will still save to:\n {dataset}\n" + f"not:\n {target}\n\n" "Please clear it before labelling the new folder." ) self.viewer.status = reason @@ -745,7 +746,7 @@ def _setup_image_layer(self, layer: Image, index: int | None = None, *, reorder: pass self._active_dlc_image_layer_id = layer_key(layer) - self._image_dataset_key = (layer.metadata or {}).get("dataset_key") + self._image_dataset_key = md.get("dataset_key") or dataset_key_for_folder(md.get("root")) context_changed = self._update_image_meta_from_layer(layer) if not self._project_path: From 7891c2c30d0efb39d962c216e710d0b65f9a27d1 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Thu, 17 Sep 2026 17:14:47 +0200 Subject: [PATCH 21/41] Fix dataset-key matching for video contexts This change fixes dataset identity checks when a video/image context lacks an explicit dataset key. The lifecycle manager now resolves the key from the containing labeled-data folder, avoids re-reporting the same mismatch, and includes both dataset folders in the mismatch message. read_video now stores the resolved dataset key in metadata so videos and annotations are treated as the same dataset when they belong to the same labeled-data folder. Added regression tests covering mismatch deduplication, folder names in warnings, and the video metadata key. --- .../_tests/core/layer_manager/test_manager.py | 62 +++++++++++++++++++ src/napari_deeplabcut/_tests/test_reader.py | 17 +++++ 2 files changed, 79 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 9cf43dbe..13ac7998 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -15,6 +15,7 @@ PointsDisplaySource, ) from napari_deeplabcut.core.layer_lifecycle.manager import PointsRuntimeResources +from napari_deeplabcut.core.project_paths import is_same_dataset from napari_deeplabcut.tracking.core.data import build_tracking_result_metadata @@ -850,6 +851,37 @@ def test_dataset_mismatch_is_reported_once_per_target_folder(qtbot): assert rec.dataset_mismatch.count == 2 + # Going back to a folder already reported is not a new fact. + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoB/imgB000.png"], + root="C:/project/labeled-data/videoB", + ) + manager._remap_frame_indices(layer) + + assert rec.dataset_mismatch.count == 2 + + +def test_dataset_mismatch_names_both_folders_in_full(qtbot): + """Two projects can hold a dataset of the same name, so bare names do not identify it.""" + layer = _points_bound_to( + ["labeled-data/mouse1/img000.png"], + root="C:/project-A/labeled-data/mouse1", + ) + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + rec = connect_signal_recorders(manager) + manager._image_meta = ImageMetadata( + paths=["labeled-data/mouse1/img000.png"], + root="C:/project-B/labeled-data/mouse1", + ) + manager._image_dataset_key = "C:/project-B/labeled-data/mouse1" + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + assert "C:/project-A/labeled-data/mouse1" in reason + assert "C:/project-B/labeled-data/mouse1" in reason + # --------------------------------------------------------------------------- # Inheriting root and paths from the open folder @@ -865,6 +897,36 @@ def _points_keyed_without_paths(*, root): return layer +def test_a_keyless_image_context_is_identified_by_its_own_folder(monkeypatch, fake_store): + """Opening a video must not read as a different dataset than the h5 beside it. + + `read_video` rewrites videos/.mp4 into labeled-data/, so its root is the + annotations' own folder. Before the key was derived from that root, a context without + one compared unequal to every keyed layer and raised a mismatch over nothing. + """ + root = "C:/project/labeled-data/videoA" + new_paths = ["labeled-data/videoA/img000.png"] + layer = _points_keyed_without_paths(root=root) + + image = make_image("videoA.mp4") + image.metadata = {"root": root} # as read_video builds one: root, no dataset_key + + manager = LayerLifecycleManager(viewer=DummyViewer([image, layer])) + monkeypatch.setattr(manager, "validate_header", lambda _layer: True) + manager._setup_image_layer(image, reorder=False) + manager._image_meta = ImageMetadata(paths=list(new_paths), root=root) + + warned = [] + monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) + + manager._sync_points_layers_from_image_meta() + + # The key is the resolved folder, so it need not match `root` character for character. + assert is_same_dataset(manager._image_dataset_key, root) + assert warned == [] + assert layer.metadata["paths"] == new_paths + + def test_sync_from_image_meta_refuses_paths_for_a_layer_from_another_dataset(monkeypatch): layer = _points_keyed_without_paths(root="C:/project/labeled-data/videoA") diff --git a/src/napari_deeplabcut/_tests/test_reader.py b/src/napari_deeplabcut/_tests/test_reader.py index 2e978166..8906e6df 100644 --- a/src/napari_deeplabcut/_tests/test_reader.py +++ b/src/napari_deeplabcut/_tests/test_reader.py @@ -1,3 +1,5 @@ +from pathlib import Path + import cv2 import dask.array as da import numpy as np @@ -379,6 +381,21 @@ def test_read_video_output(video_path): assert frame.dtype == np.uint8 +def test_read_video_keys_the_dataset_it_belongs_to(video_path): + """A video names the labeled-data folder beside it, so it must carry that folder's key. + + Without it the lifecycle manager reads the video as a dataset of its own and warns that + the annotations already open do not match it. + """ + from napari_deeplabcut.core.project_paths import dataset_key_for_folder + + _data, params, _kind = read_video(video_path)[0] + md = params["metadata"] + + assert md["dataset_key"] == dataset_key_for_folder(md["root"]) + assert Path(md["root"]).parent.name == "labeled-data" + + def test_get_video_reader_dispatch(video_path): assert _reader.get_video_reader(video_path) is not None assert is_video(str(video_path)) From 9165bbc574ad56ca6b7f2218e848107dfe24bdd1 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Fri, 18 Sep 2026 10:57:39 +0200 Subject: [PATCH 22/41] Fix keyless image context test Update the layer manager test to use a real temporary folder path for the keyless image context. The test now reflects the actual resolved folder behavior and no longer stubs header validation. --- .../_tests/core/layer_manager/test_manager.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 13ac7998..3e52a937 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -897,22 +897,22 @@ def _points_keyed_without_paths(*, root): return layer -def test_a_keyless_image_context_is_identified_by_its_own_folder(monkeypatch, fake_store): +def test_no_dataset_key_image_context_is_identified_by_folder(tmp_path, monkeypatch): """Opening a video must not read as a different dataset than the h5 beside it. `read_video` rewrites videos/.mp4 into labeled-data/, so its root is the - annotations' own folder. Before the key was derived from that root, a context without - one compared unequal to every keyed layer and raised a mismatch over nothing. + annotations' own folder. """ - root = "C:/project/labeled-data/videoA" + folder = tmp_path / "project" / "labeled-data" / "videoA" + folder.mkdir(parents=True) + root = str(folder) new_paths = ["labeled-data/videoA/img000.png"] - layer = _points_keyed_without_paths(root=root) + layer = _points_keyed_without_paths(root=root) image = make_image("videoA.mp4") image.metadata = {"root": root} # as read_video builds one: root, no dataset_key manager = LayerLifecycleManager(viewer=DummyViewer([image, layer])) - monkeypatch.setattr(manager, "validate_header", lambda _layer: True) manager._setup_image_layer(image, reorder=False) manager._image_meta = ImageMetadata(paths=list(new_paths), root=root) @@ -921,7 +921,6 @@ def test_a_keyless_image_context_is_identified_by_its_own_folder(monkeypatch, fa manager._sync_points_layers_from_image_meta() - # The key is the resolved folder, so it need not match `root` character for character. assert is_same_dataset(manager._image_dataset_key, root) assert warned == [] assert layer.metadata["paths"] == new_paths From 30ffdba42075c13f7ebaa9660414ae18fade3dc7 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Fri, 18 Sep 2026 11:20:55 +0200 Subject: [PATCH 23/41] Avoid layer clearing advice for same-folder remap When frame indices are remapped within the same opened dataset, the layer lifecycle manager no longer tells the user to clear the layer. This fixes the misleading guidance that appeared when re-extracting frames in place, where the annotations should stay with the existing folder and continue saving there. The fix distinguishes same-dataset remaps from true cross-folder mismatches and emits a clearer message in the same-folder case. A regression test covers the scenario where the folder name remains unchanged but the frame set is replaced. --- .../_tests/core/layer_manager/test_manager.py | 21 +++++++++++++++++++ .../core/layer_lifecycle/manager.py | 21 ++++++++++++------- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 3e52a937..5cbc0815 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -883,6 +883,27 @@ def test_dataset_mismatch_names_both_folders_in_full(qtbot): assert "C:/project-B/labeled-data/mouse1" in reason +def test_frames_replaced_in_the_same_folder_does_not_tell_the_user_to_clear(qtbot): + """Re-extracting frames leaves the layer where it is, so clearing it would lose labels. + + The layer belongs to the folder that was opened, so naming two folders and telling the + user to clear it would name the same folder twice and provide incorrect guidance. + """ + root = "C:/project/labeled-data/videoA" + layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root=root) + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + rec = connect_signal_recorders(manager) + manager._image_meta = ImageMetadata(paths=["labeled-data/videoA/renamed000.png"], root=root) + manager._image_dataset_key = root + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + assert "clear" not in reason.lower() + assert "still save there" in reason + + # --------------------------------------------------------------------------- # Inheriting root and paths from the open folder # --------------------------------------------------------------------------- diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index fe8de377..e9695331 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -398,13 +398,20 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: return warned.add(target) - reason = ( - f"'{getattr(layer, 'name', layer)}' could not be matched to the frames in the folder " - f"that was opened.\n\n" - f"It will still save to:\n {dataset}\n" - f"not:\n {target}\n\n" - "Please clear it before labelling the new folder." - ) + name = getattr(layer, "name", layer) + + if is_same_dataset(str(root) if root else None, target): + reason = ( + f"'{name}' does not match the frames now in {dataset}.\n\n" + "Its annotations are unchanged and still save there." + ) + else: + reason = ( + f"'{name}' could not be matched to the frames in the folder that was opened.\n\n" + f"It will still save to:\n {dataset}\n" + f"not:\n {target}\n\n" + "Please clear it before labelling the new folder." + ) self.viewer.status = reason self._single_shot_owned(0, lambda: self.layer_dataset_mismatch.emit(reason)) From ca7f51c2aedfe8a5d9de6c6b9b02b956a7df19b9 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Fri, 18 Sep 2026 12:04:26 +0200 Subject: [PATCH 24/41] Document dataset path samefile fallback Expand the `is_same_dataset` docstring to capture the Windows path aliasing bug from DeepLabCut#3348. This explains why the `samefile` fallback is required when the same project directory can appear under mapped-drive, UNC, or volume GUID paths, and warns against reducing the check to string equality. --- src/napari_deeplabcut/core/project_paths.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 3bc32ee0..49b17f92 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -763,6 +763,11 @@ def is_same_dataset(a: str | None, b: str | None) -> bool: The same folder reaches us under more than one spelling: differing case or separators on Windows, and a mapped drive against the UNC path behind it. + + The `samefile` fallback is a fix for a reported failure: DeepLabCut/DeepLabCut#3348 + had one project folder appear as `Z:/...`, as + `\\\\storage.domain/...` and as `\\\\?\\Volume{GUID}/...`, and string comparison alone + left DLC unable to find its own models. Do not simplify this to equality. """ if a is None or b is None: return False From a50eef0c573fbbcac35ff9580548d6525e6e60b3 Mon Sep 17 00:00:00 2001 From: C-Achard Date: Mon, 21 Sep 2026 14:32:25 +0200 Subject: [PATCH 25/41] Fix dataset mismatch layer warnings Use the active dataset binding instead of a layer's stale root when deciding whether a points layer still belongs to the currently opened folder. This updates the mismatch message to reference the open folder and avoids telling unbound layers to clear annotations when they can still save to the current dataset. Tests cover unbound layers and the revised warning text. --- .../_tests/core/layer_manager/test_manager.py | 57 ++++++++++++++++++- .../core/layer_lifecycle/manager.py | 7 ++- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 5cbc0815..fc65d49b 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -710,13 +710,22 @@ def attach(store, controls, resources): # --------------------------------------------------------------------------- # _remap_frame_indices # --------------------------------------------------------------------------- +NO_DATASET_KEY = object() + + def _points_bound_to(paths, *, root, dataset_key=None): """A Points layer as the readers build one: paths, root, and an immutable identity.""" layer = make_nonempty_points("bound") + if dataset_key is None: + key = root + elif dataset_key is NO_DATASET_KEY: + key = None + else: + key = dataset_key layer.metadata = { "paths": list(paths), "root": root, - "dataset_key": root if dataset_key is None else dataset_key, + "dataset_key": key, } return layer @@ -904,6 +913,52 @@ def test_frames_replaced_in_the_same_folder_does_not_tell_the_user_to_clear(qtbo assert "still save there" in reason +def test_unbound_layer_is_not_told_to_clear_when_its_root_is_stale(qtbot): + """A layer with no `dataset_key` adopts whatever is open, so it is never on another folder.""" + layer = _points_bound_to( + ["labeled-data/videoA/img000.png"], + root="C:/elsewhere/labeled-data/videoA", + dataset_key=NO_DATASET_KEY, + ) + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + rec = connect_signal_recorders(manager) + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoA/renamed000.png"], + root="C:/project/labeled-data/videoA", + ) + manager._image_dataset_key = "C:/project/labeled-data/videoA" + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + assert "clear" not in reason.lower() + assert "still save there" in reason + + +def test_dataset_mismatch_message_names_the_open_folder_not_a_stale_root(qtbot): + """The same-folder message points at the folder that was opened.""" + layer = _points_bound_to( + ["labeled-data/videoA/img000.png"], + root=None, + dataset_key=NO_DATASET_KEY, + ) + + manager = LayerLifecycleManager(viewer=DummyViewer([layer])) + rec = connect_signal_recorders(manager) + manager._image_meta = ImageMetadata( + paths=["labeled-data/videoA/renamed000.png"], + root="C:/project/labeled-data/videoA", + ) + manager._image_dataset_key = "C:/project/labeled-data/videoA" + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + assert "C:/project/labeled-data/videoA" in reason + assert "its original folder" not in reason + + # --------------------------------------------------------------------------- # Inheriting root and paths from the open folder # --------------------------------------------------------------------------- diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index e9695331..d1545e67 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -384,7 +384,8 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: folder open reaches this more than once for the same layer. Report only the first time a given layer fails to follow a given folder; the log records every pass. """ - root = (layer.metadata or {}).get("root") + metadata = layer.metadata or {} + root = metadata.get("root") dataset = str(root) if root else "its original folder" target = self._image_dataset_key or str(self._image_meta.root or "") @@ -400,9 +401,9 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: warned.add(target) name = getattr(layer, "name", layer) - if is_same_dataset(str(root) if root else None, target): + if self._belongs_to_current_dataset(metadata.get("dataset_key")): reason = ( - f"'{name}' does not match the frames now in {dataset}.\n\n" + f"'{name}' does not match the frames now in {target}.\n\n" "Its annotations are unchanged and still save there." ) else: From 5b755ae4cfe0b76ab16d1dc192f1d5aea9f45145 Mon Sep 17 00:00:00 2001 From: C-Achard Date: Mon, 21 Sep 2026 14:53:30 +0200 Subject: [PATCH 26/41] Bind adopted point layers to dataset Ensure point layers record their dataset key whenever they inherit image context from the currently open folder, whether that happens during layer setup or image metadata sync. This prevents adopted layers from gaining root/paths without being bound to the active dataset, and adds regression coverage for both entry points. --- .../_tests/core/layer_manager/test_manager.py | 40 +++++++++++++++++++ .../core/layer_lifecycle/manager.py | 10 ++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index fc65d49b..436ddaa1 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -1129,3 +1129,43 @@ def test_an_unbound_layer_binds_to_the_dataset_it_adopts(monkeypatch): assert layer.metadata["root"] == "C:/project/labeled-data/videoA" assert warned == [layer] + + +def _adopt_via_setup_points_layer(manager, layer, monkeypatch): + monkeypatch.setattr(manager, "validate_header", lambda ly: True) + manager._setup_points_layer(layer, allow_merge=False) + + +def _adopt_via_sync_from_image_meta(manager, layer, monkeypatch): + manager._sync_points_layers_from_image_meta() + + +ADOPTION_ENTRY_POINTS = { + "setup_points_layer": _adopt_via_setup_points_layer, + "sync_from_image_meta": _adopt_via_sync_from_image_meta, +} + + +@pytest.mark.parametrize("entry_point", sorted(ADOPTION_ENTRY_POINTS)) +@pytest.mark.parametrize("already_has_paths", [False, True], ids=["nothing", "paths_only"]) +def test_taking_context_from_the_open_folder_always_binds_the_layer( + qtbot, + fake_store, + monkeypatch, + entry_point, + already_has_paths, +): + """Whichever entry point hands a layer image context must also record dataset_key.""" + open_root = "C:/project/labeled-data/videoB" + open_paths = ["labeled-data/videoB/img000.png"] + + layer = make_points("unbound") + layer.metadata = {"paths": ["labeled-data/videoA/img000.png"]} if already_has_paths else {} + + manager = _manager_showing(layer, paths=open_paths, root=open_root) + + ADOPTION_ENTRY_POINTS[entry_point](manager, layer, monkeypatch) + + # Guards the assertion below against passing vacuously if adoption stops happening. + assert layer.metadata.get("root") == open_root + assert layer.metadata.get("dataset_key") == open_root diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index d1545e67..d79b0943 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -691,7 +691,9 @@ def _sync_points_layers_from_image_meta(self) -> None: if not self._may_follow_current_dataset(ly): continue - inherits_paths = not ly.metadata.get("paths") and bool(self._image_meta.paths) + inherits_context = (not ly.metadata.get("paths") and bool(self._image_meta.paths)) or ( + not ly.metadata.get("root") and bool(self._image_meta.root) + ) res = read_points_meta(ly, migrate_legacy=True, drop_controls=False, drop_header=False) if hasattr(res, "errors"): @@ -720,7 +722,7 @@ def _sync_points_layers_from_image_meta(self) -> None: ) continue - if inherits_paths: + if inherits_context: self._record_dataset_key(ly) def _cache_project_path_from_image_layer(self, layer: Image) -> None: @@ -834,10 +836,14 @@ def _wire_points_layer(self, layer: Points) -> KeypointStore | None: # Inherit only what the layer never had, and only from a folder it may follow. if self._may_follow_current_dataset(layer): + inherited = False if not layer.metadata.get("root") and self._image_meta.root: layer.metadata["root"] = self._image_meta.root + inherited = True if not layer.metadata.get("paths") and self._image_meta.paths: layer.metadata["paths"] = self._image_meta.paths + inherited = True + if inherited: self._record_dataset_key(layer) if root := layer.metadata.get("root"): From d5edcbea75e3f9945710851a1b14938b2103bd67 Mon Sep 17 00:00:00 2001 From: C-Achard Date: Mon, 21 Sep 2026 15:23:09 +0200 Subject: [PATCH 27/41] Add TODO for next refactor --- src/napari_deeplabcut/core/project_paths.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 49b17f92..3e2a58e8 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -738,6 +738,8 @@ def infer_dlc_project_from_video_path( # ----------------------------------------------------------------------------- # Lifecycle/session helpers # ----------------------------------------------------------------------------- +# TODO @C-Achard 2026-09-21: centralize dataset_key/root/paths into one frozen DatasetBinding with a single accessor, +# so a partially-set layer reads as unbound def dataset_key_for_folder(folder: str | Path | None) -> str | None: """Stable identity of the dataset folder a layer was read from. From 65137e6b252d72f5b628f0165af79a8d5343ab4f Mon Sep 17 00:00:00 2001 From: C-Achard Date: Wed, 23 Sep 2026 17:37:23 +0200 Subject: [PATCH 28/41] Document FALLBACK_H5_KEYS legacy support Add detailed comment explaining the historical context of the 'keypoints' fallback key. The 'keypoints' key was used in this package from May 2022 until April 2026, when the canonical 'df_with_missing' key was adopted. Projects labelled during that period may legitimately contain either key, so this fallback is necessary for backward compatibility with older labelling data. --- src/napari_deeplabcut/core/io.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index 92fa6323..b3ea65db 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -81,6 +81,13 @@ # ----------------------------------------------------------------------------- _SUPPORTED_SUFFIXES = {ext.lower() for ext in SUPPORTED_IMAGES} DLC_CANONICAL_H5_KEY = "df_with_missing" # TODO use this key instead of str literal in all places + +# "keypoints" is a legacy key written by this package, not an arbitrary fallback. +# `_writer.py` used `key="keypoints"` from 30f37ca (2022-05-12) through a686875 +# (2026-04-27), when the canonical key was adopted. DeepLabCut uses "df_with_missing". +# +# Projects labelled during that period may legitimately hold either key. +# Keep this to preserve compatibility with labelling data produced by earlier versions. FALLBACK_H5_KEYS = ["keypoints"] # ----------------------------------------------------------------------------- From 679723b9837f140525f0ab7dfe5d1065c9a4f551 Mon Sep 17 00:00:00 2001 From: C-Achard Date: Wed, 23 Sep 2026 19:01:04 +0200 Subject: [PATCH 29/41] Rename dataset keys to dataset folders Replace `dataset_key` with `dataset_folder` across layer metadata, lifecycle management, IO, and tests to make dataset identity terminology match the actual absolute folder being tracked. This also renames the folder resolver helper to `resolve_dataset_folder` and updates the related assertions and comments. --- .../_tests/core/layer_manager/test_manager.py | 60 +++++++++---------- .../_tests/core/test_metadata.py | 2 +- .../_tests/core/test_project_paths.py | 16 ++--- .../_tests/core/test_remap.py | 2 +- src/napari_deeplabcut/_tests/test_reader.py | 4 +- src/napari_deeplabcut/config/models.py | 4 +- src/napari_deeplabcut/core/io.py | 8 +-- .../core/layer_lifecycle/manager.py | 40 ++++++------- src/napari_deeplabcut/core/metadata.py | 2 +- src/napari_deeplabcut/core/project_paths.py | 4 +- 10 files changed, 71 insertions(+), 71 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index 436ddaa1..ba4d1f96 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -710,36 +710,36 @@ def attach(store, controls, resources): # --------------------------------------------------------------------------- # _remap_frame_indices # --------------------------------------------------------------------------- -NO_DATASET_KEY = object() +NO_DATASET_FOLDER = object() -def _points_bound_to(paths, *, root, dataset_key=None): +def _points_bound_to(paths, *, root, dataset_folder=None): """A Points layer as the readers build one: paths, root, and an immutable identity.""" layer = make_nonempty_points("bound") - if dataset_key is None: + if dataset_folder is None: key = root - elif dataset_key is NO_DATASET_KEY: + elif dataset_folder is NO_DATASET_FOLDER: key = None else: - key = dataset_key + key = dataset_folder layer.metadata = { "paths": list(paths), "root": root, - "dataset_key": key, + "dataset_folder": key, } return layer -def _manager_showing(layer, *, paths, root, dataset_key=None): +def _manager_showing(layer, *, paths, root, dataset_folder=None): """A manager whose image context is the given folder.""" manager = LayerLifecycleManager(viewer=DummyViewer([layer])) manager._image_meta = ImageMetadata(paths=list(paths), root=root) - manager._image_dataset_key = root if dataset_key is None else dataset_key + manager._image_dataset_folder = root if dataset_folder is None else dataset_folder return manager def test_remap_frame_indices_refuses_a_layer_from_another_dataset(monkeypatch): - """Identity is decided by dataset_key, whatever the frame names happen to be. + """Identity is decided by dataset_folder, whatever the frame names happen to be. These two folders share every frame name, which is the DLC norm rather than evidence that they hold the same footage. @@ -810,7 +810,7 @@ def test_remap_frame_indices_adopts_root_and_paths_together_when_frames_map(): layer = _points_bound_to( ["old/labeled-data/videoA/imgA000.png"], root="D:/moved/labeled-data/videoA", - dataset_key="C:/project/labeled-data/videoA", + dataset_folder="C:/project/labeled-data/videoA", ) manager = _manager_showing(layer, paths=new_paths, root="C:/project/labeled-data/videoA") @@ -883,7 +883,7 @@ def test_dataset_mismatch_names_both_folders_in_full(qtbot): paths=["labeled-data/mouse1/img000.png"], root="C:/project-B/labeled-data/mouse1", ) - manager._image_dataset_key = "C:/project-B/labeled-data/mouse1" + manager._image_dataset_folder = "C:/project-B/labeled-data/mouse1" manager._remap_frame_indices(layer) @@ -904,7 +904,7 @@ def test_frames_replaced_in_the_same_folder_does_not_tell_the_user_to_clear(qtbo manager = LayerLifecycleManager(viewer=DummyViewer([layer])) rec = connect_signal_recorders(manager) manager._image_meta = ImageMetadata(paths=["labeled-data/videoA/renamed000.png"], root=root) - manager._image_dataset_key = root + manager._image_dataset_folder = root manager._remap_frame_indices(layer) @@ -914,11 +914,11 @@ def test_frames_replaced_in_the_same_folder_does_not_tell_the_user_to_clear(qtbo def test_unbound_layer_is_not_told_to_clear_when_its_root_is_stale(qtbot): - """A layer with no `dataset_key` adopts whatever is open, so it is never on another folder.""" + """A layer with no `dataset_folder` adopts whatever is open, so it is never on another folder.""" layer = _points_bound_to( ["labeled-data/videoA/img000.png"], root="C:/elsewhere/labeled-data/videoA", - dataset_key=NO_DATASET_KEY, + dataset_folder=NO_DATASET_FOLDER, ) manager = LayerLifecycleManager(viewer=DummyViewer([layer])) @@ -927,7 +927,7 @@ def test_unbound_layer_is_not_told_to_clear_when_its_root_is_stale(qtbot): paths=["labeled-data/videoA/renamed000.png"], root="C:/project/labeled-data/videoA", ) - manager._image_dataset_key = "C:/project/labeled-data/videoA" + manager._image_dataset_folder = "C:/project/labeled-data/videoA" manager._remap_frame_indices(layer) @@ -941,7 +941,7 @@ def test_dataset_mismatch_message_names_the_open_folder_not_a_stale_root(qtbot): layer = _points_bound_to( ["labeled-data/videoA/img000.png"], root=None, - dataset_key=NO_DATASET_KEY, + dataset_folder=NO_DATASET_FOLDER, ) manager = LayerLifecycleManager(viewer=DummyViewer([layer])) @@ -950,7 +950,7 @@ def test_dataset_mismatch_message_names_the_open_folder_not_a_stale_root(qtbot): paths=["labeled-data/videoA/renamed000.png"], root="C:/project/labeled-data/videoA", ) - manager._image_dataset_key = "C:/project/labeled-data/videoA" + manager._image_dataset_folder = "C:/project/labeled-data/videoA" manager._remap_frame_indices(layer) @@ -966,14 +966,14 @@ def _points_keyed_without_paths(*, root): """A keyed layer with no paths, as a numeric-index h5 produces. `read_hdf` leaves `paths` empty when the frame index is numeric, but still records - `root` and `dataset_key`, so a layer can name its dataset while listing no frames. + `root` and `dataset_folder`, so a layer can name its dataset while listing no frames. """ layer = make_nonempty_points("keyed") - layer.metadata = {"paths": [], "root": root, "dataset_key": root} + layer.metadata = {"paths": [], "root": root, "dataset_folder": root} return layer -def test_no_dataset_key_image_context_is_identified_by_folder(tmp_path, monkeypatch): +def test_image_context_without_a_dataset_folder_is_identified_by_its_root(tmp_path, monkeypatch): """Opening a video must not read as a different dataset than the h5 beside it. `read_video` rewrites videos/.mp4 into labeled-data/, so its root is the @@ -986,7 +986,7 @@ def test_no_dataset_key_image_context_is_identified_by_folder(tmp_path, monkeypa layer = _points_keyed_without_paths(root=root) image = make_image("videoA.mp4") - image.metadata = {"root": root} # as read_video builds one: root, no dataset_key + image.metadata = {"root": root} # as read_video builds one: root, no dataset_folder manager = LayerLifecycleManager(viewer=DummyViewer([image, layer])) manager._setup_image_layer(image, reorder=False) @@ -997,7 +997,7 @@ def test_no_dataset_key_image_context_is_identified_by_folder(tmp_path, monkeypa manager._sync_points_layers_from_image_meta() - assert is_same_dataset(manager._image_dataset_key, root) + assert is_same_dataset(manager._image_dataset_folder, root) assert warned == [] assert layer.metadata["paths"] == new_paths @@ -1032,7 +1032,7 @@ def test_sync_from_image_meta_still_inherits_paths_for_its_own_dataset(): assert layer.metadata["paths"] == new_paths -def test_sync_from_image_meta_records_the_dataset_key_it_inherits_from(): +def test_sync_from_image_meta_records_the_dataset_folder_it_inherits_from(): layer = make_points("placeholder") layer.metadata = {"project": "C:/project"} @@ -1044,7 +1044,7 @@ def test_sync_from_image_meta_records_the_dataset_key_it_inherits_from(): manager._sync_points_layers_from_image_meta() - assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + assert layer.metadata["dataset_folder"] == "C:/project/labeled-data/videoA" def test_wire_points_layer_refuses_paths_from_another_dataset(monkeypatch, fake_store): @@ -1068,7 +1068,7 @@ def test_wire_points_layer_refuses_paths_from_another_dataset(monkeypatch, fake_ assert warned == [layer] -def test_wire_points_layer_records_the_dataset_key_it_inherits_from(monkeypatch, fake_store): +def test_wire_points_layer_records_the_dataset_folder_it_inherits_from(monkeypatch, fake_store): """A layer with no dataset of its own keeps the folder it took its paths from.""" new_paths = ["labeled-data/videoA/img000.png"] layer = make_nonempty_points("placeholder") @@ -1080,7 +1080,7 @@ def test_wire_points_layer_records_the_dataset_key_it_inherits_from(monkeypatch, manager._wire_points_layer(layer) assert layer.metadata["paths"] == new_paths - assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + assert layer.metadata["dataset_folder"] == "C:/project/labeled-data/videoA" def test_wire_points_layer_says_nothing_when_no_image_is_open(monkeypatch, fake_store): @@ -1113,14 +1113,14 @@ def test_an_unbound_layer_binds_to_the_dataset_it_adopts(monkeypatch): ) manager._remap_frame_indices(layer) - assert layer.metadata["dataset_key"] == "C:/project/labeled-data/videoA" + assert layer.metadata["dataset_folder"] == "C:/project/labeled-data/videoA" # Now a different folder reusing the same frame names must be refused. manager._image_meta = ImageMetadata( paths=["labeled-data/videoB/img000.png"], root="C:/project/labeled-data/videoB", ) - manager._image_dataset_key = "C:/project/labeled-data/videoB" + manager._image_dataset_folder = "C:/project/labeled-data/videoB" warned = [] monkeypatch.setattr(manager, "_report_layer_left_on_previous_dataset", lambda ly: warned.append(ly)) @@ -1155,7 +1155,7 @@ def test_taking_context_from_the_open_folder_always_binds_the_layer( entry_point, already_has_paths, ): - """Whichever entry point hands a layer image context must also record dataset_key.""" + """Whichever entry point hands a layer image context must also record dataset_folder.""" open_root = "C:/project/labeled-data/videoB" open_paths = ["labeled-data/videoB/img000.png"] @@ -1168,4 +1168,4 @@ def test_taking_context_from_the_open_folder_always_binds_the_layer( # Guards the assertion below against passing vacuously if adoption stops happening. assert layer.metadata.get("root") == open_root - assert layer.metadata.get("dataset_key") == open_root + assert layer.metadata.get("dataset_folder") == open_root diff --git a/src/napari_deeplabcut/_tests/core/test_metadata.py b/src/napari_deeplabcut/_tests/core/test_metadata.py index dae31b6a..9ff72572 100644 --- a/src/napari_deeplabcut/_tests/core/test_metadata.py +++ b/src/napari_deeplabcut/_tests/core/test_metadata.py @@ -167,7 +167,7 @@ def test_sync_points_from_image_never_rewrites_a_root_that_is_already_set(tmp_pa synced = metadata_mod.sync_points_from_image(image_meta, points_meta) - # Which dataset a layer belongs to is settled by dataset_key, not re-derived here. + # Which dataset a layer belongs to is settled by dataset_folder, not re-derived here. assert synced.root == str(good_points_root) diff --git a/src/napari_deeplabcut/_tests/core/test_project_paths.py b/src/napari_deeplabcut/_tests/core/test_project_paths.py index bf9ac230..efbd571e 100644 --- a/src/napari_deeplabcut/_tests/core/test_project_paths.py +++ b/src/napari_deeplabcut/_tests/core/test_project_paths.py @@ -88,28 +88,28 @@ def test_path_match_policy_ordered_depths(): assert paths_mod.PathMatchPolicy.ORDERED_DEPTHS.depths == (3, 2, 1) -def test_dataset_key_is_absolute_two_projects_stay_distinct(tmp_path: Path): +def test_dataset_folder_is_absolute_two_projects_stay_distinct(tmp_path: Path): """The dataset folder name alone repeats across projects; the resolved path does not.""" a = tmp_path / "project-A" / "labeled-data" / "mouse1" b = tmp_path / "project-B" / "labeled-data" / "mouse1" a.mkdir(parents=True) b.mkdir(parents=True) - key_a = paths_mod.dataset_key_for_folder(a) - key_b = paths_mod.dataset_key_for_folder(b) + key_a = paths_mod.resolve_dataset_folder(a) + key_b = paths_mod.resolve_dataset_folder(b) assert key_a != key_b - assert key_a == paths_mod.dataset_key_for_folder(str(a)) - assert paths_mod.dataset_key_for_folder(None) is None + assert key_a == paths_mod.resolve_dataset_folder(str(a)) + assert paths_mod.resolve_dataset_folder(None) is None -def test_points_metadata_round_trip_preserves_dataset_key(): +def test_points_metadata_round_trip_preserves_dataset_folder(): """Identity must survive the metadata sync that runs on every image insert.""" from napari_deeplabcut.config.models import PointsMetadata - meta = PointsMetadata(root="C:/p/labeled-data/videoA", dataset_key="C:/p/labeled-data/videoA") + meta = PointsMetadata(root="C:/p/labeled-data/videoA", dataset_folder="C:/p/labeled-data/videoA") - assert PointsMetadata(**meta.model_dump()).dataset_key == "C:/p/labeled-data/videoA" + assert PointsMetadata(**meta.model_dump()).dataset_folder == "C:/p/labeled-data/videoA" def test_is_same_dataset_matches_identical_and_rejects_distinct(tmp_path: Path): diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 4b3d1f1d..6ee38e6e 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -310,7 +310,7 @@ def test_ambiguous_depth1_remap_is_rejected_and_refuses_paths_update(): def test_basename_only_match_is_accepted_and_cannot_prove_identity(): - """`LayerLifecycleManager` gates on `dataset_key` for that reason.""" + """`LayerLifecycleManager` gates on `dataset_folder` for that reason.""" res = remap_layer_data_by_paths( data=np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float), old_paths=["p/labeled-data/videoA/img000.png", "p/labeled-data/videoA/img001.png"], diff --git a/src/napari_deeplabcut/_tests/test_reader.py b/src/napari_deeplabcut/_tests/test_reader.py index 8906e6df..e49c61d2 100644 --- a/src/napari_deeplabcut/_tests/test_reader.py +++ b/src/napari_deeplabcut/_tests/test_reader.py @@ -387,12 +387,12 @@ def test_read_video_keys_the_dataset_it_belongs_to(video_path): Without it the lifecycle manager reads the video as a dataset of its own and warns that the annotations already open do not match it. """ - from napari_deeplabcut.core.project_paths import dataset_key_for_folder + from napari_deeplabcut.core.project_paths import resolve_dataset_folder _data, params, _kind = read_video(video_path)[0] md = params["metadata"] - assert md["dataset_key"] == dataset_key_for_folder(md["root"]) + assert md["dataset_folder"] == resolve_dataset_folder(md["root"]) assert Path(md["root"]).parent.name == "labeled-data" diff --git a/src/napari_deeplabcut/config/models.py b/src/napari_deeplabcut/config/models.py index cc495c36..f9f624f7 100644 --- a/src/napari_deeplabcut/config/models.py +++ b/src/napari_deeplabcut/config/models.py @@ -454,7 +454,7 @@ class IOProvenance(BaseModel): kind: Whether this layer is ground-truth or machine output. dataset_key: - HDF5 key used for the keypoints table (default: ``keypoints``). + HDF5 key used for the keypoints table (default: ``df_with_missing``). """ # Keep minimal but resilient to future additions @@ -559,7 +559,7 @@ class PointsMetadata(BaseModel): shape: tuple[int, ...] | None = None name: str | None = None - dataset_key: str | None = Field( + dataset_folder: str | None = Field( default=None, description=( "Absolute folder this layer was read from, assigned once at read time and never " diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index b3ea65db..96963226 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -67,9 +67,9 @@ from napari_deeplabcut.core.metadata import attach_source_and_io_to_layer_kwargs, parse_points_metadata from napari_deeplabcut.core.project_paths import ( canonicalize_path, - dataset_key_for_folder, find_nearest_config, infer_dlc_project_from_points_meta, + resolve_dataset_folder, ) from napari_deeplabcut.core.provenance import resolve_output_path_from_metadata, should_nan_clear_existing_for_save from napari_deeplabcut.utils.debug import log_timing @@ -255,7 +255,7 @@ def read_hdf_single(file: Path, *, kind: AnnotationKind | None = None) -> list[L ) layer_props["name"] = file.stem layer_props["metadata"]["root"] = str(file.parent) - layer_props["metadata"]["dataset_key"] = dataset_key_for_folder(file.parent) + layer_props["metadata"]["dataset_folder"] = resolve_dataset_folder(file.parent) layer_props["metadata"]["name"] = layer_props["name"] layer_props["metadata"]["config_colormap"] = config_colormap @@ -861,7 +861,7 @@ def _build_image_layer_kwargs( metadata = { "paths": [canonicalize_path(fp, 3) for fp in filepaths], "root": str(filepaths[0].parent), - "dataset_key": dataset_key_for_folder(filepaths[0].parent), + "dataset_folder": resolve_dataset_folder(filepaths[0].parent), } if dlc_meta is not None: metadata["dlc"] = dlc_meta @@ -1076,7 +1076,7 @@ def _read_block(start: int, stop: int): "name": filename, "metadata": { "root": root, - "dataset_key": dataset_key_for_folder(root), + "dataset_folder": resolve_dataset_folder(root), }, } if dlc_meta is not None: diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index d79b0943..87afaf68 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -27,7 +27,7 @@ sync_points_from_image, write_points_meta, ) -from ...core.project_paths import PathMatchPolicy, dataset_key_for_folder, is_same_dataset +from ...core.project_paths import PathMatchPolicy, is_same_dataset, resolve_dataset_folder from ...core.remap import remap_layer_data_by_paths from ...napari_compat import install_add_wrapper, install_paste_patch, layer_key, unwrap from ...napari_compat.points_layer import make_paste_data @@ -111,7 +111,7 @@ def __init__(self, viewer: napari.Viewer, *, parent: QObject | None = None) -> N self._label_mode = keypoints.LabelMode.default() self._active_dlc_image_layer_id: int | None = None self._image_meta = ImageMetadata() - self._image_dataset_key: str | None = None + self._image_dataset_folder: str | None = None self._project_path: str | None = None self._dataset_mismatch_warned: WeakKeyDictionary[Layer, set[str]] = WeakKeyDictionary() @@ -366,16 +366,16 @@ def can_accept_dlc_session_image(self, layer: Image) -> tuple[bool, str | None]: "please save and clear the current layers before loading the new labeled data folder.", ) - def _belongs_to_current_dataset(self, layer_dataset_key: str | None) -> bool: + def _belongs_to_current_dataset(self, layer_dataset_folder: str | None) -> bool: """Return True if a layer may follow the image context now loaded. - A layer with no key is unbound (a config placeholder, say) and adopts what is + A layer with no folder is unbound (a config placeholder, say) and adopts what is open. """ - if layer_dataset_key is None: + if layer_dataset_folder is None: return True - return is_same_dataset(layer_dataset_key, self._image_dataset_key) + return is_same_dataset(layer_dataset_folder, self._image_dataset_folder) def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: """Report that a layer did not follow the newly opened folder. @@ -387,7 +387,7 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: metadata = layer.metadata or {} root = metadata.get("root") dataset = str(root) if root else "its original folder" - target = self._image_dataset_key or str(self._image_meta.root or "") + target = self._image_dataset_folder or str(self._image_meta.root or "") warned = self._dataset_mismatch_warned.setdefault(layer, set()) if target in warned: @@ -401,7 +401,7 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: warned.add(target) name = getattr(layer, "name", layer) - if self._belongs_to_current_dataset(metadata.get("dataset_key")): + if self._belongs_to_current_dataset(metadata.get("dataset_folder")): reason = ( f"'{name}' does not match the frames now in {target}.\n\n" "Its annotations are unchanged and still save there." @@ -432,22 +432,22 @@ def _may_follow_current_dataset(self, layer: Any) -> bool: metadata = layer.metadata or {} - if self._belongs_to_current_dataset(metadata.get("dataset_key")): + if self._belongs_to_current_dataset(metadata.get("dataset_folder")): return True logger.warning( "Layer %r belongs to %s, not to the folder just opened (%s); leaving it alone.", getattr(layer, "name", str(layer)), - metadata.get("dataset_key"), - self._image_dataset_key, + metadata.get("dataset_folder"), + self._image_dataset_folder, ) self._report_layer_left_on_previous_dataset(layer) return False - def _record_dataset_key(self, layer: Any) -> None: + def _record_dataset_folder(self, layer: Any) -> None: """Name the folder a layer just took its paths from, if it did not name one. - A config placeholder arrives with no `dataset_key`, so `_belongs_to_current_dataset` + A config placeholder arrives with no `dataset_folder`, so `_belongs_to_current_dataset` lets it follow what is currently open. Persisting the dataset key stops the next folder with the same frame names from following as well. """ @@ -455,8 +455,8 @@ def _record_dataset_key(self, layer: Any) -> None: if metadata is None: return - if metadata.get("dataset_key") is None and self._image_dataset_key is not None: - metadata["dataset_key"] = self._image_dataset_key + if metadata.get("dataset_folder") is None and self._image_dataset_folder is not None: + metadata["dataset_folder"] = self._image_dataset_folder def _reject_conflicting_dlc_image_layer(self, layer: Image, reason: str) -> None: """Reject a conflicting DLC session image safely. @@ -723,7 +723,7 @@ def _sync_points_layers_from_image_meta(self) -> None: continue if inherits_context: - self._record_dataset_key(ly) + self._record_dataset_folder(ly) def _cache_project_path_from_image_layer(self, layer: Image) -> None: """Best-effort lifecycle-owned cache of project path from an image/video layer.""" @@ -756,7 +756,7 @@ def _setup_image_layer(self, layer: Image, index: int | None = None, *, reorder: pass self._active_dlc_image_layer_id = layer_key(layer) - self._image_dataset_key = md.get("dataset_key") or dataset_key_for_folder(md.get("root")) + self._image_dataset_folder = md.get("dataset_folder") or resolve_dataset_folder(md.get("root")) context_changed = self._update_image_meta_from_layer(layer) if not self._project_path: @@ -844,7 +844,7 @@ def _wire_points_layer(self, layer: Points) -> KeypointStore | None: layer.metadata["paths"] = self._image_meta.paths inherited = True if inherited: - self._record_dataset_key(layer) + self._record_dataset_folder(layer) if root := layer.metadata.get("root"): update_save_history(root) @@ -967,7 +967,7 @@ def _handle_removed_layer(self, layer: Any) -> None: if self._active_dlc_image_layer_id == layer_key(layer): self._active_dlc_image_layer_id = None self._image_meta = ImageMetadata() - self._image_dataset_key = None + self._image_dataset_folder = None self._project_path = None paths = layer.metadata.get("paths") @@ -1011,7 +1011,7 @@ def _adopt_image_context() -> None: safe_image_meta = self._image_meta.model_dump(exclude_none=True) safe_image_meta.pop("paths", None) layer.metadata.update(safe_image_meta) - self._record_dataset_key(layer) + self._record_dataset_folder(layer) except Exception: logger.debug( "Failed to sync non-path image metadata for layer=%r", diff --git a/src/napari_deeplabcut/core/metadata.py b/src/napari_deeplabcut/core/metadata.py index 1394ebac..566b2e22 100644 --- a/src/napari_deeplabcut/core/metadata.py +++ b/src/napari_deeplabcut/core/metadata.py @@ -108,7 +108,7 @@ def sync_points_from_image(image_meta: ImageMetadata, points_meta: PointsMetadat Fill image-derived fields that the Points layer does not have yet. Only seeds what is missing. A field already set on the layer is never rewritten here: - which dataset a layer belongs to is decided by its `dataset_key`, and rewriting `root` + which dataset a layer belongs to is decided by its `dataset_folder`, and rewriting `root` behind that decision is what let annotations follow the wrong folder. """ updated = points_meta.model_dump(mode="python") diff --git a/src/napari_deeplabcut/core/project_paths.py b/src/napari_deeplabcut/core/project_paths.py index 3e2a58e8..f31e67ab 100644 --- a/src/napari_deeplabcut/core/project_paths.py +++ b/src/napari_deeplabcut/core/project_paths.py @@ -738,9 +738,9 @@ def infer_dlc_project_from_video_path( # ----------------------------------------------------------------------------- # Lifecycle/session helpers # ----------------------------------------------------------------------------- -# TODO @C-Achard 2026-09-21: centralize dataset_key/root/paths into one frozen DatasetBinding with a single accessor, +# TODO @C-Achard 2026-09-21: centralize dataset_folder/root/paths into one frozen DatasetBinding with a single accessor, # so a partially-set layer reads as unbound -def dataset_key_for_folder(folder: str | Path | None) -> str | None: +def resolve_dataset_folder(folder: str | Path | None) -> str | None: """Stable identity of the dataset folder a layer was read from. Assigned once at read time and never rewritten. ``root`` and ``paths`` cannot serve From 1a88ba9d05e78420bf93090b6d4849a3290d3265 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 09:31:49 +0200 Subject: [PATCH 30/41] Prevent other potential root/paths mismatch --- .../_tests/core/layer_manager/test_manager.py | 35 +++++++++++++++++-- .../_tests/core/test_metadata.py | 26 ++++++++++++++ .../core/layer_lifecycle/manager.py | 8 ++--- src/napari_deeplabcut/core/metadata.py | 7 +++- 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index ba4d1f96..b66b0201 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -1147,20 +1147,18 @@ def _adopt_via_sync_from_image_meta(manager, layer, monkeypatch): @pytest.mark.parametrize("entry_point", sorted(ADOPTION_ENTRY_POINTS)) -@pytest.mark.parametrize("already_has_paths", [False, True], ids=["nothing", "paths_only"]) def test_taking_context_from_the_open_folder_always_binds_the_layer( qtbot, fake_store, monkeypatch, entry_point, - already_has_paths, ): """Whichever entry point hands a layer image context must also record dataset_folder.""" open_root = "C:/project/labeled-data/videoB" open_paths = ["labeled-data/videoB/img000.png"] layer = make_points("unbound") - layer.metadata = {"paths": ["labeled-data/videoA/img000.png"]} if already_has_paths else {} + layer.metadata = {} manager = _manager_showing(layer, paths=open_paths, root=open_root) @@ -1169,3 +1167,34 @@ def test_taking_context_from_the_open_folder_always_binds_the_layer( # Guards the assertion below against passing vacuously if adoption stops happening. assert layer.metadata.get("root") == open_root assert layer.metadata.get("dataset_folder") == open_root + + +@pytest.mark.parametrize("entry_point", sorted(ADOPTION_ENTRY_POINTS)) +def test_a_layer_holding_its_own_paths_takes_no_context_before_remap( + qtbot, + fake_store, + monkeypatch, + entry_point, +): + """Neither entry point hands a root to a layer that already lists frames. + + `root` and `paths` route a save together, so a layer that took one folder's root + while listing another's frames would save into the first and be indexed against the + second. Such a layer takes its root from `_remap_frame_indices`, once the paths + update is verified; binding it here would also mark it as belonging to a folder its + frames were never checked against, which silences the mismatch report. + """ + open_root = "C:/project/labeled-data/videoB" + open_paths = ["labeled-data/videoB/img000.png"] + own_paths = ["labeled-data/videoA/img000.png"] + + layer = make_points("unbound") + layer.metadata = {"paths": list(own_paths)} + + manager = _manager_showing(layer, paths=open_paths, root=open_root) + + ADOPTION_ENTRY_POINTS[entry_point](manager, layer, monkeypatch) + + assert layer.metadata.get("root") is None + assert layer.metadata.get("dataset_folder") is None + assert layer.metadata.get("paths") == own_paths diff --git a/src/napari_deeplabcut/_tests/core/test_metadata.py b/src/napari_deeplabcut/_tests/core/test_metadata.py index 9ff72572..28872ea7 100644 --- a/src/napari_deeplabcut/_tests/core/test_metadata.py +++ b/src/napari_deeplabcut/_tests/core/test_metadata.py @@ -171,6 +171,32 @@ def test_sync_points_from_image_never_rewrites_a_root_that_is_already_set(tmp_pa assert synced.root == str(good_points_root) +def test_sync_points_from_image_does_not_seed_root_beside_existing_paths(tmp_path: Path): + project_root = tmp_path / "project" + layer_dataset = project_root / "labeled-data" / "mouse1" + opened_dataset = project_root / "labeled-data" / "mouse2" + layer_dataset.mkdir(parents=True) + opened_dataset.mkdir(parents=True) + + image_meta = ImageMetadata( + root=str(opened_dataset), + paths=[str(opened_dataset / "img001.png")], + shape=[100, 200], + name="images", + ) + points_meta = PointsMetadata(paths=[str(layer_dataset / "img001.png")]) + + synced = metadata_mod.sync_points_from_image(image_meta, points_meta) + + # root routes the save, paths says what it is indexed against: seeding one beside the + # other is the split that sends annotations to a folder they did not come from. + assert synced.root is None + assert synced.paths == [str(layer_dataset / "img001.png")] + # Fields that do not route a save are still seeded. + assert tuple(synced.shape) == (100, 200) + assert synced.name == "images" + + def test_ensure_metadata_models_accepts_dicts_and_models(): ImageMetadata(root="img-root") pts_model = PointsMetadata(root="pts-root") diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 87afaf68..03ed8d04 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -691,9 +691,7 @@ def _sync_points_layers_from_image_meta(self) -> None: if not self._may_follow_current_dataset(ly): continue - inherits_context = (not ly.metadata.get("paths") and bool(self._image_meta.paths)) or ( - not ly.metadata.get("root") and bool(self._image_meta.root) - ) + inherits_context = not ly.metadata.get("paths") and bool(self._image_meta.paths or self._image_meta.root) res = read_points_meta(ly, migrate_legacy=True, drop_controls=False, drop_header=False) if hasattr(res, "errors"): @@ -834,8 +832,8 @@ def _wire_points_layer(self, layer: Points) -> KeypointStore | None: if proj: self._project_path = proj - # Inherit only what the layer never had, and only from a folder it may follow. - if self._may_follow_current_dataset(layer): + # Inherit only when the layer has no paths of its own, and only from a folder it may follow. + if self._may_follow_current_dataset(layer) and not layer.metadata.get("paths"): inherited = False if not layer.metadata.get("root") and self._image_meta.root: layer.metadata["root"] = self._image_meta.root diff --git a/src/napari_deeplabcut/core/metadata.py b/src/napari_deeplabcut/core/metadata.py index 566b2e22..ae0ab318 100644 --- a/src/napari_deeplabcut/core/metadata.py +++ b/src/napari_deeplabcut/core/metadata.py @@ -110,10 +110,15 @@ def sync_points_from_image(image_meta: ImageMetadata, points_meta: PointsMetadat Only seeds what is missing. A field already set on the layer is never rewritten here: which dataset a layer belongs to is decided by its `dataset_folder`, and rewriting `root` behind that decision is what let annotations follow the wrong folder. + + A layer that already has `paths` is not given a `root` either: the two route saves + together and must name one dataset. Such a layer takes its `root` from + `_remap_frame_indices`, once the paths update is verified. """ updated = points_meta.model_dump(mode="python") - for key in ("root", "paths", "shape", "name"): + keys = ("shape", "name") if updated.get("paths") else ("root", "paths", "shape", "name") + for key in keys: if updated.get(key) in (None, "", []): value = getattr(image_meta, key, None) if value not in (None, "", []): From 88af72d5d48379f74f2dd86cc554e7f259a17b01 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 09:49:27 +0200 Subject: [PATCH 31/41] Reject colliding basenames earlier --- .../_tests/core/test_remap.py | 24 +++++++++++++++++++ src/napari_deeplabcut/core/remap.py | 14 +++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 6ee38e6e..f1a3419c 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -309,6 +309,30 @@ def test_ambiguous_depth1_remap_is_rejected_and_refuses_paths_update(): assert res.changed is False +def test_fully_colliding_basenames_are_rejected_not_read_as_aligned(): + """Equal key lists at depth=1 are a name collision, not a match. + + Both sides canonicalize to ["img0.png", "img0.png"], so equality alone cannot tell + two folders of identically named frames from one folder in its original order. + """ + old_paths = ["projA/labeled-data/mouse1/img0.png", "projA/labeled-data/mouse2/img0.png"] + new_paths = ["projB/labeled-data/catX/img0.png", "projB/labeled-data/catY/img0.png"] + + data = np.array([[0.0, 1.0, 2.0], [1.0, 3.0, 4.0]], dtype=float) + + res = remap_layer_data_by_paths( + data=data, + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + policy=PathMatchPolicy.ORDERED_DEPTHS, + ) + + assert res.depth_used == 1 + assert res.is_ambiguous is True + assert res.accept_paths_update is False + + def test_basename_only_match_is_accepted_and_cannot_prove_identity(): """`LayerLifecycleManager` gates on `dataset_folder` for that reason.""" res = remap_layer_data_by_paths( diff --git a/src/napari_deeplabcut/core/remap.py b/src/napari_deeplabcut/core/remap.py index db5f5d37..126f853c 100644 --- a/src/napari_deeplabcut/core/remap.py +++ b/src/napari_deeplabcut/core/remap.py @@ -283,8 +283,16 @@ def remap_layer_data_by_paths( False, False, False, False, None, 0, "No overlap between old and new paths; skipping remap.", None ) + dup_old = _find_duplicates(old_keys) + dup_new = _find_duplicates(new_keys) + non_bijective = len(set(idx_map.values())) < len(idx_map) + + # Bare filenames repeat across dataset folders, so equal key lists at depth=1 are no + # evidence that the two sides hold the same frames. + ambiguous_depth1 = depth == 1 and (bool(dup_old) or bool(dup_new) or non_bijective) + # If ordering already matches, accept metadata paths update but no data remap needed. - if old_keys == new_keys: + if old_keys == new_keys and not ambiguous_depth1: return RemapResult( changed=False, applied=False, @@ -298,8 +306,6 @@ def remap_layer_data_by_paths( warnings: list[str] = [] - dup_old = _find_duplicates(old_keys) - dup_new = _find_duplicates(new_keys) if dup_old: examples = ", ".join(list(dup_old.keys())[:_SAMPLE_N]) warnings.append(f"Duplicate canonical keys in old_paths at depth={depth} (examples: {examples}).") @@ -316,7 +322,6 @@ def remap_layer_data_by_paths( if mapped_ratio < _WARN_MAPPED_RATIO: warnings.append(f"Low mapping coverage: {mapped_ratio:.2f} (mapped={len(idx_map)} of old={len(old_keys)}).") - non_bijective = len(set(idx_map.values())) < len(idx_map) if non_bijective: warnings.append("Non-bijective mapping detected (multiple old indices map to the same new index).") @@ -324,7 +329,6 @@ def remap_layer_data_by_paths( logger.warning("Remap may be ambiguous/risky: %s", w) # Reject ambiguous basename-only remaps. - ambiguous_depth1 = depth == 1 and (bool(dup_old) or bool(dup_new) or non_bijective) if ambiguous_depth1: msg = "Rejected ambiguous depth=1 remap; keeping original frame indices and paths." logger.warning(msg) From 575cf3b1ea5d93962ec50eb909453d6e15e92ca6 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 09:51:17 +0200 Subject: [PATCH 32/41] Fix stale-root mismatch warnings Treat layers with a stale `root` as saving elsewhere even when `dataset_folder` is unset, so the mismatch warning names the actual save path instead of the currently open folder. Update the layer-manager test to cover this unbound-layer case. --- .../_tests/core/layer_manager/test_manager.py | 14 ++++++++++---- .../core/layer_lifecycle/manager.py | 6 +++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py index b66b0201..367ba26c 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -913,8 +913,13 @@ def test_frames_replaced_in_the_same_folder_does_not_tell_the_user_to_clear(qtbo assert "still save there" in reason -def test_unbound_layer_is_not_told_to_clear_when_its_root_is_stale(qtbot): - """A layer with no `dataset_folder` adopts whatever is open, so it is never on another folder.""" +def test_unbound_layer_with_a_stale_root_is_told_where_it_will_actually_save(qtbot): + """Having no `dataset_folder` does not mean the layer saves into the open folder. + + A rejected remap leaves `root` untouched, so this layer still writes to + `C:/elsewhere`. Naming the opened folder here would point the user at a folder that + is never written to. + """ layer = _points_bound_to( ["labeled-data/videoA/img000.png"], root="C:/elsewhere/labeled-data/videoA", @@ -932,8 +937,9 @@ def test_unbound_layer_is_not_told_to_clear_when_its_root_is_stale(qtbot): manager._remap_frame_indices(layer) reason = rec.dataset_mismatch.calls[0][0] - assert "clear" not in reason.lower() - assert "still save there" in reason + assert "C:/elsewhere/labeled-data/videoA" in reason + assert "C:/project/labeled-data/videoA" in reason + assert "still save there" not in reason def test_dataset_mismatch_message_names_the_open_folder_not_a_stale_root(qtbot): diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 03ed8d04..b389bdd3 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -401,7 +401,11 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: warned.add(target) name = getattr(layer, "name", layer) - if self._belongs_to_current_dataset(metadata.get("dataset_folder")): + # Not `dataset_folder`: an unbound layer passes that check while its `root` names + # somewhere else entirely, and it is `root` that receives the save. + saves_elsewhere = bool(root) and not is_same_dataset(resolve_dataset_folder(root), target) + + if not saves_elsewhere: reason = ( f"'{name}' does not match the frames now in {target}.\n\n" "Its annotations are unchanged and still save there." From fbdb1fa1857390e502115101e2c503195a17f7c2 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 09:54:29 +0200 Subject: [PATCH 33/41] Fix dataset mismatch warning dedupe Deduplicate dataset-mismatch notifications using the unwrapped layer object instead of the proxy layer. This keeps repeated warnings from being tracked separately when the same layer is wrapped, reducing duplicate notifications. --- src/napari_deeplabcut/core/layer_lifecycle/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index b389bdd3..504f4b34 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -389,7 +389,7 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: dataset = str(root) if root else "its original folder" target = self._image_dataset_folder or str(self._image_meta.root or "") - warned = self._dataset_mismatch_warned.setdefault(layer, set()) + warned = self._dataset_mismatch_warned.setdefault(unwrap(layer), set()) if target in warned: logger.debug( "Extra dataset-mismatch notification for layer=%r folder=%r", From 445c0f87f8466b1da59347d058ccc28a9790bdf6 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 10:06:31 +0200 Subject: [PATCH 34/41] Normalize dataset paths in save check Resolve both the layer root and target through `resolve_dataset_folder()` before comparing them. This avoids falsely treating equivalent dataset paths as different save locations when deciding whether a layer saves elsewhere. --- src/napari_deeplabcut/core/layer_lifecycle/manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 504f4b34..73517fdc 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -403,7 +403,9 @@ def _report_layer_left_on_previous_dataset(self, layer: Any) -> None: # Not `dataset_folder`: an unbound layer passes that check while its `root` names # somewhere else entirely, and it is `root` that receives the save. - saves_elsewhere = bool(root) and not is_same_dataset(resolve_dataset_folder(root), target) + saves_elsewhere = bool(root) and not is_same_dataset( + resolve_dataset_folder(root), resolve_dataset_folder(target) + ) if not saves_elsewhere: reason = ( From b3abeec19b7fa61aef252f6ffc484f6fc875dca5 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 10:18:26 +0200 Subject: [PATCH 35/41] Warn when remapped frames lose paths Detect annotated frames whose old path disappears during remapping and emit a warning instead of silently accepting the change. The remap logic logs the issue when a frame keeps its index while its path is replaced, and the layer lifecycle manager surfaces the warning to the user. Added regression coverage for the renamed-frame case so this misalignment is visible and actionable. --- .../_tests/core/test_remap.py | 36 +++++++++++++++++ .../core/layer_lifecycle/manager.py | 7 +++- src/napari_deeplabcut/core/remap.py | 40 +++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index f1a3419c..582a27cf 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -4,6 +4,7 @@ from napari_deeplabcut.core.project_paths import PathMatchPolicy from napari_deeplabcut.core.remap import ( + LOST_ANNOTATED_FRAMES, build_frame_index_map, remap_layer_data_by_paths, remap_time_indices, @@ -333,6 +334,41 @@ def test_fully_colliding_basenames_are_rejected_not_read_as_aligned(): assert res.accept_paths_update is False +def test_annotated_frame_losing_its_path_is_logged(caplog): + """A renamed frame is accepted silently, so the log is the only trace. + + The point keeps its index while `paths` is replaced, so it now refers to whatever + path holds that position. + """ + old_paths = [f"p/labeled-data/vidA/img{i:03d}.png" for i in range(5)] + new_paths = list(old_paths) + new_paths[2] = "p/labeled-data/vidA/renamed.png" + + with caplog.at_level(logging.WARNING, logger="napari_deeplabcut.core.remap"): + res = remap_layer_data_by_paths( + data=np.array([[2.0, 10.0, 10.0]]), + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + ) + + assert res.accept_paths_update is True + assert "lost their path" in caplog.text + # The manager selects this warning out of res.warnings to notify the user. + assert any(w.startswith(LOST_ANNOTATED_FRAMES) for w in res.warnings) + + caplog.clear() + with caplog.at_level(logging.WARNING, logger="napari_deeplabcut.core.remap"): + remap_layer_data_by_paths( + data=np.array([[0.0, 10.0, 10.0]]), + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + ) + + assert "lost their path" not in caplog.text + + def test_basename_only_match_is_accepted_and_cannot_prove_identity(): """`LayerLifecycleManager` gates on `dataset_folder` for that reason.""" res = remap_layer_data_by_paths( diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 73517fdc..83cf175a 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -12,6 +12,7 @@ from napari.layers import Image, Layer, Points, Tracks from napari.utils.events import Event from napari.utils.history import update_save_history +from napari.utils.notifications import show_warning from qtpy.QtCore import QObject, Signal from ...config.keybinds import install_points_layer_keybindings, install_viewer_keybindings @@ -28,7 +29,7 @@ write_points_meta, ) from ...core.project_paths import PathMatchPolicy, is_same_dataset, resolve_dataset_folder -from ...core.remap import remap_layer_data_by_paths +from ...core.remap import LOST_ANNOTATED_FRAMES, remap_layer_data_by_paths from ...napari_compat import install_add_wrapper, install_paste_patch, layer_key, unwrap from ...napari_compat.points_layer import make_paste_data from ...tracking.core.data import TRACKING_LAYER_METADATA_KEY, is_tracking_result_points_layer @@ -1075,6 +1076,10 @@ def _adopt_image_context() -> None: if isinstance(layer, Points): mark_layer_presentation_changed(layer) + for warning in res.warnings: + if warning.startswith(LOST_ANNOTATED_FRAMES): + show_warning(f"'{getattr(layer, 'name', layer)}' — {warning}") + else: # Either no overlap at all, or a match too ambiguous to trust logger.warning( diff --git a/src/napari_deeplabcut/core/remap.py b/src/napari_deeplabcut/core/remap.py index 126f853c..f113eab0 100644 --- a/src/napari_deeplabcut/core/remap.py +++ b/src/napari_deeplabcut/core/remap.py @@ -18,6 +18,9 @@ _WARN_MAPPED_RATIO = 0.80 # Warn if mapping coverage of old paths is below this ratio (mapped / old). _SAMPLE_N = 5 # Number of examples to include in warnings about duplicate keys. +# Prefix identifying the one remap warning the user is notified about, rather than only logged. +LOST_ANNOTATED_FRAMES = "Annotated frames lost their path" + @dataclass(frozen=True) class RemapResult: @@ -57,6 +60,38 @@ class RemapResult: warnings: tuple[str, ...] = () +def _annotated_frames_without_a_path( + *, + data: Any, + time_col: int, + idx_map: Mapping[int, int], + n_old: int, +) -> str | None: + """Warn about annotated frames whose old path did not survive the remap. + + Unmapped indices are left unchanged by `_remap_array`, so such a frame keeps its + index while `paths` is replaced, and now refers to whatever path holds that position. + """ + unmapped = set(range(n_old)) - set(idx_map) + if not unmapped: + return None + + try: + annotated = sorted(unmapped & {int(t) for t in np.asarray(data)[:, time_col]}) + except Exception: + logger.debug("Could not determine annotated frames for remap diagnostics", exc_info=True) + return None + + if not annotated: + return None + + return ( + f"{LOST_ANNOTATED_FRAMES}: {len(annotated)} annotated frame(s) are no longer in the folder " + f"(frames {annotated[:_SAMPLE_N]}). Their keypoints now sit on whichever frame took that " + f"position, and will save there." + ) + + def _remap_array(values: np.ndarray, idx_map: Mapping[int, int]) -> np.ndarray: """ Remap time indices in an array of indices. @@ -344,6 +379,11 @@ def remap_layer_data_by_paths( warnings=tuple(warnings), ) + lost = _annotated_frames_without_a_path(data=data, time_col=time_col, idx_map=idx_map, n_old=len(old_keys)) + if lost: + logger.warning(lost) + warnings.append(lost) + res = remap_time_indices(data=data, time_col=time_col, idx_map=idx_map) return RemapResult( From 4b54a1f7509a42c879a0fb305aeaca4e229b10ba Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 10:32:18 +0200 Subject: [PATCH 36/41] Prevent duplicate annotation layers Drop a reloaded Points layer when the same annotation file is already open, avoiding two layers writing back to the same H5. Added an end-to-end regression test for reopening the same folder. --- .../_tests/e2e/test_overwrite_and_merge.py | 31 ++++++++++++++++ .../core/layer_lifecycle/manager.py | 37 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py b/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py index fffbd708..60b9134c 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py +++ b/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py @@ -655,3 +655,34 @@ def _gt_has_expected_rows() -> bool: machine_before, check_dtype=False, ) + + +@pytest.mark.usefixtures("qtbot") +def test_opening_the_same_folder_twice_keeps_one_annotation_layer( + viewer, + keypoint_controls, + qtbot, + tmp_path, +) -> None: + """A folder open re-reads its h5, so opening one twice would load it twice. + + Two layers on one file both save to it, and whichever is saved last wins. Save + routing branches on the selection, so the duplicate does not block a save; it makes + the stale copy selectable and indistinguishable from the live one. + """ + _project, _config_path, labeled, _h5_path = _make_minimal_dlc_project(tmp_path) + + viewer.open(str(labeled), plugin="napari-deeplabcut") + qtbot.waitUntil(lambda: len([ly for ly in viewer.layers if isinstance(ly, Points)]) == 1, timeout=10_000) + original = next(ly for ly in viewer.layers if isinstance(ly, Points)) + + for image_layer in [ly for ly in viewer.layers if not isinstance(ly, Points)]: + viewer.layers.remove(image_layer) + qtbot.wait(100) + + viewer.open(str(labeled), plugin="napari-deeplabcut") + qtbot.wait(600) # the reloaded copy is removed on a deferred timer + + points_layers = [ly for ly in viewer.layers if isinstance(ly, Points)] + assert len(points_layers) == 1, f"Expected the reloaded copy to be dropped, got {[ly.name for ly in points_layers]}" + assert points_layers[0] is original, "The surviving layer must be the one already loaded, not the reloaded copy" diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index 83cf175a..d07dd9cc 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -465,6 +465,40 @@ def _record_dataset_folder(self, layer: Any) -> None: if metadata.get("dataset_folder") is None and self._image_dataset_folder is not None: metadata["dataset_folder"] = self._image_dataset_folder + def _reject_reloaded_annotations(self, layer: Points) -> bool: + """Drop a second copy of an annotation file that is already loaded. + + Opening a folder re-reads its h5, so opening one twice yields two layers on the + same file. Both then save to it, and whichever is saved last wins, silently. + + Keyed on `source_h5` alone: it is the absolute file both layers came from, so a + GT and a machine h5 in one folder stay distinct, and a layer without one (a config + placeholder, a tracking result) is never matched. + """ + source = (layer.metadata or {}).get("source_h5") + if not source: + return False + + for managed, _store in self.iter_managed_points(): + if unwrap(managed) is unwrap(layer): + continue + if (managed.metadata or {}).get("source_h5") != source: + continue + + logger.warning( + "Annotations from %s are already loaded as %r; dropping the reloaded copy.", + source, + getattr(managed, "name", managed), + ) + show_warning( + f"These annotations are already open as '{getattr(managed, 'name', managed)}'.\n" + "The second copy was closed so both cannot save over each other." + ) + self._single_shot_owned(LAYER_REMOVAL_DELAY_MS, lambda ly=layer: self._remove_layer_if_present(ly)) + return True + + return False + def _reject_conflicting_dlc_image_layer(self, layer: Image, reason: str) -> None: """Reject a conflicting DLC session image safely. @@ -916,6 +950,9 @@ def _setup_points_layer( # KEEP_AS_SEPARATE_LAYER means continue normal setup below. + if self._reject_reloaded_annotations(layer): + return PointsInsertResult.SKIPPED + store = self._wire_points_layer(layer) if store is None: return PointsInsertResult.SKIPPED From 746aaa009652fa334c759cb6816160d06f133b11 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 10:58:03 +0200 Subject: [PATCH 37/41] Refuse remaps for missing annotated frames Raise a dedicated error when remapping would strand keypoints on frames that no longer exist in the opened folder, instead of quietly reassigning them by position. The layer manager now surfaces that refusal as an error and leaves the layer on its original frame set, and tests were updated to cover both the refusal case and the allowed case where only unannotated frames disappear. --- .../_tests/core/test_remap.py | 54 ++++++++++--------- .../core/layer_lifecycle/manager.py | 18 ++++--- src/napari_deeplabcut/core/remap.py | 18 +++++-- 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 582a27cf..aa74efd5 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -1,10 +1,12 @@ import logging import numpy as np +import pytest from napari_deeplabcut.core.project_paths import PathMatchPolicy from napari_deeplabcut.core.remap import ( LOST_ANNOTATED_FRAMES, + AnnotationFramesMissingError, build_frame_index_map, remap_layer_data_by_paths, remap_time_indices, @@ -334,39 +336,41 @@ def test_fully_colliding_basenames_are_rejected_not_read_as_aligned(): assert res.accept_paths_update is False -def test_annotated_frame_losing_its_path_is_logged(caplog): - """A renamed frame is accepted silently, so the log is the only trace. +def test_annotated_frame_losing_its_path_is_refused(caplog): + """A keypoint on a vanished frame has no correct destination, so the remap refuses. - The point keeps its index while `paths` is replaced, so it now refers to whatever - path holds that position. + Frame association is positional: re-keying the layer would move those keypoints onto + whichever path took their position, and the next save would write them there. """ old_paths = [f"p/labeled-data/vidA/img{i:03d}.png" for i in range(5)] - new_paths = list(old_paths) - new_paths[2] = "p/labeled-data/vidA/renamed.png" + new_paths = [p for p in old_paths if not p.endswith("img002.png")] with caplog.at_level(logging.WARNING, logger="napari_deeplabcut.core.remap"): - res = remap_layer_data_by_paths( - data=np.array([[2.0, 10.0, 10.0]]), - old_paths=old_paths, - new_paths=new_paths, - time_col=0, - ) - - assert res.accept_paths_update is True + with pytest.raises(AnnotationFramesMissingError) as excinfo: + remap_layer_data_by_paths( + data=np.array([[2.0, 10.0, 10.0]]), + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + ) + + assert str(excinfo.value).startswith(LOST_ANNOTATED_FRAMES) assert "lost their path" in caplog.text - # The manager selects this warning out of res.warnings to notify the user. - assert any(w.startswith(LOST_ANNOTATED_FRAMES) for w in res.warnings) - caplog.clear() - with caplog.at_level(logging.WARNING, logger="napari_deeplabcut.core.remap"): - remap_layer_data_by_paths( - data=np.array([[0.0, 10.0, 10.0]]), - old_paths=old_paths, - new_paths=new_paths, - time_col=0, - ) - assert "lost their path" not in caplog.text +def test_frames_vanishing_without_keypoints_are_not_refused(): + """Only frames carrying keypoints have anything to lose.""" + old_paths = [f"p/labeled-data/vidA/img{i:03d}.png" for i in range(5)] + new_paths = [p for p in old_paths if not p.endswith("img002.png")] + + res = remap_layer_data_by_paths( + data=np.array([[0.0, 10.0, 10.0]]), + old_paths=old_paths, + new_paths=new_paths, + time_col=0, + ) + + assert res.accept_paths_update is True def test_basename_only_match_is_accepted_and_cannot_prove_identity(): diff --git a/src/napari_deeplabcut/core/layer_lifecycle/manager.py b/src/napari_deeplabcut/core/layer_lifecycle/manager.py index d07dd9cc..938ed407 100644 --- a/src/napari_deeplabcut/core/layer_lifecycle/manager.py +++ b/src/napari_deeplabcut/core/layer_lifecycle/manager.py @@ -12,7 +12,7 @@ from napari.layers import Image, Layer, Points, Tracks from napari.utils.events import Event from napari.utils.history import update_save_history -from napari.utils.notifications import show_warning +from napari.utils.notifications import show_error, show_warning from qtpy.QtCore import QObject, Signal from ...config.keybinds import install_points_layer_keybindings, install_viewer_keybindings @@ -29,7 +29,7 @@ write_points_meta, ) from ...core.project_paths import PathMatchPolicy, is_same_dataset, resolve_dataset_folder -from ...core.remap import LOST_ANNOTATED_FRAMES, remap_layer_data_by_paths +from ...core.remap import AnnotationFramesMissingError, remap_layer_data_by_paths from ...napari_compat import install_add_wrapper, install_paste_patch, layer_key, unwrap from ...napari_compat.points_layer import make_paste_data from ...tracking.core.data import TRACKING_LAYER_METADATA_KEY, is_tracking_result_points_layer @@ -1113,10 +1113,6 @@ def _adopt_image_context() -> None: if isinstance(layer, Points): mark_layer_presentation_changed(layer) - for warning in res.warnings: - if warning.startswith(LOST_ANNOTATED_FRAMES): - show_warning(f"'{getattr(layer, 'name', layer)}' — {warning}") - else: # Either no overlap at all, or a match too ambiguous to trust logger.warning( @@ -1139,6 +1135,16 @@ def _adopt_image_context() -> None: res.message, ) + except AnnotationFramesMissingError as exc: + # Refused, not failed: the layer keeps its own paths and data, so a save still + # writes where its keypoints came from. Say so rather than degrade quietly. + logger.error( + "Refused to remap %s: %s", + getattr(layer, "name", str(layer)), + exc, + ) + show_error(f"'{getattr(layer, 'name', layer)}' — {exc}\n\nThe layer was left on its own frames.") + except Exception: logger.exception("Failed to remap frame indices for layer %s", getattr(layer, "name", str(layer))) diff --git a/src/napari_deeplabcut/core/remap.py b/src/napari_deeplabcut/core/remap.py index f113eab0..0910b626 100644 --- a/src/napari_deeplabcut/core/remap.py +++ b/src/napari_deeplabcut/core/remap.py @@ -18,10 +18,19 @@ _WARN_MAPPED_RATIO = 0.80 # Warn if mapping coverage of old paths is below this ratio (mapped / old). _SAMPLE_N = 5 # Number of examples to include in warnings about duplicate keys. -# Prefix identifying the one remap warning the user is notified about, rather than only logged. +# Prefix of the message carried by AnnotationFramesMissingError. LOST_ANNOTATED_FRAMES = "Annotated frames lost their path" +class AnnotationFramesMissingError(RuntimeError): + """A layer holds keypoints on a frame the opened folder does not contain. + + Frame association is positional, so re-keying the layer would move those keypoints + onto whichever path took their position, and the next save would write them there. + No correct mapping exists, so the remap refuses rather than guessing. + """ + + @dataclass(frozen=True) class RemapResult: """ @@ -87,8 +96,8 @@ def _annotated_frames_without_a_path( return ( f"{LOST_ANNOTATED_FRAMES}: {len(annotated)} annotated frame(s) are no longer in the folder " - f"(frames {annotated[:_SAMPLE_N]}). Their keypoints now sit on whichever frame took that " - f"position, and will save there." + f"(frames {annotated[:_SAMPLE_N]}). Keypoints on them cannot be matched to the frames now " + f"open, and moving them would save annotations onto a frame they do not belong to." ) @@ -383,6 +392,9 @@ def remap_layer_data_by_paths( if lost: logger.warning(lost) warnings.append(lost) + # Re-keying the layer would move these rows onto whichever path took their + # position, and the next save would write them there. + raise AnnotationFramesMissingError(lost) res = remap_time_indices(data=data, time_col=time_col, idx_map=idx_map) From 0fa814f4dba9a76eb111fcb6336270ad245fd6a7 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 28 Sep 2026 11:11:23 +0200 Subject: [PATCH 38/41] Add folder switch integrity tests Covers a regression where missing labeled-data frames could be re-keyed onto the wrong image and then spread on save. Also verifies the existing remap behavior still works when frames are only added to a folder. --- .../e2e/test_folder_switch_integrity.py | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py index aeceb18b..d121ff80 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py +++ b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py @@ -11,6 +11,7 @@ from pathlib import Path +import numpy as np import pandas as pd import pytest from napari.layers import Image, Points @@ -19,6 +20,8 @@ _make_project_with_two_labeled_folders, _make_two_projects_sharing_a_video_name, _read_h5_keypoints, + _write_dlc_config, + _write_frames, ) @@ -183,3 +186,85 @@ def test_layer_does_not_follow_a_different_project_using_the_same_video_name( stray = sorted(p.name for p in proj.folder_b.glob("CollectedData*")) assert not stray, f"Annotations from {project_before} were written into project-B: {stray}" + + +def _write_multi_row_gt(path: Path, *, scorer: str, folder_name: str, rows: dict[str, list[float]]) -> Path: + """GT file whose row keys may name frames that are not on disk.""" + cols = pd.MultiIndex.from_product( + [[scorer], ["bodypart1", "bodypart2"], ["x", "y"]], + names=["scorer", "bodyparts", "coords"], + ) + index = pd.MultiIndex.from_tuples([("labeled-data", folder_name, name) for name in rows]) + df = pd.DataFrame(list(rows.values()), index=index, columns=cols) + path.parent.mkdir(parents=True, exist_ok=True) + df.to_hdf(path, key="df_with_missing", mode="w") + df.to_csv(str(path).replace(".h5", ".csv")) + return path + + +@pytest.mark.usefixtures("qtbot") +def test_keypoints_on_a_deleted_frame_do_not_spread_to_other_frames( + viewer, + keypoint_controls, + qtbot, + tmp_path, + overwrite_confirm, +) -> None: + """A row key with no file on disk must not be re-keyed onto a frame that does exist. + + Frame association is positional. Re-keying moves those keypoints onto whichever path + took their index, and each save walks them one frame further down the folder. + """ + overwrite_confirm.capture() + + project = tmp_path / "project" + folder = _write_frames(project / "labeled-data" / "videoA", ("img001.png", "img002.png")) + _write_dlc_config(project, bodyparts=("bodypart1", "bodypart2")) + + gt_path = _write_multi_row_gt( + folder / "CollectedData_John.h5", + scorer="John", + folder_name="videoA", + rows={ + "img000.png": [10.0, 20.0, 30.0, 40.0], # deleted from disk, carries keypoints + "img001.png": [np.nan, np.nan, np.nan, np.nan], + "img002.png": [np.nan, np.nan, np.nan, np.nan], + }, + ) + + _open_folder(viewer, qtbot, folder, expect_points=True) + layer = _points_layers(viewer)[0] + + assert len(layer.metadata.get("paths") or []) == 3, "The layer must keep its own frame list, not the folder's" + + viewer.layers.selection.select_only(layer) + keypoint_controls._save_layers_dialog(selected=True) + qtbot.wait(300) + + df = _read_h5_keypoints(gt_path) + annotated = {str(idx[-1]) for idx, row in df.iterrows() if np.isfinite(row.to_numpy(dtype=float)).any()} + assert annotated == {"img000.png"}, f"Keypoints spread to frames they were never placed on: {annotated}" + + +@pytest.mark.usefixtures("qtbot") +def test_frames_added_to_the_folder_still_remap(viewer, keypoint_controls, qtbot, tmp_path) -> None: + """The DLC refine loop only adds frames, so it must keep working. + + `extract_outlier_frames` writes new frames into labeled-data/