Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/source/user_guide/gui.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,22 @@ And for a bounding boxes dataset, you will see a view more like the one below:
Note the additional bounding boxes layer that is loaded for bounding boxes datasets. For both poses and bounding boxes datasets, you can toggle the visibility of any of these layers by clicking on the eye icon.


### Reset pose edits

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is currently a PR #1126 updating the user guide with the edit timeline widget. Your reset feature should be described inside that section once that PR is approved.

Based on the style used for that part of the user guide, I would suggest the following:

### Reset edits

If you want to discard your corrections, the `Edit tracked data` menu
provides two buttons:

- `Reset current frame` restores the moved or removed keypoints in the frame currently shown in the viewer. Edits in other frames are kept.
- `Reset all frames` restores all edited keypoints in the loaded dataset.

Both buttons restore the dataset to the state it was in when you
[loaded it](#load-the-tracked-dataset) into the viewer. This means
that if you [resumed an editing session](#resume-an-editing-session),
the edits already saved in the file are kept, and their keypoints are
still marked as edited.

Resetting does not change the file on disk. To keep the restored state,
[save the dataset](#save-the-edited-data) again.

Individual edits cannot be undone one at a time: you can only reset a whole frame or the whole dataset.

Take this as a suggestion, we can further discussed once #1126 is merged.


In **Edit tracked data**, use **Reset current frame** to restore dragged or
deleted pose points in the displayed frame. Edits in other frames are kept.
Use **Reset all frames** to restore all pose points in the selected dataset.

Both controls restore the dataset as it was loaded into the viewer. Any edits
already saved in the loaded file are retained, including their edited flags.
Resetting does not change the file on disk. Save the dataset separately to
persist the restored state.

Reset is available for pose datasets with their companion tracks layer still
present. The current frame control is disabled when the time axis is displayed
as a spatial axis. Bounding boxes and individual edit history are not supported.
The loaded baseline is held in memory for each pose layer.

### The points layer
The points layer shows the data for the current frame.

Expand Down
81 changes: 79 additions & 2 deletions movement/napari/edit_timeline_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@
from napari.utils.theme import get_theme
from napari.viewer import Viewer
from qtpy.QtCore import QTimer, Signal
from qtpy.QtWidgets import QCheckBox, QLabel, QVBoxLayout, QWidget
from qtpy.QtWidgets import QCheckBox, QLabel, QPushButton, QVBoxLayout, QWidget

from movement.napari.layer_wiring import (
MAX_FRAME_IDX_KEY,
POINTS_BASELINE_KEY,
POINTS_PROPERTIES_KEY,
TRACKS_LAYER_KEY,
active_movement_points_layer,
frame_axis_is_sliced,
is_movement_points_layer,
reset_points_to_baseline,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -51,9 +55,10 @@ class EditControlsWidget(QWidget):

show_individuals_toggled = Signal(bool)

def __init__(self, parent=None):
def __init__(self, parent=None, napari_viewer=None):
"""Initialise the instructions label and display checkbox."""
super().__init__(parent=parent)
self.viewer = napari_viewer
instructions = QLabel(
"Use the points layer controls to move or delete keypoints. "
"Frames with edited points are flagged as coloured bars on the "
Expand All @@ -73,8 +78,65 @@ def __init__(self, parent=None):
layout = QVBoxLayout()
layout.addWidget(instructions)
layout.addWidget(self.show_individuals_checkbox)
self.reset_frame_button = QPushButton("Reset current frame")
self.reset_all_button = QPushButton("Reset all frames")
for button in (self.reset_frame_button, self.reset_all_button):
button.setToolTip(
"Restore pose points to the file as loaded, including "
"any previously saved edits."
)
layout.addWidget(button)
self.reset_frame_button.clicked.connect(self._reset_frame)
self.reset_all_button.clicked.connect(self._reset_all)
if self.viewer is not None:
for event in (
self.viewer.layers.events.inserted,
self.viewer.layers.events.removed,
self.viewer.layers.selection.events.active,
self.viewer.dims.events.order,
self.viewer.dims.events.ndisplay,
):
event.connect(self._schedule_reset_enabled)
self._update_reset_enabled()
self.setLayout(layout)

def _reset_layer(self):
"""Find a loaded pose layer with its companion still in the viewer."""
if self.viewer is None:
return None
layer = active_movement_points_layer(self.viewer)
if (
layer is not None
and POINTS_BASELINE_KEY in layer.metadata
and layer.metadata.get(TRACKS_LAYER_KEY) in self.viewer.layers
):
return layer
return None

def _schedule_reset_enabled(self, event=None):
"""Wait until loading both layers and their baseline has finished."""
QTimer.singleShot(0, self._update_reset_enabled)

def _update_reset_enabled(self):
"""Disable restoration when no supported pose layer is loaded."""
enabled = self._reset_layer() is not None
self.reset_all_button.setEnabled(enabled)
self.reset_frame_button.setEnabled(
enabled and frame_axis_is_sliced(self.viewer)
)

def _reset_frame(self):
"""Reset only the current frame, leaving other frames unchanged."""
layer = self._reset_layer()
if layer is not None and frame_axis_is_sliced(self.viewer):
reset_points_to_baseline(layer, self.viewer.dims.current_step[0])

def _reset_all(self):
"""Reset all frames in the active pose layer."""
layer = self._reset_layer()
if layer is not None:
reset_points_to_baseline(layer)


class EditTimelineWidget(QWidget):
"""Dock widget flagging frames with edited points.
Expand Down Expand Up @@ -283,6 +345,21 @@ def _on_layer_data_changed(self, event):
"""
if event.source is not self.active_layer:
return
if getattr(event, "movement_reset", False) is True:
frame = event.frame
previous = self._reconstruct_previously_removed_points(
self.active_layer
)
if frame is None:
self._removed_points = previous
else:
self._removed_points = [
point
for point in self._removed_points
if point[0] != frame
] + [point for point in previous if point[0] == frame]
self._redraw_bars()
return
if event.action == ActionType.REMOVING:
self._capture_removed_points(event)
self._redraw_bars()
Expand Down
89 changes: 89 additions & 0 deletions movement/napari/layer_wiring.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
DATASET_ATTRS_KEY: str = "movement_dataset_attrs"
TRACKS_LAYER_KEY: str = "movement_tracks_layer"
MAX_FRAME_IDX_KEY: str = "movement_max_frame_idx"
POINTS_BASELINE_KEY: str = "movement_points_baseline"

# Keep a set of viewers already wired by connect_viewer_callbacks,
# so we don't wire them twice. We use a WeakSet so tracking a viewer here
Expand Down Expand Up @@ -186,6 +187,8 @@ def on_points_data_changed(event):
layer = event.source
if not isinstance(layer, Points):
return
if getattr(event, "movement_reset", False) is True:
return

if event.action == ActionType.CHANGED:
moved_indices = list(event.data_indices)
Expand Down Expand Up @@ -267,3 +270,89 @@ def set_tracks_layer_data(tracks_layer, data, properties):
tracks_layer.data = data
tracks_layer.properties = properties
tracks_layer.color_by = color_by


def capture_points_baseline(points_layer: Points) -> None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of storing a separate snapshot here, could we reuse the load-time state we already keep on the layer?

POINTS_PROPERTIES_KEY already holds, for every row (incl. NaN rows), the original confidence, edited flags and position_is_nan. It's also what napari_layers_to_ds compares the live layer against on save. The only thing missing for a reset is the original positions. So my suggestion is:

  1. In DataLoader._add_points_layer, for poses datasets, also store the loader's full Tracks array (self.data, incl. NaN rows) in the layer metadata, e.g. under a POINTS_POSITION_KEY (we can discuss the name). It's row-aligned with POINTS_PROPERTIES_KEY, since both come from ds_to_napari_layers.
  2. In the reset function, rebuild the rows of the frame being reset (or of all frames) from those two: positions from the new key, properties from POINTS_PROPERTIES_KEY, skipping rows where position_is_nan is True. Rows of other frames are kept from the current layers, and the merged rows are sorted as on load, as the PR already does.
  3. Derive the symbols of restored points from their loaded edited flag (ring if edited, default otherwise), as on load, instead of storing a copy of the symbols. Points in frames that aren't reset keep their current symbol.

The reset buttons would then check for the new key instead of POINTS_BASELINE_KEY.

I tried this on a branch to check that it works: pr-1137-reset-from-original-position. Let me know what you think of this. We can further discuss this.

Therefore, I would remove this capture_points_baseline function.

"""Store independent copies of pose data immediately after loading.

The baseline includes edits saved in the input file. It is owned by the
layer, not the loader widget, and is never updated by subsequent edits.
"""
tracks = points_layer.metadata[TRACKS_LAYER_KEY]
points_layer.metadata[POINTS_BASELINE_KEY] = {
"data": tracks.data.copy(),
"points_properties": {
key: value.copy() for key, value in points_layer.properties.items()
},
"tracks_properties": {
key: value.copy() for key, value in tracks.properties.items()
},
"symbol": points_layer.symbol.copy(),
}


def reset_points_to_baseline(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is more a personal taste, but what about changing this name to reset_edits(), I think that might be more clear.

points_layer: Points, frame: int | None = None
) -> None:
"""Restore loaded pose points for one frame, or all frames if None.

Deleted points are restored from the baseline, rather than inferred
from surviving edited flags. Other frames retain their current data.
Identity swaps and individual edit history are not supported.
"""
baseline = points_layer.metadata[POINTS_BASELINE_KEY]
tracks = points_layer.metadata[TRACKS_LAYER_KEY]
restore = (
baseline["data"][:, 1] == frame
if frame is not None
else np.ones(len(baseline["data"]), dtype=bool)
)
keep = (
tracks.data[:, 1] != frame
if frame is not None
else np.zeros(len(tracks.data), dtype=bool)
)
data = np.concatenate([tracks.data[keep], baseline["data"][restore]])
order = np.lexsort((data[:, 1], data[:, 0]))
data = data[order]

def merge_properties(current, original):
keys = current.keys() | original.keys()
return {
key: np.concatenate(
[
current.get(key, np.zeros(len(tracks.data), dtype=bool))[
keep
],
original.get(
key, np.zeros(len(baseline["data"]), dtype=bool)
)[restore],
]
)[order]
for key in keys
}

properties = merge_properties(
points_layer.properties, baseline["points_properties"]
)
track_properties = merge_properties(
tracks.properties, baseline["tracks_properties"]
)
symbols = np.concatenate(
[points_layer.symbol[keep], baseline["symbol"][restore]]
)[order]
# Avoid interpreting restoration as a user drag or deletion. Notify
# observers once, after Points and Tracks are consistent again.
with points_layer.events.data.blocker():
points_layer.data = data[:, 1:].copy()
points_layer.properties = properties
points_layer.symbol = symbols
points_layer.selected_data = set()
set_tracks_layer_data(tracks, data, track_properties)
points_layer.events.data(
value=points_layer.data,
action=ActionType.CHANGED,
data_indices=(),
movement_reset=True,
frame=frame,
)
3 changes: 3 additions & 0 deletions movement/napari/loader_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
POINTS_LAYER_KEY,
POINTS_PROPERTIES_KEY,
TRACKS_LAYER_KEY,
capture_points_baseline,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do what I suggested, we would remove this because it would no longer exist.

Instead, we would add POINTS_POSITION_KEY (name to discuss) and points_layer_properties.

connect_viewer_callbacks,
frame_axis_is_sliced,
on_points_data_changed,
Expand Down Expand Up @@ -212,6 +213,8 @@ def _on_load_clicked(self):
# and a boxes layer if the dataset is a bounding boxes one
self._add_points_layer()
self._add_tracks_layer()
if self.data_bboxes is None:
capture_points_baseline(self.points_layer)
if self.data_bboxes is not None:
self._add_boxes_layer()

Expand Down
4 changes: 3 additions & 1 deletion movement/napari/meta_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ def __init__(self, napari_viewer: Viewer, parent=None):
# A collapsible "edit controls" widget that can be used
# to show/hide and configure the edit timeline docked
# to the bottom of the viewer.
self.edit_controls = EditControlsWidget(parent=self)
self.edit_controls = EditControlsWidget(
parent=self, napari_viewer=napari_viewer
)
self.edit_controls.show_individuals_toggled.connect(
self._on_show_individuals_toggled
)
Expand Down
Loading