Switch back to original marker API and add arguments to methods

Author: mwcraig

astrowidgets/interface_definition.py

@@ -58,31 +58,43 @@ def save(self, filename):
 
     # Marker-related methods
     @abstractmethod
-    def start_marking(self):
+    def start_marking(self, marker_name=None):
         raise NotImplementedError
 
     @abstractmethod
-    def stop_marking(self):
+    def stop_marking(self, clear_markers=False):
         raise NotImplementedError
 
     @abstractmethod
-    def add_markers(self):
+    def add_markers(self, table, x_colname='x', y_colname='y',
+                    skycoord_colname='coord', use_skycoord=False,
+                    marker_name=None):
         raise NotImplementedError
 
-    @abstractmethod
-    def remove_all_markers(self):
-        raise NotImplementedError
+    # @abstractmethod
+    # def remove_all_markers(self):
+    #     raise NotImplementedError
 
     @abstractmethod
-    def remove_markers_by_name(self, marker_name=None):
+    def reset_markers(self):
         raise NotImplementedError
 
+    # @abstractmethod
+    # def remove_markers_by_name(self, marker_name=None):
+    #     raise NotImplementedError
+
     @abstractmethod
-    def get_all_markers(self):
+    def remove_markers(self, marker_name=None):
         raise NotImplementedError
 
+    # @abstractmethod
+    # def get_all_markers(self):
+    #     raise NotImplementedError
+
     @abstractmethod
-    def get_markers_by_name(self, marker_name=None):
+    def get_markers(self, x_colname='x', y_colname='y',
+                    skycoord_colname='coord',
+                    marker_name=None):
         raise NotImplementedError
 
     # Methods that modify the view

astrowidgets/tests/widget_api_test.py

@@ -102,7 +102,7 @@ def test_zoom(self):
         assert self.image.zoom_level == 6  # 3 x 2
 
     def test_marking_operations(self):
-        marks = self.image.get_all_markers()
+        marks = self.image.get_markers(marker_name="all")
         assert marks is None
         assert not self.image.is_marking
 
@@ -141,7 +141,7 @@ def test_marking_operations(self):
         # Regression test for GitHub Issue 97:
         # Marker name with no markers should give warning.
         with pytest.warns(UserWarning, match='is empty') as warning_lines:
-            t = self.image.get_markers_by_name('markymark')
+            t = self.image.get_markers(marker_name='markymark')
         assert t is None
         assert len(warning_lines) == 1
 
@@ -150,15 +150,21 @@ def test_marking_operations(self):
         assert not self.image.click_drag
 
         # Simulate a mouse click to add default marker name to the list.
-        self.image._mouse_click_cb(self.image.viewer, None, 50, 50)
-        assert self.image.get_marker_names() == [self.image._interactive_marker_set_name, 'markymark']
+        try:
+            self.image._mouse_click_cb(self.image.viewer, None, 50, 50)
+            assert self.image.get_marker_names() == [self.image._interactive_marker_set_name, 'markymark']
+        except AttributeError:
+            pass
 
         # Clear markers to not pollute other tests.
         self.image.stop_marking(clear_markers=True)
 
         assert self.image.is_marking is False
-        assert self.image.get_all_markers() is None
-        assert len(self.image.get_marker_names()) == 0
+        assert self.image.get_markers(marker_name="all") is None
+
+        # Hate this, should add to public API
+        marknames = self.image._marktags
+        assert len(marknames) == 0
 
         # Make sure that click_drag is restored as expected
         assert self.image.click_drag
@@ -181,27 +187,32 @@ def test_add_markers(self):
         # Add more markers under different name.
         self.image.add_markers(tab, x_colname='x', y_colname='y',
                                skycoord_colname='coord', marker_name='test2')
-        assert self.image.get_marker_names() == ['test1', 'test2']
+
+        marknames = self.image._marktags
+        assert marknames == set(['test1', 'test2'])
+        # assert self.image.get_marker_names() == ['test1', 'test2']
 
         # No guarantee markers will come back in the same order, so sort them.
-        t1 = self.image.get_markers_by_name('test1')
+        t1 = self.image.get_markers(marker_name='test1')
         # Sort before comparing
         t1.sort('x')
         tab.sort('x')
         assert np.all(t1['x'] == tab['x'])
         assert (t1['y'] == tab['y']).all()
 
         # That should have given us two copies of the input table
-        t2 = self.image.get_all_markers()
+        t2 = self.image.get_markers(marker_name="all")
         expected = vstack([tab, tab], join_type='exact')
         # Sort before comparing
         t2.sort(['x', 'y'])
         expected.sort(['x', 'y'])
         assert (t2['x'] == expected['x']).all()
         assert (t2['y'] == expected['y']).all()
 
-        self.image.remove_markers_by_name('test1')
-        assert self.image.get_marker_names() == ['test2']
+        self.image.remove_markers(marker_name='test1')
+        marknames = self.image._marktags
+        assert marknames == set(['test2'])
+        # assert self.image.get_marker_names() == ['test2']
 
         # Ensure unable to mark with reserved name
         for name in self.image.RESERVED_MARKER_SET_NAMES:
@@ -215,24 +226,28 @@ def test_add_markers(self):
                                skycoord_colname='coord')
         # Don't care about the order of the marker names so use set instead of
         # list.
-        assert (set(self.image.get_marker_names()) ==
+        marknames = self.image._marktags
+        assert (set(marknames) ==
                 set(['test2', self.image._default_mark_tag_name]))
+        # assert (set(self.image.get_marker_names()) ==
+        #         set(['test2', self.image._default_mark_tag_name]))
 
         # Clear markers to not pollute other tests.
-        self.image.remove_all_markers()
-        assert len(self.image.get_marker_names()) == 0
-        assert self.image.get_all_markers() is None
+        self.image.reset_markers()
+        marknames = self.image._marktags
+        assert len(marknames) == 0
+        assert self.image.get_markers(marker_name="all") is None
         with pytest.warns(UserWarning, match='is empty'):
-            assert self.image.get_markers_by_name(self.image._default_mark_tag_name) is None
+            assert self.image.get_markers(marker_name=self.image._default_mark_tag_name) is None
 
         with pytest.raises(ValueError, match="No markers named 'test1'"):
-            self.image.get_markers_by_name('test1')
+            self.image.get_markers(marker_name='test1')
         with pytest.raises(ValueError, match="No markers named 'test2'"):
-            self.image.get_markers_by_name('test2')
+            self.image.get_markers(marker_name='test2')
 
     def test_remove_markers(self):
         with pytest.raises(ValueError, match='arf'):
-            self.image.remove_markers_by_name('arf')
+            self.image.remove_markers(marker_name='arf')
 
     def test_adding_markers_as_world(self, data, wcs):
         ndd = NDData(data=data, wcs=wcs)
@@ -273,7 +288,7 @@ def test_cuts(self, data):
         with pytest.raises(ValueError, match='must be one of'):
             self.image.cuts = 'not a valid value'
 
-        with pytest.raises(ValueError, match=r'must be given as \(low, high\)'):
+        with pytest.raises(ValueError, match='must have length 2'):
             self.image.cuts = (1, 10, 100)
 
         assert 'histogram' in self.image.autocut_options
@@ -312,9 +327,9 @@ def test_click_drag(self):
         assert not self.image.click_center
 
         # If is_marking is true then trying to enable click_drag should fail
-        self.image._is_marking = True
         self.image.click_drag = False
-        with pytest.raises(ValueError, match='[Ii]nteractive marking'):
+        self.image._is_marking = True
+        with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while in marking mode)'):
             self.image.click_drag = True
         self.image._is_marking = False
 
@@ -333,7 +348,7 @@ def test_click_center(self):
         # If is_marking is true then trying to enable click_center should fail
         self.image._is_marking = True
         self.image.click_center = False
-        with pytest.raises(ValueError, match='[Ii]nteractive marking'):
+        with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while in marking mode)'):
             self.image.click_center = True
         self.image._is_marking = False
 
@@ -343,14 +358,6 @@ def test_scroll_pan(self):
             self.image.scroll_pan = value
             assert self.image.scroll_pan is value
 
-    def test_save(self, tmpdir):
-        with pytest.raises(ValueError, match='not supported'):
-            self.image.save(str(tmpdir.join('woot.jpg')))
-
-        filename = str(tmpdir.join('woot.png'))
+    def test_save(self, tmp_path):
+        filename = tmp_path / 'woot.png'
         self.image.save(filename)
-
-        with pytest.raises(ValueError, match='exists'):
-            self.image.save(filename)
-
-        self.image.save(filename, overwrite=True)