Merge pull request #207 from desihub/numpy-1.23

Add bounds checks to radec_to_desiname

.github/workflows/python-package.yml

@@ -17,17 +17,17 @@ jobs:
             fail-fast: true
             matrix:
                 os: [ubuntu-latest]
-                python-version: ['3.9', '3.10']  #, '3.11'] There are still issues with Numpy <1.23 and 3.11.
-                numpy-version: ['<1.23']
+                python-version: ['3.10']  # There are still issues with Numpy <1.23 and 3.11.
+                numpy-version: ['<1.23', '<1.24', '<2.0']
                 astropy-version: ['<5.1', '<6.0', '<7.0']
 
         steps:
             - name: Checkout code
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
               with:
                 fetch-depth: 0
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v4
+              uses: actions/setup-python@v5
               with:
                 python-version: ${{ matrix.python-version }}
             - name: Install Python dependencies
@@ -47,15 +47,15 @@ jobs:
             matrix:
                 os: [ubuntu-latest]
                 python-version: ['3.10']
-                astropy-version: ['<5.1']
+                # astropy-version: ['<5.1'] not used at this time.
 
         steps:
             - name: Checkout code
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
               with:
                 fetch-depth: 0
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v4
+              uses: actions/setup-python@v5
               with:
                 python-version: ${{ matrix.python-version }}
             - name: Install Python dependencies
@@ -81,11 +81,11 @@ jobs:
 
         steps:
             - name: Checkout code
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
               with:
                 fetch-depth: 0
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v4
+              uses: actions/setup-python@v5
               with:
                 python-version: ${{ matrix.python-version }}
             - name: Install Python dependencies
@@ -104,11 +104,11 @@ jobs:
 
         steps:
             - name: Checkout code
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
               with:
                   fetch-depth: 0
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v4
+              uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Install Python dependencies
@@ -131,11 +131,11 @@ jobs:
 
         steps:
             - name: Checkout code
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
               with:
                 fetch-depth: 0
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v4
+              uses: actions/setup-python@v5
               with:
                 python-version: ${{ matrix.python-version }}
             - name: Install Python dependencies

doc/changes.rst

@@ -14,7 +14,9 @@ Change Log
 3.4.3 (unreleased)
 ------------------
 
-* No changes yet.
+* Check input bounds in :func:`~desiutil.names.radec_to_desiname` (PR `#207`_).
+
+.. _`#207`: https://github.com/desihub/desiutil/pull/207
 
 3.4.2 (2023-11-29)
 ------------------

py/desiutil/names.py

@@ -14,30 +14,45 @@
 def radec_to_desiname(target_ra, target_dec):
     """Convert the right ascension and declination of a DESI target
     into the corresponding "DESINAME" for reference in publications.
-    Length of target_ra and target_dec must be the same if providing an
+    Length of `target_ra` and `target_dec` must be the same if providing an
     array or list. Note that these names are not unique for roughly
     one percent of DESI targets, so also including TARGETID in
     publications is highly recommended for uniqueness.
 
     Parameters
     ----------
-    target_ra: array of :class:`float64`
+    target_ra: array of :class:`~numpy.float64`
         Right ascension in degrees of target object(s). Can be float, double,
-         or array/list of floats or doubles
-    target_dec: array of :class:`float64`
+        or array/list of floats or doubles.
+    target_dec: array of :class:`~numpy.float64`
         Declination in degrees of target object(s). Can be float, double,
-         or array/list of floats or doubles
+        or array/list of floats or doubles.
 
     Returns
     -------
     array of :class:`str`
         The DESI names referring to the input target RA and DEC's. Array is
         the same length as the input arrays.
 
+    Raises
+    ------
+    ValueError
+        If any input values are out of bounds.
     """
     # Convert to numpy array in case inputs are scalars or lists
     target_ra, target_dec = np.atleast_1d(target_ra), np.atleast_1d(target_dec)
 
+    base_tests = [('NaN values', np.isnan),
+                  ('Infinite values', np.isinf),]
+    inputs = {'target_ra': {'data': target_ra,
+                            'tests': base_tests + [('RA not in range [0, 360)', lambda x: (x < 0) | (x >= 360))]},
+              'target_dec': {'data': target_dec,
+                             'tests': base_tests + [('Dec not in range [-90, 90]', lambda x: (x < -90) | (x > 90))]}}
+    for coord in inputs:
+        for message, check in inputs[coord]['tests']:
+            if check(inputs[coord]['data']).any():
+                raise ValueError(f"{message} detected in {coord}!")
+
     # Number of decimal places in final naming convention
     precision = 4
 

py/desiutil/test/test_names.py

@@ -4,6 +4,7 @@
 """
 import unittest
 import numpy as np
+from ..names import radec_to_desiname
 
 
 class TestNames(unittest.TestCase):
@@ -19,9 +20,8 @@ def tearDownClass(cls):
         pass
 
     def test_radec_to_desiname(self):
-        """Test MaskedArrayWithLimits
+        """Test computation of desiname.
         """
-        from ..names import radec_to_desiname
         ras = [6.2457354547234, 23.914121939862518, 36.23454570972834,
                235.25235223446, 99.9999999999999]
         decs = [29.974787585945496, -42.945872347904356, -0.9968423456,
@@ -44,3 +44,35 @@ def test_radec_to_desiname(self):
         outnames = radec_to_desiname(np.array(ras),
                                      np.array(decs))
         self.assertTrue(np.alltrue(outnames == correct_names))
+
+    def test_radec_to_desiname_bad_values(self):
+        """Test exceptions when running radec_to_desiname with bad values.
+        """
+        ras = [6.2457354547234, 23.914121939862518, 36.23454570972834,
+               235.25235223446, 99.9999999999999]
+        decs = [29.974787585945496, -42.945872347904356, -0.9968423456,
+                8.45677345352345, 89.234958294953]
+
+        original_ra = ras[2]
+        for message, value in [("NaN values detected in target_ra!", np.nan),
+                               ("Infinite values detected in target_ra!", np.inf),
+                               ("RA not in range [0, 360) detected in target_ra!", -23.914121939862518),
+                               ("RA not in range [0, 360) detected in target_ra!", 360.23454570972834)]:
+            ras[2] = value
+            with self.assertRaises(ValueError) as e:
+                outnames = radec_to_desiname(ras, decs)
+            self.assertEqual(str(e.exception), message)
+
+        ras[2] = original_ra
+
+        original_dec = decs[2]
+        for message, value in [("NaN values detected in target_dec!", np.nan),
+                               ("Infinite values detected in target_dec!", np.inf),
+                               ("Dec not in range [-90, 90] detected in target_dec!", -90.9968423456),
+                               ("Dec not in range [-90, 90] detected in target_dec!", 90.9968423456)]:
+            decs[2] = value
+            with self.assertRaises(ValueError) as e:
+                outnames = radec_to_desiname(ras, decs)
+            self.assertEqual(str(e.exception), message)
+
+        decs[2] = original_dec

setup.cfg

@@ -28,12 +28,12 @@ package_dir =
     =py
 packages = find:
 include_package_data = True
-python_requires = >=3.8
+python_requires = >=3.9
 # setup_requires = setuptools_scm
 install_requires =
     requests
     pyyaml
-    numpy<1.23
+    numpy<2.0
     astropy>=5.0
     healpy
     matplotlib