Do not apply offset when double clicking a PredictedInstance (#1888)

* Add offset argument to newInstance and AddInstance

* Apply offset of 10 for Add Instance menu button (Ctrl + I)

* Add offset for docks Add Instance button

* Make the QtVideoPlayer context menu unit-testable

* Add test for creating a new instance

* Add test for "New Instance" button in `InstancesDock`

* Fix typo in docstring

* Add docstrings and typehinting

* Remove unused imports and sort imports

sleap/gui/app.py

@@ -704,13 +704,17 @@ def prev_vid():
         )
 
         def new_instance_menu_action():
+            """Determine which action to use when using Ctrl + I or menu Add Instance.
+
+            We always add an offset of 10.
+            """
             method_key = [
                 key
                 for (key, val) in instance_adding_methods.items()
                 if val == self.state["instance_init_method"]
             ]
             if method_key:
-                self.commands.newInstance(init_method=method_key[0])
+                self.commands.newInstance(init_method=method_key[0], offset=10)
 
         labelMenu = self.menuBar().addMenu("Labels")
         add_menu_item(

sleap/gui/commands.py

@@ -504,6 +504,7 @@ def newInstance(
         init_method: str = "best",
         location: Optional[QtCore.QPoint] = None,
         mark_complete: bool = False,
+        offset: int = 0,
     ):
         """Creates a new instance, copying node coordinates as appropriate.
 
@@ -513,13 +514,16 @@ def newInstance(
             init_method: Method to use for positioning nodes.
             location: The location where instance should be added (if node init
                 method supports custom location).
+            mark_complete: Whether to mark the instance as complete.
+            offset: Offset to apply to the location if given.
         """
         self.execute(
             AddInstance,
             copy_instance=copy_instance,
             init_method=init_method,
             location=location,
             mark_complete=mark_complete,
+            offset=offset,
         )
 
     def setPointLocations(
@@ -2858,6 +2862,7 @@ def do_action(cls, context: CommandContext, params: dict):
         init_method = params.get("init_method", "best")
         location = params.get("location", None)
         mark_complete = params.get("mark_complete", False)
+        offset = params.get("offset", 0)
 
         if context.state["labeled_frame"] is None:
             return
@@ -2881,6 +2886,7 @@ def do_action(cls, context: CommandContext, params: dict):
             init_method=init_method,
             location=location,
             from_prev_frame=from_prev_frame,
+            offset=offset,
         )
 
         # Add the instance
@@ -2898,6 +2904,7 @@ def create_new_instance(
         init_method: str,
         location: Optional[QtCore.QPoint],
         from_prev_frame: bool,
+        offset: int = 0,
     ) -> Instance:
         """Create new instance."""
 
@@ -2915,6 +2922,7 @@ def create_new_instance(
             mark_complete=mark_complete,
             init_method=init_method,
             location=location,
+            offset=offset,
         )
 
         if has_missing_nodes:
@@ -2988,6 +2996,7 @@ def set_visible_nodes(
         mark_complete: bool,
         init_method: str,
         location: Optional[QtCore.QPoint] = None,
+        offset: int = 0,
     ) -> bool:
         """Sets visible nodes for new instance.
 
@@ -2996,6 +3005,9 @@ def set_visible_nodes(
             copy_instance: The instance to copy from.
             new_instance: The new instance.
             mark_complete: Whether to mark the instance as complete.
+            init_method: The initialization method.
+            location: The location of the mouse click if any.
+            offset: The offset to apply to all nodes.
 
         Returns:
             Whether the new instance has missing nodes.
@@ -3014,24 +3026,19 @@ def set_visible_nodes(
         scale_width = new_size_width / old_size_width
         scale_height = new_size_height / old_size_height
 
-        # Default the offset is 0
-        offset_x = 0
-        offset_y = 0
-
-        # Using the menu or the hotkey
-        if init_method == "best":
-            offset_x = 10
-            offset_y = 10
+        # The offset is 0, except when using Ctrl + I or Add Instance button.
+        offset_x = offset
+        offset_y = offset
 
-            # Using right click and context menu
-            if location is not None:
-                reference_node = next(
-                    (node for node in copy_instance if not node.isnan()), None
-                )
-                reference_x = reference_node.x
-                reference_y = reference_node.y
-                offset_x = location.x() - (reference_x * scale_width)
-                offset_y = location.y() - (reference_y * scale_height)
+        # Using right click and context menu with option "best"
+        if (init_method == "best") and (location is not None):
+            reference_node = next(
+                (node for node in copy_instance if not node.isnan()), None
+            )
+            reference_x = reference_node.x
+            reference_y = reference_node.y
+            offset_x = location.x() - (reference_x * scale_width)
+            offset_y = location.y() - (reference_y * scale_height)
 
         # Go through each node in skeleton.
         for node in context.state["skeleton"].node_names:

sleap/gui/widgets/docks.py

@@ -557,7 +557,7 @@ def create_table_edit_buttons(self) -> QWidget:
 
         hb = QHBoxLayout()
         self.add_button(
-            hb, "New Instance", lambda x: main_window.commands.newInstance()
+            hb, "New Instance", lambda x: main_window.commands.newInstance(offset=10)
         )
         self.add_button(
             hb, "Delete Instance", main_window.commands.deleteSelectedInstance

sleap/gui/widgets/video.py

@@ -240,6 +240,8 @@ def __init__(
 
         self._register_shortcuts()
 
+        self.context_menu = None
+        self._menu_actions = dict()
         if self.context:
             self.setContextMenuPolicy(QtCore.Qt.CustomContextMenu)
             self.customContextMenuRequested.connect(self.show_contextual_menu)
@@ -358,44 +360,54 @@ def add_shortcut(key, step):
     def setSeekbarSelection(self, a: int, b: int):
         self.seekbar.setSelection(a, b)
 
-    def show_contextual_menu(self, where: QtCore.QPoint):
-        if not self.is_menu_enabled:
-            return
+    def create_contextual_menu(self, scene_pos: QtCore.QPointF) -> QtWidgets.QMenu:
+        """Create the context menu for the viewer.
 
-        scene_pos = self.view.mapToScene(where)
-        menu = QtWidgets.QMenu()
+        This is called when the user right-clicks in the viewer. This function also
+        stores the menu actions in the `_menu_actions` attribute so that they can be
+        accessed later and stores the context menu in the `context_menu` attribute.
 
-        menu.addAction("Add Instance:").setEnabled(False)
+        Args:
+            scene_pos: The position in the scene where the menu was requested.
 
-        menu.addAction(
-            "Default",
-            lambda: self.context.newInstance(init_method="best", location=scene_pos),
-        )
+        Returns:
+            The created context menu.
+        """
 
-        menu.addAction(
-            "Average",
-            lambda: self.context.newInstance(
-                init_method="template", location=scene_pos
-            ),
-        )
+        self.context_menu = QtWidgets.QMenu()
+        self.context_menu.addAction("Add Instance:").setEnabled(False)
+
+        self._menu_actions = dict()
+        params_by_action_name = {
+            "Default": {"init_method": "best", "location": scene_pos},
+            "Average": {"init_method": "template", "location": scene_pos},
+            "Force Directed": {"init_method": "force_directed", "location": scene_pos},
+            "Copy Prior Frame": {"init_method": "prior_frame"},
+            "Random": {"init_method": "random", "location": scene_pos},
+        }
+        for action_name, params in params_by_action_name.items():
+            self._menu_actions[action_name] = self.context_menu.addAction(
+                action_name, lambda params=params: self.context.newInstance(**params)
+            )
 
-        menu.addAction(
-            "Force Directed",
-            lambda: self.context.newInstance(
-                init_method="force_directed", location=scene_pos
-            ),
-        )
+        return self.context_menu
 
-        menu.addAction(
-            "Copy Prior Frame",
-            lambda: self.context.newInstance(init_method="prior_frame"),
-        )
+    def show_contextual_menu(self, where: QtCore.QPoint):
+        """Show the context menu at the given position in the viewer.
 
-        menu.addAction(
-            "Random",
-            lambda: self.context.newInstance(init_method="random", location=scene_pos),
-        )
+        This is called when the user right-clicks in the viewer. This function calls
+        `create_contextual_menu` to create the menu and then shows the menu at the
+        given position.
+
+        Args:
+            where: The position in the viewer where the menu was requested.
+        """
 
+        if not self.is_menu_enabled:
+            return
+
+        scene_pos = self.view.mapToScene(where)
+        menu = self.create_contextual_menu(scene_pos)
         menu.exec_(self.mapToGlobal(where))
 
     def load_video(self, video: Video, plot=True):

tests/gui/test_commands.py

@@ -3,11 +3,15 @@
 import sys
 import time
 
+import numpy as np
 from pathlib import PurePath, Path
+from qtpy import QtCore
 from typing import List
 
 from sleap import Skeleton, Track, PredictedInstance
+from sleap.gui.app import MainWindow
 from sleap.gui.commands import (
+    AddInstance,
     CommandContext,
     ExportAnalysisFile,
     ExportDatasetWithImages,
@@ -922,3 +926,102 @@ def no_gui_ask(cls, context, params):
     # Case 3: Export all frames and suggested frames with image data.
     context.exportFullPackage()
     assert_loaded_package_similar(path_to_pkg, sugg=True, pred=True)
+
+
+def test_newInstance(qtbot, centered_pair_predictions: Labels):
+
+    # Get the data
+    labels = centered_pair_predictions
+    lf = labels[0]
+    pred_inst = lf.instances[0]
+    video = labels.video
+
+    # Set-up command context
+    main_window = MainWindow(labels=labels)
+    context = main_window.commands
+    context.state["labeled_frame"] = lf
+    context.state["frame_idx"] = lf.frame_idx
+    context.state["skeleton"] = labels.skeleton
+    context.state["video"] = labels.videos[0]
+
+    # Case 1: Double clicking a prediction results in no offset for new instance
+
+    # Double click on prediction
+    assert len(lf.instances) == 2
+    main_window._handle_instance_double_click(instance=pred_inst)
+
+    # Check new instance
+    assert len(lf.instances) == 3
+    new_inst = lf.instances[-1]
+    assert new_inst.from_predicted is pred_inst
+    assert np.array_equal(new_inst.numpy(), pred_inst.numpy())  # No offset
+
+    # Case 2: Using Ctrl + I (or menu "Add Instance" button)
+
+    # Connect the action to a slot
+    add_instance_menu_action = main_window._menu_actions["add instance"]
+    triggered = False
+
+    def on_triggered():
+        nonlocal triggered
+        triggered = True
+
+    add_instance_menu_action.triggered.connect(on_triggered)
+
+    # Find which instance we are going to copy from
+    (
+        copy_instance,
+        from_predicted,
+        from_prev_frame,
+    ) = AddInstance.find_instance_to_copy_from(
+        context, copy_instance=None, init_method="best"
+    )
+
+    # Click on the menu action
+    assert len(lf.instances) == 3
+    add_instance_menu_action.trigger()
+    assert triggered, "Action not triggered"
+
+    # Check new instance
+    assert len(lf.instances) == 4
+    new_inst = lf.instances[-1]
+    offset = 10
+    np.nan_to_num(new_inst.numpy() - copy_instance.numpy(), nan=offset)
+    assert np.all(
+        np.nan_to_num(new_inst.numpy() - copy_instance.numpy(), nan=offset) == offset
+    )
+
+    # Case 3: Using right click and "Default" option
+
+    # Find which instance we are going to copy from
+    (
+        copy_instance,
+        from_predicted,
+        from_prev_frame,
+    ) = AddInstance.find_instance_to_copy_from(
+        context, copy_instance=None, init_method="best"
+    )
+
+    video_player = main_window.player
+    right_click_location_x = video.shape[2] / 2
+    right_click_location_y = video.shape[1] / 2
+    right_click_location = QtCore.QPointF(
+        right_click_location_x, right_click_location_y
+    )
+    video_player.create_contextual_menu(scene_pos=right_click_location)
+    default_action = video_player._menu_actions["Default"]
+    default_action.trigger()
+
+    # Check new instance
+    assert len(lf.instances) == 5
+    new_inst = lf.instances[-1]
+    reference_node_idx = np.where(
+        np.all(
+            new_inst.numpy() == [right_click_location_x, right_click_location_y], axis=1
+        )
+    )[0][0]
+    offset = (
+        new_inst.numpy()[reference_node_idx] - copy_instance.numpy()[reference_node_idx]
+    )
+    diff = np.nan_to_num(new_inst.numpy() - copy_instance.numpy(), nan=offset)
+    assert np.all(diff == offset)