From a7f5312940bc9d75bb9701babc12ceb654913ebb Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 09:07:00 +0200 Subject: [PATCH 01/18] Prevent GT deletions from machine saves Add an `allow_deletions` flag to `merge_save_df` so merge behavior can preserve existing values when needed. In `write_hdf`, set this flag to false when the source annotations are machine-generated, ensuring machine-to-GT promotion only overwrites with actual machine values and does not treat missing machine values as delete requests. Also add merge debug logging with source/destination kinds and row counts. --- src/napari_deeplabcut/core/dataframes.py | 14 ++++++++++++-- src/napari_deeplabcut/core/io.py | 13 ++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index a3d43d7a..5c4c8748 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -513,6 +513,8 @@ def complete_df_for_save( def merge_save_df( df_old: pd.DataFrame, df_new: pd.DataFrame, + *, + allow_deletions: bool = True, ) -> pd.DataFrame: """ Merge an existing DLC dataframe with a new save dataframe. @@ -521,6 +523,7 @@ def merge_save_df( - 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 + - Machine labels to GT save are not allowed deletions. """ df_new2, df_old2 = harmonize_keypoint_row_index(df_new, df_old) df_new2 = harmonize_keypoint_column_index(df_new2) @@ -538,9 +541,16 @@ def merge_save_df( cols = df_old2.columns.union(df_new2.columns) df_out = df_old2.reindex(index=idx, columns=cols) + incoming = df_new2.reindex(index=idx, columns=cols) - # Critical: assign df_new values directly, including NaN. - df_out.loc[df_new2.index, df_new2.columns] = df_new2 + if allow_deletions: + # Critical: assign df_new values directly, including NaN. + df_out.loc[df_new2.index, df_new2.columns] = df_new2 + else: + # Machine-to-GT promotion semantics: only actual machine annotations + # may modify GT. Missing machine values are not deletion requests. + incoming_has_value = incoming.notna() + df_out = df_out.where(~incoming_has_value, incoming) return df_out diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index be327a6e..af933be7 100644 --- a/src/napari_deeplabcut/core/io.py +++ b/src/napari_deeplabcut/core/io.py @@ -549,7 +549,18 @@ def writer(path: str, data: Any, attributes: dict) -> List[str] ) pass - df_out = merge_save_df(df_old, df_new) + allow_deletions = source_kind != AnnotationKind.MACHINE + + logger.debug( + "Merging save dataframe source_kind=%s destination_kind=%s allow_deletions=%s old_rows=%d new_rows=%d", + source_kind, + destination_kind, + allow_deletions, + len(df_old.index), + len(df_new.index), + ) + + df_out = merge_save_df(df_old, df_new, allow_deletions=allow_deletions) else: df_out = df_new From fd0ae2638e112a04dd0422de2232ea6a529be16d Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 09:38:48 +0200 Subject: [PATCH 02/18] Add regressions for safe machine-to-GT merge Adds two dataframe regression tests covering machine-label promotion into GT data. The new tests reproduce a remapped `metadata.paths` scenario where save completion can introduce all-NaN rows, and assert that merging with `allow_deletions=False` preserves existing manual GT labels while still adding corrected outlier frames. It also verifies merge semantics explicitly: finite incoming values overwrite GT values, while incoming NaNs must not delete existing coordinates. --- .../_tests/core/test_dataframes.py | 218 ++++++++++++++++++ 1 file changed, 218 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/test_dataframes.py b/src/napari_deeplabcut/_tests/core/test_dataframes.py index 3339b460..f8118110 100644 --- a/src/napari_deeplabcut/_tests/core/test_dataframes.py +++ b/src/napari_deeplabcut/_tests/core/test_dataframes.py @@ -994,6 +994,224 @@ 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, allow_deletions=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, + allow_deletions=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 + ) + + # ----------------------------------------------------------------------------- # 13) drop_likelihood_columns # ----------------------------------------------------------------------------- From 8a5653b4af3644b43396dbff3f2c8faaa8b009dc Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 09:39:18 +0200 Subject: [PATCH 03/18] Add e2e regression for machine label promotion Adds a comprehensive end-to-end test for the machine-to-GT save flow after frame remapping, covering overwrite behavior, schema integrity, preservation of existing GT rows, promotion of machine rows, and protection of the source machine HDF. Introduces shared e2e utilities to seed mixed GT/machine datasets and normalize dataframe row-path matching for robust assertions across Index/MultiIndex row keys. --- .../_tests/e2e/test_overwrite_and_merge.py | 328 +++++++++++++++++- src/napari_deeplabcut/_tests/e2e/utils.py | 208 +++++++++++ 2 files changed, 533 insertions(+), 3 deletions(-) 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..4d352e82 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,315 @@ 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") + + if isinstance(machine_io, dict): + assert machine_io.get("kind") is (AnnotationKind.MACHINE) + else: + assert ( + getattr( + machine_io, + "kind", + None, + ) + is 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) + 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, + ) From e8249a45218d38ec306079327fca209c9ef7129b Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 10:21:48 +0200 Subject: [PATCH 04/18] Avoid asserting on Enum instance Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../_tests/e2e/test_overwrite_and_merge.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) 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 4d352e82..cef7fa36 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py +++ b/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py @@ -463,18 +463,8 @@ def test_machine_label_promotion_preserves_existing_gt_after_frame_remap( assert machine_layer.name == ("machinelabels-iter0") machine_io = (machine_layer.metadata or {}).get("io") - - if isinstance(machine_io, dict): - assert machine_io.get("kind") is (AnnotationKind.MACHINE) - else: - assert ( - getattr( - machine_io, - "kind", - None, - ) - is AnnotationKind.MACHINE - ) + 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. From cfc1ea694f3e8d05e36bfceab1d006cdf816bdf4 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 10:36:26 +0200 Subject: [PATCH 05/18] Relax GT kind check and clarify merge docs Update the e2e overwrite/merge test to accept either `AnnotationKind.GT` or the serialized string `"gt"` for saved metadata kind values. Also refine `merge_save_df` docstring semantics to clarify that NaN-based deletions only apply when `allow_deletions=True`, with GT-layer saves remaining the only deletion source. --- src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py | 2 +- src/napari_deeplabcut/core/dataframes.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) 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 cef7fa36..fffbd708 100644 --- a/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py +++ b/src/napari_deeplabcut/_tests/e2e/test_overwrite_and_merge.py @@ -529,7 +529,7 @@ def _gt_has_expected_rows() -> bool: assert save_target is not None if isinstance(save_target, dict): - assert save_target.get("kind") is (AnnotationKind.GT) + 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: diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index 5c4c8748..418293ee 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -521,9 +521,10 @@ def merge_save_df( Semantics: - rows/columns outside df_new scope are preserved from df_old - - rows/columns inside df_new scope replace df_old, including NaN + - rows/columns inside df_new scope replace df_old, including NaN, unless allow_deletions=False + - Notably, machine labels to GT save are not allowed deletions. + The GT layer itself is the only source of any deletions. - NaN in df_new therefore clears/deletes an old saved keypoint - - Machine labels to GT save are not allowed deletions. """ df_new2, df_old2 = harmonize_keypoint_row_index(df_new, df_old) df_new2 = harmonize_keypoint_column_index(df_new2) From f9d7801714c21e4eb2a97dfa46ec61170a4b1415 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 10:36:56 +0200 Subject: [PATCH 06/18] Centralize deletion policy for point saves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `allow_deletions_for_save()` in provenance and use it in both save merging and overwrite conflict reporting. This keeps machine→GT saves as non-deleting patches while allowing authoritative GT edits to delete keypoints, ensuring conflict warnings and write behavior stay consistent. --- src/napari_deeplabcut/core/conflicts.py | 5 ++++- src/napari_deeplabcut/core/io.py | 7 +++++-- src/napari_deeplabcut/core/provenance.py | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/napari_deeplabcut/core/conflicts.py b/src/napari_deeplabcut/core/conflicts.py index ed8b53aa..64489ea9 100644 --- a/src/napari_deeplabcut/core/conflicts.py +++ b/src/napari_deeplabcut/core/conflicts.py @@ -20,6 +20,7 @@ from napari_deeplabcut.core.metadata import parse_points_metadata from napari_deeplabcut.core.project_paths import infer_dlc_project_from_points_meta from napari_deeplabcut.core.provenance import ( + allow_deletions_for_save, resolve_output_path_from_metadata, ) @@ -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) + + allow_deletions = allow_deletions_for_save(source_kind, destination_kind) + deletion_conflict = keypoint_deletions(df_old, df_new) if allow_deletions else None report = build_overwrite_conflict_report( key_conflict, diff --git a/src/napari_deeplabcut/core/io.py b/src/napari_deeplabcut/core/io.py index af933be7..91e571d4 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 allow_deletions_for_save, resolve_output_path_from_metadata from napari_deeplabcut.utils.debug import log_timing logger = logging.getLogger(__name__) @@ -549,7 +549,10 @@ def writer(path: str, data: Any, attributes: dict) -> List[str] ) pass - allow_deletions = source_kind != AnnotationKind.MACHINE + allow_deletions = allow_deletions_for_save( + source_kind=source_kind, + destination_kind=destination_kind, + ) logger.debug( "Merging save dataframe source_kind=%s destination_kind=%s allow_deletions=%s old_rows=%d new_rows=%d", diff --git a/src/napari_deeplabcut/core/provenance.py b/src/napari_deeplabcut/core/provenance.py index 18c410b1..47be1c47 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 allow_deletions_for_save( + *, + source_kind: AnnotationKind | None, + 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 # ---------------------------------------- From 2ff0e1149c88c2a113400d3d29687874f3c55796 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 10:41:55 +0200 Subject: [PATCH 07/18] Use keyword args in save deletion check --- src/napari_deeplabcut/core/conflicts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/conflicts.py b/src/napari_deeplabcut/core/conflicts.py index 64489ea9..098f4927 100644 --- a/src/napari_deeplabcut/core/conflicts.py +++ b/src/napari_deeplabcut/core/conflicts.py @@ -158,7 +158,7 @@ def compute_overwrite_report_for_points_save( key_conflict = keypoint_conflicts(df_old, df_new) - allow_deletions = allow_deletions_for_save(source_kind, destination_kind) + allow_deletions = allow_deletions_for_save(source_kind=source_kind, destination_kind=destination_kind) deletion_conflict = keypoint_deletions(df_old, df_new) if allow_deletions else None report = build_overwrite_conflict_report( From 8df4aab88e4c24c1ab8bc410e72e6cb338b6be07 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 10:44:13 +0200 Subject: [PATCH 08/18] Compute incoming only if needed --- src/napari_deeplabcut/core/dataframes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index 418293ee..3836d8bf 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -542,7 +542,6 @@ def merge_save_df( cols = df_old2.columns.union(df_new2.columns) df_out = df_old2.reindex(index=idx, columns=cols) - incoming = df_new2.reindex(index=idx, columns=cols) if allow_deletions: # Critical: assign df_new values directly, including NaN. @@ -550,6 +549,7 @@ def merge_save_df( else: # Machine-to-GT promotion semantics: only actual machine annotations # may modify GT. Missing machine values are not deletion requests. + incoming = df_new2.reindex(index=idx, columns=cols) incoming_has_value = incoming.notna() df_out = df_out.where(~incoming_has_value, incoming) From a449d1ca99a498c28c05c7a1bd7978cf56a4be60 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 11:04:53 +0200 Subject: [PATCH 09/18] Clarify merge_save_df docs and patch behavior Expands `merge_save_df` documentation with explicit parameter, return, and mode-specific semantics for deletion vs non-deletion merges. Also simplifies the `allow_deletions=False` branch by using `DataFrame.update`, ensuring only non-missing incoming values are applied while NaNs remain no-ops that preserve existing annotations. --- src/napari_deeplabcut/core/dataframes.py | 55 ++++++++++++++++++------ 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index 3836d8bf..ba49783a 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -517,14 +517,48 @@ def merge_save_df( allow_deletions: bool = True, ) -> pd.DataFrame: """ - Merge an existing DLC dataframe with a new save dataframe. - - Semantics: - - rows/columns outside df_new scope are preserved from df_old - - rows/columns inside df_new scope replace df_old, including NaN, unless allow_deletions=False - - Notably, machine labels to GT save are not allowed deletions. - The GT layer itself is the only source of any deletions. - - NaN in df_new therefore clears/deletes an old saved keypoint + Merge a new DLC save dataframe into an existing DLC dataframe. + + Parameters + ---------- + df_old: + Existing on-disk DLC annotations. + df_new: + Incoming annotations to save. + allow_deletions: + Controls how missing values in ``df_new`` are interpreted. + + When True, ``df_new`` is authoritative within its row and column + scope. Incoming values, including NaN, replace existing values. + This supports intentional deletion when saving a directly edited + GT layer. + + When False, ``df_new`` is treated 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 the old and new row and column + indexes, with ``df_new`` applied according to ``allow_deletions``. + + Semantics + --------- + In both modes: + + - rows and columns outside ``df_new``'s scope are preserved from ``df_old`` + - non-missing values in ``df_new`` add or overwrite values in ``df_old`` + + With ``allow_deletions=True``: + + - NaN values in ``df_new`` clear existing values in ``df_old`` + + With ``allow_deletions=False``: + + - NaN values in ``df_new`` are no-ops and preserve existing values in + ``df_old`` """ df_new2, df_old2 = harmonize_keypoint_row_index(df_new, df_old) df_new2 = harmonize_keypoint_column_index(df_new2) @@ -549,10 +583,7 @@ def merge_save_df( else: # Machine-to-GT promotion semantics: only actual machine annotations # may modify GT. Missing machine values are not deletion requests. - incoming = df_new2.reindex(index=idx, columns=cols) - incoming_has_value = incoming.notna() - df_out = df_out.where(~incoming_has_value, incoming) - + df_out.update(df_new2, overwrite=True) return df_out From c6d5f6726dcf4e93493d0eb7f9996f387a98a9d0 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 11:08:35 +0200 Subject: [PATCH 10/18] Adjust overlap guard in merge_save_df Update the overlap-check condition in `merge_save_df` so the intersection validation is gated by explicit index-length checks before attempting overlap detection. --- src/napari_deeplabcut/core/dataframes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index ba49783a..f1be7e4b 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -564,7 +564,7 @@ def merge_save_df( 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 len(df_old2.index) and len(df_new2.index) and len(df_old2.index): overlap = df_old2.index.intersection(df_new2.index) if overlap.empty: raise ValueError( From 02cbb1dad992e7b0a8229af02277b6353350ffd4 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 11:08:52 +0200 Subject: [PATCH 11/18] Add disjoint-row merge guard test Adds a regression test for `merge_save_df` to ensure non-deleting patches reject completely disjoint row indexes. The test covers machine annotations with no frame overlap and asserts a `ValueError` with the expected overlap-related message. --- .../_tests/core/test_dataframes.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/test_dataframes.py b/src/napari_deeplabcut/_tests/core/test_dataframes.py index f8118110..53457ccc 100644 --- a/src/napari_deeplabcut/_tests/core/test_dataframes.py +++ b/src/napari_deeplabcut/_tests/core/test_dataframes.py @@ -1212,6 +1212,59 @@ def test_machine_to_gt_merge_overwrites_finite_values_but_does_not_delete(): ) +def test_merge_save_df_non_deleting_patch_adds_completely_disjoint_rows(): + """ + A non-deleting patch may safely add rows that do not overlap the existing + dataframe. + + This represents machine annotations whose frames are entirely disjoint + from the existing GT dataset. + """ + 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, + ) + + with pytest.raises(ValueError, match="no row-index overlap after harmonization"): + merge_save_df( + df_old, + df_new, + allow_deletions=False, + ) + + # ----------------------------------------------------------------------------- # 13) drop_likelihood_columns # ----------------------------------------------------------------------------- From 14b48dfd8bf6aa4ab7ea844813edde1c5456757e Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 11:11:43 +0200 Subject: [PATCH 12/18] Add test for machine-to-GT overwrite semantics Adds coverage for overwrite preflight behavior when promoting machine labels to GT. The new test ensures deletion detection is skipped (keypoint_deletions must not run), while finite overwrite conflicts are still reported and passed to the report builder with deletion_conflict set to None. --- .../_tests/core/test_conflicts.py | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/test_conflicts.py b/src/napari_deeplabcut/_tests/core/test_conflicts.py index 3d04a727..720c8938 100644 --- a/src/napari_deeplabcut/_tests/core/test_conflicts.py +++ b/src/napari_deeplabcut/_tests/core/test_conflicts.py @@ -451,6 +451,118 @@ 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_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) From 77bfaaa0e3680d7f7f1dbd8252cf51d7f4212d4d Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Mon, 13 Jul 2026 11:14:14 +0200 Subject: [PATCH 13/18] Add disjoint machine-to-GT promotion test Adds a regression test for overwrite-report preflight when promoting machine labels to GT with disjoint data. The test verifies that additions-only updates return `None` (no warning) and that GT deletion detection is not invoked in this scenario. --- .../_tests/core/test_conflicts.py | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/napari_deeplabcut/_tests/core/test_conflicts.py b/src/napari_deeplabcut/_tests/core/test_conflicts.py index 720c8938..2db2a76d 100644 --- a/src/napari_deeplabcut/_tests/core/test_conflicts.py +++ b/src/napari_deeplabcut/_tests/core/test_conflicts.py @@ -563,6 +563,78 @@ def fake_build_report( ) +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) From bcf89e5af3c9b0aba35bd414cf579c20b734b7f1 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 14 Jul 2026 09:09:43 +0200 Subject: [PATCH 14/18] Fix incorrect condition --- src/napari_deeplabcut/core/dataframes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index f1be7e4b..75c0ca41 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -564,7 +564,7 @@ def merge_save_df( 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) and len(df_old2.index): + if allow_deletions and len(df_new2.index) and len(df_old2.index): overlap = df_old2.index.intersection(df_new2.index) if overlap.empty: raise ValueError( From 29a5af8ac642a01e2f40b07e5d795c54419d9ee0 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Tue, 14 Jul 2026 09:13:04 +0200 Subject: [PATCH 15/18] Fix docstring style --- src/napari_deeplabcut/core/dataframes.py | 66 +++++++++++------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/src/napari_deeplabcut/core/dataframes.py b/src/napari_deeplabcut/core/dataframes.py index 75c0ca41..ffe7c0d0 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -516,49 +516,41 @@ def merge_save_df( *, allow_deletions: bool = True, ) -> pd.DataFrame: - """ - Merge a new DLC save dataframe into an existing DLC dataframe. + """Merge incoming DLC annotations into an existing DLC dataframe. - Parameters - ---------- - df_old: - Existing on-disk DLC annotations. - df_new: - Incoming annotations to save. - allow_deletions: - Controls how missing values in ``df_new`` are interpreted. - - When True, ``df_new`` is authoritative within its row and column - scope. Incoming values, including NaN, replace existing values. - This supports intentional deletion when saving a directly edited - GT layer. - - When False, ``df_new`` is treated 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 the old and new row and column + Args: + df_old: Existing on-disk DLC annotations. + df_new: Incoming annotations to save. + allow_deletions: 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 ``allow_deletions``. - Semantics - --------- - In both modes: - - - rows and columns outside ``df_new``'s scope are preserved from ``df_old`` - - non-missing values in ``df_new`` add or overwrite values in ``df_old`` - - With ``allow_deletions=True``: + Raises: + ValueError: If ``allow_deletions=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. - - NaN values in ``df_new`` clear existing values in ``df_old`` + 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 ``allow_deletions=False``: + With ``allow_deletions=True``: + - NaN values in ``df_new`` clear existing values in ``df_old``. - - NaN values in ``df_new`` are no-ops and preserve existing values in - ``df_old`` + With ``allow_deletions=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) From d0937e28adf25594c4cd5ad58ad2cb836fec88bd Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Wed, 15 Jul 2026 14:29:45 +0200 Subject: [PATCH 16/18] Clarify NaN overwrite semantics in save merge Renamed save-merge provenance APIs from `allow_deletions` to `should_nan_clear_existing`/`nan_clears_existing` to better reflect behavior. Updated conflict reporting and HDF save paths to use the new naming, including logging. `merge_save_df` now applies writes through an explicit value-selection step so NaNs only clear existing values when that mode is enabled, while preserving prior values when disabled. --- src/napari_deeplabcut/core/conflicts.py | 6 ++--- src/napari_deeplabcut/core/dataframes.py | 29 ++++++++++++------------ src/napari_deeplabcut/core/io.py | 10 ++++---- src/napari_deeplabcut/core/provenance.py | 4 ++-- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/napari_deeplabcut/core/conflicts.py b/src/napari_deeplabcut/core/conflicts.py index 098f4927..7dfd7dd6 100644 --- a/src/napari_deeplabcut/core/conflicts.py +++ b/src/napari_deeplabcut/core/conflicts.py @@ -20,8 +20,8 @@ from napari_deeplabcut.core.metadata import parse_points_metadata from napari_deeplabcut.core.project_paths import infer_dlc_project_from_points_meta from napari_deeplabcut.core.provenance import ( - allow_deletions_for_save, resolve_output_path_from_metadata, + should_nan_clear_existing_for_save, ) @@ -158,8 +158,8 @@ def compute_overwrite_report_for_points_save( key_conflict = keypoint_conflicts(df_old, df_new) - allow_deletions = allow_deletions_for_save(source_kind=source_kind, destination_kind=destination_kind) - deletion_conflict = keypoint_deletions(df_old, df_new) if allow_deletions else None + 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 ffe7c0d0..482f897a 100644 --- a/src/napari_deeplabcut/core/dataframes.py +++ b/src/napari_deeplabcut/core/dataframes.py @@ -514,14 +514,14 @@ def merge_save_df( df_old: pd.DataFrame, df_new: pd.DataFrame, *, - allow_deletions: bool = True, + nan_clears_existing: bool = True, ) -> pd.DataFrame: """Merge incoming DLC annotations into an existing DLC dataframe. Args: df_old: Existing on-disk DLC annotations. df_new: Incoming annotations to save. - allow_deletions: Controls how missing values in ``df_new`` are interpreted. + 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 @@ -533,11 +533,11 @@ def merge_save_df( Returns: pandas.DataFrame: A dataframe containing the union of old and new row/column - indexes, with ``df_new`` applied according to ``allow_deletions``. + indexes, with ``df_new`` applied according to ``nan_clears_existing``. Raises: - ValueError: If ``allow_deletions=True`` and row indexes in ``df_old`` and - ``df_new`` do not overlap after harmonization. This guards against + 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. @@ -546,17 +546,17 @@ def merge_save_df( - 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 ``allow_deletions=True``: + With ``nan_clears_existing=True``: - NaN values in ``df_new`` clear existing values in ``df_old``. - With ``allow_deletions=False``: + 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 allow_deletions and len(df_new2.index) and len(df_old2.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( @@ -569,13 +569,12 @@ def merge_save_df( df_out = df_old2.reindex(index=idx, columns=cols) - if allow_deletions: - # Critical: assign df_new values directly, including NaN. - df_out.loc[df_new2.index, df_new2.columns] = df_new2 - else: - # Machine-to-GT promotion semantics: only actual machine annotations - # may modify GT. Missing machine values are not deletion requests. - df_out.update(df_new2, overwrite=True) + 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 91e571d4..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 allow_deletions_for_save, 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,21 +549,21 @@ def writer(path: str, data: Any, attributes: dict) -> List[str] ) pass - allow_deletions = allow_deletions_for_save( + 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 allow_deletions=%s old_rows=%d new_rows=%d", + "Merging save dataframe source_kind=%s destination_kind=%s nan_clears_existing=%s old_rows=%d new_rows=%d", source_kind, destination_kind, - allow_deletions, + nan_clears_existing, len(df_old.index), len(df_new.index), ) - df_out = merge_save_df(df_old, df_new, allow_deletions=allow_deletions) + 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 47be1c47..cfe66de9 100644 --- a/src/napari_deeplabcut/core/provenance.py +++ b/src/napari_deeplabcut/core/provenance.py @@ -149,9 +149,9 @@ def is_projectless_folder_association_candidate( return True -def allow_deletions_for_save( +def should_nan_clear_existing_for_save( *, - source_kind: AnnotationKind | None, + source_kind: AnnotationKind, destination_kind: AnnotationKind, ) -> bool: """ From 4f16c7eaf7ffcc96eb31876171ee256b0b009500 Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Wed, 15 Jul 2026 14:30:06 +0200 Subject: [PATCH 17/18] Add comment for future refactor --- src/napari_deeplabcut/tracking/core/merge.py | 4 ++++ 1 file changed, 4 insertions(+) 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.""" From 950d891ff8ec7c04686dc15cf2b904411aa26f7e Mon Sep 17 00:00:00 2001 From: Cyril Achard Date: Wed, 15 Jul 2026 14:30:23 +0200 Subject: [PATCH 18/18] Update merge_save_df tests for NaN policy Adjusts dataframe merge tests to use the new `nan_clears_existing` flag instead of `allow_deletions`, clarifies test names/docs around authoritative vs non-deleting merge behavior, and updates expectations so non-deleting merges can add disjoint incoming rows while preserving existing GT rows. --- .../_tests/core/test_dataframes.py | 81 ++++++++++++++----- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/src/napari_deeplabcut/_tests/core/test_dataframes.py b/src/napari_deeplabcut/_tests/core/test_dataframes.py index 53457ccc..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")]) @@ -1110,7 +1113,7 @@ def test_remapped_machine_layer_does_not_clear_initial_gt_frames_outside_origina 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, allow_deletions=False) + 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 @@ -1176,7 +1179,7 @@ def test_machine_to_gt_merge_overwrites_finite_values_but_does_not_delete(): merged = merge_save_df( df_old, df_new, - allow_deletions=False, + nan_clears_existing=False, ) # Finite machine correction overwrites the existing nose coordinates. @@ -1214,11 +1217,12 @@ def test_machine_to_gt_merge_overwrites_finite_values_but_does_not_delete(): def test_merge_save_df_non_deleting_patch_adds_completely_disjoint_rows(): """ - A non-deleting patch may safely add rows that do not overlap the existing + A non-deleting save may safely add rows that do not overlap the existing dataframe. - This represents machine annotations whose frames are entirely disjoint - from the existing GT dataset. + 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", @@ -1257,12 +1261,45 @@ def test_merge_save_df_non_deleting_patch_adds_completely_disjoint_rows(): columns=cols, ) - with pytest.raises(ValueError, match="no row-index overlap after harmonization"): - merge_save_df( - df_old, - df_new, - allow_deletions=False, - ) + 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 + ) # -----------------------------------------------------------------------------