Draft
Conversation
Introduce safe save/merge behavior and overwrite confirmation for DeepLabCut HDF output. - write_hdf: implemented merge-on-save for CollectedData files (preserve old values where new are NaN, new values overwrite old) and retained special handling for machine/refinement outputs. Writes CSV alongside HDF and marks controls as saved (updates last_saved_label when available). - Added user confirmation flow: detect keypoint-level conflicts, summarize them, and prompt the user via a new OverwriteConflictsDialog (napari_deeplabcut.ui.dialogs). Headless/scripted saves skip the dialog. - misc: added utilities to align dataframes, detect keypoint conflicts, format image/keypoint summaries, and produce human-readable conflict text. - _reader: switch numeric check to pd.api.types.is_numeric_dtype for robustness. - _widgets/_writer: added module docstrings and small import/cleanup changes; writer now imports the new dialogs module and datetime for last-saved text. These changes aim to prevent accidental data loss by surfacing potential overwrites and merging labels safely when saving ground-truth label files.
Add comprehensive end-to-end tests for overwrite/merge behavior when saving keypoints: new tests exercise config-first and folder load flows, ensure existing labels are preserved when filling NaNs, verify overwrite warning is shown with image+keypoint details on true conflicts, and confirm cancelling the warning aborts the write. Introduces autouse fixtures to auto-accept QMessageBox and to control/capture OverwriteConflictsDialog.confirm calls, plus helpers to create a minimal DLC-like project and manipulate Points layers. Also add unit tests for conflict statistics and building overwrite-warning text in test_misc.py, and a small header/comment change in test_widgets.py. These tests aim to prevent regressions around saves and user prompts.
Rework end-to-end tests to avoid modal hangs and provide deterministic control over overwrite confirmation: autouse fixture stubs QMessageBox, and overwrite_confirm now patches napari_deeplabcut._writer._maybe_confirm_overwrite to capture/forbid/cancel confirmation requests. Add helper utilities for stable HDF assertions, point manipulation, and robust layer selection, and reorganize tests to cover filling-NaNs, real conflicts, and cancel behavior. In _writer.py, read controls from metadata["metadata"] to find KeypointControls attached to a layer. In keypoints.py, make _add robust: ensure coords are 2D, append data and populate/extend properties (label, id, likelihood) for new points, handle multi-row inserts, and fix QUICK-mode assignment by squeezing coord. These changes improve test reliability and correctness of point append/save behavior.
Refactor KeypointStore._add to robustly align point properties with layer.data when appending points. The new logic ensures properties ('label', 'id', 'likelihood') are treated as arrays (creating them if missing), truncates/pads existing values to match the previous point count, and then appends values for the newly added row(s) (likelihood defaults to 1.0). This handles multi-row inserts and guards against mismatched lengths. Also remove the now-unneeded test helper _ensure_last_point_has_label from tests and tidy up minor whitespace.
Prevent get_folder_parser from parsing arbitrary directories by returning None for paths that don't resemble a DeepLabCut dataset. Adds _looks_like_dlc_folder(path) which checks the path is an existing directory and either contains a 'labeled-data' segment or has CollectedData*/machinelabels* files (.h5/.csv); get_folder_parser uses this to short-circuit non-DLC folders.
Add heuristics and plumbing to prefer the napari-deeplabcut reader for DLC-like paths (config.yaml or labeled-data folders) and fall back to builtins otherwise. Introduce helper functions to detect DLC assets and a qt_viewer._qt_open wrapper to force the DLC plugin for drag/drop when appropriate. Add layer adoption logic (_adopt_existing_layers, _adopt_layer, _handle_existing_image_layer, _handle_existing_points_layer) so the widget initializes correctly when opened after layers exist, plus _infer_image_root to robustly determine image roots. Make metadata updates safer by skipping None/empty values and only setting project when present. Also add a module logger for debugging.
Add logging and defensive handling in write_hdf: import logging and create a module logger; use metadata.get(...) with safe defaults for 'metadata', 'name', and 'root'; attempt to infer root from stored paths when root is missing; raise a clear KeyError if root cannot be determined. Also handle cases where an existing GT HDF5 file does not contain a 'keypoints' key by logging a warning and falling back to reading the whole file. These changes make the writer more resilient to missing or differently-structured layer metadata and provide clearer diagnostics.
Adjust test_reader.py to better mimic a DLC labeled-data folder: create labeled-data/test, write images into that subfolder, and add a CollectedData_me.csv artifact so the folder parser recognizes it. Update get_folder_parser usage and assertions to handle a valid parser or None when no images exist. Also change HDF test keys from "data" to the correct "keypoints" key and relax some assertions to check for list/parser existence rather than exact layer counts.
Add syncing of Points layer metadata from image-level metadata to ensure DLC-required keys (root, paths, shape, name) are present and updated in-place to avoid dropping non-serializable objects. Call the sync when image metadata is updated and set root/paths defaults when registering new layers. Make HDF writer more robust by accepting layer metadata either nested under metadata['metadata'] or at the top-level (root/paths), normalize names, infer root from paths when possible, and provide a clearer KeyError message when root cannot be determined. Rename local variables for clarity (layer_meta/layer_name). Harden keypoint store addition logic to tolerate missing 'controls' in layer.metadata by safely extracting label_mode (or using a default), preventing crashes and preserving expected advance/loop behavior when controls are absent.
Add a truth-safe _has_value helper to avoid crashes with numpy arrays and similar objects when checking root/paths/shape/name. Use it for the early-exit condition and when deciding whether to populate layer.metadata fields, so empty strings/None are treated as absent while scalars without __len__ are treated as present. Minor comment tweak to clarify in-place metadata updates.
Introduce a cross-version helper _set_or_add_bodypart_xy in test_e2e.py that updates an existing keypoint row if present or adds a new point (via store.current_keypoint and Points.add) when a placeholder NaN row is absent. Update affected tests to pass the store fixture and call the new helper. Adjust label handling to safely handle missing property values. Also add an empty ui package init (src/napari_deeplabcut/ui/__init__.py) to register the ui package.
Add three new modules to centralize DLC metadata and path logic: - src/napari_deeplabcut/config/models.py: Pydantic models and enums (DLCHeaderModel, ImageMetadata, PointsMetadata, PathMatchPolicy, MetadataKind) to represent authoritative header and layer metadata. - src/napari_deeplabcut/core/metadata.py: Helpers for inferring image root (infer_image_root), normalizing/ensuring models (ensure_metadata_models), merging metadata without clobbering existing values (merge_image_metadata, merge_points_metadata), and syncing points metadata from image metadata (sync_points_from_image). - src/napari_deeplabcut/core/paths.py: Path canonicalization and matching utilities (canonicalize_path, PathMatchPolicy, find_matching_depth) and DLC-specific heuristics (is_config_yaml, has_dlc_datafiles, looks_like_dlc_labeled_folder, should_force_dlc_reader). The implementations intentionally preserve legacy behavior for canonicalization and matching depths and centralize migration/synchronization logic for napari-deeplabcut.
Replace ad-hoc dict-based image metadata with typed ImageMetadata/PointsMetadata models and centralized sync helpers in KeypointControls. Add robust layer->image metadata merging, syncing of Points layer metadata in-place, and a project_path field for config/crop handling. Use core.paths.should_force_dlc_reader and core.metadata helpers (infer_image_root, sync_points_from_image) instead of prior local heuristics. Improve overwrite-confirmation test fixture to patch the writer import site, record richer call info, and tighten assertions; update tests to reflect metadata model changes and adjust waits/selection checks. Misc: rename _images_meta -> _image_meta, small cleanup and defensive guards for legacy/malformed metadata.
Remove the custom QtWelcomeWidget and related Qt painting imports/logic, and disable the monkeypatch that forced the DeepLabCut reader on drag/drop (code commented out). Update logging to use repr for image metadata. Add an ImageMetadata.__repr__ implementation that shows only non-None fields and truncates long path lists for clearer debug output.
Introduce a new core/remap.py module that centralizes path-based frame/time remapping logic (RemapResult, build_frame_index_map, remap_time_indices, remap_layer_data_by_paths). Replace the large inline remapping implementation in _widgets.py with calls to remap_layer_data_by_paths and import PathMatchPolicy; update metadata handling to store project path in _project_path and use the new remap result for conditional layer updates and logging. The new implementation unifies canonicalization/depth selection, handles list- and array-like napari layers, and provides clearer debug messages when remapping is skipped or applied.
Add comprehensive unit tests for remapping utilities in napari_deeplabcut: build_frame_index_map, remap_layer_data_by_paths, and remap_time_indices. Covers ordered-depth matching (PathMatchPolicy.ORDERED_DEPTHS), depth fallback, reordering/overlap mapping, array-like Points/Tracks with different time_col positions, list-like Shapes, no-overlap and already-aligned early exits, missing path lists, unmapped indices preservation, and handling of empty/None inputs.
Add ambiguity detection and warning support to remapping logic. Introduces heuristic thresholds (_WARN_OVERLAP_RATIO, _WARN_MAPPED_RATIO, _SAMPLE_N), duplicate-key detection (_find_duplicates), and emits logged warnings for duplicate canonical keys, low overlap/mapping coverage, and non-bijective mappings. RemapResult now carries a warnings tuple. Improve robustness of _remap_array and remap_time_indices comparisons. remap_layer_data_by_paths now derives canonical keys, builds an index map, logs warnings (without changing remap behavior), and includes warnings in the result. Tests updated to assert warnings for duplicate canonical keys and low overlap ratios.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
In _reader.py, the OSError raised when no supported images are found was replaced with a temporary `raise None` and a FIXME comment referencing potential h5/csv handling. In _widgets.py, the logger.setLevel call was annotated with a `# FIXME @C-Achard temp` comment. These appear to be short-term/debugging edits and the `raise None` should be reverted to a proper exception or handled logic before merging (raising None will cause a TypeError at runtime).
Add logging and video-aware behavior to the folder parser. Introduces a module logger and checks for supported video files when no supported images are found, emitting an info message if a video is present or a warning if neither images nor videos are present, then returns None. Also reorganizes a few imports (natsorted, yaml, logging). This provides clearer feedback to users instead of silently returning when no images exist.
Introduce helpers to create/read DLC-style keypoints H5 files and to build minimal projects with multiple ground-truth (GT) and optional machine label files. Add snapshot and assertion utilities for comparing keypoint signatures. Add end-to-end tests (marked xfail) that exercise save routing, machine-file isolation, layer-rename behavior, ambiguous placeholder save, and folder loading when multiple H5 files exist. These tests codify desired provenance-based routing and deterministic failure modes for ambiguous saves.
Introduce deterministic IO/provenance support and related utilities. Adds IOProvenance and AnnotationKind to config.models (with POSIX path normalization), exposes an io field on PointsMetadata, and relaxes model extras. New core.errors defines typed domain exceptions. New core.io discovers and classifies DLC H5/CSV artifacts deterministically. core.metadata gains helpers to parse/merge metadata, normalize/resolve provenance, and enforce unique save targets. core.paths gains root-anchor inference utilities to pick/locate project roots. These changes enable safer, OS-agnostic IO routing and unambiguous save/load behavior.
Add unit tests for napari_deeplabcut.core.io discovery utilities. Tests cover: grouping of HDF5 and CSV annotation artifacts with kind inference and pairing behavior; preference for .h5 over .csv when both exist; support for CSV-only artifacts; and iter_annotation_candidates expanding folders and files with deterministic filename ordering.
Make folder parsing deterministic and more robust by discovering all DLC annotation H5 artifacts (use discover_annotations) instead of loading the first .h5 encountered. The reader now: - imports logging and logs H5 load failures - collects errors while attempting to load every discovered H5 and only raises if no points layers were successfully loaded - preserves legacy image-only behavior when no H5s exist - adds authoritative source metadata (source_h5, source_h5_name, source_h5_stem) to point layers for safer routing/debug In core/io.py the discovery API was renamed to discover_annotations and callers/tests updated accordingly. Tests updated to reflect deterministic loading (removed an xfail). Minor housekeeping comments and module header additions included.
Attach and propagate IO provenance for point layers and add migration helpers for legacy metadata. - reader: import IOProvenance and add _attach_source_and_io() to populate metadata.metadata.io (and legacy source_h5 fields) using canonicalized relative paths and inferred kind; call it when reading HDF layers. Replace misc.canonicalize_path usages with core.paths.canonicalize_path. - widgets: add _ensure_io_from_source_h5() to synthesize IOProvenance from existing source_h5 for legacy layers and call it when initializing/inspecting keypoint layers to maintain provenance continuity. - writer: change output resolution to prefer provenance-derived paths via _resolve_output_path_from_metadata(), fall back to a single CollectedData*.h5 candidate when unambiguous, then to legacy root/name naming; move kind-detection to prefer PointsMetadata.io and use filename-based fallback only if missing. These changes keep provenance authoritative, ease migration from older sessions, and make path handling more deterministic.
Add an optional save_target: IOProvenance field to PointsMetadata to record where a Points layer was saved. Adjust test_e2e to include machine_path in the set of expected H5 paths (when present) when building expected_by_stem, so the test accounts for layers originating from the machine-specific file as well as ground-truth paths.
Add safe promotion workflow and sidecar defaults so machine/prediction Points layers are not overwritten. - UI: prompt for a scorer when saving a prediction layer (with config.yaml lookup, deterministic human_xxxxxx fallback, and user confirmation for generic names). Persist chosen scorer in a folder-scoped sidecar (.napari-deeplabcut.json). - Widgets: ensure a save_target (CollectedData_<scorer>.h5) is set for machine sources before saving; abort if the user cancels or if an anchor cannot be determined. - Writer: add _set_df_scorer to rewrite the scorer level when promoting; change _resolve_output_path_from_metadata to return (path, scorer, source_kind) and avoid writing back to machine sources unless a save_target exists (raise MissingProvenanceError otherwise). Support promotion save_target resolution and preserve existing GT behavior. - Metadata: add sidecar read/write helpers and get_default_scorer/set_default_scorer for storing folder-scoped preferences. Also includes helper utilities for finding config scorers, generating placeholders, and computing safe folder anchors from Points layer metadata. These changes aim to prevent accidental overwrites of model predictions and to streamline promotion to ground-truth CollectedData files.
Add unit tests for core sidecar metadata and writer promotion behavior. test_metadata.py covers reading a missing sidecar, setting/getting the default scorer, schema version presence, and validation against empty scorer names. test_writer_promotion.py verifies writer behavior: it raises MissingProvenanceError for machine sources without a save_target, and it correctly promotes machine labels to a GT CollectedData file (rewriting scorer), while leaving the original machine file unchanged; includes helpers for constructing minimal metadata and reading keypoints.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Guard the conversion of layer.data and the verbose logger.debug call with logger.isEnabledFor(logging.DEBUG). This prevents constructing the numpy array and computing min/max frame values when debug logging is not enabled, reducing unnecessary overhead during normal runs.
Extract detach logic and add visibility checks for crop autorefresh. Added _panel_is_displayed to verify panel (and parent) visibility, introduced _detach_crop_autorefresh to cleanly disconnect previous data-change handlers and stop timers, and simplified sync_crop_layer_autorefresh to use the detach helper and fast no-op paths. update_video_panel_context now detaches autorefresh when the panel is not displayed to avoid orphaned listeners/timers.
Add best-effort visibility checks and update gating to prevent unnecessary legend resolves/repaints when the color scheme dock is hidden. Introduce _is_effectively_visible() and use it in schedule_update, the update timer stop logic, and the main update path. Route selection.active events to a new _on_current_step handler that no-ops in CONFIG mode and when the panel is hidden. Cache the last applied color scheme (_last_scheme) to skip redundant display updates, and refresh once on showEvent to catch missed updates while hidden. Include defensive RuntimeError handling for deleted/closed Qt objects.
In KeypointsDropdownMenu, add a visibility check (with RuntimeError guard) and safely fetch _keypoints via getattr to handle missing/empty keypoint lists. Iterate over the local keypoints list and only assign store.current_keypoint when the target differs to avoid redundant setter calls and unnecessary event churn (e.g., on every frame step).
Relocate the _is_effectively_visible visibility check from ColorSchemeResolver into ColorSchemePanel. Centralizing this dock visibility check in the panel allows update logic to no-op when the color scheme dock is hidden, avoiding unnecessary legend resolution/repaints during frame stepping or point edits. The method implementation is unchanged; the duplicate in ColorSchemeResolver was removed. Also includes a minor whitespace adjustment near the update timer setup.
Replace Path.glob-based expansion with os.scandir and fnmatch for _expand_image_paths. Adds fnmatch/os imports and a small _has_glob_magic helper, filters by _SUPPORTED_SUFFIXES, handles OSErrors, and sorts results for deterministic ordering. This improves performance and robustness when listing large directories and when expanding glob patterns, while preserving file-type filtering.
Add a new config module (config/supported_files.py) that defines SUPPORTED_IMAGES and SUPPORTED_VIDEOS, and update core/io.py to import these constants instead of redefining them. This refactor centralizes supported image/video formats to avoid duplication and make future format changes easier; no functional behavior changes intended.
Replace isVisible() checks with isHidden() to more reliably skip work for widgets that are hidden (avoiding false negatives when not yet shown). In ColorSchemePanel, also walk up parentWidget() chain and return False if any parent is hidden, ensuring nested/inactive UI is respected. Preserve existing RuntimeError guards. Add clarifying comment in KeypointsDropdownMenu about skipping work for explicitly hidden/inactive menus while allowing calls before top-level show (useful for tests and construction paths).
Introduce force_show(widget, qtbot) in conftest to reliably show a widget and its Qt parents (uses QApplication.processEvents) so tests are stable under offscreen/QPA. Import QApplication and call force_show in multiple tests (e2e/test_layer_coloring.py, test_widgets.py, ui/test_color_scheme.py) to ensure viewer, docks, panels, and widgets are visible before assertions. Also add empty UI package __init__.py and adjust a test to use the viewer fixture. These changes reduce flakiness from unseen/offscreen widgets during UI testing.
Rename attach_source_and_io to attach_source_and_io_to_layer_kwargs and update call sites and tests accordingly. Adjust the function signature and parameter name (metadata -> layer_kwargs) and change how metadata dict is accessed (layer_kwargs.setdefault("metadata", ...)). Update imports and references in core/io.py and tests to use the new name, and expand the docstring to clarify the function expects the middle dict of a napari LayerData tuple and writes into layer_kwargs["metadata"].
Implement a compatibility paste handler for napari Points layers and add end-to-end tests. The compat make_paste_data normalizes IDs, resolves current/copy slice points across napari versions, and applies offsets when pasting across time frames. It also handles differences in edge/border color and width keys, guards missing clipboard payloads, preserves feature properties, and pastes into private color managers when present. Two e2e tests were added to verify copying/pasting to a new frame (offsets frame and preserves labels) and that pasting on the same frame does not duplicate existing keypoints.
Replace make_napari_viewer usage with a shared viewer fixture in e2e tests and remove explicit make_napari_viewer() calls. Updated tests in src/napari_deeplabcut/_tests/e2e/* to accept viewer instead of make_napari_viewer, simplifying setup and centralizing viewer creation across tests.
Move compatibility helpers and robust paste handling for napari Points layers. Adds small utilities (_first_present, _first_attr, _normalize_id, _get_current_slice_point, _get_clipboard_slice_point, _get_not_displayed_axes, _filter_clipboard_payload, _paste_text_payload, _append_widths, _paste_colors, _offset_pasted_data) and compat constants to handle differences across napari private APIs (slice representations, border/edge naming, color managers, etc.). Refactors apply_points_layer_ui_tweaks to hide widgets via a list and to create a colormap selector more defensibly. Rewrites make_paste_data to: filter out already-annotated keypoints, shift pasted coordinates onto the viewer's current non-displayed slice(s), preserve/paste text, widths, and colors across versions, and use a safer clipboard mutation strategy. Minor docstring and install_* doc tweaks included. These changes make paste behavior more resilient to napari internals changes and avoid duplicating annotations.
Improve stability of e2e tests by waiting for Points layers to be registered with KeypointControls before asserting. test_layer_coloring: format waitUntil call and add waits to ensure the placeholder layer is in controls._stores and that controls.color_mode becomes "individual". test_points_layers: replace a direct attribute check with a qtbot.waitUntil that verifies the added Points layer appears in controls._stores. These changes reduce flakiness by ensuring UI state is ready before assertions.
Add a helper (_get_existing_keypoint_controls) to locate the viewer-provided KeypointControls dock widget (imports QDockWidget). Replace direct construction/docking of KeypointControls in several end-to-end tests with the existing instance, add/adjust qtbot waitUntil checks (wiring, color_mode, face_color properties) and make the placeholder layer active before adding keypoints. These changes reduce duplication and flakiness by ensuring tests operate on the actual controls instance provided by the viewer fixture.
Introduce three pytest fixtures to conftest.py to simplify access to the plugin UI during tests: keypoint_controls_and_dock (returns controls and dock from add_plugin_dock_widget), keypoint_controls (returns only controls), and keypoint_controls_dock (returns only dock). These provide convenient, reusable handles for testing the napari-deeplabcut keypoint controls and their dock widget.
Refactor tests to use the shared keypoint_controls fixture instead of creating KeypointControls inline. Updated test function signatures and replaced local instantiation/add_dock_widget calls with the fixture (or assigned controls = keypoint_controls where needed). Applies to multiple test modules to remove duplicated setup and ensure consistent widget usage across tests.
Change the KeypointControls checkbox label from "Confirm overwrite saves" to "Warn on overwrite" to clarify the control's intent and make the wording more concise; tooltip and behavior remain unchanged.
Revamp the Tutorial QDialog UI: apply a dark stylesheet for dialog, labels and buttons (rounded corners, hover/pressed/disabled states), set window title to "💡Tutorial" and tweak opacity to 0.96. Improve layout spacing and constraints: message label is now a wrapped QLabel (messageLabel) with min/max widths, adjusted margins, and a footer layout for navigation. Navigation buttons changed to arrow glyphs (←/→), spacing updated, count label renamed and reformatted to "Tip X of Y", and update_nav_buttons is invoked after layout setup. Also minor text formatting fixes for multi-line strings.
Comment out the 'Viewer crop' line in update_video_panel_context (src/napari_deeplabcut/ui/cropping.py) because it was considered too verbose (TMI). Other context lines (frame, output folder, crop source, config crop) remain unchanged.
Change the [project.optional-dependencies] group name from 'testing' to 'dev' in pyproject.toml. This renames the development dependency group (pillow, pytest, pytest-cov) to the more conventional 'dev' label for clarity when installing development dependencies.
Plugin refactor [4-II]: Cropping widgets and DLC project utilities
Plugin refactor [4-III]:: Centralize keybinds config, improve shortcuts window
Plugin refactor [4-IV]: Allow users to specify config.yaml location for loose labeled folders
Plugin refactor [5]: coverage pass, split code, check performance
Update tox.ini to install the 'dev' extras instead of 'testing'. This aligns the test environment with the project's development dependency group; test command and other settings remain unchanged.
Collaborator
Author
|
Note : current CI failures are fixed in #182 |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plugin refactor
Central PR grouping all refactor efforts into a single all-in-one merge.
While review efforts can be focused on the PRs listed below, this central branch is meant to enable centralized testing, feedback and git maneuvers if required.
Scope
The refactor involves a full reorganization and re-writing of the plugin codebase, with the aim of :
Not planned for this particular PR
Removed
vertices.csvPlanning
Merge into this branch, in order:
Documentation
Related PRs
Adapt to the refactor and merge:
New features
Add the following features :
Optional features
Useful features that could be added in this refactor or later as needed.