Merge pull request #117 from astronomy-commons/issue/100/accept-ra-le…

…-180

Validate input coordinates for cone / polygonal searches

src/lsdb/catalog/catalog.py

@@ -9,6 +9,7 @@
 from hipscat.pixel_math import HealpixPixel
 from hipscat.pixel_math.healpix_pixel_function import get_pixel_argsort
 from hipscat.pixel_math.polygon_filter import SphericalCoordinates
+from hipscat.pixel_math.validators import validate_declination_values, validate_radius
 
 from lsdb import io
 from lsdb.catalog.association_catalog import AssociationCatalog
@@ -186,13 +187,6 @@ def crossmatch(
         hc_catalog = hc.catalog.Catalog(new_catalog_info, alignment.pixel_tree)
         return Catalog(ddf, ddf_map, hc_catalog)
 
-    @staticmethod
-    def _check_ra_dec_values_valid(ra: float, dec: float):
-        if ra < -180 or ra > 180:
-            raise ValueError("ra must be between -180 and 180")
-        if dec > 90 or dec < -90:
-            raise ValueError("dec must be between -90 and 90")
-
     def cone_search(self, ra: float, dec: float, radius: float):
         """Perform a cone search to filter the catalog
 
@@ -208,9 +202,8 @@ def cone_search(self, ra: float, dec: float, radius: float):
             A new Catalog containing the points filtered to those within the cone, and the partitions that
             overlap the cone.
         """
-        if radius < 0:
-            raise ValueError("Cone radius must be non negative")
-        self._check_ra_dec_values_valid(ra, dec)
+        validate_radius(radius)
+        validate_declination_values(dec)
         filtered_hc_structure = self.hc_structure.filter_by_cone(ra, dec, radius)
         pixels_in_cone = filtered_hc_structure.get_healpix_pixels()
         partitions = self._ddf.to_delayed()
@@ -238,6 +231,8 @@ def polygon_search(self, vertices: List[SphericalCoordinates]) -> Catalog:
             A new catalog containing the points filtered to those within the
             polygonal region, and the partitions that have some overlap with it.
         """
+        _, dec = np.array(vertices).T
+        validate_declination_values(dec)
         polygon, vertices_xyz = get_cartesian_polygon(vertices)
         filtered_hc_structure = self.hc_structure.filter_by_polygon(vertices_xyz)
         pixels_in_polygon = filtered_hc_structure.get_healpix_pixels()

tests/lsdb/catalog/test_cone_search.py

@@ -1,5 +1,6 @@
 import pytest
 from astropy.coordinates import SkyCoord
+from hipscat.pixel_math.validators import ValidatorsErrors
 
 
 def test_cone_search_filters_correct_points(small_sky_order1_catalog, assert_divisions_are_correct):
@@ -34,17 +35,18 @@ def test_cone_search_filters_partitions(small_sky_order1_catalog):
         assert pixel in consearch_catalog._ddf_pixel_map
 
 
-def test_negative_radius_errors(small_sky_order1_catalog):
-    with pytest.raises(ValueError):
-        small_sky_order1_catalog.cone_search(0, 0, -1)
+def test_cone_search_wrapped_ra(small_sky_order1_catalog):
+    # RA is inside the [0,360] degree range
+    small_sky_order1_catalog.cone_search(200.3, 0, 1.2)
+    # RA is outside the [0,360] degree range, but they are wrapped
+    small_sky_order1_catalog.cone_search(400.9, 0, 1.3)
+    small_sky_order1_catalog.cone_search(-100.1, 0, 1.5)
 
 
-def test_invalid_ra_dec(small_sky_order1_catalog):
-    with pytest.raises(ValueError):
-        small_sky_order1_catalog.cone_search(-200, 0, 1)
-    with pytest.raises(ValueError):
-        small_sky_order1_catalog.cone_search(200, 0, 1)
-    with pytest.raises(ValueError):
-        small_sky_order1_catalog.cone_search(0, -100, 1)
-    with pytest.raises(ValueError):
-        small_sky_order1_catalog.cone_search(0, 100, 1)
+def test_invalid_dec_and_negative_radius(small_sky_order1_catalog):
+    with pytest.raises(ValueError, match=ValidatorsErrors.INVALID_DEC):
+        small_sky_order1_catalog.cone_search(0, -100.3, 1.2)
+    with pytest.raises(ValueError, match=ValidatorsErrors.INVALID_DEC):
+        small_sky_order1_catalog.cone_search(0, 100.4, 1.3)
+    with pytest.raises(ValueError, match=ValidatorsErrors.INVALID_RADIUS):
+        small_sky_order1_catalog.cone_search(0, 0, -1.5)

tests/lsdb/catalog/test_polygon_search.py

@@ -1,5 +1,7 @@
 import numpy as np
+import numpy.testing as npt
 import pytest
+from hipscat.pixel_math.validators import ValidatorsErrors
 
 from lsdb.core.search.polygon_search import get_cartesian_polygon
 
@@ -37,8 +39,48 @@ def test_polygon_search_empty(small_sky_order1_catalog):
     assert len(polygon_search_catalog.hc_structure.pixel_tree) == 1
 
 
+def test_polygon_search_invalid_dec(small_sky_order1_catalog):
+    # Some declination values are out of the [-90,90] bounds
+    vertices = [(-20, 100), (-20, -1), (20, -1), (20, 100)]
+    with pytest.raises(ValueError, match=ValidatorsErrors.INVALID_DEC):
+        small_sky_order1_catalog.polygon_search(vertices)
+
+
 def test_polygon_search_invalid_shape(small_sky_order1_catalog):
     """The polygon is not convex, so the shape is invalid"""
     vertices = [(0, 1), (1, 0), (1, 1), (0, 0), (1, 1)]
     with pytest.raises(RuntimeError):
         small_sky_order1_catalog.polygon_search(vertices)
+
+
+def test_polygon_search_wrapped_right_ascension():
+    """Tests the scenario where the polygon edges intersect the
+    discontinuity of the RA [0,360] degrees range. For the same
+    polygon we have several possible combination of coordinates
+    (here with some float-fudging)."""
+    vertices = [(-20.1, 1), (-20.2, -1), (20.3, -1)]
+    all_vertices_combinations = [
+        [(-20.1, 1), (-20.2, -1), (20.3, -1)],
+        [(-20.1, 1), (339.8, -1), (20.3, -1)],
+        [(-20.1, 1), (-380.2, -1), (20.3, -1)],
+        [(-20.1, 1), (-20.2, -1), (380.3, -1)],
+        [(-20.1, 1), (-20.2, -1), (-339.7, -1)],
+        [(339.9, 1), (-20.2, -1), (20.3, -1)],
+        [(339.9, 1), (339.8, -1), (20.3, -1)],
+        [(339.9, 1), (-380.2, -1), (20.3, -1)],
+        [(339.9, 1), (-20.2, -1), (380.3, -1)],
+        [(339.9, 1), (-20.2, -1), (-339.7, -1)],
+        [(-380.1, 1), (-20.2, -1), (20.3, -1)],
+        [(-380.1, 1), (339.8, -1), (20.3, -1)],
+        [(-380.1, 1), (-380.2, -1), (20.3, -1)],
+        [(-380.1, 1), (-20.2, -1), (380.3, -1)],
+        [(-380.1, 1), (-20.2, -1), (-339.7, -1)],
+        [(-20.1, 1), (339.8, -1), (380.3, -1)],
+        [(-20.1, 1), (339.8, -1), (-339.7, -1)],
+        [(-20.1, 1), (-380.2, -1), (380.3, -1)],
+        [(-20.1, 1), (-380.2, -1), (-339.7, -1)],
+    ]
+    _, vertices_xyz = get_cartesian_polygon(vertices)
+    for v in all_vertices_combinations:
+        _, wrapped_v_xyz = get_cartesian_polygon(v)
+        npt.assert_allclose(vertices_xyz, wrapped_v_xyz, rtol=1e-7)