Repository: RadioAstronomySoftwareGroup/pyuvdata (python)

Issue:
Allow JDs as input to `phase_to_time`
A PR brought this up as a possible enhancement to `phase_to_time`, allow the user to possibly give a float JD as another input time other than astropy.Time objects.

Pull request:
Phase to jd
<!--- Provide a general summary of your changes in the Title above -->
Allows the input to `phase_to_time` to be a float as well as an astropy Time object. assumes time is a JD
## Description
<!--- Describe your changes in detail -->
Allows the input to `phase_to_time` to be a float as well as an astropy Time object. assumes time is a JD
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. If this PR closes an issue, put the word 'closes' before the issue link to auto-close the issue when the PR is merged. -->
closes #659 
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation change (documentation changes only)
- [ ] Version change
- [ ] Build or continuous integration change


## Checklist:
<!--- You may remove the checklists that don't apply to your change type(s) or just leave them empty -->
<!--- Go over all the following points, and replace the space with an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] I have read the [contribution guide](https://github.com/RadioAstronomySoftwareGroup/pyuvdata/blob/master/.github/CONTRIBUTING.md).
- [x] My code follows the code style of this project.

New feature checklist:
- [x] I have added or updated the docstrings associated with my feature using the [numpy docstring format](https://numpydoc.readthedocs.io/en/latest/format.html).
- [x] I have updated the tutorial to highlight my new feature (if appropriate).
- [x] I have added tests to cover my new feature.
- [x] All new and existing tests pass in both python 2 and python 3.
- [x] I have updated the [CHANGELOG](https://github.com/RadioAstronomySoftwareGroup/pyuvdata/blob/master/CHANGELOG.md).

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94cc954e..98e4d539 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file.
 - `UVFlag` information on Read The Docs
 
 ### Changed
+- `UVData.phase_to_time` now accepts a float as an input. Assumes float represents a JD.
 - Added optional acceptability check for `utils.LatLonAlt_from_XYZ`
 - Use `astropy_healpix` rather than `healpy` for HEALPix functionality because `healpy` has a GPL license which is incompatible with ours.
 - `h5py` is now a required package instead of an optional one.
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
index 01b5c2af..ae337a9e 100644
--- a/docs/tutorial.rst
+++ b/docs/tutorial.rst
@@ -286,9 +286,21 @@ Phasing/unphasing data
   # Undo phasing to try another phase center
   >>> UV.unphase_to_drift()
 
+  # Phase the data to the zenith at first time step
+  # The time_array can be used directly now as input floats
+  # are assumed to be a JD
+  >>> UV.phase_to_time(UV.time_array[0])
+  >>> print(UV.phase_type)
+  phased
+
+  # Undo phasing to try another phase center
+  >>> UV.unphase_to_drift()
+
   # Phase to a specific ra/dec/epoch (in radians)
   >>> UV.phase(5.23368, 0.710940, epoch="J2000")
 
+
+
 UVData: Plotting
 ------------------
 Making a simple waterfall plot.
diff --git a/pyuvdata/uvdata.py b/pyuvdata/uvdata.py
index bca38167..d3c87160 100644
--- a/pyuvdata/uvdata.py
+++ b/pyuvdata/uvdata.py
@@ -943,8 +943,8 @@ class UVData(UVBase):
 
         Parameters
         ----------
-        time : astropy.time.Time object
-            The time to phase to, an astropy Time object.
+        time : astropy.time.Time object or float
+            The time to phase to, an astropy Time object or a float Julian Date
         phase_frame : str
             The astropy frame to phase to. Either 'icrs' or 'gcrs'.
             'gcrs' accounts for precession & nutation,
@@ -958,7 +958,7 @@ class UVData(UVBase):
         ValueError
             If the phase_type is not 'drift'
         TypeError
-            If time is not an astropy.time.Time object
+            If time is not an astropy.time.Time object or Julian Date as a float
         """
         if self.phase_type == 'drift':
             pass
@@ -970,8 +970,11 @@ class UVData(UVBase):
                              'Set the phase_type to drift or phased to '
                              'reflect the phasing status of the data')
 
+        if isinstance(time, (float, np.float32)):
+            time = Time(time, format='jd')
+
         if not isinstance(time, Time):
-            raise TypeError("time must be an astropy.time.Time object")
+            raise TypeError("time must be an astropy.time.Time object or a float")
 
         # Generate ra/dec of zenith at time in the phase_frame coordinate system
         # to use for phasing


Test patch:
diff --git a/pyuvdata/tests/test_uvdata.py b/pyuvdata/tests/test_uvdata.py
index 6b296de9..38e81c6c 100644
--- a/pyuvdata/tests/test_uvdata.py
+++ b/pyuvdata/tests/test_uvdata.py
@@ -446,31 +446,51 @@ def test_generic_read():
     pytest.raises(ValueError, uv_in.read, 'foo')
 
 
-def test_phase_unphaseHERA():
-    """
-    Read in drift data, phase to an RA/DEC, unphase and check for object equality.
-    """
+@pytest.fixture
+def uv_phase_and_raw():
     testfile = os.path.join(DATA_PATH, 'zen.2458661.23480.HH.uvh5')
     UV_raw = UVData()
-    UV_raw.read(testfile)
-
+    # Note the RA/DEC values in the raw file were calculated from the lat/long
+    # in the file, which don't agree with our known_telescopes.
+    # So for this test we use the lat/lon in the file.
+    UV_raw.read_uvh5(testfile)
+    # uvtest.checkWarnings(UV_raw.read_miriad, [testfile], {'correct_lat_lon': False},
+    #                      message='Altitude is not present in file and latitude and '
+    #                              'longitude values do not match')
     UV_phase = UVData()
-    UV_phase.read(testfile)
-    UV_phase.phase(0., 0., epoch="J2000")
-    UV_phase.unphase_to_drift()
-    # check that phase + unphase gets back to raw
-    assert UV_raw == UV_phase
+    UV_phase.read_uvh5(testfile)
 
-    # check that phase + unphase work using gcrs
-    UV_phase.phase(Angle('5d').rad, Angle('30d').rad, phase_frame='gcrs')
-    UV_phase.unphase_to_drift()
-    assert UV_raw == UV_phase
+    yield UV_phase, UV_raw
+
+    del UV_phase, UV_raw
 
-    # check that phase + unphase work using a different epoch
-    UV_phase.phase(Angle('180d').rad, Angle('90d'), epoch=Time('2010-01-01T00:00:00', format='isot', scale='utc'))
+    return
+
+
+@pytest.mark.parametrize(
+    "phase_kwargs",
+    [
+        {"ra": 0., "dec": 0., "epoch": "J2000"},
+        {"ra": Angle('5d').rad, "dec": Angle('30d').rad, "phase_frame": "gcrs"},
+        {"ra": Angle('180d').rad, "dec": Angle('90d'),
+         "epoch": Time('2010-01-01T00:00:00', format='isot', scale='utc')
+         },
+
+    ]
+)
+def test_phase_unphaseHERA(uv_phase_and_raw, phase_kwargs):
+    """
+    Read in drift data, phase to an RA/DEC, unphase and check for object equality.
+    """
+    UV_phase, UV_raw = uv_phase_and_raw
+    UV_phase.phase(**phase_kwargs)
     UV_phase.unphase_to_drift()
+    # check that phase + unphase gets back to raw
     assert UV_raw == UV_phase
 
+
+def test_phase_unphaseHERA_one_bl(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
     # check that phase + unphase work with one baseline
     UV_raw_small = UV_raw.select(blt_inds=[0], inplace=False)
     UV_phase_small = copy.deepcopy(UV_raw_small)
@@ -478,6 +498,9 @@ def test_phase_unphaseHERA():
     UV_phase_small.unphase_to_drift()
     assert UV_raw_small == UV_phase_small
 
+
+def test_phase_unphaseHERA_antpos(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
     # check that they match if you phase & unphase using antenna locations
     # first replace the uvws with the right values
     antenna_enu = uvutils.ENU_from_ECEF((UV_raw.antenna_positions + UV_raw.telescope_location),
@@ -509,6 +532,9 @@ def test_phase_unphaseHERA():
     UV_phase.unphase_to_drift(use_ant_pos=True)
     assert UV_raw_new == UV_phase
 
+
+def test_phase_unphaseHERA_zenith_timestamp(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
     # check that phasing to zenith with one timestamp has small changes
     # (it won't be identical because of precession/nutation changing the coordinate axes)
     # use gcrs rather than icrs to reduce differences (don't include abberation)
@@ -520,34 +546,90 @@ def test_phase_unphaseHERA():
     # it's unclear to me how close this should be...
     assert np.allclose(UV_phase_simple_small.uvw_array, UV_raw_small.uvw_array, atol=1e-1)
 
+
+def test_phase_to_time_jd_input(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
+    UV_phase.phase_to_time(UV_raw.time_array[0])
+    UV_phase.unphase_to_drift()
+    assert UV_phase == UV_raw
+
+
+def test_phase_to_time_error(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
     # check error if not passing a Time object to phase_to_time
     with pytest.raises(TypeError) as cm:
-        UV_raw.phase_to_time(UV_raw.time_array[0])
-    assert str(cm.value).startswith('time must be an astropy.time.Time object')
+        UV_phase.phase_to_time('foo')
+    assert str(cm.value).startswith("time must be an astropy.time.Time object")
+
+
+def test_unphase_drift_data_error(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
+    # check error if not passing a Time object to phase_to_time
+    with pytest.raises(ValueError) as cm:
+        UV_phase.unphase_to_drift()
+    assert str(cm.value).startswith("The data is already drift scanning;")
+
+
+@pytest.mark.parametrize(
+    "phase_func,phase_kwargs,err_msg",
+    [("unphase_to_drift", {},
+      "The phasing type of the data is unknown. Set the phase_type"),
+     ("phase", {"ra": 0, "dec": 0, "epoch": "J2000"},
+      "The phasing type of the data is unknown. Set the phase_type"),
+     ("phase_to_time", {"time": 0},
+      "The phasing type of the data is unknown. Set the phase_type")
+     ]
+)
+def test_unknown_phase_unphaseHERA_errors(
+    uv_phase_and_raw, phase_func, phase_kwargs, err_msg
+):
+    UV_phase, UV_raw = uv_phase_and_raw
 
-    # check errors when trying to unphase drift or unknown data
-    pytest.raises(ValueError, UV_raw.unphase_to_drift)
+    # Set phase type to unkown on some tests, ignore on others.
     UV_raw.set_unknown_phase_type()
-    pytest.raises(ValueError, UV_raw.unphase_to_drift)
-    UV_raw.set_drift()
+    # if this is phase_to_time, use this index set in the dictionary and
+    # assign the value of the time_array associated with that index
+    # this is a little hacky, but we cannot acces UV_raw.time_array in the parametrize
+    if phase_func == "phsae_to_time":
+        phase_kwargs["time"] = UV_raw.time_array[phase_kwargs["time"]]
 
-    # check errors when trying to phase phased or unknown data
-    UV_phase.phase(0., 0., epoch="J2000")
-    pytest.raises(ValueError, UV_phase.phase, 0., 0., epoch="J2000")
-    pytest.raises(ValueError, UV_phase.phase_to_time,
-                  UV_phase.time_array[0])
+    with pytest.raises(ValueError) as cm:
+        getattr(UV_raw, phase_func)(**phase_kwargs)
+    assert str(cm.value).startswith(err_msg)
+
+
+@pytest.mark.parametrize(
+    "phase_func,phase_kwargs,err_msg",
+    [("phase", {"ra": 0, "dec": 0, "epoch": "J2000"},
+      "The data is already phased;"),
+     ("phase_to_time", {"time": 0},
+      "The data is already phased;")
+     ]
+)
+def test_phase_rephaseHERA_errors(
+    uv_phase_and_raw, phase_func, phase_kwargs, err_msg
+):
+    UV_phase, UV_raw = uv_phase_and_raw
 
-    UV_phase.set_unknown_phase_type()
-    pytest.raises(ValueError, UV_phase.phase, 0., 0., epoch="J2000")
-    pytest.raises(ValueError, UV_phase.phase_to_time,
-                  UV_phase.time_array[0])
+    # Set phase type to unkown on some tests, ignore on others.
+    UV_raw.phase(0., 0., epoch="J2000")
+    # if this is phase_to_time, use this index set in the dictionary and
+    # assign the value of the time_array associated with that index
+    # this is a little hacky, but we cannot acces UV_raw.time_array in the parametrize
+    if phase_func == "phsae_to_time":
+        phase_kwargs["time"] = UV_raw.time_array[phase_kwargs["time"]]
+
+    with pytest.raises(ValueError) as cm:
+        getattr(UV_raw, phase_func)(**phase_kwargs)
+    assert str(cm.value).startswith(err_msg)
 
-    # check errors when trying to phase to an unsupported frame
-    UV_phase = copy.deepcopy(UV_raw)
-    pytest.raises(ValueError, UV_phase.phase, 0., 0., epoch="J2000", phase_frame='cirs')
 
-    del(UV_phase)
-    del(UV_raw)
+def test_phase_unphaseHERA_bad_frame(uv_phase_and_raw):
+    UV_phase, UV_raw = uv_phase_and_raw
+    # check errors when trying to phase to an unsupported frame
+    with pytest.raises(ValueError) as cm:
+        UV_phase.phase(0., 0., epoch="J2000", phase_frame='cirs')
+    assert str(cm.value).startswith("phase_frame can only be set to icrs or gcrs.")
 
 
 def test_phasing():