made all changes suggested by @eerovaher

Author: emolter

astroquery/solarsystem/pds/__init__.py

@@ -15,7 +15,7 @@ class Conf(_config.ConfigNamespace):
 
     # server settings
     pds_server = _config.ConfigItem(
-        ["https://pds-rings.seti.org/cgi-bin/tools/viewer3_xxx.pl?"], "Ring Node"
+        "https://pds-rings.seti.org/cgi-bin/tools/viewer3_xxx.pl?", "Ring Node"
     )
 
     # implement later: other pds tools

astroquery/solarsystem/pds/core.py

@@ -1,6 +1,5 @@
 # 1. standard library imports
 
-from collections import OrderedDict
 import re
 import warnings
 
@@ -11,7 +10,6 @@
 import astropy.units as u
 from astropy.coordinates import EarthLocation, Angle
 from bs4 import BeautifulSoup
-from astroquery import log
 
 # 3. local imports - use relative imports
 # all Query classes should inherit from BaseQuery.
@@ -29,27 +27,17 @@ class RingNodeClass(BaseQuery):
     <https://pds-rings.seti.org/tools/>
     """
 
-    TIMEOUT = conf.timeout
-
-    def __init__(self):
+    def __init__(self, timeout=None):
         '''
         Instantiate Planetary Ring Node query
         '''
         super().__init__()
-        self.URL = conf.pds_server
+        self.url = conf.pds_server
+        self.timeout = conf.timeout
         self.planet_defaults = conf.planet_defaults
 
     def __str__(self):
-        """
-        String representation of `~RingNodeClass` object instance
 
-        Examples
-        --------
-        >>> from astroquery.solarsystem.pds import RingNode
-        >>> nodeobj = RingNode
-        >>> print(nodeobj)  # doctest: +SKIP
-        PDSRingNode instance
-        """
         return "PDSRingNode instance"
 
     def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel=3,
@@ -89,7 +77,7 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel
         --------
         >>> from astroquery.solarsystem.pds import RingNode
         >>> import astropy.units as u
-        >>> systemtable, bodytable, ringtable = RingNode.ephemeris(planet='Uranus',
+        >>> bodytable, ringtable = RingNode.ephemeris(planet='Uranus',
         ...                 epoch='2024-05-08 22:39',
         ...                 location = (-23.029 * u.deg, -67.755 * u.deg, 5000 * u.m))  # doctest: +REMOTE_DATA
         >>> print(ringtable)  # doctest: +REMOTE_DATA
@@ -120,12 +108,10 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel
             epoch = Time(epoch, format='iso')
         elif epoch is None:
             epoch = Time.now()
-            log.warning("Observation time not set. Using current time.")
 
         if location is None:
             viewpoint = "observatory"
             latitude, longitude, altitude = "", "", ""
-            log.warning("Observatory coordinates not set. Using center of Earth.")
         else:
             viewpoint = "latlon"
             if isinstance(location, EarthLocation):
@@ -134,8 +120,8 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel
                 latitude = loc[1].deg
                 altitude = loc[2].to(u.m).value
             elif hasattr(location, "__iter__"):
-                latitude = Angle(location[0]).deg
-                longitude = Angle(location[1]).deg
+                longitude = Angle(location[0]).deg
+                latitude = Angle(location[1]).deg
                 altitude = u.Quantity(location[2]).to("m").value
 
         if int(neptune_arcmodel) not in [1, 2, 3]:
@@ -145,11 +131,11 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel
 
         # configure request_payload for ephemeris query
         # thankfully, adding extra planet-specific keywords here does not break query for other planets
-        request_payload = OrderedDict(
+        request_payload = dict(
             [
                 ("abbrev", planet[:3]),
                 ("ephem", self.planet_defaults[planet]["ephem"]),
-                ("time", epoch.utc.to_value("iso", subfmt="date_hm")),  # UTC. this should be enforced when checking inputs
+                ("time", epoch.utc.iso[:16]),
                 ("fov", 10),  # next few are figure options, can be hardcoded and ignored
                 ("fov_unit", planet.capitalize() + " radii"),
                 ("center", "body"),
@@ -192,7 +178,7 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel
 
         # query and parse
         response = self._request(
-            "GET", self.URL, params=request_payload, timeout=self.TIMEOUT, cache=cache
+            "GET", self.url, params=request_payload, timeout=self.timeout, cache=cache
         )
 
         return response
@@ -210,21 +196,19 @@ def _parse_result(self, response, verbose=None):
 
         Returns
         -------
-        systemtable : dict
-        bodytable : `astropy.Table`
-        ringtable : `astropy.Table`
+        bodytable : `astropy.QTable`
+        ringtable : `astropy.QTable`
         """
         self.last_response = response
         try:
             self._last_query.remove_cache_file(self.cache_location)
-        except OSError:
+        except FileNotFoundError:
             # this is allowed: if `cache` was set to False, this
             # won't be needed
             pass
 
         soup = BeautifulSoup(response.text, "html.parser")
         text = soup.get_text()
-        # print(repr(text))
         textgroups = re.split("\n\n|\n \n", text)
         ringtable = None
         for group in textgroups:
@@ -354,15 +338,17 @@ def _parse_result(self, response, verbose=None):
                 pass
 
         # do some cleanup from the parsing job
+        # and make system-wide parameters metadata of bodytable and ringtable
+        systemtable["epoch"] = Time(epoch, format="iso", scale="utc")  # add obs time to systemtable
         if ringtable is not None:
             ringtable.add_index("ring")
+            ringtable.meta = systemtable
 
         bodytable = table.join(bodytable, bodytable2)  # concatenate minor body table
         bodytable.add_index("Body")
+        bodytable.meta = systemtable
 
-        systemtable["epoch"] = Time(epoch, format="iso", scale="utc")  # add obs time to systemtable
-
-        return systemtable, bodytable, ringtable
+        return bodytable, ringtable
 
 
 RingNode = RingNodeClass()

astroquery/solarsystem/pds/tests/test_ringnode.py

@@ -2,7 +2,6 @@
 import os
 import numpy as np
 
-from astropy.tests.helper import assert_quantity_allclose
 import astropy.units as u
 
 from astroquery.utils.mocks import MockResponse
@@ -45,13 +44,14 @@ def patch_request(request):
 
 def test_ephemeris_query_Uranus(patch_request):
 
-    systemtable, bodytable, ringtable = pds.RingNode.ephemeris(
+    bodytable, ringtable = pds.RingNode.ephemeris(
         planet="Uranus",
         epoch="2022-05-03 00:00",
-        location=(10.0 * u.deg, -120.355 * u.deg, 1000 * u.m),
+        location=(-120.355 * u.deg, 10.0 * u.deg, 1000 * u.m),
     )
     # check system table
-    assert_quantity_allclose(
+    systemtable = bodytable.meta
+    assert np.allclose(
         [
             -56.12233,
             -56.13586,
@@ -83,7 +83,7 @@ def test_ephemeris_query_Uranus(patch_request):
     mab = bodytable[bodytable.loc_indices["Mab"]]
     assert mab["NAIF ID"] == 726
     assert mab["Body"] == "Mab"
-    assert_quantity_allclose(
+    assert np.allclose(
         [
             42.011201,
             15.801323,
@@ -119,14 +119,13 @@ def test_ephemeris_query_Uranus(patch_request):
 
 def test_ephemeris_query_Pluto(patch_request):
 
-    systemtable, bodytable, ringtable = pds.RingNode.ephemeris(
+    bodytable, ringtable = pds.RingNode.ephemeris(
         planet="Pluto",
         epoch="2021-10-07 07:25",
     )
-    print(systemtable)
-    print(bodytable[bodytable.loc_indices["Styx"]])
+    systemtable = bodytable.meta
     # check system table
-    assert_quantity_allclose(
+    assert np.allclose(
         [
             57.57737,
              57.56961,
@@ -158,7 +157,7 @@ def test_ephemeris_query_Pluto(patch_request):
     styx = bodytable[bodytable.loc_indices["Styx"]]
     assert styx["NAIF ID"] == 905
     assert styx["Body"] == "Styx"
-    assert_quantity_allclose(
+    assert np.allclose(
         [
             296.212477,
             -22.93533,
@@ -193,15 +192,13 @@ def test_ephemeris_query_Pluto(patch_request):
 def test_ephemeris_query_Neptune(patch_request):
     '''Verify that the Neptune ring arcs are queried properly'''
 
-    systemtable, bodytable, ringtable = pds.RingNode.ephemeris(
+    bodytable, ringtable = pds.RingNode.ephemeris(
         planet="Neptune",
         epoch="2021-10-07 07:25",
         neptune_arcmodel=2
     )
 
-    print(ringtable[ringtable.loc_indices["Courage"]])
-
-    assert_quantity_allclose(
+    assert np.allclose(
                     [63.81977,
                      55.01978,
                      44.21976,
@@ -227,13 +224,12 @@ def test_ephemeris_query_Neptune(patch_request):
 
 def test_ephemeris_query_Saturn(patch_request):
     '''Check Saturn F ring is queried properly'''
-    systemtable, bodytable, ringtable = pds.RingNode.ephemeris(
+    bodytable, ringtable = pds.RingNode.ephemeris(
         planet="Saturn",
         epoch="2021-10-07 07:25",
     )
-    print(ringtable)
 
-    assert_quantity_allclose(
+    assert np.allclose(
                     [249.23097,
                      250.34081
                      ],
@@ -248,7 +244,7 @@ def test_ephemeris_query_payload():
         planet="Neptune",
         epoch="2022-05-03 00:00",
         neptune_arcmodel=1,
-        location=(10.0 * u.deg, -120.355 * u.deg, 1000 * u.m),
+        location=(-120.355 * u.deg, 10.0 * u.deg, 1000 * u.m),
         get_query_payload=True,
     )
 
@@ -301,3 +297,12 @@ def test_ephemeris_query_payload():
             ("output", "html"),
         ]
     )
+
+
+def test_bad_query_raise():
+
+    with pytest.raises(ValueError):
+        bodytable, ringtable = pds.RingNode.ephemeris(
+            planet="Venus",
+            epoch="2021-10-07 07:25",
+            )

astroquery/solarsystem/pds/tests/test_ringnode_remote.py

@@ -1,30 +1,22 @@
 import pytest
-from astropy.tests.helper import assert_quantity_allclose
 import numpy as np
 import astropy.units as u
 
 from ... import pds
 
 
-# Horizons has machinery here to mock request from a text file
-# is that necessary? why is that done?
-# wouldn't we want to know if the website we are querying changes something that makes code fail?
-
-
-# --------------------------------- actual test functions
-
-
 @pytest.mark.remote_data
 class TestRingNodeClass:
     def test_ephemeris_query(self):
 
-        systemtable, bodytable, ringtable = pds.RingNode.ephemeris(
+        bodytable, ringtable = pds.RingNode.ephemeris(
             planet="Uranus",
             epoch="2022-05-03 00:00",
-            location=(10.0 * u.deg, -120.355 * u.deg, 1000 * u.m),
+            location=(-120.355 * u.deg, 10.0 * u.deg, 1000 * u.m),
         )
         # check system table
-        assert_quantity_allclose(
+        systemtable = bodytable.meta
+        assert np.allclose(
             [
                 -56.12233,
                 -56.13586,
@@ -56,7 +48,7 @@ def test_ephemeris_query(self):
         mab = bodytable[bodytable.loc_indices["Mab"]]
         assert mab["NAIF ID"] == 726
         assert mab["Body"] == "Mab"
-        assert_quantity_allclose(
+        assert np.allclose(
             [
                 42.011201,
                 15.801323,

docs/solarsystem/pds/pds.rst

@@ -22,9 +22,9 @@ ephemerides of the rings and small moons around Uranus as viewed from ALMA:
 
    >>> from astroquery.solarsystem.pds import RingNode
    >>> import astropy.units as u
-   >>> systemtable, bodytable, ringtable = RingNode.ephemeris(planet='Uranus',
+   >>> bodytable, ringtable = RingNode.ephemeris(planet='Uranus',
    ...                 epoch='2024-05-08 22:39',
-   ...                 location = (-23.029 * u.deg, -67.755 * u.deg, 5000 * u.m))  # doctest: +REMOTE_DATA
+   ...                 location = (-67.755 * u.deg, -23.029 * u.deg, 5000 * u.m))  # doctest: +REMOTE_DATA
    >>> print(ringtable)
          ring  pericenter ascending node
                   deg          deg
@@ -42,21 +42,23 @@ ephemerides of the rings and small moons around Uranus as viewed from ALMA:
 
 ``planet`` must be one of ['mars', 'jupiter', 'uranus', 'saturn', 'neptune', 'pluto'] (case-insensitive)
 
+.. code-block:: python
+
+   >>> bodytable, ringtable = RingNode.ephemeris(planet='Venus',
+   ...                 epoch='2024-05-08 22:39',
+   ...                 location = (-67.755 * u.deg, -23.029 * u.deg, 5000 * u.m)) # doctest: +IGNORE_EXCEPTION_DETAIL
+   Traceback (most recent call last):
+   ...
+   ValueError: illegal value for 'planet' parameter (must be 'Mars', 'Jupiter', 'Saturn', 'Uranus', 'Neptune', or 'Pluto')
+
 ``epoch`` is the datetime to query. Accepts a string in format 'YYYY-MM-DD HH:MM' (UTC assumed), or a `~astropy.time.Time` object. If no epoch is provided, the current time is used.
 
-``location`` is the observer's location. Accepts an `~astropy.coordinates.EarthLocation`, or any 3-element array-like (e.g. list, tuple) of format (latitude, longitude, elevation). Longitude and latitude should be anything that initializes an `~astropy.coordinates.Angle` object, and altitude should initialize a `~astropy.units.Quantity` object (with units of length).  If ``None``, then the geocenter is used.
+``location`` is the observer's location. Accepts an `~astropy.coordinates.EarthLocation`, or any 3-element array-like (e.g. list, tuple) of format (longitude, latitude, elevation). Longitude and latitude should be anything that initializes an `~astropy.coordinates.Angle` object, and altitude should initialize a `~astropy.units.Quantity` object (with units of length).  If ``None``, then the geocenter is used.
 
 ``neptune_arcmodel`` is the choice of which ephemeris to assume for Neptune's ring arcs. accepts a float. must be one of 1, 2, or 3 (see https://pds-rings.seti.org/tools/viewer3_nep.shtml for details). default 3. has no effect if planet != 'Neptune'
 
 Outputs
 ---------
-``systemtable`` is a dict containing system-wide ephemeris data. Every value is a `~astropy.units.Quantity`. We can get a list of all the keys in this dictionary with:
-
-.. code-block:: python
-
-	>>> print(systemtable.keys())
-	dict_keys(['sub_sun_lat', 'sub_sun_lat_min', 'sub_sun_lat_max', 'opening_angle', 'phase_angle', 'sub_sun_lon', 'sub_obs_lon', 'd_sun', 'd_obs', 'light_time', 'epoch'])
-
 ``bodytable`` is a `~astropy.table.QTable` containing ephemeris information on the moons in the planetary system. Every column is assigned a unit from `~astropy.units`. We can get a list of all the columns in this table with:
 
 .. code-block:: python
@@ -75,7 +77,7 @@ Note that the behavior of ``ringtable`` changes depending on the planet you quer
 
 .. code-block:: python
 
-	>>> systemtable, bodytable, ringtable = RingNode.ephemeris(planet='Neptune', epoch='2022-05-24 00:00') # doctest: +REMOTE_DATA
+	>>> bodytable, ringtable = RingNode.ephemeris(planet='Neptune', epoch='2022-05-24 00:00') # doctest: +REMOTE_DATA
 	>>> print(ringtable)
     	   ring    min_angle max_angle
     	              deg       deg   
@@ -85,6 +87,14 @@ Note that the behavior of ``ringtable`` changes depending on the planet you quer
     	 Egalite A  33.88179  34.88179
     	 Egalite B   30.0818   33.0818
     	Fraternite   16.0818  25.68181
+		
+System-wide data are available as metadata in both ``bodytable`` and ``ringtable`` (if ``ringtable`` exists), e.g.:
+
+.. code-block:: python
+
+	>>> systemtable = bodytable.meta
+	>>> print(systemtable.keys())
+	dict_keys(['sub_sun_lat', 'sub_sun_lat_min', 'sub_sun_lat_max', 'opening_angle', 'phase_angle', 'sub_sun_lon', 'sub_obs_lon', 'd_sun', 'd_obs', 'light_time', 'epoch'])
 
 
 Acknowledgements