Merge pull request #79 from mwcraig/minor-api-cleanup

Minor api cleanup

Author: mwcraig

src/astro_image_display_api/api_test.py

@@ -79,8 +79,8 @@ def _assert_empty_catalog_table(self, table):
         assert len(table) == 0
         assert sorted(table.colnames) == sorted(["x", "y", "coord"])
 
-    def _get_catalog_names_as_set(self):
-        marks = self.image.catalog_names
+    def _get_catalog_labels_as_set(self):
+        marks = self.image.catalog_labels
         return set(marks)
 
     @pytest.mark.parametrize("load_type", ["fits", "nddata", "array"])
@@ -362,7 +362,7 @@ def test_set_catalog_style_before_catalog_data_raises_error(self):
         with pytest.raises(
             ValueError, match="Must load a catalog before setting a catalog style"
         ):
-            self.image.set_catalog_style(color="red", shape="x", size=10)
+            self.image.set_catalog_style(color="red", shape="circle", size=10)
 
     def test_set_get_catalog_style_no_labels(self, catalog):
         # Check that getting without setting returns a dict that contains
@@ -376,7 +376,7 @@ def test_set_get_catalog_style_no_labels(self, catalog):
         # Add some data before setting a style
         self.image.load_catalog(catalog)
         # Check that setting a marker style works
-        marker_settings = dict(color="red", shape="x", size=10)
+        marker_settings = dict(color="red", shape="crosshair", size=10)
         self.image.set_catalog_style(**marker_settings.copy())
 
         retrieved_style = self.image.get_catalog_style()
@@ -421,7 +421,7 @@ def test_set_get_catalog_style_preserves_extra_keywords(self, catalog):
         # The only required keywords are color, shape, and size.
         # Add some extra keyword to the style.
         style = dict(
-            color="blue", shape="x", size=10, extra_kw="extra_value", alpha=0.5
+            color="blue", shape="circle", size=10, extra_kw="extra_value", alpha=0.5
         )
         self.image.set_catalog_style(**style.copy())
 
@@ -489,7 +489,7 @@ def test_load_multiple_catalogs(self, catalog):
             catalog_label="test2",
         )
 
-        assert sorted(self.image.catalog_names) == ["test1", "test2"]
+        assert sorted(self.image.catalog_labels) == ["test1", "test2"]
 
         # No guarantee markers will come back in the same order, so sort them.
         t1 = self.image.get_catalog(catalog_label="test1")
@@ -513,7 +513,7 @@ def test_load_multiple_catalogs(self, catalog):
         # other one without a label.
         self.image.remove_catalog(catalog_label="test1")
         # Make sure test1 is really gone.
-        assert self.image.catalog_names == ("test2",)
+        assert self.image.catalog_labels == ("test2",)
 
         # Get without a catalog
         t2 = self.image.get_catalog()
@@ -534,7 +534,7 @@ def test_load_catalog_multiple_same_label(self, catalog):
         self.image.load_catalog(catalog, catalog_label="test1")
 
         retrieved_catalog = self.image.get_catalog(catalog_label="test1")
-        assert len(retrieved_catalog) == 2 * len(catalog)
+        assert len(retrieved_catalog) == len(catalog)
 
     def test_load_catalog_with_skycoord_no_wcs(self, catalog, data):
         # Check that loading a catalog with skycoord but no x/y and
@@ -543,13 +543,10 @@ def test_load_catalog_with_skycoord_no_wcs(self, catalog, data):
 
         # Remove x/y columns from the catalog
         del catalog["x", "y"]
-        self.image.load_catalog(catalog)
-        # Retrieve the catalog and check that the x and y columns are None
-        retrieved_catalog = self.image.get_catalog()
-        assert "x" in retrieved_catalog.colnames
-        assert "y" in retrieved_catalog.colnames
-        assert all(rc is None for rc in retrieved_catalog["x"])
-        assert all(rc is None for rc in retrieved_catalog["y"])
+        with pytest.raises(
+            ValueError, match="Cannot use pixel coordinates without pixel columns"
+        ):
+            self.image.load_catalog(catalog)
 
     def test_load_catalog_with_use_skycoord_no_skycoord_no_wcs(self, catalog, data):
         # Check that loading a catalog with use_skycoord=True but no
@@ -605,7 +602,7 @@ def test_load_catalog_with_no_style_has_a_style(self, catalog):
     def test_load_catalog_with_style_sets_style(self, catalog):
         # Check that loading a catalog with a style sets the style
         # for that catalog.
-        style = dict(color="blue", shape="x", size=10)
+        style = dict(color="blue", shape="square", size=10)
         self.image.load_catalog(
             catalog, catalog_label="test1", catalog_style=style.copy()
         )

src/astro_image_display_api/image_viewer_logic.py

@@ -9,7 +9,7 @@
 from astropy import units as u
 from astropy.coordinates import SkyCoord
 from astropy.nddata import CCDData, NDData
-from astropy.table import Table, vstack
+from astropy.table import Table
 from astropy.units import Quantity
 from astropy.visualization import (
     AsymmetricPercentileInterval,
@@ -492,9 +492,16 @@ def load_catalog(
                 x, y = self._wcs.world_to_pixel(coords)
                 to_add[x_colname] = x
                 to_add[y_colname] = y
+                xy = (x, y)
             else:
                 to_add[x_colname] = to_add[y_colname] = None
 
+        if not use_skycoord and xy is None:
+            raise ValueError(
+                "Cannot use pixel coordinates without pixel columns or both "
+                "coordinates and a WCS."
+            )
+
         if coords is None:
             if use_skycoord and self._wcs is None:
                 raise ValueError(
@@ -509,17 +516,8 @@ def load_catalog(
 
         catalog_label = self._resolve_catalog_label(catalog_label)
 
-        # Either set new data or append to existing data
-        if (
-            catalog_label in self._catalogs
-            and self._catalogs[catalog_label].data is not None
-        ):
-            # If the catalog already exists, we append to it
-            old_table = self._catalogs[catalog_label].data
-            self._catalogs[catalog_label].data = vstack([old_table, to_add])
-        else:
-            # If the catalog does not exist, we create a new one
-            self._catalogs[catalog_label].data = to_add
+        # Set the new data
+        self._catalogs[catalog_label].data = to_add
 
         # Ensure a catalog always has a style
         if catalog_style is None:
@@ -590,7 +588,7 @@ def get_catalog(
         return result
 
     @property
-    def catalog_names(self) -> tuple[str, ...]:
+    def catalog_labels(self) -> tuple[str, ...]:
         return tuple(self._user_catalog_labels())
 
     # Methods that modify the view
@@ -712,13 +710,15 @@ def get_viewport(
                         "sky coordinates."
                     )
                 else:
-                    center = viewport.wcs.pixel_to_world(
-                        viewport.center[0], viewport.center[1]
-                    )
-                    pixel_scale = proj_plane_pixel_scales(viewport.wcs)[
-                        viewport.largest_dimension
-                    ]
-                    fov = pixel_scale * viewport.fov * u.degree
+                    if center is None:
+                        center = viewport.wcs.pixel_to_world(
+                            viewport.center[0], viewport.center[1]
+                        )
+                    if fov is None:
+                        pixel_scale = proj_plane_pixel_scales(viewport.wcs)[
+                            viewport.largest_dimension
+                        ]
+                        fov = pixel_scale * viewport.fov * u.degree
         else:
             # Pixel coordinates
             if isinstance(viewport.center, SkyCoord):

src/astro_image_display_api/interface_definition.py

@@ -345,6 +345,9 @@ def load_catalog(
         """
         Add catalog entries to the viewer at positions given by the catalog.
 
+        Loading a catalog using a ``catalog_label`` that already exists will
+        overwrite the existing catalog with the new one.
+
         Parameters
         ----------
         table : `astropy.table.Table`
@@ -543,7 +546,7 @@ def get_catalog(
 
     @property
     @abstractmethod
-    def catalog_names(self) -> tuple[str, ...]:
+    def catalog_labels(self) -> tuple[str, ...]:
         """
         Names of the loaded catalogs.