Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate coordinates.get_moon() #14354

Merged
merged 1 commit into from Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions astropy/coordinates/orbital_elements.py
Expand Up @@ -170,12 +170,12 @@

@deprecated(
since="5.0",
alternative="astropy.coordinates.get_moon",
alternative="astropy.coordinates.get_body('moon')",
message=(
"The private calc_moon function has been deprecated, as its functionality is"
" now available in ERFA. Note that the coordinate system was not interpreted"
" quite correctly, leading to small inaccuracies. Please use the public"
" get_moon or get_body functions instead."
" get_body() function instead."
),
)
def calc_moon(t):
Expand Down
1 change: 1 addition & 0 deletions astropy/coordinates/solar_system.py
Expand Up @@ -500,6 +500,7 @@ def get_body(body, time, location=None, ephemeris=None):
return SkyCoord(gcrs)


@deprecated("5.3", alternative='get_body("moon")')
def get_moon(time, location=None, ephemeris=None):
"""
Get a `~astropy.coordinates.SkyCoord` for the Earth's Moon as observed
Expand Down
7 changes: 3 additions & 4 deletions astropy/coordinates/tests/test_regression.py
Expand Up @@ -38,7 +38,6 @@
SphericalRepresentation,
UnitSphericalRepresentation,
get_body,
get_moon,
get_sun,
)
from astropy.coordinates.sites import get_builtin_sites
Expand Down Expand Up @@ -313,7 +312,7 @@ def test_regression_4926():
times = Time("2010-01-1") + np.arange(20) * u.day
green = get_builtin_sites()["greenwich"]
# this is the regression test
moon = get_moon(times, green)
moon = get_body("moon", times, green)

# this is an additional test to make sure the GCRS->ICRS transform works for complex shapes
moon.transform_to(ICRS())
Expand All @@ -326,7 +325,7 @@ def test_regression_4926():
def test_regression_5209():
"check that distances are not lost on SkyCoord init"
time = Time("2015-01-01")
moon = get_moon(time)
moon = get_body("moon", time)
new_coord = SkyCoord([moon])
assert_quantity_allclose(new_coord[0].distance, moon.distance)

Expand Down Expand Up @@ -446,7 +445,7 @@ def test_regression_5889_5890():
*u.Quantity([3980608.90246817, -102.47522911, 4966861.27310067], unit=u.m)
)
times = Time("2017-03-20T12:00:00") + np.linspace(-2, 2, 3) * u.hour
moon = get_moon(times, location=greenwich)
moon = get_body("moon", times, location=greenwich)
targets = SkyCoord([350.7 * u.deg, 260.7 * u.deg], [18.4 * u.deg, 22.4 * u.deg])
targs2d = targets[:, np.newaxis]
targs2d.transform_to(moon)
Expand Down
55 changes: 15 additions & 40 deletions astropy/coordinates/tests/test_solar_system.py
Expand Up @@ -28,12 +28,12 @@
from astropy.units import allclose as quantity_allclose
from astropy.utils.compat.optional_deps import HAS_JPLEPHEM, HAS_SKYFIELD
from astropy.utils.data import download_file, get_pkg_data_filename
from astropy.utils.exceptions import AstropyDeprecationWarning

if HAS_SKYFIELD:
from skyfield.api import Loader, Topos

de432s_separation_tolerance_planets = 5 * u.arcsec
de432s_separation_tolerance_moon = 5 * u.arcsec
de432s_distance_tolerance = 20 * u.km

skyfield_angular_separation_tolerance = 1 * u.arcsec
Expand Down Expand Up @@ -99,7 +99,7 @@ def test_positions_skyfield(tmp_path):
)

# planet positions w.r.t true equator and equinox
moon_astropy = get_moon(t, location, ephemeris="de430").transform_to(frame)
moon_astropy = get_body("moon", t, location, ephemeris="de430").transform_to(frame)
mercury_astropy = get_body("mercury", t, location, ephemeris="de430").transform_to(
frame
)
Expand Down Expand Up @@ -196,7 +196,7 @@ def test_erfa_planet(self, body, sep_tol, dist_tol):

@pytest.mark.remote_data
@pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
@pytest.mark.parametrize("body", ("mercury", "jupiter", "sun"))
@pytest.mark.parametrize("body", ("mercury", "jupiter", "sun", "moon"))
def test_de432s_planet(self, body):
astropy = get_body(body, self.t, ephemeris="de432s")
horizons = self.horizons[body]
Expand All @@ -212,23 +212,6 @@ def test_de432s_planet(self, body):
astropy.distance, horizons.distance, atol=de432s_distance_tolerance
)

@pytest.mark.remote_data
@pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
def test_de432s_moon(self):
astropy = get_moon(self.t, ephemeris="de432s")
horizons = self.horizons["moon"]

# convert to true equator and equinox
astropy = astropy.transform_to(self.apparent_frame)

# Assert sky coordinates are close.
assert astropy.separation(horizons) < de432s_separation_tolerance_moon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the definition of de432s_separation_tolerance_moon can also be removed (and it is the same as for planets, so the tests indeed stay the same)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed de432s_separation_tolerance_moon.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhvk, I've implemented your suggestion. Can this be merged now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, completely missed that, thanks for pinging. Yes, let's get this in!


# Assert distances are close.
assert_quantity_allclose(
astropy.distance, horizons.distance, atol=de432s_distance_tolerance
)


class TestPositionKittPeak:
"""
Expand Down Expand Up @@ -293,7 +276,7 @@ def test_erfa_planet(self, body, sep_tol, dist_tol):

@pytest.mark.remote_data
@pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
@pytest.mark.parametrize("body", ("mercury", "jupiter"))
@pytest.mark.parametrize("body", ("mercury", "jupiter", "moon"))
def test_de432s_planet(self, body):
astropy = get_body(body, self.t, ephemeris="de432s")
horizons = self.horizons[body]
Expand All @@ -309,23 +292,6 @@ def test_de432s_planet(self, body):
astropy.distance, horizons.distance, atol=de432s_distance_tolerance
)

@pytest.mark.remote_data
@pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
def test_de432s_moon(self):
astropy = get_moon(self.t, ephemeris="de432s")
horizons = self.horizons["moon"]

# convert to true equator and equinox
astropy = astropy.transform_to(self.apparent_frame)

# Assert sky coordinates are close.
assert astropy.separation(horizons) < de432s_separation_tolerance_moon

# Assert distances are close.
assert_quantity_allclose(
astropy.distance, horizons.distance, atol=de432s_distance_tolerance
)

@pytest.mark.remote_data
@pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
@pytest.mark.parametrize("bodyname", ("mercury", "jupiter"))
Expand Down Expand Up @@ -440,15 +406,15 @@ def test_get_sun_consistency(time):
assert sep < 0.1 * u.arcsec


def test_get_moon_nonscalar_regression():
def test_get_body_nonscalar_regression():
"""
Test that the builtin ephemeris works with non-scalar times.
See Issue #5069.
"""
times = Time(["2015-08-28 03:30", "2015-09-05 10:30"])
# the following line will raise an Exception if the bug recurs.
get_moon(times, ephemeris="builtin")
get_body("moon", times, ephemeris="builtin")


def test_barycentric_pos_posvel_same():
Expand Down Expand Up @@ -593,3 +559,12 @@ def test_regression_10271():

difference = (icrs_sun_from_alma - icrs_sun_from_geocentre).norm()
assert_quantity_allclose(difference, 0.13046941 * u.m, atol=1 * u.mm)


def test_get_moon_deprecation():
time_now = Time.now()
with pytest.warns(
AstropyDeprecationWarning, match=r'Use get_body\("moon"\) instead\.$'
):
moon = get_moon(time_now)
assert moon == get_body("moon", time_now)
3 changes: 3 additions & 0 deletions docs/changes/coordinates/14354.api.rst
@@ -0,0 +1,3 @@
``get_moon()`` is deprecated and may be removed in a future version of
``astropy``. Calling ``get_moon(...)`` should be replaced with
``get_body("moon", ...)``.
13 changes: 6 additions & 7 deletions docs/coordinates/solarsystem.rst
Expand Up @@ -19,11 +19,10 @@ use cases you may have (see the examples below).
most conveniently achieved via ``pip install jplephem``, although whatever
package management system you use might have it as well.

Three functions are provided; :meth:`~astropy.coordinates.get_body`,
:meth:`~astropy.coordinates.get_moon` and
:meth:`~astropy.coordinates.get_body_barycentric`. The first two functions
return |SkyCoord| objects in the `~astropy.coordinates.GCRS` frame, while the
latter returns a `~astropy.coordinates.CartesianRepresentation` of the
Two functions are provided; :func:`~astropy.coordinates.get_body` and
:func:`~astropy.coordinates.get_body_barycentric`.
The first returns |SkyCoord| objects in the `~astropy.coordinates.GCRS` frame,
while the latter returns a `~astropy.coordinates.CartesianRepresentation` of the
barycentric position of a body (i.e., in the `~astropy.coordinates.ICRS` frame).

Examples
Expand All @@ -38,7 +37,7 @@ without the need to download a large ephemerides file)::

>>> from astropy.time import Time
>>> from astropy.coordinates import solar_system_ephemeris, EarthLocation
>>> from astropy.coordinates import get_body_barycentric, get_body, get_moon
>>> from astropy.coordinates import get_body_barycentric, get_body
>>> t = Time("2014-09-22 23:22")
>>> loc = EarthLocation.of_site('greenwich') # doctest: +REMOTE_DATA
>>> with solar_system_ephemeris.set('builtin'):
Expand All @@ -64,7 +63,7 @@ ephemeris is set):
>>> get_body('jupiter', t, loc) # doctest: +REMOTE_DATA, +FLOAT_CMP
<SkyCoord (GCRS: obstime=2014-09-22 23:22:00.000, obsgeoloc=(3949481.69182405, -550931.91022387, 4961151.73597633) m, obsgeovel=(40.159527, 287.47873161, -0.04597922) m / s): (ra, dec, distance) in (deg, deg, km)
(136.90234846, 17.03160654, 8.89196021e+08)>
>>> get_moon(t, loc) # doctest: +REMOTE_DATA, +FLOAT_CMP
>>> get_body('moon', t, loc) # doctest: +REMOTE_DATA, +FLOAT_CMP
<SkyCoord (GCRS: obstime=2014-09-22 23:22:00.000, obsgeoloc=(3949481.69182405, -550931.91022387, 4961151.73597633) m, obsgeovel=(40.159527, 287.47873161, -0.04597922) m / s): (ra, dec, distance) in (deg, deg, km)
(165.51854528, 2.32861794, 407229.55638763)>
>>> get_body_barycentric('moon', t) # doctest: +REMOTE_DATA, +FLOAT_CMP
Expand Down
6 changes: 3 additions & 3 deletions examples/coordinates/plot_obs-planning.py
Expand Up @@ -119,13 +119,13 @@


##############################################################################
# Do the same with `~astropy.coordinates.get_moon` to find when the moon is
# Do the same with `~astropy.coordinates.get_body` to find when the moon is
# up. Be aware that this will need to download a 10MB file from the internet
# to get a precise location of the moon.

from astropy.coordinates import get_moon
from astropy.coordinates import get_body

moon_July12_to_13 = get_moon(times_July12_to_13)
moon_July12_to_13 = get_body("moon", times_July12_to_13)
moonaltazs_July12_to_13 = moon_July12_to_13.transform_to(frame_July12_to_13)

##############################################################################
Expand Down