Better separate id and location coordinate formatting and test.

Author: mkelley

astroquery/jplhorizons/core.py

@@ -615,7 +615,7 @@ def ephemerides_async(self, *, airmass_lessthan=99,
         if self.id is None:
             raise ValueError("'id' parameter not set. Query aborted.")
         elif isinstance(self.id, dict):
-            commandline = "g:" + self._format_site_coords(self.id)
+            commandline = self._format_id_coords(self.id)
         else:
             commandline = str(self.id)
         if self.location is None:
@@ -850,16 +850,20 @@ def elements_async(self, *, get_query_payload=False,
 
         URL = conf.horizons_server
 
-        # check for required information
+        # check for required information and assemble commandline stub
         if self.id is None:
             raise ValueError("'id' parameter not set. Query aborted.")
+        elif isinstance(self.id, dict):
+            commandline = self._format_id_coords(self.id)
+        else:
+            commandline = str(self.id)
+
         if self.location is None:
             self.location = '500@10'
         if self.epochs is None:
             self.epochs = Time.now().jd
 
-        # assemble commandline based on self.id_type
-        commandline = str(self.id)
+        # expand commandline based on self.id_type
         if self.id_type in ['designation', 'name',
                             'asteroid_name', 'comet_name']:
             commandline = ({'designation': 'DES=',
@@ -878,7 +882,7 @@ def elements_async(self, *, get_query_payload=False,
                 commandline += ' NOFRAG;'
 
         if isinstance(self.location, dict):
-            raise ValueError(('cannot use topographic position in orbital'
+            raise ValueError(('cannot use topographic position in orbital '
                               'elements query'))
 
         # configure request_payload for ephemerides query
@@ -1099,7 +1103,7 @@ def vectors_async(self, *, get_query_payload=False,
         if self.id is None:
             raise ValueError("'id' parameter not set. Query aborted.")
         elif isinstance(self.id, dict):
-            commandline = "g:" + self._format_site_coords(self.id)
+            commandline = self._format_id_coords(self.id)
         else:
             commandline = str(self.id)
         if self.location is None:
@@ -1211,6 +1215,7 @@ def _prep_loc_dict(loc_dict, attr_name):
     @staticmethod
     def _location_to_params(loc_dict):
         """translate a 'location' dict to request parameters"""
+
         location = {
             "CENTER": f"coord@{loc_dict['body']}",
             "COORD_TYPE": "GEODETIC",
@@ -1219,10 +1224,20 @@ def _location_to_params(loc_dict):
         return location
 
     @staticmethod
-    def _format_site_coords(coords):
-        # formats lon/lat/elevation/body (e.g., id and location dictionaries) for the Horizons API
+    def _format_coords(coords):
+        """Dictionary to Horizons API formatted lon/lat/elevation coordinate triplet."""
         return (f"{coords['lon'].to_value('deg')},{coords['lat'].to_value('deg')},"
-                f"{coords['elevation'].to_value('km')}@{coords['body']}")
+                f"{coords['elevation'].to_value('km')}")
+
+    @staticmethod
+    def _format_site_coords(coords):
+        """`location` dictionary to SITE_COORDS parameter."""
+        return HorizonsClass._format_coords(coords)
+
+    @staticmethod
+    def _format_id_coords(coords):
+        """`id` dictionary to COMMAND parameter's coordinate format."""
+        return f"g:{HorizonsClass._format_coords(coords)}@{coords['body']}"
 
     def _parse_result(self, response, verbose=None):
         """

astroquery/jplhorizons/tests/test_jplhorizons.py

@@ -7,6 +7,7 @@
 from numpy.ma import is_masked
 from astropy.tests.helper import assert_quantity_allclose
 from astropy.utils.exceptions import AstropyDeprecationWarning
+from astropy import units as u
 
 from astroquery.utils.mocks import MockResponse
 from ...query import AstroQuery
@@ -291,3 +292,61 @@ def test_id_type_deprecation():
 
     with pytest.warns(AstropyDeprecationWarning):
         jplhorizons.Horizons(id='Ceres', id_type='majorbody')
+
+
+def test_id_geodetic_coords():
+    """Test target based on geodetic coordinates.
+
+    From the Horizons manual:
+
+    For example, while 301 specifies the target to be the center of the Moon,
+    and Apollo-11 @ 301 specifies the Apollo-11 landing site as target, the
+    following:
+
+        g: 348.8, -43.3, 0 @ 301
+
+    specifies an ephemeris for the crater Tycho on the Moon (body 301), at
+    geodetic (planetodetic) coordinates 348.8 degrees east longitude, -43.3
+    degrees latitude (south), and zero km altitude with respect to the Moon’s
+    mean-Earth reference frame and ellipsoid surface.
+
+    """
+
+    target = {
+        "lon": 348.8 * u.deg,
+        "lat": -43.3 * u.deg,
+        "elevation": 0 * u.m,
+        "body": 301
+    }
+
+    q = jplhorizons.Horizons(id=target)
+    for payload in (q.ephemerides(get_query_payload=True),
+                    q.vectors(get_query_payload=True),
+                    q.elements(get_query_payload=True)):
+        assert payload["COMMAND"] == '"g:348.8,-43.3,0.0@301"'
+
+
+def test_location_topocentric_coords():
+    """Test location from topocentric coordinates.
+
+    Similar to `test_id_geodetic_coords`.
+
+    """
+
+    location = {
+        "lon": 348.8 * u.deg,
+        "lat": -43.3 * u.deg,
+        "elevation": 0 * u.m,
+        "body": 301
+    }
+
+    q = jplhorizons.Horizons(id=399, location=location)
+    for payload in (q.ephemerides(get_query_payload=True),
+                    q.vectors(get_query_payload=True)):
+        assert payload["CENTER"] == 'coord@301'
+        assert payload["COORD_TYPE"] == "GEODETIC"
+        assert payload["SITE_COORD"] == "'348.8,-43.3,0.0'"
+
+    # not allowed for elements
+    with pytest.raises(ValueError):
+        q.elements(get_query_payload=True)