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..45bf590f 100644 --- a/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py +++ b/src/napari_deeplabcut/_tests/core/layer_manager/test_manager.py @@ -8,13 +8,14 @@ import pytest from napari.layers import Image, Points -from napari_deeplabcut.config.models import AnnotationKind +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, 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 @@ -105,6 +106,7 @@ def connect_signal_recorders(manager): inserted=SignalRecorder(), removed=SignalRecorder(), conflicts=SignalRecorder(), + dataset_mismatch=SignalRecorder(), ) manager.refresh_video_panel_requested.connect(rec.refresh_video) @@ -119,6 +121,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 @@ -702,3 +705,590 @@ 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 +# --------------------------------------------------------------------------- +NO_DATASET_FOLDER = object() + + +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_folder is None: + key = root + elif dataset_folder is NO_DATASET_FOLDER: + key = None + else: + key = dataset_folder + layer.metadata = { + "paths": list(paths), + "root": root, + "dataset_folder": key, + } + return layer + + +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_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_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. + """ + 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 = _manager_showing( + layer, + 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_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 = _manager_showing( + layer, + paths=["labeled-data/mouse1/img000.png"], + root="C:/project-B/labeled-data/mouse1", + ) + + 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-A/labeled-data/mouse1" + assert warned == [layer] + + +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 = _manager_showing( + layer, + paths=["labeled-data/videoA/renamed000.png"], + root="C:/project/labeled-data/videoA", + ) + + 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_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_folder="C:/project/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"] == "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): + """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", + ) + + manager._remap_frame_indices(layer) + manager._remap_frame_indices(layer) + + 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( + paths=["labeled-data/videoC/imgC000.png"], + root="C:/project/labeled-data/videoC", + ) + manager._remap_frame_indices(layer) + + 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_folder = "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 + + +def test_frames_replaced_in_the_same_folder_names_one_folder_and_reports_the_lock(qtbot): + """Same folder: one folder named, and the lock stated.""" + 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_folder = root + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + assert "not:" not in reason + assert "still save to their original folder" in reason + assert "has been locked" in reason + + +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", + dataset_folder=NO_DATASET_FOLDER, + ) + + 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_folder = "C:/project/labeled-data/videoA" + + manager._remap_frame_indices(layer) + + reason = rec.dataset_mismatch.calls[0][0] + 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): + """The same-folder message points at the folder that was opened.""" + layer = _points_bound_to( + ["labeled-data/videoA/img000.png"], + root=None, + dataset_folder=NO_DATASET_FOLDER, + ) + + 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_folder = "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 + + +# --------------------------------------------------------------------------- +# Locking a layer that no longer matches the frames on screen +# --------------------------------------------------------------------------- +def test_a_layer_left_on_another_folder_cannot_be_labelled(): + """Frame association is positional, so an edit would land on the layer's own frame.""" + layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root="C:/project/labeled-data/videoA") + + manager = _manager_showing( + layer, + paths=["labeled-data/videoB/imgB000.png"], + root="C:/project/labeled-data/videoB", + ) + + manager._remap_frame_indices(layer) + + assert layer.editable is False + + +def test_a_layer_whose_frames_were_replaced_in_place_cannot_be_labelled(): + """Same folder, different frame list: the indices are stale too.""" + root = "C:/project/labeled-data/videoA" + layer = _points_bound_to(["labeled-data/videoA/imgA000.png"], root=root) + + manager = _manager_showing(layer, paths=["labeled-data/videoA/renamed000.png"], root=root) + + manager._remap_frame_indices(layer) + + assert layer.editable is False + + +def test_reopening_the_layers_own_folder_gives_editing_back(): + """The lock follows the open folder, not the layer.""" + own_paths = ["labeled-data/videoA/imgA000.png"] + layer = _points_bound_to(own_paths, root="C:/project/labeled-data/videoA") + + manager = _manager_showing( + layer, + paths=["labeled-data/videoB/imgB000.png"], + root="C:/project/labeled-data/videoB", + ) + manager._remap_frame_indices(layer) + assert layer.editable is False + + manager._image_meta = ImageMetadata(paths=own_paths, root="C:/project/labeled-data/videoA") + manager._image_dataset_folder = "C:/project/labeled-data/videoA" + manager._remap_frame_indices(layer) + + assert layer.editable is True + + +def test_a_matching_layer_is_never_locked(): + """A layer that follows the folder stays editable.""" + new_paths = ["labeled-data/videoA/img001.png", "labeled-data/videoA/img000.png"] + layer = _points_bound_to( + ["labeled-data/videoA/img000.png", "labeled-data/videoA/img001.png"], + root="C:/project/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.editable is True + + +def test_frames_pruned_from_the_layers_own_folder_lock_and_give_the_remedy(qtbot): + """No in-plugin action clears this, so the message carries the fix from the DLC docs. + + The layer holds a keypoint on `imgA001.png`, which is no longer in the folder. + """ + root = "C:/project/labeled-data/videoA" + layer = _points_bound_to( + ["labeled-data/videoA/imgA000.png", "labeled-data/videoA/imgA001.png"], + root=root, + ) + layer.data = np.array([[0, 1, 2], [1, 3, 4]], dtype=float) + layer.metadata["source_h5_stem"] = "CollectedData_John" + + manager = _manager_showing(layer, paths=["labeled-data/videoA/imgA000.png"], root=root) + rec = connect_signal_recorders(manager) + + manager._remap_frame_indices(layer) + + assert layer.editable is False + + reason = rec.dataset_mismatch.calls[0][0] + assert "CollectedData_John.csv" in reason + assert "convertcsv2h5" in reason + + +# --------------------------------------------------------------------------- +# 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_folder`, so a layer can name its dataset while listing no frames. + """ + layer = make_nonempty_points("keyed") + layer.metadata = {"paths": [], "root": root, "dataset_folder": root} + return layer + + +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 + annotations' own folder. + """ + 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) + image = make_image("videoA.mp4") + 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) + 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() + + assert is_same_dataset(manager._image_dataset_folder, 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") + + 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_folder_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_folder"] == "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_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") + 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_folder"] == "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") + 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_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_folder = "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] + + +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)) +def test_taking_context_from_the_open_folder_always_binds_the_layer( + qtbot, + fake_store, + monkeypatch, + entry_point, +): + """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 = {} + + 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_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 d6cbad65..28872ea7 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,43 +152,49 @@ 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): +def test_sync_points_from_image_never_rewrites_a_root_that_is_already_set(tmp_path: Path): project_root = tmp_path / "project" - dataset_root = project_root / "labeled-data" / "mouse1" - dataset_root.mkdir(parents=True) + good_points_root = project_root / "labeled-data" / "mouse1" + other_dataset_root = project_root / "labeled-data" / "mouse2" + good_points_root.mkdir(parents=True) + other_dataset_root.mkdir(parents=True) - image_meta = ImageMetadata( - root=str(dataset_root), - paths=[str(dataset_root / "img001.png")], - name="images", - ) + image_meta = ImageMetadata(root=str(other_dataset_root)) points_meta = PointsMetadata( - root=str(project_root), # stale / wrong + root=str(good_points_root), project=str(project_root), ) synced = metadata_mod.sync_points_from_image(image_meta, points_meta) - assert synced.root == str(dataset_root) + # Which dataset a layer belongs to is settled by dataset_folder, not re-derived here. + assert synced.root == str(good_points_root) -def test_sync_points_from_image_keeps_existing_dataset_root_when_already_good(tmp_path: Path): +def test_sync_points_from_image_does_not_seed_root_beside_existing_paths(tmp_path: Path): project_root = tmp_path / "project" - good_points_root = project_root / "labeled-data" / "mouse1" - other_dataset_root = project_root / "labeled-data" / "mouse2" - good_points_root.mkdir(parents=True) - other_dataset_root.mkdir(parents=True) + 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(other_dataset_root)) - points_meta = PointsMetadata( - root=str(good_points_root), - project=str(project_root), + 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) - # already a valid dataset root -> do not overwrite - assert synced.root == str(good_points_root) + # 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(): diff --git a/src/napari_deeplabcut/_tests/core/test_project_paths.py b/src/napari_deeplabcut/_tests/core/test_project_paths.py index 92c19245..efbd571e 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 @@ -87,6 +88,70 @@ def test_path_match_policy_ordered_depths(): assert paths_mod.PathMatchPolicy.ORDERED_DEPTHS.depths == (3, 2, 1) +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.resolve_dataset_folder(a) + key_b = paths_mod.resolve_dataset_folder(b) + + assert key_a != key_b + 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_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_folder="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): + 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/_tests/core/test_remap.py b/src/napari_deeplabcut/_tests/core/test_remap.py index 869864ad..aa74efd5 100644 --- a/src/napari_deeplabcut/_tests/core/test_remap.py +++ b/src/napari_deeplabcut/_tests/core/test_remap.py @@ -1,9 +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, @@ -281,6 +284,123 @@ 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_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_annotated_frame_losing_its_path_is_refused(caplog): + """A keypoint on a vanished frame has no correct destination, so the remap refuses. + + 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 = [p for p in old_paths if not p.endswith("img002.png")] + + with caplog.at_level(logging.WARNING, logger="napari_deeplabcut.core.remap"): + 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 + + +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(): + """`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"], + new_paths=["p/labeled-data/videoB/img000.png", "p/labeled-data/videoB/img001.png"], + time_col=0, + policy=PathMatchPolicy.ORDERED_DEPTHS, + ) + + assert res.depth_used == 1 + assert res.accept_paths_update is True + + +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/_tests/e2e/test_folder_switch_integrity.py b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py new file mode 100644 index 00000000..128bec57 --- /dev/null +++ b/src/napari_deeplabcut/_tests/e2e/test_folder_switch_integrity.py @@ -0,0 +1,273 @@ +# 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, + _make_two_projects_sharing_a_video_name, + _read_h5_keypoints, + _write_dlc_config, + _write_frames, +) + + +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_identical_frame_names_in_another_folder_do_not_migrate_the_layer( + viewer, + keypoint_controls, + qtbot, + tmp_path, + overwrite_confirm, +) -> None: + """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 == "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"} + + +@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}" + + +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": [50.0, 60.0, 70.0, 80.0], + "img002.png": [90.0, 100.0, 110.0, 120.0], + }, + ) + + _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) + by_frame = {str(idx[-1]): row.to_numpy(dtype=float).tolist() for idx, row in df.iterrows()} + + # Spreading overwrites a real frame with the deleted frame's coordinates. + assert by_frame["img000.png"] == [10.0, 20.0, 30.0, 40.0] + assert by_frame["img001.png"] == [50.0, 60.0, 70.0, 80.0] + assert by_frame["img002.png"] == [90.0, 100.0, 110.0, 120.0] + + +@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/