diff --git a/src/napari_deeplabcut/_tests/core/test_conflicts.py b/src/napari_deeplabcut/_tests/core/test_conflicts.py index 3d04a727..2db2a76d 100644 --- a/src/napari_deeplabcut/_tests/core/test_conflicts.py +++ b/src/napari_deeplabcut/_tests/core/test_conflicts.py @@ -451,6 +451,190 @@ def fake_build_report(conflicts, *, deletion_conflict, layer_name, destination_p ) +def test_compute_overwrite_report_skips_deletions_for_machine_to_gt_promotion( + monkeypatch, + tmp_path, +): + """ + Machine-to-GT promotion uses non-deleting save semantics. + + The completed machine dataframe may contain NaN values for existing GT + frames after remapping/save scope creation. Those NaNs are not applied as + deletions by the writer and therefore must not be reported as deletions by + the overwrite preflight. + + Valid machine overwrites of GT keypoints must still be reported. + """ + out = tmp_path / "CollectedData_target.h5" + out.touch() + + old_df = pd.DataFrame({"old": [1]}) + new_df = pd.DataFrame({"new": [1]}) + completed_df = pd.DataFrame({"completed": [1]}) + + key_conflict = pd.DataFrame( + [[True]], + index=["img000.png"], + columns=["nose"], + ) + report = SimpleNamespace( + has_conflicts=True, + n_overwrites=1, + n_deletions=0, + ) + + # The presence of save_target makes the destination GT, while io.kind + # identifies the selected source layer as MACHINE. + pts_meta = _make_points_meta( + io_kind=AnnotationKind.MACHINE, + save_target=object(), + ) + + _stub_validation_pipeline( + monkeypatch, + pts_meta=pts_meta, + df_new=new_df, + completed_df=completed_df, + ) + + monkeypatch.setattr( + conflicts_mod, + "resolve_output_path_from_metadata", + lambda attributes: ( + str(out), + "target_scorer", + AnnotationKind.MACHINE, + ), + ) + monkeypatch.setattr( + pd, + "read_hdf", + lambda path, key=None: old_df, + ) + monkeypatch.setattr( + conflicts_mod, + "keypoint_conflicts", + lambda df_old, df_new: key_conflict, + ) + + def fail_keypoint_deletions(*args, **kwargs): + pytest.fail("keypoint_deletions() must not be called for machine-to-GT promotion.") + + monkeypatch.setattr( + conflicts_mod, + "keypoint_deletions", + fail_keypoint_deletions, + ) + + seen = {} + + def fake_build_report( + conflicts, + *, + deletion_conflict, + layer_name, + destination_path, + ): + seen["args"] = ( + conflicts, + deletion_conflict, + layer_name, + destination_path, + ) + return report + + monkeypatch.setattr( + conflicts_mod, + "build_overwrite_conflict_report", + fake_build_report, + ) + + result = conflicts_mod.compute_overwrite_report_for_points_save( + data=[[0, 1, 2]], + attributes={"name": "machinelabels-iter0"}, + ) + + assert result is report + assert seen["args"] == ( + key_conflict, + None, + "machinelabels-iter0", + str(out), + ) + + +def test_compute_overwrite_report_returns_none_for_disjoint_machine_to_gt_promotion( + monkeypatch, + tmp_path, +): + """ + A disjoint machine-to-GT promotion has only additions. + + Missing machine values are not deletions, so if there are no finite + overwrite conflicts the preflight must return None and show no warning. + """ + out = tmp_path / "CollectedData_target.h5" + out.touch() + + old_df = pd.DataFrame({"old": [1]}) + new_df = pd.DataFrame({"new": [1]}) + completed_df = pd.DataFrame({"completed": [1]}) + + no_overwrites = pd.DataFrame( + [[False]], + index=["img000.png"], + columns=["nose"], + ) + + pts_meta = _make_points_meta( + io_kind=AnnotationKind.MACHINE, + save_target=object(), + ) + + _stub_validation_pipeline( + monkeypatch, + pts_meta=pts_meta, + df_new=new_df, + completed_df=completed_df, + ) + + monkeypatch.setattr( + conflicts_mod, + "resolve_output_path_from_metadata", + lambda attributes: ( + str(out), + "target_scorer", + AnnotationKind.MACHINE, + ), + ) + monkeypatch.setattr( + pd, + "read_hdf", + lambda path, key=None: old_df, + ) + monkeypatch.setattr( + conflicts_mod, + "keypoint_conflicts", + lambda df_old, df_new: no_overwrites, + ) + + def fail_keypoint_deletions(*args, **kwargs): + pytest.fail("Disjoint machine promotion must not calculate GT deletions.") + + monkeypatch.setattr( + conflicts_mod, + "keypoint_deletions", + fail_keypoint_deletions, + ) + + result = conflicts_mod.compute_overwrite_report_for_points_save( + data=[[0, 1, 2]], + attributes={"name": "machinelabels-iter0"}, + ) + + assert result is None + + def test_compute_overwrite_report_raises_when_gt_fallback_has_no_root_and_no_dataset_dir(monkeypatch): pts_meta = _make_points_meta(io_kind=AnnotationKind.GT, root=None) _stub_validation_pipeline(monkeypatch, pts_meta=pts_meta) diff --git a/src/napari_deeplabcut/_tests/core/test_dataframes.py b/src/napari_deeplabcut/_tests/core/test_dataframes.py index 3339b460..5718ea37 100644 --- a/src/napari_deeplabcut/_tests/core/test_dataframes.py +++ b/src/napari_deeplabcut/_tests/core/test_dataframes.py @@ -584,7 +584,7 @@ def test_complete_df_for_save_without_paths_preserves_existing_rows_and_complete # ----------------------------------------------------------------------------- -# 9) merge_save_df: save-scope overlay preserves intentional NaN deletions +# 9) merge_save_df: NaN merge policy and row harmonization # ----------------------------------------------------------------------------- @@ -600,7 +600,7 @@ def test_merge_save_df_nan_in_new_clears_old_value(): df_old = pd.DataFrame([[10.0, 20.0]], index=idx, columns=cols) df_new = pd.DataFrame([[np.nan, np.nan]], index=idx, columns=cols) - out = merge_save_df(df_old, df_new) + out = merge_save_df(df_old, df_new, nan_clears_existing=True) assert pd.isna(out.loc[idx[0], ("S", "", "nose", "x")]) assert pd.isna(out.loc[idx[0], ("S", "", "nose", "y")]) @@ -635,7 +635,7 @@ def test_merge_save_df_preserves_rows_outside_new_save_scope(): columns=cols, ) - out = merge_save_df(df_old, df_new) + out = merge_save_df(df_old, df_new, nan_clears_existing=True) assert pd.isna(out.loc[("labeled-data", "test", "img000.png"), ("S", "", "nose", "x")]) assert out.loc[("labeled-data", "test", "img999.png"), ("S", "", "nose", "x")] == 30.0 @@ -660,7 +660,7 @@ def test_merge_save_df_preserves_old_columns_outside_new_columns(): df_old = pd.DataFrame([[10.0, 20.0, 30.0, 40.0]], index=idx, columns=old_cols) df_new = pd.DataFrame([[11.0, 22.0]], index=idx, columns=new_cols) - out = merge_save_df(df_old, df_new) + out = merge_save_df(df_old, df_new, nan_clears_existing=True) assert out.loc[idx[0], ("S", "", "nose", "x")] == 11.0 assert out.loc[idx[0], ("S", "", "nose", "y")] == 22.0 @@ -702,7 +702,7 @@ def test_merge_save_df_nan_clears_old_value_after_row_harmonization(): ) guarantee_multiindex_rows(df_new) - out = merge_save_df(df_old, df_new) + out = merge_save_df(df_old, df_new, nan_clears_existing=True) # harmonize_keypoint_row_index() collapses the deep row to basename so the # assignment hits the existing row and NaN clears the old value. @@ -712,12 +712,15 @@ def test_merge_save_df_nan_clears_old_value_after_row_harmonization(): assert pd.isna(out.loc[row, ("S", "", "nose", "y")]) -def test_merge_save_df_refuses_no_row_overlap_after_harmonization(): +def test_merge_save_df_merge_refuses_no_row_overlap_after_harmonization(): """ - If row labels do not overlap after harmonization, df_new cannot overwrite or - delete existing values. merge_save_df() refuses this case rather than silently - preserving old labels when deletion/overwrite semantics were expected. + An authoritative merge requires overlapping row indices. + + When incoming NaN values may clear existing annotations, a complete lack + of overlap implies incompatible row representations or unrelated + sets. """ + cols = cols_4level( scorer="S", individuals=("",), @@ -740,7 +743,7 @@ def test_merge_save_df_refuses_no_row_overlap_after_harmonization(): ) with pytest.raises(ValueError, match="no row-index overlap after harmonization"): - merge_save_df(df_old, df_new) + merge_save_df(df_old, df_new, nan_clears_existing=True) # ----------------------------------------------------------------------------- @@ -983,7 +986,7 @@ def test_deleted_keypoint_roundtrip_complete_then_merge_clears_old_value(): header=header, ) - out = merge_save_df(df_old, df_new) + out = merge_save_df(df_old, df_new, nan_clears_existing=True) # Deleted nose should be cleared. assert pd.isna(out.loc[idx[0], ("S", "", "nose", "x")]) @@ -994,6 +997,311 @@ def test_deleted_keypoint_roundtrip_complete_then_merge_clears_old_value(): assert out.loc[idx[0], ("S", "", "tail", "y")] == 41.0 +def test_remapped_machine_layer_does_not_clear_initial_gt_frames_outside_original_scope(): + """ + Regression test for destructive machine-to-GT promotion after frame remapping. + + Real-world failure reproduced + ----------------------------- + 1. CollectedData_S contains manually labeled data for 30 frames. + 2. A machine-label file contains corrected labels for 20 additional + outlier frames. + 3. Both layers are displayed in one 50-frame napari image stack. + 4. During layer remapping, the machine layer's metadata.paths is expanded + from its original 20 source paths to all 50 viewer paths. + 5. complete_df_for_save() interprets metadata.paths as the editable save + scope and creates NaN rows for the 30 frames absent from the machine + layer. + 6. merge_save_df() assigns those NaNs into the existing GT dataframe, + deleting the original manual labels. + + The trigger is therefore this mismatch: + + actual machine annotation scope: outlier_paths (20 frames) + metadata.paths at save time: viewer_paths (50 frames) + + Expected behavior + ----------------- + Promoting the corrected machine labels should preserve the original + 30 manually labeled frames and add the 20 corrected outlier frames. + """ + scorer = "S" + individuals = tuple(str(i) for i in range(1, 11)) + bodyparts = ("head", "body", "tail") + + header_cols = cols_4level( + scorer=scorer, + individuals=individuals, + bodyparts=bodyparts, + coords=("x", "y"), + ) + header = DLCHeaderModel(columns=header_cols) + + initial_paths = [f"labeled-data/test/img{i:03d}.png" for i in range(30)] + outlier_paths = [f"labeled-data/test/img{i:03d}.png" for i in range(30, 50)] + viewer_paths = initial_paths + outlier_paths + + # Existing manually labeled GT dataset: + # 30 frames x 10 individuals x 3 bodyparts x 2 coordinates. + initial_index_df = pd.DataFrame(index=pd.Index(initial_paths)) + guarantee_multiindex_rows(initial_index_df) + initial_index = initial_index_df.index + + old_values = np.arange( + 30 * len(header_cols), + dtype=float, + ).reshape(30, len(header_cols)) + + old_values += 1000.0 + + df_old = pd.DataFrame( + old_values, + index=initial_index, + columns=header_cols, + ) + df_old_before_save = df_old.copy(deep=True) + + points_data: list[list[float]] = [] + labels: list[str] = [] + ids: list[str] = [] + + for frame_index in range(30, 50): + for individual_index, individual in enumerate(individuals): + for bodypart_index, bodypart in enumerate(bodyparts): + x = 2000.0 + frame_index * 100.0 + individual_index * 10.0 + bodypart_index + y = x + 0.5 + + points_data.append([float(frame_index), y, x]) + labels.append(bodypart) + ids.append(individual) + + # The machine layer originally represented only outlier_paths, but its + # metadata.paths now contains every path in the 50-frame viewer. + remapped_machine_meta = PointsMetadata( + header=header, + paths=viewer_paths, + ) + + machine_ctx = PointsWriteInputModel( + points={"data": np.asarray(points_data, dtype=float)}, + meta=remapped_machine_meta, + props={ + "label": labels, + "id": ids, + "likelihood": [1.0] * len(points_data), + }, + ) + + sparse_machine_df = form_df_from_validated(machine_ctx) + + # The finite machine annotations themselves cover only the 20 outliers. + assert len(sparse_machine_df.index) == 20 + assert set(sparse_machine_df.index) == set( + pd.MultiIndex.from_tuples([("labeled-data", "test", f"img{i:03d}.png") for i in range(30, 50)]) + ) + + # complete_df_for_save() currently treats all 50 remapped viewer paths as + # editable rows. This creates 30 all-NaN rows for the original frames. + df_new = complete_df_for_save( + sparse_machine_df, + pts_meta=remapped_machine_meta, + header=header, + ) + + assert len(df_new.index) == 50 + + initial_rows_in_incoming_save = df_new.loc[initial_index] + assert initial_rows_in_incoming_save.isna().all().all() + + merged = merge_save_df(df_old, df_new, nan_clears_existing=False) + + # Intended result: all 50 frame rows should exist. + assert len(merged.index) == 50 + + # Regression expectation: + # frames outside the machine file's original 20-frame annotation scope + # must retain their manually labeled GT coordinates. + pd.testing.assert_frame_equal( + merged.loc[initial_index, header_cols], + df_old_before_save, + ) + + # The 20 corrected outlier frames should also be present and finite. + outlier_index_df = pd.DataFrame(index=pd.Index(outlier_paths)) + guarantee_multiindex_rows(outlier_index_df) + outlier_index = outlier_index_df.index + + assert merged.loc[outlier_index, header_cols].notna().all().all() + + +def test_machine_to_gt_merge_overwrites_finite_values_but_does_not_delete(): + """ + A machine-to-GT merge is a non-deleting patch: + + - finite incoming values overwrite existing GT values; + - incoming NaN values preserve existing GT values. + """ + cols = cols_4level( + scorer="S", + individuals=("animal1",), + bodyparts=("nose", "tail"), + coords=("x", "y"), + ) + index = pd.MultiIndex.from_tuples( + [ + ( + "labeled-data", + "test", + "img000.png", + ) + ] + ) + + df_old = pd.DataFrame( + [[10.0, 20.0, 30.0, 40.0]], + index=index, + columns=cols, + ) + + df_new = pd.DataFrame( + [ + [ + 11.0, + 21.0, + np.nan, + np.nan, + ] + ], + index=index, + columns=cols, + ) + + merged = merge_save_df( + df_old, + df_new, + nan_clears_existing=False, + ) + + # Finite machine correction overwrites the existing nose coordinates. + assert ( + merged.loc[ + index[0], + ("S", "animal1", "nose", "x"), + ] + == 11.0 + ) + assert ( + merged.loc[ + index[0], + ("S", "animal1", "nose", "y"), + ] + == 21.0 + ) + + # Missing machine tail coordinates preserve existing GT. + assert ( + merged.loc[ + index[0], + ("S", "animal1", "tail", "x"), + ] + == 30.0 + ) + assert ( + merged.loc[ + index[0], + ("S", "animal1", "tail", "y"), + ] + == 40.0 + ) + + +def test_merge_save_df_non_deleting_patch_adds_completely_disjoint_rows(): + """ + A non-deleting save may safely add rows that do not overlap the existing + dataframe. + + This represents machine annotations for frames that are entirely disjoint + from the existing GT dataset. Existing GT rows must remain unchanged and + finite incoming machine values must be added. + """ + cols = cols_4level( + scorer="S", + individuals=("animal1",), + bodyparts=("nose",), + coords=("x", "y"), + ) + + old_index = pd.MultiIndex.from_tuples( + [ + ( + "labeled-data", + "test", + "img000.png", + ) + ] + ) + new_index = pd.MultiIndex.from_tuples( + [ + ( + "labeled-data", + "test", + "img001.png", + ) + ] + ) + + df_old = pd.DataFrame( + [[10.0, 20.0]], + index=old_index, + columns=cols, + ) + df_new = pd.DataFrame( + [[30.0, 40.0]], + index=new_index, + columns=cols, + ) + + result = merge_save_df( + df_old, + df_new, + nan_clears_existing=False, + ) + + assert result.index.equals(old_index.union(new_index)) + + # Existing GT row is preserved. + assert ( + result.loc[ + old_index[0], + ("S", "animal1", "nose", "x"), + ] + == 10.0 + ) + assert ( + result.loc[ + old_index[0], + ("S", "animal1", "nose", "y"), + ] + == 20.0 + ) + + # Disjoint incoming machine row is added. + assert ( + result.loc[ + new_index[0], + ("S", "animal1", "nose", "x"), + ] + == 30.0 + ) + assert ( + result.loc[ + new_index[0], + ("S", "animal1", "nose", "y"), + ] + == 40.0 + ) + + # ----------------------------------------------------------------------------- # 13) drop_likelihood_columns # ----------------------------------------------------------------------------- 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 4c5b679e..fffbd708 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py +++ b/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py @@ -5,9 +5,19 @@ import pytest from napari.layers import Points -from napari_deeplabcut.config.models import DLCHeaderModel - -from .utils import _get_coord_from_df, _get_points_layer_with_data, _make_minimal_dlc_project, _set_or_add_bodypart_xy +from napari_deeplabcut.config.models import AnnotationKind, DLCHeaderModel +from napari_deeplabcut.core.io import _read_hdf_any_key +from napari_deeplabcut.core.layers import is_machine_layer + +from .utils import ( + _dataframe_rows_by_path, + _get_coord_from_df, + _get_points_layer_with_data, + _make_minimal_dlc_project, + _make_project_config_and_frames_no_gt, + _seed_gt_and_machine_outlier_dataset, + _set_or_add_bodypart_xy, +) logger = logging.getLogger(__name__) @@ -343,3 +353,305 @@ def test_overwrite_warning_cancel_aborts_write(viewer, keypoint_controls, qtbot, post = pd.read_hdf(h5_path, key="df_with_missing") assert _get_coord_from_df(post, "bodypart1", "x") == b1x_pre assert _get_coord_from_df(post, "bodypart1", "y") == b1y_pre + + +@pytest.mark.usefixtures("qtbot") +def test_machine_label_promotion_preserves_existing_gt_after_frame_remap( + viewer, + keypoint_controls, + qtbot, + tmp_path, + overwrite_confirm, +): + """ + End-to-end regression for destructive machine-to-GT promotion. + + Reproduced workflow + ------------------- + 1. A labeled-data folder contains 50 extracted images. + 2. CollectedData_John.h5 contains manually labeled GT for the first + 30 frames. + 3. machinelabels-iter0.h5 contains machine annotations for the final + 20 outlier frames. + 4. Opening the folder aligns both annotation layers to one 50-frame + image stack. + 5. The machine layer is selected and saved through the plugin UI. + 6. The save workflow promotes the machine source into the existing + CollectedData_John.h5 target. + + Regression assertions + --------------------- + - the original 30 GT rows remain unchanged; + - the 20 machine rows are promoted into GT; + - the resulting GT file contains 50 rows; + - machine likelihood values are not written to GT; + - the machine source file is not overwritten; + - the on-disk GT file remains canonical single-animal DLC data. + """ + overwrite_confirm.capture() + + ( + _project, + _config_path, + labeled_folder, + ) = _make_project_config_and_frames_no_gt(tmp_path) + + ( + gt_path, + machine_path, + initial_paths, + outlier_paths, + expected_outlier_xy, + ) = _seed_gt_and_machine_outlier_dataset( + labeled_folder, + scorer="John", + bodypart="bodypart1", + n_initial_frames=30, + n_outlier_frames=20, + ) + + # Save exact copies for post-operation comparisons. + gt_before = _read_hdf_any_key(gt_path).sort_index() + machine_before = _read_hdf_any_key(machine_path).sort_index() + + assert len(gt_before.index) == 30 + assert len(machine_before.index) == 20 + + assert gt_before.notna().all().all() + assert machine_before.notna().all().all() + + # Canonical disk-state sanity checks before loading the plugin. + assert gt_before.columns.nlevels == 3 + assert list(gt_before.columns.names) == [ + "scorer", + "bodyparts", + "coords", + ] + + assert machine_before.columns.nlevels == 3 + assert "likelihood" in set(machine_before.columns.get_level_values("coords")) + + # Open the complete labeled-data folder through the real reader and + # lifecycle manager. + viewer.open( + str(labeled_folder), + plugin="napari-deeplabcut", + ) + + qtbot.waitUntil( + lambda: len([layer for layer in viewer.layers if isinstance(layer, Points)]) >= 2, + timeout=10_000, + ) + + qtbot.waitUntil( + lambda: any(isinstance(layer, Points) and layer.name == "machinelabels-iter0" for layer in viewer.layers), + timeout=10_000, + ) + + points_layers = [layer for layer in viewer.layers if isinstance(layer, Points)] + + machine_layers = [layer for layer in points_layers if is_machine_layer(layer)] + + assert len(machine_layers) == 1, ( + "Expected exactly one machine annotation layer. " + "Loaded Points layers were: " + f"{[(layer.name, layer.metadata) for layer in points_layers]}" + ) + + machine_layer = machine_layers[0] + + assert machine_layer.name == ("machinelabels-iter0") + + machine_io = (machine_layer.metadata or {}).get("io") + machine_kind = machine_io.get("kind") if isinstance(machine_io, dict) else getattr(machine_io, "kind", None) + assert machine_kind in ("machine", "MACHINE", AnnotationKind.MACHINE) + + # The source machine HDF has 20 annotation rows. After remapping, finite + # machine points should still occupy exactly 20 frame positions. + machine_data = np.asarray(machine_layer.data) + + assert machine_data.ndim == 2 + assert machine_data.shape[1] == 3 + + machine_frame_indices = {int(frame_index) for frame_index in machine_data[:, 0]} + + assert len(machine_frame_indices) == 20 + + # Reproduce and document the exact dangerous state: + # + # - finite machine annotations: 20 frames + # - machine metadata paths: combined 50-frame viewer context + remapped_paths = list((machine_layer.metadata or {}).get("paths") or []) + + assert len(remapped_paths) == 50, ( + "The regression setup did not reproduce the post-remap state. " + "Expected the machine layer to have 50 shared viewer paths, " + f"but got {len(remapped_paths)}." + ) + + normalized_remapped_paths = {str(path).replace("\\", "/") for path in remapped_paths} + + assert set(initial_paths).issubset(normalized_remapped_paths) + assert set(outlier_paths).issubset(normalized_remapped_paths) + + # Before saving, the machine source should not yet have a promotion target. + assert (machine_layer.metadata or {}).get("save_target") is None + + # Exercise the real selected-layer save workflow. This should: + # + # 1. detect the MACHINE source; + # 2. discover config.yaml and scorer John; + # 3. attach a GT save target; + # 4. preflight against CollectedData_John.h5; + # 5. write through write_hdf(); + # 6. use allow_deletions=False for the MACHINE source. + viewer.layers.selection.active = machine_layer + keypoint_controls.viewer.layers.selection.select_only(machine_layer) + + keypoint_controls._save_layers_dialog(selected=True) + + # Wait for the final combined file rather than relying on a fixed sleep. + def _gt_has_expected_rows() -> bool: + try: + saved = _read_hdf_any_key(gt_path) + return len(saved.index) == 50 + except Exception: + return False + + qtbot.waitUntil( + _gt_has_expected_rows, + timeout=10_000, + ) + + # Promotion target should now be attached to the live machine layer. + save_target = (machine_layer.metadata or {}).get("save_target") + + assert save_target is not None + + if isinstance(save_target, dict): + assert save_target.get("kind") is (AnnotationKind.GT) or save_target.get("kind") == "gt" + assert save_target.get("scorer") == ("John") + assert save_target.get("source_relpath_posix") == "CollectedData_John.h5" + else: + assert ( + getattr( + save_target, + "kind", + None, + ) + is AnnotationKind.GT + ) + assert ( + getattr( + save_target, + "scorer", + None, + ) + == "John" + ) + assert ( + getattr( + save_target, + "source_relpath_posix", + None, + ) + == "CollectedData_John.h5" + ) + + assert gt_path.exists() + assert gt_path.with_suffix(".csv").exists() + + gt_after = _read_hdf_any_key(gt_path).sort_index() + + # ------------------------------------------------------------------ + # Validate final GT schema. + # ------------------------------------------------------------------ + + assert isinstance( + gt_after.columns, + pd.MultiIndex, + ) + assert gt_after.columns.nlevels == 3 + assert list(gt_after.columns.names) == [ + "scorer", + "bodyparts", + "coords", + ] + + assert set(gt_after.columns.get_level_values("scorer")) == {"John"} + + assert tuple(dict.fromkeys(gt_after.columns.get_level_values("bodyparts"))) == ("bodypart1",) + + assert set(gt_after.columns.get_level_values("coords")) == {"x", "y"} + + assert "likelihood" not in set(gt_after.columns.get_level_values("coords")) + + # Expected final dataset: + # 30 manual GT frames + 20 promoted machine frames. + assert len(gt_after.index) == 50 + + before_rows = _dataframe_rows_by_path(gt_before) + after_rows = _dataframe_rows_by_path(gt_after) + + assert set(initial_paths).issubset(after_rows) + assert set(outlier_paths).issubset(after_rows) + + # ------------------------------------------------------------------ + # Core regression assertion: original GT is unchanged. + # ------------------------------------------------------------------ + + for path in initial_paths: + before_row = gt_before.loc[before_rows[path]] + after_row = gt_after.loc[after_rows[path]] + + pd.testing.assert_series_equal( + after_row, + before_row, + check_dtype=False, + check_names=False, + ) + + # ------------------------------------------------------------------ + # Promoted machine values were added with the target GT scorer. + # ------------------------------------------------------------------ + + for path, ( + expected_x, + expected_y, + ) in expected_outlier_xy.items(): + row_key = after_rows[path] + + actual_x = gt_after.loc[ + row_key, + ( + "John", + "bodypart1", + "x", + ), + ] + actual_y = gt_after.loc[ + row_key, + ( + "John", + "bodypart1", + "y", + ), + ] + + assert actual_x == pytest.approx(expected_x) + assert actual_y == pytest.approx(expected_y) + + # There should be no fully empty rows in the final combined dataset. + assert not gt_after.isna().all(axis=1).any() + + # ------------------------------------------------------------------ + # Saving refined annotations must not rewrite the machine source HDF. + # ------------------------------------------------------------------ + + machine_after = _read_hdf_any_key(machine_path).sort_index() + + pd.testing.assert_frame_equal( + machine_after, + machine_before, + check_dtype=False, + ) diff --git a/src/napari_deeplabcut/_tests/e2e/utils.py b/src/napari_deeplabcut/_tests/e2e/utils.py index 49fd319e..21d81624 100644 --- a/src/napari_deeplabcut/_tests/e2e/utils.py +++ b/src/napari_deeplabcut/_tests/e2e/utils.py @@ -343,3 +343,211 @@ def _scheme_from_policy(layer, prop: str, names: list[str]) -> dict[str, str]: cycles = _cycles_from_policy(layer) mapping = cycles.get(prop, {}) return {name: _to_hex(mapping[name]) for name in names if name in mapping} + + +def _row_key_to_posix(row_key) -> str: + """Normalize a DLC dataframe row key to a POSIX-style string.""" + if isinstance(row_key, tuple): + return "/".join( + str(part).replace("\\", "/").strip("/") for part in row_key if part is not None and str(part) != "" + ) + + return str(row_key).replace("\\", "/").strip("/") + + +def _dataframe_rows_by_path( + df: pd.DataFrame, +) -> dict[str, object]: + """ + Map normalized DLC row paths to their original pandas index values. + + The original values are retained so callers can safely use them with + df.loc regardless of whether the dataframe has an Index or MultiIndex. + """ + rows: dict[str, object] = {} + + for row_key in df.index: + normalized = _row_key_to_posix(row_key) + + if normalized in rows: + raise AssertionError(f"Duplicate normalized DLC row key: {normalized}") + + rows[normalized] = row_key + + return rows + + +def _seed_gt_and_machine_outlier_dataset( + labeled_folder: Path, + *, + scorer: str = "John", + model_scorer: str = "DLC_model", + bodypart: str = "bodypart1", + n_initial_frames: int = 30, + n_outlier_frames: int = 20, +) -> tuple[ + Path, + Path, + list[str], + list[str], + dict[str, tuple[float, float]], +]: + """ + Seed the exact disk layout involved in machine-to-GT refinement. + + Creates + ------- + CollectedData_.h5 + Finite manual GT annotations on the first ``n_initial_frames``. + + machinelabels-iter0.h5 + Finite machine annotations on the following ``n_outlier_frames``. + + PNG files + One shared image set containing both groups of frames. + + Returns + ------- + gt_path + machine_path + initial_paths + Canonical DLC row paths for the initial GT frames. + outlier_paths + Canonical DLC row paths for the machine frames. + expected_outlier_xy + Expected promoted coordinates keyed by canonical DLC row path. + """ + if n_initial_frames <= 0: + raise ValueError("n_initial_frames must be positive.") + + if n_outlier_frames <= 0: + raise ValueError("n_outlier_frames must be positive.") + + total_frames = n_initial_frames + n_outlier_frames + + existing_images = sorted(labeled_folder.glob("*.png")) + assert existing_images, f"The project helper must create at least one readable PNG in {labeled_folder}." + + # Reuse the bytes of an image created by the existing project fixture. + # This gives us 50 valid image files without introducing an image-writing + # dependency into this test. + template_image_bytes = existing_images[0].read_bytes() + + for image in existing_images: + image.unlink() + + image_names = [f"img{i:03d}.png" for i in range(total_frames)] + + for image_name in image_names: + destination = labeled_folder / image_name + destination.write_bytes(template_image_bytes) + + dataset_name = labeled_folder.name + + initial_paths = [f"labeled-data/{dataset_name}/{image_name}" for image_name in image_names[:n_initial_frames]] + outlier_paths = [f"labeled-data/{dataset_name}/{image_name}" for image_name in image_names[n_initial_frames:]] + + # ------------------------------------------------------------------ + # Existing human ground truth: canonical single-animal DLC format. + # ------------------------------------------------------------------ + + gt_columns = pd.MultiIndex.from_product( + [ + [scorer], + [bodypart], + ["x", "y"], + ], + names=[ + "scorer", + "bodyparts", + "coords", + ], + ) + + gt_index = pd.MultiIndex.from_tuples([tuple(path.split("/")) for path in initial_paths]) + + gt_values = np.empty( + (n_initial_frames, len(gt_columns)), + dtype=float, + ) + + for frame_index in range(n_initial_frames): + gt_values[frame_index, 0] = 1000.0 + frame_index + gt_values[frame_index, 1] = 2000.0 + frame_index + + gt_df = pd.DataFrame( + gt_values, + index=gt_index, + columns=gt_columns, + ) + + gt_path = labeled_folder / f"CollectedData_{scorer}.h5" + + gt_df.to_hdf( + gt_path, + key="df_with_missing", + mode="w", + ) + gt_df.to_csv(gt_path.with_suffix(".csv")) + + # ------------------------------------------------------------------ + # Machine labels: x/y/likelihood on the 20 outlier frames. + # ------------------------------------------------------------------ + + machine_columns = pd.MultiIndex.from_product( + [ + [model_scorer], + [bodypart], + ["x", "y", "likelihood"], + ], + names=[ + "scorer", + "bodyparts", + "coords", + ], + ) + + machine_index = pd.MultiIndex.from_tuples([tuple(path.split("/")) for path in outlier_paths]) + + machine_values = np.empty( + (n_outlier_frames, len(machine_columns)), + dtype=float, + ) + + expected_outlier_xy: dict[ + str, + tuple[float, float], + ] = {} + + for local_index, path in enumerate(outlier_paths): + x = 5000.0 + local_index + y = 6000.0 + local_index + likelihood = 0.95 + + machine_values[local_index, 0] = x + machine_values[local_index, 1] = y + machine_values[local_index, 2] = likelihood + + expected_outlier_xy[path] = (x, y) + + machine_df = pd.DataFrame( + machine_values, + index=machine_index, + columns=machine_columns, + ) + + machine_path = labeled_folder / "machinelabels-iter0.h5" + + machine_df.to_hdf( + machine_path, + key="df_with_missing", + mode="w", + ) + + return ( + gt_path, + machine_path, + initial_paths, + outlier_paths, + expected_outlier_xy, + ) diff --git a/src/napari_deeplabcut/core/conflicts.py b/src/napari_deeplabcut/core/conflicts.py index ed8b53aa..7dfd7dd6 100644 --- a/src/napari_deeplabcut/core/conflicts.py +++ b/src/napari_deeplabcut/core/conflicts.py @@ -21,6 +21,7 @@ from napari_deeplabcut.core.project_paths import infer_dlc_project_from_points_meta from napari_deeplabcut.core.provenance import ( resolve_output_path_from_metadata, + should_nan_clear_existing_for_save, ) @@ -156,7 +157,9 @@ def compute_overwrite_report_for_points_save( df_old = pd.read_hdf(out) key_conflict = keypoint_conflicts(df_old, df_new) - deletion_conflict = keypoint_deletions(df_old, df_new) + + nan_clears_existing = should_nan_clear_existing_for_save(source_kind=source_kind, destination_kind=destination_kind) + deletion_conflict = keypoint_deletions(df_old, df_new) if nan_clears_existing else None report = build_overwrite_conflict_report( key_conflict, diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index a3d43d7a..482f897a 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -513,20 +513,50 @@ def complete_df_for_save( def merge_save_df( df_old: pd.DataFrame, df_new: pd.DataFrame, + *, + nan_clears_existing: bool = True, ) -> pd.DataFrame: - """ - Merge an existing DLC dataframe with a new save dataframe. + """Merge incoming DLC annotations into an existing DLC dataframe. - Semantics: - - rows/columns outside df_new scope are preserved from df_old - - rows/columns inside df_new scope replace df_old, including NaN - - NaN in df_new therefore clears/deletes an old saved keypoint + Args: + df_old: Existing on-disk DLC annotations. + df_new: Incoming annotations to save. + nan_clears_existing: Controls how missing values in ``df_new`` are interpreted. + + - If ``True``, ``df_new`` is authoritative within its row/column scope. + Incoming values, including NaN, replace existing values. This enables + intentional deletion when saving a directly edited GT layer. + - If ``False``, ``df_new`` is applied as a non-deleting patch. + Only non-missing incoming values are applied; incoming NaN values + preserve existing values. This is used when promoting machine + annotations into GT. + + Returns: + pandas.DataFrame: A dataframe containing the union of old and new row/column + indexes, with ``df_new`` applied according to ``nan_clears_existing``. + + Raises: + ValueError: If ``nan_clears_existing=True`` and row indexes in ``df_old`` + and ``df_new`` do not overlap after harmonization. This guards against + accidental destructive saves when incoming data does not cover any + existing rows. + + Notes: + In both modes: + - Rows/columns outside ``df_new`` scope are preserved from ``df_old``. + - Non-missing values in ``df_new`` add or overwrite values in ``df_old``. + + With ``nan_clears_existing=True``: + - NaN values in ``df_new`` clear existing values in ``df_old``. + + With ``nan_clears_existing=False``: + - NaN values in ``df_new`` are no-ops and preserve existing values. """ df_new2, df_old2 = harmonize_keypoint_row_index(df_new, df_old) df_new2 = harmonize_keypoint_column_index(df_new2) df_old2 = harmonize_keypoint_column_index(df_old2) - if len(df_old2.index) and len(df_new2.index): + if nan_clears_existing and len(df_new2.index) and len(df_old2.index): overlap = df_old2.index.intersection(df_new2.index) if overlap.empty: raise ValueError( @@ -539,9 +569,12 @@ def merge_save_df( df_out = df_old2.reindex(index=idx, columns=cols) - # Critical: assign df_new values directly, including NaN. - df_out.loc[df_new2.index, df_new2.columns] = df_new2 - + existing = df_out.loc[df_new2.index, df_new2.columns] + values_to_write = df_new2 + if not nan_clears_existing: + # Keep existing values where the new df contains NaN + values_to_write = df_new2.where(df_new2.notna(), existing) + df_out.loc[df_new2.index, df_new2.columns] = values_to_write return df_out diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index be327a6e..699e8234 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -70,7 +70,7 @@ find_nearest_config, infer_dlc_project_from_points_meta, ) -from napari_deeplabcut.core.provenance import resolve_output_path_from_metadata +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 logger = logging.getLogger(__name__) @@ -549,7 +549,21 @@ def writer(path: str, data: Any, attributes: dict) -> List[str] ) pass - df_out = merge_save_df(df_old, df_new) + nan_clears_existing = should_nan_clear_existing_for_save( + source_kind=source_kind, + destination_kind=destination_kind, + ) + + logger.debug( + "Merging save dataframe source_kind=%s destination_kind=%s nan_clears_existing=%s old_rows=%d new_rows=%d", + source_kind, + destination_kind, + nan_clears_existing, + len(df_old.index), + len(df_new.index), + ) + + df_out = merge_save_df(df_old, df_new, nan_clears_existing=nan_clears_existing) else: df_out = df_new diff --git a/src/napari_deeplabcut/core/provenance.py b/src/napari_deeplabcut/core/provenance.py index 18c410b1..cfe66de9 100644 --- a/src/napari_deeplabcut/core/provenance.py +++ b/src/napari_deeplabcut/core/provenance.py @@ -149,6 +149,20 @@ def is_projectless_folder_association_candidate( return True +def should_nan_clear_existing_for_save( + *, + source_kind: AnnotationKind, + destination_kind: AnnotationKind, +) -> bool: + """ + Return whether missing incoming values may delete stored annotations. + + Direct GT edits are authoritative and may delete keypoints. + Machine-to-GT promotion is a non-deleting patch. + """ + return not (source_kind == AnnotationKind.MACHINE and destination_kind == AnnotationKind.GT) + + # ---------------------------------------- # Core provenance logic # ---------------------------------------- diff --git a/src/napari_deeplabcut/tracking/core/merge.py b/src/napari_deeplabcut/tracking/core/merge.py index 80cf45f7..cefadecd 100644 --- a/src/napari_deeplabcut/tracking/core/merge.py +++ b/src/napari_deeplabcut/tracking/core/merge.py @@ -22,6 +22,10 @@ _COORD_TOL_DEFAULT = 1e-6 +# NOTE: @C-Achard 2026-07-15 This system could be reused for machine labels -> GT merges, +# and would give more control to users to choose whether to overwrite existing GT or only fill missing slots. +# It would also simplify the whole machinery for machine to GT promotion. + class TrackingMergePolicy(str, Enum): """Supported merge policies for tracking-result -> DLC points merges."""