From 6448ab871ec071a4f111fe151daf57900f71b86b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brigitta=20Sipo=CC=8Bcz?= Date: Mon, 12 Dec 2022 02:10:01 -0800 Subject: [PATCH] Addressing final review items --- astroquery/solarsystem/pds/core.py | 165 ++++++++----------- astroquery/solarsystem/pds/tests/test_pds.py | 54 +++--- 2 files changed, 101 insertions(+), 118 deletions(-) diff --git a/astroquery/solarsystem/pds/core.py b/astroquery/solarsystem/pds/core.py index d27640bc6d..ecf2ac5309 100644 --- a/astroquery/solarsystem/pds/core.py +++ b/astroquery/solarsystem/pds/core.py @@ -4,7 +4,7 @@ # 2. third party imports from astropy.time import Time -from astropy import table +from astropy.table import QTable, join import astropy.units as u from astropy.coordinates import EarthLocation, Angle from bs4 import BeautifulSoup @@ -93,7 +93,8 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel Parameters ---------- - planet : str, required. one of Mars, Jupiter, Saturn, Uranus, Neptune, or Pluto + planet : str + One of "Mars", "Jupiter", "Saturn", "Uranus", "Neptune", or "Pluto". epoch : `~astropy.time.Time` object, or str in format YYYY-MM-DD hh:mm, optional. If str is provided then UTC is assumed. If no epoch is provided, the current time is used. @@ -107,8 +108,9 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel `~astropy.coordinates.Angle` object, and altitude should initialize an `~astropy.units.Quantity` object (with units of length). If ``None``, then the geofocus is used. - neptune_arcmodel : float, optional. which ephemeris to assume for Neptune's ring arcs - must be one of 1, 2, or 3 (see https://pds-rings.seti.org/tools/viewer3_nep.shtml for details) + neptune_arcmodel : int, optional. + which ephemeris to assume for Neptune's ring arcs + Must be one of 1, 2, or 3 (see https://pds-rings.seti.org/tools/viewer3_nep.shtml for details) has no effect if planet != 'Neptune' get_query_payload : boolean, optional When set to `True` the method returns the HTTP request parameters as @@ -147,10 +149,9 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel """ planet = planet.lower() - if planet not in ["mars", "jupiter", "saturn", "uranus", "neptune", "pluto", ]: - raise ValueError( - "illegal value for 'planet' parameter (must be 'Mars', 'Jupiter', 'Saturn', 'Uranus', 'Neptune', or 'Pluto')" - ) + if planet not in planet_defaults: + raise ValueError("illegal value for 'planet' parameter " + "(must be 'Mars', 'Jupiter', 'Saturn', 'Uranus', 'Neptune', or 'Pluto')") if isinstance(epoch, (int, float)): epoch = Time(epoch, format='jd') @@ -174,13 +175,13 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel loc = location.geodetic longitude = loc[0].deg latitude = loc[1].deg - altitude = loc[2].to(u.m).value + altitude = loc[2].to_value(u.m) elif hasattr(location, "__iter__"): longitude = Angle(location[0]).deg latitude = Angle(location[1]).deg - altitude = u.Quantity(location[2]).to("m").value + altitude = u.Quantity(location[2]).to_value(u.m) - if int(neptune_arcmodel) not in [1, 2, 3]: + if neptune_arcmodel not in [1, 2, 3]: raise ValueError( f"Illegal Neptune arc model {neptune_arcmodel}. must be one of 1, 2, or 3 (see https://pds-rings.seti.org/tools/viewer3_nep.shtml for details)" ) @@ -209,7 +210,7 @@ def ephemeris_async(self, planet, *, epoch=None, location=None, neptune_arcmodel ("altitude", altitude), ("moons", planet_defaults[planet]["moons"]), ("rings", planet_defaults[planet]["rings"]), - ("arcmodel", neptune_arcmodels[int(neptune_arcmodel)]), + ("arcmodel", neptune_arcmodels[neptune_arcmodel]), ("extra_ra", ""), # figure options below this line, can all be hardcoded and ignored ("extra_ra_type", "hours"), ("extra_dec", ""), @@ -251,15 +252,9 @@ def _parse_result(self, response, verbose=None): Returns ------- - bodytable : `astropy.QTable` - ringtable : `astropy.QTable` + bodytable : `~astropy.table.QTable` + ringtable : `~astropy.table.QTable` """ - try: - self._last_query.remove_cache_file(self.cache_location) - 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() @@ -278,13 +273,11 @@ def _parse_result(self, response, verbose=None): group = "NAIF " + group # fixing lack of header for NAIF ID bodytable_names = ("NAIF ID", "Body", "RA", "Dec", "RA (deg)", "Dec (deg)", "dRA", "dDec") bodytable_units = [None, None, None, None, u.deg, u.deg, u.arcsec, u.arcsec] - bodytable = table.QTable.read(group, format="ascii.fixed_width", + bodytable = QTable.read(group, format="ascii.fixed_width", col_starts=(0, 4, 18, 35, 54, 68, 80, 91), col_ends=(4, 18, 35, 54, 68, 80, 91, 102), - names=bodytable_names) - # units=(bodytable_units)) # this much cleaner way of adding units is supported in later versions but not in 3.7 - for name, unit in zip(bodytable_names, bodytable_units): - bodytable[name].unit = unit + names=bodytable_names, + units=(bodytable_units)) # minor body table part 2 elif group.startswith("Sub-"): @@ -292,23 +285,20 @@ def _parse_result(self, response, verbose=None): group = os.linesep.join(group.splitlines()[2:]) # removing two-row header entirely bodytable2_names = ("NAIF ID", "Body", "sub_obs_lon", "sub_obs_lat", "sub_sun_lon", "sub_sun_lat", "phase", "distance") bodytable2_units = [None, None, u.deg, u.deg, u.deg, u.deg, u.deg, u.km * 1e6] - bodytable2 = table.QTable.read(group, format="ascii.fixed_width", - col_starts=(0, 4, 18, 28, 37, 49, 57, 71), - col_ends=(4, 18, 28, 37, 49, 57, 71, 90), - names=bodytable2_names, - data_start=0) - # units=(bodytable_units)) # this much cleaner way of adding units is supported in later versions but not in 3.7 - for name, unit in zip(bodytable2_names, bodytable2_units): - bodytable2[name].unit = unit + bodytable2 = QTable.read(group, format="ascii.fixed_width", + col_starts=(0, 4, 18, 28, 37, 49, 57, 71), + col_ends=(4, 18, 28, 37, 49, 57, 71, 90), + names=bodytable2_names, + data_start=0, + units=(bodytable2_units)) # ring plane data elif group.startswith("Ring s"): - lines = group.splitlines() - for line in lines: - l = line.split(":") - if "Ring sub-solar latitude" in l[0]: - [sub_sun_lat, sub_sun_lat_min, sub_sun_lat_max] = [ - float(s.strip("()")) for s in re.split(r"\(|to", l[1]) + for line in group.splitlines(): + lines = line.split(":") + if "Ring sub-solar latitude" in lines[0]: + sub_sun_lat, sub_sun_lat_min, sub_sun_lat_max = [ + float(s.strip("()")) for s in re.split(r"\(|to", lines[1]) ] systemtable = { "sub_sun_lat": sub_sun_lat * u.deg, @@ -316,100 +306,83 @@ def _parse_result(self, response, verbose=None): "sub_sun_lat_max": sub_sun_lat_min * u.deg, } - elif "Ring plane opening angle" in l[0]: + elif "Ring plane opening angle" in lines[0]: systemtable["opening_angle"] = ( - float(re.sub("[a-zA-Z]+", "", l[1]).strip("()")) * u.deg + float(re.sub("[a-zA-Z]+", "", lines[1]).strip("()")) * u.deg ) - elif "Ring center phase angle" in l[0]: - systemtable["phase_angle"] = float(l[1].strip()) * u.deg - elif "Sub-solar longitude" in l[0]: + elif "Ring center phase angle" in lines[0]: + systemtable["phase_angle"] = float(lines[1].strip()) * u.deg + elif "Sub-solar longitude" in lines[0]: systemtable["sub_sun_lon"] = ( - float(re.sub("[a-zA-Z]+", "", l[1]).strip("()")) * u.deg + float(re.sub("[a-zA-Z]+", "", lines[1]).strip("()")) * u.deg ) - elif "Sub-observer longitude" in l[0]: - systemtable["sub_obs_lon"] = float(l[1].strip()) * u.deg - else: - pass + elif "Sub-observer longitude" in lines[0]: + systemtable["sub_obs_lon"] = float(lines[1].strip()) * u.deg # basic info about the planet elif group.startswith("Sun-planet"): - lines = group.splitlines() - for line in lines: - l = line.split(":") - if "Sun-planet distance (AU)" in l[0]: + for line in group.splitlines(): + lines = line.split(":") + if "Sun-planet distance (AU)" in lines[0]: # this is redundant with sun distance in km pass - elif "Observer-planet distance (AU)" in l[0]: + elif "Observer-planet distance (AU)" in lines[0]: # this is redundant with observer distance in km pass - elif "Sun-planet distance (km)" in l[0]: + elif "Sun-planet distance (km)" in lines[0]: systemtable["d_sun"] = ( - float(l[1].split("x")[0].strip()) * 1e6 * u.km + float(lines[1].split("x")[0].strip()) * 1e6 * u.km ) - elif "Observer-planet distance (km)" in l[0]: + elif "Observer-planet distance (km)" in lines[0]: systemtable["d_obs"] = ( - float(l[1].split("x")[0].strip()) * 1e6 * u.km + float(lines[1].split("x")[0].strip()) * 1e6 * u.km ) - elif "Light travel time" in l[0]: - systemtable["light_time"] = float(l[1].strip()) * u.second - else: - pass + elif "Light travel time" in lines[0]: + systemtable["light_time"] = float(lines[1].strip()) * u.second # --------- below this line, planet-specific info ------------ # Uranus individual rings data elif group.startswith("Ring "): ringtable_names = ("ring", "pericenter", "ascending node") ringtable_units = [None, u.deg, u.deg] - ringtable = table.QTable.read(" " + group, format="ascii.fixed_width", - col_starts=(5, 18, 29), - col_ends=(18, 29, 36), - names=ringtable_names) - # units=(ringtable_units)) # this much cleaner way of adding units is supported in later versions but not in 3.7 - for name, unit in zip(ringtable_names, ringtable_units): - ringtable[name].unit = unit + ringtable = QTable.read(" " + group, format="ascii.fixed_width", + col_starts=(5, 18, 29), + col_ends=(18, 29, 36), + names=ringtable_names, + units=(ringtable_units)) # Saturn F-ring data elif group.startswith("F Ring"): - lines = group.splitlines() - for line in lines: - l = line.split(":") - if "F Ring pericenter" in l[0]: - peri = float(re.sub("[a-zA-Z]+", "", l[1]).strip("()")) - elif "F Ring ascending node" in l[0]: - ascn = float(l[1].strip()) + for line in group.splitlines(): + lines = line.split(":") + if "F Ring pericenter" in lines[0]: + peri = float(re.sub("[a-zA-Z]+", "", lines[1]).strip("()")) + elif "F Ring ascending node" in lines[0]: + ascn = float(lines[1].strip()) ringtable_names = ("ring", "pericenter", "ascending node") ringtable_units = [None, u.deg, u.deg] - ringtable = table.QTable( - [["F"], [peri], [ascn]], - names=ringtable_names) - # units=(ringtable_units) # this much cleaner way of adding units is supported in later versions but not in 3.7 - for name, unit in zip(ringtable_names, ringtable_units): - ringtable[name].unit = unit + ringtable = QTable([["F"], [peri], [ascn]], + names=ringtable_names, + units=(ringtable_units)) # Neptune ring arcs data elif group.startswith("Courage"): - lines = group.splitlines() - for i in range(len(lines)): - l = lines[i].split(":") - ring = l[0].split("longitude")[0].strip() + for i, line in enumerate(group.splitlines()): + lines = line.split(":") + ring = lines[0].split("longitude")[0].strip() [min_angle, max_angle] = [ float(s.strip()) - for s in re.sub("[a-zA-Z]+", "", l[1]).strip("()").split() + for s in re.sub("[a-zA-Z]+", "", lines[1]).strip("()").split() ] if i == 0: ringtable_names = ("ring", "min_angle", "max_angle") ringtable_units = [None, u.deg, u.deg] - ringtable = table.QTable( - [[ring], [min_angle], [max_angle]], - names=ringtable_names) - for name, unit in zip(ringtable_names, ringtable_units): - ringtable[name].unit = unit - # units=(ringtable_units) # this much cleaner way of adding units is supported in later versions but not in 3.7 + ringtable = QTable([[ring], [min_angle], [max_angle]], + names=ringtable_names, + units=ringtable_units) else: ringtable.add_row([ring, min_angle*u.deg, max_angle*u.deg]) - else: - pass # do some cleanup from the parsing job # and make system-wide parameters metadata of bodytable and ringtable @@ -418,7 +391,7 @@ def _parse_result(self, response, verbose=None): ringtable.add_index("ring") ringtable.meta = systemtable - bodytable = table.join(bodytable, bodytable2) # concatenate minor body table + bodytable = join(bodytable, bodytable2) # concatenate minor body table bodytable.add_index("Body") bodytable.meta = systemtable diff --git a/astroquery/solarsystem/pds/tests/test_pds.py b/astroquery/solarsystem/pds/tests/test_pds.py index 49a3fe8246..471a6b10a6 100644 --- a/astroquery/solarsystem/pds/tests/test_pds.py +++ b/astroquery/solarsystem/pds/tests/test_pds.py @@ -3,6 +3,8 @@ import numpy as np import astropy.units as u +from astropy.table import QTable + from ....query import AstroQuery from astroquery.utils.mocks import MockResponse @@ -62,21 +64,27 @@ def test_ephemeris_query_Uranus(patch_request): ) # check system table systemtable = bodytable.meta + assert isinstance(systemtable, dict) + assert np.allclose( [-56.12233, -56.13586, -56.13586, -56.01577, 0.10924, 354.11072, 354.12204, 2947896667.0, 3098568884.0, 10335.713263, ], [systemtable["sub_sun_lat"].to(u.deg).value, systemtable["sub_sun_lat_min"].to(u.deg).value, systemtable["sub_sun_lat_max"].to(u.deg).value, systemtable["opening_angle"].to(u.deg).value, systemtable["phase_angle"].to(u.deg).value, systemtable["sub_sun_lon"].to(u.deg).value, systemtable["sub_obs_lon"].to(u.deg).value, systemtable["d_sun"].to(u.km).value, systemtable["d_obs"].to(u.km).value, systemtable["light_time"].to(u.second).value, ], rtol=1e-2, ) - # check a moon in body table - mab = bodytable[bodytable.loc_indices["Mab"]] + expected_mab = """ +NAIF ID,Body, RA,Dec,RA (deg),Dec (deg),dRA,dDec,sub_obs_lon,sub_obs_lat,sub_sun_lon,sub_sun_lat,phase,distance +726,Mab,2h 48m 02.6883s,15d 48m 04.764s,42.011201,15.801323,5.368,0.6233,223.976,55.906,223.969,56.013,0.10932,3098.514 + """ + expected_mab = QTable.read(expected_mab, format='ascii.csv', + units=[None] * 4 + ['deg'] * 2 + ['arcsec'] * 2 + ['deg'] * 5 + ['Gm']) + + # check the moon Mab in body table. Slicing to make sure we get a 1 long QTable instead of a Row + mab = bodytable[16:17] + assert mab["NAIF ID"] == 726 assert mab["Body"] == "Mab" - assert np.allclose( - [42.011201, 15.801323, 5.368, 0.623, 223.976, 55.906, 223.969, 56.013, 0.10932, 3098.514, ], - [mab["RA (deg)"].to(u.deg).value, mab["Dec (deg)"].to(u.deg).value, mab["dRA"].to(u.arcsec).value, mab["dDec"].to(u.arcsec).value, mab["sub_obs_lon"].to(u.deg).value, mab["sub_obs_lat"].to(u.deg).value, mab["sub_sun_lon"].to(u.deg).value, mab["sub_sun_lat"].to(u.deg).value, mab["phase"].to(u.deg).value, mab["distance"].to(u.km * 1e6).value, ], - rtol=1e-2, - ) + assert expected_mab.values_equal(mab) # check a ring in ringtable beta = ringtable[ringtable.loc_indices["Beta"]] @@ -110,7 +118,6 @@ def test_ephemeris_query_Pluto(patch_request): rtol=1e-2, ) - # check ringtable is None assert ringtable is None @@ -125,11 +132,16 @@ def test_ephemeris_query_Neptune(patch_request): neptune_arcmodel=2 ) - assert np.allclose( - [63.81977, 55.01978, 44.21976, 40.41978, 26.41978, 64.81977, 59.11976, 45.21976, 43.41978, 36.01978], - [ringtable[ringtable.loc_indices["Courage"]]["min_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Liberte"]]["min_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Egalite A"]]["min_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Egalite B"]]["min_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Fraternite"]]["min_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Courage"]]["max_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Liberte"]]["max_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Egalite A"]]["max_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Egalite B"]]["max_angle"].to(u.deg).value, ringtable[ringtable.loc_indices["Fraternite"]]["max_angle"].to(u.deg).value, ], - rtol=1e-3 - ) + expected = """ring,min_angle,max_angle +Courage,63.81977,64.81977 +Liberte,55.01978,59.11976 +Egalite A,44.21976,45.21976 +Egalite B,40.41978,43.41978 +Fraternite,26.41978,36.01978 + """ + expected = QTable.read(expected, format='ascii.csv', units=(None, 'deg', 'deg')) + + assert (expected == ringtable).all() def test_ephemeris_query_Saturn(patch_request): @@ -141,11 +153,12 @@ def test_ephemeris_query_Saturn(patch_request): epoch="2021-10-07 07:25", ) - assert np.allclose( - [249.23097, 250.34081], - [ringtable[ringtable.loc_indices["F"]]["pericenter"].to(u.deg).value, ringtable[ringtable.loc_indices["F"]]["ascending node"].to(u.deg).value], - rtol=1e-3 - ) + expected = """ring,pericenter,ascending node +F,249.23097,250.34081 + """ + + expected = QTable.read(expected, format='ascii.csv', units=(None, 'deg', 'deg')) + assert (expected == ringtable).all() def test_ephemeris_query_payload(): @@ -213,7 +226,4 @@ def test_ephemeris_query_payload(): def test_bad_query_raise(): with pytest.raises(ValueError): - bodytable, ringtable = pds.RingNode.ephemeris( - planet="Venus", - epoch="2021-10-07 07:25", - ) + bodytable, ringtable = pds.RingNode.ephemeris(planet="Venus", epoch="2021-10-07 07:25")