Plugin refactor [7]: Fix trajectory plot, better interactions#183
Draft
C-Achard wants to merge 25 commits intocy/refactor-ux-tweaksfrom
Draft
Plugin refactor [7]: Fix trajectory plot, better interactions#183C-Achard wants to merge 25 commits intocy/refactor-ux-tweaksfrom
C-Achard wants to merge 25 commits intocy/refactor-ux-tweaksfrom
Conversation
Introduce a PointsInteractionObserver (and PointsInteractionEvent) to coalesce napari Points-layer events and drive UI updates. Integrate the observer into KeypointControls to keep the matplotlib trajectory plot in sync with Points selection and layer changes, and add a deferred refresh path to populate the plot when layers pre-exist. Improve trajectory widget robustness: safer icon handling, axis theming, correct Y-axis orientation to match napari coordinates, optional force-update of the plot range, and new helpers to show/hide keypoint lines based on Points selection. Also add capture_points_state utility and simplify _store_crop_coordinates to use the first Shapes layer.
Introduce tests for point-layer interactions and trajectory UI behavior: add PointsInteractionObserver tests to ensure selection events emit and the observer rebinds when the active layer changes; tighten matplotlib canvas behavior in widgets tests so plot refresh does nothing when hidden (and can be forced); add new UI tests for KeypointMatplotlibCanvas.sync_visible_lines_to_points_selection covering no-selection, label filtering, and multi-selection visibility. Also add future annotations and import the observer in tests.
Re-apply napari styling to the trajectory matplotlib canvas and toolbar. Call _apply_napari_theme when the canvas is docked and during canvas initialization, connect to viewer theme change events, and redraw the canvas. Replace the previous toolbar icon-replacement call with clearer tooltips and add a minimal stylesheet and label styling that adapts to light/dark napari themes. Includes a safe no-op connection for older napari versions and keeps the icon-replacement implementation commented out for future use.
Adjust sizing and layout of the KeypointMatplotlibCanvas: import QSizePolicy and apply Expanding/Fixed policies to canvas, toolbar, slider, and label; set layout margins, spacing and stretch factors so the plot gets most vertical space; replace setMinimumHeight with setMinimumSize. Comment out a fixed figure size call and call updateGeometry. Add resizeEvent to trigger canvas redraw and provide sizeHint/minimumSizeHint to suggest comfortable default and minimum sizes.
Improve resilience of the trajectory plot when the viewer contains non-DLC or empty Points layers. Add helper methods: _get_plot_points_layer (find first plottable DLC Points), _clear_plot (reset plot state) and _has_refreshable_df (guard df usage). _load_dataframe now skips/clears the plot for missing or incomplete Points layers and handles KeyError/debug-logs instead of raising; it also clears on other failures. Fix numeric handling so single-row dataframes don't squeeze to scalars (use np.atleast_1d), and avoid refreshing the canvas when there is nothing to plot or the widget is not visible. Minor import added for Points/Image/Labels. Update test to assert on layer name rather than identity to avoid brittle identity checks.
Rename the matplotlib canvas class and update usages across the codebase and tests. KeypointMatplotlibCanvas -> TrajectoryMatplotlibCanvas, _matplotlib_canvas -> _traj_mpl_canvas, and associated methods (_ensure_mpl_canvas_docked, silently_dock_matplotlib_canvas, _show_matplotlib_canvas) renamed and wired to the new names. Add a safe getter _safe_get_traj_canvas to handle deleted Qt objects, adjust docking/showing logic to use the new canvas, and update all affected tests. Minor file header comments/whitespace tweaks also included.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the napari-deeplabcut trajectory plotting UI and introduces a points-layer interaction observer to keep trajectory visibility synchronized with points selection.
Changes:
- Replaces the prior keypoint plot canvas with
TrajectoryMatplotlibCanvasand updates widget wiring/docking behavior. - Adds
PointsInteractionObserver/PointsInteractionEventto coalesce selection/layer-change events. - Adds/updates UI + e2e tests covering selection-driven trajectory visibility and observer behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/napari_deeplabcut/ui/plots/trajectory.py |
New/refactored matplotlib trajectory canvas, theming, layer refresh, and selection-based visibility logic. |
src/napari_deeplabcut/core/layers.py |
Adds PointsInteractionObserver and related event/snapshot helpers. |
src/napari_deeplabcut/_widgets.py |
Switches to the new trajectory canvas, docks/shows it, and wires the observer callback. |
src/napari_deeplabcut/_tests/ui/test_traj_select.py |
New tests for selection → trajectory visibility synchronization. |
src/napari_deeplabcut/_tests/test_widgets.py |
Updates widget tests to use TrajectoryMatplotlibCanvas and new docking method name/behavior. |
src/napari_deeplabcut/_tests/e2e/test_points_layers.py |
Updates existing assertions to the new canvas field name and adds observer tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update trajectory canvas tests to use pytest-qt's add_widget API (replace deprecated addWidget) and add qtbot.wait(0) after plotting. This ensures Matplotlib line artists are fully initialized before being assigned to canvas._lines, reducing flakiness in test_traj_select.py.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Ensure Points interactions are closed during KeypointControls teardown and make the docking debug message less verbose. Clarify PointsInteractionEvent 'reasons' docstring by adding additional typical values (e.g. "install" and "content") and rephrasing for readability. Reduce TrajectoryMatplotlibCanvas minimum height from 440 to 340 to avoid overly large cramped layouts.
Add defensive logging and cleanup to apply_points_layer_ui_tweaks: resolve point controls failures with a debug log, log failures when hiding individual widgets, normalize colormap names to strings, construct the dropdown from the normalized names, call update_to only if available, and catch errors when adding the selector. Update tests: remove an xfail marker, replace a SimpleNamespace with a small DummyLayer class in the compat integration test, and add a module docstring/note to test_widgets. These changes improve robustness of UI wiring and make tests more explicit.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add `# pragma: no cover` to three except clauses in src/napari_deeplabcut/napari_compat/color.py. These changes exclude the napari import failure, guess_continuous fallback, and color_manager assignment failure paths from test coverage reporting; they do not alter runtime behavior, only test coverage bookkeeping.
Avoid attribute errors and non-deterministic failures when closing widgets, computing layer state, and loading toolbar icons. _widgets.py: guard against missing _points_interactions before calling close() on widget close. core/layers.py: compute n_points defensively (handle None or objects raising on len()) when capturing a Points layer state. ui/plots/trajectory.py: prevent setting the layout unconditionally, load toolbar icons only if the file exists (use QIcon), and centralize/clarify how the Points layer is chosen for selection-driven visibility to keep plotting and selection policies in sync.
Add extensive tests covering cropping UI behavior and plan/execute logic. Tests include: ensuring DLC crop Shapes layer is reused or created correctly; DLC config Y-extent fallbacks (active, last, none); finding valid rectangles in Shapes with selected/last fallback; summarizing crop source from active Shapes or none for invalid shapes; validation errors in plan_frame_extraction for missing image, output root, required points layer, and required rectangle when cropping; validation in plan_crop_save for missing project config or missing rectangle; execute_crop_save handling of non-dict video_sets entries by replacing them with a dict containing a crop string; and update_video_panel_context behavior for hidden and no-image cases. These cover edge cases and error messages to improve robustness and prevent regressions.
Pass a get_color_mode accessor into TrajectoryMatplotlibCanvas and update plotting/selection logic to support an "individual" color mode. Trajectories are now keyed by (individual, bodypart) instead of just bodypart, and plotting iterates per (individual, bodypart) series. Selection syncing maps Points selections to (id,label) pairs when in individual mode (falls back to bodypart grouping otherwise). Added helper methods (_plot_mode, _df_has_individuals, _line_color_for, _legend_text_for, _iter_series) and made refresh after color-mode changes more robust with exception handling.
Resolve and normalize color cycles for trajectory plots using DLC header metadata. Add robust header parsing (_get_header_model_from_metadata), multi-animal detection (_is_multianimal_layer), and config colormap resolution (_get_config_colormap). Use build_color_cycles to derive label/id cycles (with DEFAULT_MULTI_ANIMAL_INDIVIDUAL_CMAP fallback for multi-animal layers) and normalize keys/individual names for reliable lookups (_normalized_cycle, _normalized_individual_name, _resolved_face_color_cycles). Revamp _line_color_for to prefer id cycles in individual mode and label cycles in bodypart mode with safe fallbacks. Also: provide a default color-mode lambda in __init__, dedupe/preserve order of individuals when building plots, normalize yielded individual names, and fix a show/hide race in on_doubleclick. Adds TODOs to consider centralizing duplicate logic.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adjust traj selection tests to match changes in TrajectoryMatplotlibCanvas: pass get_color_mode=lambda: "bodypart" for bodypart-mode tests, update _lines keys from simple labels to tuple keys ("", label), and force canvas._get_plot_points_layer to return the test layer directly. Also rename two tests to indicate they run in bodypart mode. These changes ensure the tests align with the canvas' updated color-mode handling and line key format.
Prefer the active plottable napari Points layer when selecting data for DLC trajectory plots. Extracts an _is_plottable_points_layer helper and uses it to validate a layer (requires a 'header' in metadata and non-empty data). Falls back to the first suitable Points layer in the viewer if the active layer is not valid, and returns None if no plottable layer is found. This refactors and clarifies the selection logic for better UX and readability.
Rename the infer_frame_count parameter to preferred_paths and prefer it over layer metadata when determining frame count. Update compute_label_progress to pass the argument through to the revised function. Also expand the layer stats tooltip to note that percentage progress assumes all keypoints are visible on every frame and represents a theoretical maximum (actual progress may differ if keypoints are occluded or missing).
Compute richer labeling progress and surface it in the UI. - Extend LabelProgress with completed_frames, completed_percent, incomplete_frames, incomplete_frames_by_individual and missing_points_by_individual. - Add helpers to infer observed bodypart/individual names and iterate labeled slots; normalize slot ids. - Update compute_label_progress to calculate frame-level completion, per-individual missing counts, and return the new fields while preserving the original overall percent semantics. - Update KeypointControls to pass the new progress object to the status panel and fix color radio button label/case handling (capitalize buttons, use lowercase when reading). - Update LayerStatusPanel to accept the LabelProgress object, show a richer tooltip summary (including incomplete frames and per-individual details), support right-click to copy the progress details to clipboard, and clear cached details for no/invalid layers. - Add small typing/import adjustments (TYPE_CHECKING, QApplication, QMenu). These changes provide more actionable diagnostics about labeling completeness while keeping the top-line percentage unchanged.
Add a QToolButton "ℹ" next to the progress label (styled, hover/tooltip, context-menu enabled) and wrap the label+button in a container for layout. Set cursor/interaction flags and propagate tooltips so the detailed progress summary can be accessed from the info button, label, or container. Clarify progress wording throughout (e.g. use "fully labeled" / "non fully labeled" instead of "complete"/"incomplete") and expand the tip text to note visibility limitations. Also wire up clearing of tooltips for no/invalid layer states and add required Qt imports.
Replace a literal placeholder with an f-string to correctly show the per-individual missing keypoints count, and insert a newline in the details/tooltip text to improve sentence separation and readability.
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
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.
Automated summary
This pull request refactors the trajectory plotting and points layer selection synchronization in the napari-deeplabcut plugin.
It introduces a new
TrajectoryMatplotlibCanvas, replaces the old keypoint plot canvas, and adds robust synchronization between the points layer selection and trajectory plot visibility.The changes also include new end-to-end and UI tests to verify the improved behavior.
Key changes:
Trajectory Plot Refactor & Synchronization
KeypointMatplotlibCanvaswithTrajectoryMatplotlibCanvasthroughout the codebase, updating all references, tests, and widget logic for clarity and maintainability. [1] [2] [3]_traj_mpl_canvas,_ensure_traj_canvas_docked,_show_traj_canvas, etc.) and ensures that the trajectory plot is properly docked, shown, or hidden as needed. [1] [2] [3] [4] [5] [6] [7]_refresh_trajectory_plot_from_layersto update the trajectory plot when layers are adopted, fixing issues when data is loaded before the plugin UI is opened.Points Layer Selection Synchronization
PointsInteractionObserverandPointsInteractionEventto observe and respond to selection changes in points layers, keeping the trajectory plot in sync with the current selection. [1] [2] [3]_on_points_interactionmethod to update trajectory plot visibility based on the selection state of the active points layer.Testing Improvements
TrajectoryMatplotlibCanvasand related methods. [1] [2]Minor Codebase Cleanups
These changes improve the user experience of trajectory visualization and interaction within the plugin.