Reducing cleverness, adding test coverage

pyuvdata/tests/test_utils.py

@@ -4261,3 +4261,78 @@ def test_uvw_track_generator_moon():
 
     # Check that the total lengths all match 1
     assert np.allclose((gen_results["uvw"] ** 2.0).sum(1), 2.0)
+
+
+@pytest.mark.parametrize("err_state", ["err", "warn", "none"])
+@pytest.mark.parametrize("tel_loc", ["Center", "Moon", "Earth", "Space"])
+@pytest.mark.parametrize("check_frame", ["Moon", "Earth"])
+@pytest.mark.parametrize("del_tel_loc", [False, None, True])
+def test_check_surface_based_positions(err_state, tel_loc, check_frame, del_tel_loc):
+    tel_loc_dict = {
+        "Center": np.array([0, 0, 0]),
+        "Moon": np.array([0, 0, 1.737e6]),
+        "Earth": np.array([0, 6.37e6, 0]),
+        "Space": np.array([4.22e7, 0, 0]),
+    }
+    tel_frame_dict = {"Moon": "mcmf", "Earth": "itrs"}
+
+    ant_pos = np.array(
+        [[0.0, 0.0, 0.0], [1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0]]
+    )
+    if del_tel_loc:
+        ant_pos += tel_loc_dict[tel_loc]
+
+    fail_type = err_msg = err_type = None
+    err_check = uvtest.check_warnings
+    if (tel_loc != check_frame) and (err_state != "none"):
+        if tel_loc == "Center":
+            fail_type = "below"
+        elif tel_loc == "Space":
+            fail_type = "above"
+        else:
+            fail_type = "above" if tel_loc == "Earth" else "below"
+
+    if fail_type is not None:
+        err_msg = (
+            f"{tel_frame_dict[check_frame]} position vector magnitudes must be "
+            f"on the order of the radius of {check_frame} -- they appear to lie well "
+            f"{fail_type} this."
+        )
+        if err_state == "err":
+            err_type = ValueError
+            err_check = pytest.raises
+        else:
+            err_type = UserWarning
+
+        with err_check(err_type, match=err_msg):
+            status = uvutils.check_surface_based_positions(
+                telescope_loc=None if (del_tel_loc) else tel_loc_dict[tel_loc],
+                antenna_positions=None if (del_tel_loc is None) else ant_pos,
+                telescope_frame=tel_frame_dict[check_frame],
+                raise_error=err_state == "err",
+                raise_warning=err_state == "warn",
+            )
+
+        assert (err_state == "err") or (status == (tel_loc == check_frame))
+
+
+@pytest.mark.skipif(not hasmoon, reason="lunarsky not installed")
+@pytest.mark.parametrize("tel_loc", ["Earth", "Moon"])
+@pytest.mark.parametrize("check_frame", ["Earth", "Moon"])
+def test_check_surface_based_positions_earthmoonloc(tel_loc, check_frame):
+    frame = "mcmf" if (check_frame == "Moon") else "itrs"
+
+    if tel_loc == "Earth":
+        loc = EarthLocation.from_geodetic(0, 0, 0)
+    else:
+        loc = MoonLocation.from_selenodetic(0, 0, 0)
+
+    if tel_loc == check_frame:
+        assert uvutils.check_surface_based_positions(
+            telescope_loc=loc, telescope_frame=frame
+        )
+    else:
+        with pytest.raises(ValueError, match=(f"{frame} position vector")):
+            uvutils.check_surface_based_positions(
+                telescope_loc=loc, telescope_frame=frame
+            )

pyuvdata/utils.py

@@ -120,7 +120,7 @@
 ANGLE_TIME_EQUIV = [(units.s, units.arcsec, lambda x: x * 15.0, lambda x: x / 15.0)]
 
 _range_dict = {
-    "itrs": (6.35e6, 6.39e6, "Earth"), "mcmf": (1717100.0, 1757100.0, "Earth")
+    "itrs": (6.35e6, 6.39e6, "Earth"), "mcmf": (1717100.0, 1757100.0, "Moon")
 }
 
 
@@ -4105,7 +4105,7 @@ def check_surface_based_positions(
     whereas for theMoon the range is 1717.1 to 1757.1 km.
 
     telescope_loc : tuple or EarthLocation or MoonLocation
-        Telescope location, specified as a 3-elemnemt tuple (specifying geocentric
+        Telescope location, specified as a 3-element tuple (specifying geo/selenocentric
         position in meters) or as an astropy EarthLocation (or lunarsky MoonLocation).
     telescope_frame : str, optional
         Reference frame for latitude/longitude/altitude. Options are itrs (default) or
@@ -4138,7 +4138,7 @@ def check_surface_based_positions(
         antenna_positions = antenna_positions + (
             telescope_loc.x.to("m").value,
             telescope_loc.y.to("m").value,
-            telescope_loc.x.to("m").value,
+            telescope_loc.z.to("m").value,
         )
     elif telescope_loc is not None:
         antenna_positions = antenna_positions + telescope_loc
@@ -4155,7 +4155,7 @@ def check_surface_based_positions(
         return True
 
     err_msg = (
-        f"{telescope_frame} antenna position vector magnitudes must be on the order of "
+        f"{telescope_frame} position vector magnitudes must be on the order of "
         f"the radius of {world} -- they appear to lie well {err_type} this."
     )
 

pyuvdata/uvcal/uvcal.py

@@ -9,7 +9,6 @@
 import warnings
 
 import numpy as np
-from astropy import units
 from docstring_parser import DocstringStyle
 
 from .. import parameter as uvp
@@ -1195,7 +1194,7 @@ def check(
         check_extra=True,
         run_check_acceptability=True,
         check_freq_spacing=False,
-        lst_tol="1ms",
+        lst_tol=uvutils.LST_RAD_TOL,
     ):
         """
         Add some extra checks on top of checks on UVBase class.
@@ -1213,14 +1212,13 @@ def check(
             Option to check if frequencies are evenly spaced and the spacing is
             equal to their channel_width. This is not required for UVCal
             objects in general but is required to write to calfits files.
-        lst_tol : str or float or Quantity
+        lst_tol : float or None
             Tolerance level at which to test LSTs against their expected values. If
-            provided as a float, must be in units of radians, otherwise a string
-            parsable by the astropy `Quantity` class (or a `Quantity` object itself)
-            with a time-based value can also be used. Default value is 1 millisecond,
-            which is set by the predictive uncertainty in IERS calculations of DUT1,
-            which for some observatories sets the precision with which these values
-            are written. Note that this will raise a warning if the check fails.
+            provided as a float, must be in units of radians. If set to None, the
+            default precision tolerance from the `lst_array` parameter is used (1 mas).
+            Default value is 15 mas,  which is set by the predictive uncertainty in IERS
+            calculations of DUT1 (of order 1 ms), which for some observatories sets the
+            precision with which these values are written.
 
         Returns
         -------
@@ -1326,30 +1324,14 @@ def check(
                 raise_error=False,
             )
 
-            # Figure out our LST tols
-            if lst_tol is None:
-                lst_tols = self._lst_array.tols
-            else:
-                if isinstance(lst_tol, str):
-                    lst_tol = units.Quantity(lst_tol)
-                if isinstance(lst_tol, units.Quantity):
-                    lst_tol = lst_tol.to(
-                        "rad", equivalencies=uvutils.ANGLE_TIME_EQUIV
-                    ).value
-                if not isinstance(lst_tol, float):
-                    raise ValueError(
-                        "lst_tol must be of type float, angle Quantity, or None."
-                    )
-                lst_tols = [0, lst_tol]
-
             lat, lon, alt = self.telescope_location_lat_lon_alt_degrees
             uvutils.check_lsts_against_times(
                 jd_array=self.time_array,
                 lst_array=self.lst_array,
                 latitude=lat,
                 longitude=lon,
                 altitude=alt,
-                lst_tols=lst_tols,
+                lst_tols=self._lst_array.tols if lst_tol is None else [0, lst_tol],
                 frame=self._telescope_location.frame,
             )
 

pyuvdata/uvdata/tests/test_uvdata.py

@@ -5518,7 +5518,7 @@ def test_telescope_loc_xyz_check(paper_uvh5, tmp_path):
         UserWarning,
         [
             "The uvw_array does not match the expected",
-            "itrs antenna position vector magnitudes must be on the order "
+            "itrs position vector magnitudes must be on the order "
             "of the radius of Earth -- they appear to lie well below this.",
         ],
     ):

pyuvdata/uvdata/uvdata.py

@@ -3204,7 +3204,7 @@ def check(
         allow_flip_conj=False,
         check_autos=False,
         fix_autos=False,
-        lst_tol="1ms",
+        lst_tol=uvutils.LST_RAD_TOL,
     ):
         """
         Add some extra checks on top of checks on UVBase class.
@@ -3237,14 +3237,13 @@ def check(
         fix_autos : bool
             If auto-correlations with imaginary values are found, fix those values so
             that they are real-only in data_array. Default is True.
-        lst_tol : str or float or Quantity
+        lst_tol : float or None
             Tolerance level at which to test LSTs against their expected values. If
-            provided as a float, must be in units of radians, otherwise a string
-            parsable by the astropy `Quantity` class (or a `Quantity` object itself)
-            with a time-based value can also be used. Default value is 1 millisecond,
-            which is set by the predictive uncertainty in IERS calculations of DUT1,
-            which for some observatories sets the precision with which these values
-            are written. Note that this will raise a warning if the check fails.
+            provided as a float, must be in units of radians. If set to None, the
+            default precision tolerance from the `lst_array` parameter is used (1 mas).
+            Default value is 15 mas,  which is set by the predictive uncertainty in IERS
+            calculations of DUT1 (of order 1 ms), which for some observatories sets the
+            precision with which these values are written.
 
         Returns
         -------
@@ -3384,22 +3383,6 @@ def check(
                 raise_error=False,
             )
 
-            # Figure out our LST tols
-            if lst_tol is None:
-                lst_tols = self._lst_array.tols
-            else:
-                if isinstance(lst_tol, str):
-                    lst_tol = units.Quantity(lst_tol)
-                if isinstance(lst_tol, units.Quantity):
-                    lst_tol = lst_tol.to(
-                        "rad", equivalencies=uvutils.ANGLE_TIME_EQUIV
-                    ).value
-                if not isinstance(lst_tol, float):
-                    raise ValueError(
-                        "lst_tol must be of type float, angle Quantity, or None."
-                    )
-                lst_tols = [0, lst_tol]
-
             lat, lon, alt = self.telescope_location_lat_lon_alt_degrees
             # Check the LSTs against what we expect given up-to-date IERS data
             uvutils.check_lsts_against_times(
@@ -3408,7 +3391,7 @@ def check(
                 latitude=lat,
                 longitude=lon,
                 altitude=alt,
-                lst_tols=lst_tols,
+                lst_tols=self._lst_array.tols if lst_tol is None else [0, lst_tol],
                 frame=self._telescope_location.frame,
             )
 

pyuvdata/uvflag/uvflag.py

@@ -11,7 +11,6 @@
 
 import h5py
 import numpy as np
-from astropy import units
 
 from .. import parameter as uvp
 from .. import telescopes as uvtel
@@ -927,7 +926,12 @@ def _set_type_waterfall(self):
         if not self.future_array_shapes:
             self._freq_array.form = ("Nfreqs",)
 
-    def check(self, check_extra=True, run_check_acceptability=True, lst_tol="1ms"):
+    def check(
+        self,
+        check_extra=True,
+        run_check_acceptability=True,
+        lst_tol=uvutils.LST_RAD_TOL,
+    ):
         """
         Add some extra checks on top of checks on UVBase class.
 
@@ -940,14 +944,13 @@ def check(self, check_extra=True, run_check_acceptability=True, lst_tol="1ms"):
             If true, check all parameters, otherwise only check required parameters.
         run_check_acceptability : bool
             Option to check if values in parameters are acceptable.
-        lst_tol : str or float or Quantity
+        lst_tol : float or None
             Tolerance level at which to test LSTs against their expected values. If
-            provided as a float, must be in units of radians, otherwise a string
-            parsable by the astropy `Quantity` class (or a `Quantity` object itself)
-            with a time-based value can also be used. Default value is 1 millisecond,
-            which is set by the predictive uncertainty in IERS calculations of DUT1,
-            which for some observatories sets the precision with which these values
-            are written. Note that this will raise a warning if the check fails.
+            provided as a float, must be in units of radians. If set to None, the
+            default precision tolerance from the `lst_array` parameter is used (1 mas).
+            Default value is 15 mas,  which is set by the predictive uncertainty in IERS
+            calculations of DUT1 (of order 1 ms), which for some observatories sets the
+            precision with which these values are written.
 
         Returns
         -------
@@ -1036,30 +1039,14 @@ def check(self, check_extra=True, run_check_acceptability=True, lst_tol="1ms"):
                 raise_error=False,
             )
 
-            # Figure out our LST tols
-            if lst_tol is None:
-                lst_tols = self._lst_array.tols
-            else:
-                if isinstance(lst_tol, str):
-                    lst_tol = units.Quantity(lst_tol)
-                if isinstance(lst_tol, units.Quantity):
-                    lst_tol = lst_tol.to(
-                        "rad", equivalencies=uvutils.ANGLE_TIME_EQUIV
-                    ).value
-                if not isinstance(lst_tol, float):
-                    raise ValueError(
-                        "lst_tol must be of type float, angle Quantity, or None."
-                    )
-                lst_tols = [0, lst_tol]
-
             lat, lon, alt = self.telescope_location_lat_lon_alt_degrees
             uvutils.check_lsts_against_times(
                 jd_array=self.time_array,
                 lst_array=self.lst_array,
                 latitude=lat,
                 longitude=lon,
                 altitude=alt,
-                lst_tols=lst_tols,
+                lst_tols=self._lst_array.tols if lst_tol is None else [0, lst_tol],
                 frame=self._telescope_location.frame,
             )