Merge pull request #2161 from mkelley/jplhorizons-fix-id_type-behavior

Revise id_type behavior in jplhorizons.

Author: bsipocz

CHANGES.rst

@@ -34,6 +34,14 @@ jplhorizons
   Included in this update, the default reference system is changed from
   J2000 to ICRF, following API documentation. [#2154]
 
+- Query ``id_type`` behavior has changed:
+  - ``'majorbody'`` and ``'id'`` have been removed and the equivalent functionality
+    replaced with ``None``.  ``None`` implements the Horizons default, which is
+    to search for major bodies first, then fall back to a small body search when
+    no matches are found.  Horizons does not have a major body only search.
+  - The default value was ``'smallbody'`` but it is now ``None``, which
+    follows Horizons's default behavior.
+
 Infrastructure, Utility and Other Changes and Additions
 -------------------------------------------------------
 

astroquery/jplhorizons/core.py

@@ -12,6 +12,7 @@
 from astropy.table import Table, Column
 from astropy.io import ascii
 from astropy.time import Time
+from astropy.utils.exceptions import AstropyDeprecationWarning
 
 # 3. local imports - use relative imports
 # commonly required local imports shown below as example
@@ -29,13 +30,13 @@
 class HorizonsClass(BaseQuery):
     """
     A class for querying the
-    `JPL Horizons <https://ssd.jpl.nasa.gov/horizons.cgi>`_ service.
+    `JPL Horizons <https://ssd.jpl.nasa.gov/horizons/>`_ service.
     """
 
     TIMEOUT = conf.timeout
 
     def __init__(self, id=None, location=None, epochs=None,
-                 id_type='smallbody'):
+                 id_type=None):
         """Instantiate JPL query.
 
         Parameters
@@ -47,7 +48,7 @@ def __init__(self, id=None, location=None, epochs=None,
             orbital element or vector queries. Uses the same codes as JPL
             Horizons. If no location is provided, Earth's center is used for
             ephemerides queries and the Sun's center for elements and vectors
-            queries. Arbitrary topocentic coordinates for ephemerides queries
+            queries. Arbitrary topocentric coordinates for ephemerides queries
             can be provided in the format of a dictionary. The dictionary has to
             be of the form {``'lon'``: longitude in deg (East positive, West
             negative), ``'lat'``: latitude in deg (North positive, South
@@ -64,11 +65,18 @@ def __init__(self, id=None, location=None, epochs=None,
             element and vector queries. If no epochs are provided, the current
             time is used.
         id_type : str, optional
-            Identifier type, options: ``'smallbody'``, ``'majorbody'`` (planets
-            but also anything that is not a small body), ``'designation'``,
-            ``'name'``, ``'asteroid_name'``, ``'comet_name'``, ``'id'``
-            (Horizons id number), or ``'smallbody'`` (find the closest match
-            under any id_type), default: ``'smallbody'``
+            Controls Horizons's object selection for ``id``
+            [HORIZONSDOC_SELECTION]_ .  Options: ``'designation'`` (small body
+            designation), ``'name'`` (asteroid or comet name),
+            ``'asteroid_name'``, ``'comet_name'``, ``'smallbody'`` (asteroid
+            and comet search), or ``None`` (first search search planets,
+            natural satellites, spacecraft, and special cases, and if no
+            matches, then search small bodies).
+
+        References
+        ----------
+
+        .. [HORIZONSDOC_SELECTION] https://ssd.jpl.nasa.gov/horizons/manual.html#select (retrieved 2021 Sep 23).
 
 
         Examples
@@ -79,7 +87,7 @@ def __init__(self, id=None, location=None, epochs=None,
         ...                      'stop':'2017-02-01',
         ...                      'step':'1d'})
         >>> print(eros)  # doctest: +SKIP
-        JPLHorizons instance "433"; location=568, epochs={'start': '2017-01-01', 'step': '1d', 'stop': '2017-02-01'}, id_type=smallbody
+        JPLHorizons instance "433"; location=568, epochs={'start': '2017-01-01', 'step': '1d', 'stop': '2017-02-01'}, id_type=None
         """
         super(HorizonsClass, self).__init__()
         self.id = id
@@ -101,9 +109,13 @@ def __init__(self, id=None, location=None, epochs=None,
         self.epochs = epochs
 
         # check for id_type
-        if id_type not in ['smallbody', 'majorbody',
-                           'designation', 'name',
-                           'asteroid_name', 'comet_name', 'id']:
+        if id_type in ['majorbody', 'id']:
+            warnings.warn("``id_type``s 'majorbody' and 'id' are deprecated "
+                          "and replaced with ``None``, which has the same "
+                          "functionality.", AstropyDeprecationWarning)
+            id_type = None
+        if id_type not in [None, 'smallbody', 'designation', 'name',
+                           'asteroid_name', 'comet_name']:
             raise ValueError('id_type ({:s}) not allowed'.format(id_type))
         self.id_type = id_type
 
@@ -128,7 +140,7 @@ def __str__(self):
         ...                         'stop':'2017-02-01',
         ...                         'step':'1d'})
         >>> print(eros)  # doctest: +SKIP
-        JPLHorizons instance "433"; location=568, epochs={'start': '2017-01-01', 'step': '1d', 'stop': '2017-02-01'}, id_type=smallbody
+        JPLHorizons instance "433"; location=568, epochs={'start': '2017-01-01', 'step': '1d', 'stop': '2017-02-01'}, id_type=None
         """
         return ('JPLHorizons instance \"{:s}\"; location={:s}, '
                 'epochs={:s}, id_type={:s}').format(

astroquery/jplhorizons/tests/test_jplhorizons.py

@@ -6,16 +6,17 @@
 from collections import OrderedDict
 
 from numpy.ma import is_masked
-from ...utils.testing_tools import MockResponse
 from astropy.tests.helper import assert_quantity_allclose
+from astropy.utils.exceptions import AstropyDeprecationWarning
 
+from ...utils.testing_tools import MockResponse
 from ... import jplhorizons
 
 # files in data/ for different query types
 DATA_FILES = {'ephemerides': 'ceres_ephemerides.txt',
               'elements': 'ceres_elements.txt',
               'vectors': 'ceres_vectors.txt',
-              '"1935 UZ;"': 'no_H.txt'}
+              '"1935 UZ"': 'no_H.txt'}
 
 
 def data_path(filename):
@@ -26,7 +27,7 @@ def data_path(filename):
 # monkeypatch replacement request function
 def nonremote_request(self, request_type, url, **kwargs):
 
-    if kwargs['params']['COMMAND'] == '"Ceres;"':
+    if kwargs['params']['COMMAND'] == '"Ceres"':
         # pick DATA_FILE based on query type
         query_type = {'OBSERVER': 'ephemerides',
                       'ELEMENTS': 'elements',
@@ -181,7 +182,7 @@ def test_elements_query_payload():
         ('EPHEM_TYPE', 'ELEMENTS'),
         ('MAKE_EPHEM', 'YES'),
         ('OUT_UNITS', 'AU-D'),
-        ('COMMAND', '"Ceres;"'),
+        ('COMMAND', '"Ceres"'),
         ('CENTER', "'500@10'"),
         ('CSV_FORMAT', 'YES'),
         ('ELEM_LABELS', 'YES'),
@@ -201,7 +202,7 @@ def test_vectors_query_payload():
         ('format', 'text'),
         ('EPHEM_TYPE', 'VECTORS'),
         ('OUT_UNITS', 'AU-D'),
-        ('COMMAND', '"Ceres;"'),
+        ('COMMAND', '"Ceres"'),
         ('CENTER', "'500@10'"),
         ('CSV_FORMAT', '"YES"'),
         ('REF_PLANE', 'ECLIPTIC'),
@@ -218,3 +219,17 @@ def test_no_H(patch_request):
     """testing missing H value (also applies for G, M1, k1, M2, k2)"""
     res = jplhorizons.Horizons(id='1935 UZ').ephemerides()[0]
     assert 'H' not in res
+
+
+def test_id_type_deprecation():
+    """Test deprecation warnings based on issue 1742.
+
+    https://github.com/astropy/astroquery/pull/2161
+
+    """
+
+    with pytest.warns(AstropyDeprecationWarning):
+        res = jplhorizons.Horizons(id='Ceres', id_type='id')
+
+    with pytest.warns(AstropyDeprecationWarning):
+        res = jplhorizons.Horizons(id='Ceres', id_type='majorbody')

astroquery/jplhorizons/tests/test_jplhorizons_remote.py

@@ -15,6 +15,7 @@ def test_ephemerides_query(self):
         # check values of Ceres for a given epoch
         # orbital uncertainty of Ceres is basically zero
         res = jplhorizons.Horizons(id='Ceres', location='500',
+                                   id_type='smallbody',
                                    epochs=2451544.5).ephemerides()[0]
 
         assert res['targetname'] == "1 Ceres (A801 AA)"
@@ -152,7 +153,7 @@ def test_ephemerides_query_five(self):
 
     def test_ephemerides_query_six(self):
         # tests optional constrains for ephemerides queries
-        obj = jplhorizons.Horizons(id='3552',
+        obj = jplhorizons.Horizons(id='3552', id_type='smallbody',
                                    location='I33',
                                    epochs={'start': '2018-05-01',
                                            'stop': '2018-08-01',
@@ -169,13 +170,14 @@ def test_ephemerides_query_six(self):
 
     def test_ephemerides_query_raw(self):
         res = (jplhorizons.Horizons(id='Ceres', location='500',
-                                    epochs=2451544.5).
+                                    id_type='smallbody', epochs=2451544.5).
                ephemerides(get_raw_response=True))
 
         assert len(res) >= 15400
 
     def test_elements_query(self):
         res = jplhorizons.Horizons(id='Ceres', location='500@10',
+                                   id_type='smallbody',
                                    epochs=[2451544.5,
                                            2451545.5]).elements()[0]
 
@@ -204,6 +206,7 @@ def test_elements_query(self):
 
     def test_elements_query_two(self):
         obj = jplhorizons.Horizons(id='Ceres', location='500@10',
+                                   id_type='smallbody',
                                    epochs=[2451544.5,
                                            2451545.5])
 
@@ -219,6 +222,7 @@ def test_elements_query_two(self):
 
     def test_elements_query_raw(self):
         res = jplhorizons.Horizons(id='Ceres', location='500@10',
+                                   id_type='smallbody',
                                    epochs=2451544.5).elements(
                                        get_raw_response=True)
 
@@ -228,6 +232,7 @@ def test_vectors_query(self):
         # check values of Ceres for a given epoch
         # orbital uncertainty of Ceres is basically zero
         res = jplhorizons.Horizons(id='Ceres', location='500@10',
+                                   id_type='smallbody',
                                    epochs=2451544.5).vectors()[0]
 
         assert res['targetname'] == "1 Ceres (A801 AA)"
@@ -251,28 +256,25 @@ def test_vectors_query(self):
 
     def test_vectors_query_raw(self):
         res = jplhorizons.Horizons(id='Ceres', location='500@10',
+                                   id_type='smallbody',
                                    epochs=2451544.5).vectors(
                                        get_raw_response=True)
 
         assert len(res) >= 6412
 
     def test_unknownobject(self):
-        try:
+        with pytest.raises(ValueError):
             jplhorizons.Horizons(id='spamspamspameggsspam', location='500',
                                  epochs=2451544.5).ephemerides()
-        except ValueError:
-            pass
 
     def test_multipleobjects(self):
-        try:
-            jplhorizons.Horizons(id='73P', location='500',
+        with pytest.raises(ValueError):
+            jplhorizons.Horizons(id='73P', location='500', id_type='smallbody',
                                  epochs=2451544.5).ephemerides()
-        except ValueError:
-            pass
 
     def test_uri(self):
         target = jplhorizons.Horizons(id='3552', location='500',
-                                      epochs=2451544.5)
+                                      id_type='smallbody', epochs=2451544.5)
         assert target.uri is None
 
         target.ephemerides()
@@ -299,10 +301,12 @@ def test__userdefinedlocation_ephemerides_query(self):
 
         am_res = jplhorizons.Horizons(id='Ceres',
                                       location='688',
+                                      id_type='smallbody',
                                       epochs=2451544.5).ephemerides()[0]
 
         user_res = jplhorizons.Horizons(id='Ceres',
                                         location=anderson_mesa,
+                                        id_type='smallbody',
                                         epochs=2451544.5).ephemerides()[0]
 
         assert_quantity_allclose([am_res['RA'], am_res['DEC']],
@@ -342,6 +346,7 @@ def test_airmass(self):
 
         # verify data['a-mass'].filled(99) works:
         target = jplhorizons.Horizons('Ceres', location='I41',
+                                      id_type='smallbody',
                                       epochs=[2458300.5])
         eph = target.ephemerides(quantities='1,8')
         assert len(eph) == 1
@@ -352,7 +357,8 @@ def test_airmass(self):
 
     def test_vectors_aberrations(self):
         """Check functionality of `aberrations` options"""
-        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='500@0')
+        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='500@0',
+                                   id_type='smallbody')
 
         vec = obj.vectors(aberrations='geometric')
         assert_quantity_allclose(vec['x'][0], -2.086487005013347)
@@ -364,7 +370,8 @@ def test_vectors_aberrations(self):
         assert_quantity_allclose(vec['x'][0], -2.086576286974797)
 
     def test_vectors_delta_T(self):
-        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='500@0')
+        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='500@0',
+                                   id_type='smallbody')
 
         vec = obj.vectors(delta_T=False)
         assert 'delta_T' not in vec.columns
@@ -373,7 +380,8 @@ def test_vectors_delta_T(self):
         assert_quantity_allclose(vec['delta_T'][0], 69.184373)
 
     def test_ephemerides_extraprecision(self):
-        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='G37')
+        obj = jplhorizons.Horizons(id='1', epochs=2458500, location='G37',
+                                   id_type='smallbody')
 
         vec_simple = obj.ephemerides(extra_precision=False)
         vec_highprec = obj.ephemerides(extra_precision=True)