Merge pull request astropy#41 from mwcraig/remove-interavtivity-api

mwcraig · web-flow · commit b9891ac0b93f · 2025-05-22T09:33:43.000-05:00
Remove interactive marking API
diff --git a/src/astro_image_display_api/dummy_viewer.py b/src/astro_image_display_api/dummy_viewer.py
@@ -28,7 +28,6 @@ class ImageViewer:
     image_width: int = 0
     image_height: int = 0
     zoom_level: float = 1
-    _is_marking: bool = False
     stretch_options: tuple = ("linear", "log", "sqrt")
     autocut_options: tuple = ("minmax", "zscale", "asinh", "percentile", "histogram")
     _cursor: str = ImageViewerInterface.ALLOWED_CURSOR_LOCATIONS[0]
@@ -46,9 +45,6 @@ class ImageViewer:
     # Default marker name for marking via API
     DEFAULT_MARKER_NAME: str = ImageViewerInterface.DEFAULT_MARKER_NAME
 
-    # Default marker name for interactive marking
-    DEFAULT_INTERACTIVE_MARKER_NAME: str = ImageViewerInterface.DEFAULT_INTERACTIVE_MARKER_NAME
-
     # some internal variable for keeping track of viewer state
     _interactive_marker_name: str = ""
     _previous_click_center: bool = False
@@ -60,18 +56,12 @@ class ImageViewer:
     _center: tuple[float, float] = (0.0, 0.0)
 
     # Some properties where we need to control what happens
-    @property
-    def is_marking(self) -> bool:
-        return self._is_marking
-
     @property
     def click_center(self) -> bool:
         return self._click_center
 
     @click_center.setter
     def click_center(self, value: bool) -> None:
-        if self.is_marking:
-            raise ValueError("Cannot set click_center while marking is active.")
         self._click_center = value
         self._click_drag = not value
 
@@ -80,8 +70,6 @@ def click_drag(self) -> bool:
         return self._click_drag
     @click_drag.setter
     def click_drag(self, value: bool) -> None:
-        if self.is_marking:
-            raise ValueError("Cannot set click_drag while marking is active.")
         self._click_drag = value
         self._click_center = not value
 
@@ -198,45 +186,6 @@ def save(self, filename: str | os.PathLike, overwrite: bool = False) -> None:
         p.write_text("This is a dummy file. The viewer does not save anything.")
 
     # Marker-related methods
-    def start_marking(self, marker_name: str | None = None, marker: Any = None) -> None:
-        """
-        Start interactive marking of points on the image.
-
-        Parameters
-        ----------
-        marker_name : str, optional
-            The name of the marker set to use. If not given, a unique
-            name will be generated.
-        """
-        self._is_marking = True
-        self._previous_click_center = self.click_center
-        self._previous_click_drag = self.click_drag
-        self._previous_marker = self.marker
-        self._previous_scroll_pan = self.scroll_pan
-        self._click_center = False
-        self._click_drag = False
-        self.scroll_pan = True
-        self._interactive_marker_name = marker_name if marker_name else self.DEFAULT_INTERACTIVE_MARKER_NAME
-        self.marker = marker if marker else self.DEFAULT_INTERACTIVE_MARKER_NAME
-
-    def stop_marking(self, clear_markers: bool = False) -> None:
-        """
-        Stop interactive marking of points on the image.
-
-        Parameters
-        ----------
-        clear_markers : bool, optional
-            If `True`, clear the markers that were created during
-            interactive marking. Default is `False`.
-        """
-        self._is_marking = False
-        self.click_center = self._previous_click_center
-        self.click_drag = self._previous_click_drag
-        self.scroll_pan = self._previous_scroll_pan
-        self.marker = self._previous_marker
-        if clear_markers:
-            self.remove_markers(self._interactive_marker_name)
-
     def add_markers(self, table: Table, x_colname: str = 'x', y_colname: str = 'y',
                     skycoord_colname: str = 'coord', use_skycoord: bool = False,
                     marker_name: str | None = None) -> None:
diff --git a/src/astro_image_display_api/interface_definition.py b/src/astro_image_display_api/interface_definition.py
@@ -13,7 +13,6 @@
 ALLOWED_CURSOR_LOCATIONS = ('top', 'bottom', None)
 
 DEFAULT_MARKER_NAME = 'default-marker-name'
-DEFAULT_INTERACTIVE_MARKER_NAME = 'interactive-markers'
 
 # List of marker names that are for internal use only
 RESERVED_MARKER_SET_NAMES = ('all',)
@@ -34,7 +33,6 @@ class ImageViewerInterface(Protocol):
     image_width: int
     image_height: int
     zoom_level: float
-    is_marking: bool
     stretch_options: tuple
     autocut_options: tuple
     cursor: str
@@ -52,9 +50,6 @@ class ImageViewerInterface(Protocol):
     # Default marker name for marking via API
     DEFAULT_MARKER_NAME: str = DEFAULT_MARKER_NAME
 
-    # Default marker name for interactive marking
-    DEFAULT_INTERACTIVE_MARKER_NAME: str = DEFAULT_INTERACTIVE_MARKER_NAME
-
     # The methods, grouped loosely by purpose
 
     # Methods for loading data
@@ -113,37 +108,6 @@ def save(self, filename: str | os.PathLike, overwrite: bool = False) -> None:
         """
         raise NotImplementedError
 
-    # Marker-related methods
-    @abstractmethod
-    def start_marking(self, marker_name: str | None = None, marker: Any = None) -> None:
-        """
-        Start interactive marking of points on the image.
-
-        Parameters
-        ----------
-        marker_name : str, optional
-            The name of the marker set to use. If not given, a unique
-            name will be generated.
-
-        marker : Any, optional
-            The marker to use. If not given, a default marker will be
-            used.
-        """
-        raise NotImplementedError
-
-    @abstractmethod
-    def stop_marking(self, clear_markers: bool = False) -> None:
-        """
-        Stop interactive marking of points on the image.
-
-        Parameters
-        ----------
-        clear_markers : bool, optional
-            If `True`, clear the markers that were created during
-            interactive marking. Default is `False`.
-        """
-        raise NotImplementedError
-
     @abstractmethod
     def add_markers(self, table: Table, x_colname: str = 'x', y_colname: str = 'y',
                     skycoord_colname: str = 'coord', use_skycoord: bool = False,
diff --git a/src/astro_image_display_api/widget_api_test.py b/src/astro_image_display_api/widget_api_test.py
@@ -1,5 +1,4 @@
 # TODO: How to enable switching out backend and still run the same tests?
-import warnings
 
 import pytest
 
@@ -65,7 +64,6 @@ def _get_marker_names_as_set(self):
     def test_default_marker_names(self):
         # Check only that default names are set to a non-empty string
         assert self.image.DEFAULT_MARKER_NAME
-        assert self.image.DEFAULT_INTERACTIVE_MARKER_NAME
 
     def test_width_height(self):
         assert self.image.image_width == 250
@@ -125,75 +123,15 @@ def test_zoom(self):
         self.image.zoom(2)
         assert self.image.zoom_level == 6  # 3 x 2
 
-    def test_marking_operations(self):
-        marks = self.image.get_markers(marker_name="all")
-        self._assert_empty_marker_table(marks)
-        assert not self.image.is_marking
-
-        # Ensure you cannot set it like this.
-        with pytest.raises(AttributeError):
-            self.image.is_marking = True
-
-        # Setting these to check that start_marking affects them.
-        self.image.click_center = True  # Disables click_drag
-        assert self.image.click_center
-        self.image.scroll_pan = False
-        assert not self.image.scroll_pan
-
+    def test_marker_properties(self):
         # Set the marker style
         marker_style = {'color': 'yellow', 'radius': 10, 'type': 'cross'}
         self.image.marker = marker_style
         m_str = str(self.image.marker)
         for key in marker_style.keys():
             assert key in m_str
 
-        self.image.start_marking(marker_name='markymark', marker=marker_style)
-        assert self.image.is_marking
-        assert self.image.marker == marker_style
-        assert not self.image.click_center
-        assert not self.image.click_drag
-
-        # scroll_pan better activate when marking otherwise there is
-        # no way to pan while interactively marking
-        assert self.image.scroll_pan
-
-        # Make sure that when we stop_marking we get our old controls back.
-        self.image.stop_marking()
-        assert self.image.click_center
-        assert not self.image.click_drag
-        assert not self.image.scroll_pan
-
-        # Make sure no warning is issued when trying to retrieve markers
-        # with a name that does not exist.
-        with warnings.catch_warnings():
-            warnings.simplefilter("error")
-            t = self.image.get_markers(marker_name='markymark')
-
-        self._assert_empty_marker_table(t)
-
-        self.image.click_drag = True
-        self.image.start_marking()
-        assert not self.image.click_drag
-
-        # Add a marker to the interactive marking table
-        self.image.add_markers(
-            Table(data=[[50], [50]], names=['x', 'y'], dtype=('float', 'float')),
-            marker_name=self.image.DEFAULT_INTERACTIVE_MARKER_NAME,
-        )
-        assert self._get_marker_names_as_set() == set([self.image.DEFAULT_INTERACTIVE_MARKER_NAME])
-
-        # Clear markers to not pollute other tests.
-        self.image.stop_marking(clear_markers=True)
-
-        assert self.image.is_marking is False
-        self._assert_empty_marker_table(self.image.get_markers(marker_name="all"))
-
-        # Hate this, should add to public API
-        marknames = self._get_marker_names_as_set()
-        assert len(marknames) == 0
-
-        # Make sure that click_drag is restored as expected
-        assert self.image.click_drag
+    # TODO: add test that checks that retrieving markers with an unknown name issues no error
 
     def test_add_markers(self):
         original_marker_name = self.image.DEFAULT_MARKER_NAME
@@ -391,29 +329,21 @@ def test_click_drag(self):
 
         # If is_marking is true then trying to enable click_drag should fail
         self.image.click_drag = False
-        self.image.start_marking()
-        with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while in marking mode)|(while marking is active)'):
-            self.image.click_drag = True
-        self.image.stop_marking()
 
     def test_click_center(self):
         # Set this to ensure that click_center turns it off
         self.image.click_drag = True
 
         # Make sure that setting click_center to False does not turn off
-        # click_draf.
+        # click_drag.
         self.image.click_center = False
         assert self.image.click_drag
 
         self.image.click_center = True
         assert not self.image.click_drag
 
         self.image.click_center = False
-        # If is_marking is true then trying to enable click_center should fail
-        self.image.start_marking()
-        with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while marking is active)'):
-            self.image.click_center = True
-        self.image.stop_marking()
+
 
     def test_scroll_pan(self):
         # Make sure scroll_pan is actually settable
