Skip to content

feat: add direct ITRS observed transforms (swev-id: astropy__astropy-13398) - #130

Open
casey-brooks wants to merge 2 commits into
astropy__astropy-13398from
noa/issue-129
Open

feat: add direct ITRS observed transforms (swev-id: astropy__astropy-13398)#130
casey-brooks wants to merge 2 commits into
astropy__astropy-13398from
noa/issue-129

Conversation

@casey-brooks

Copy link
Copy Markdown

Summary

  • implement direct ITRSAltAz/HADec transforms with rotation helper, obstime validation, and refraction/unity warnings
  • expose the transforms by importing the module in builtin_frames.__init__
  • add regression tests covering round-trips, zenith alignment, unit-spherical behaviour, and comparison to the ICRS path

Testing

  • SETUPTOOLS_USE_DISTUTILS=stdlib PYTHONPATH=. LD_LIBRARY_PATH=/nix/store/h4qhxh7vwmxgy6w05g0xsf6r1bfi9vga-gcc-15.2.0-lib/lib:/nix/store/dvb9i3s69j29md1hpx3gpaqqds6530df-gcc-15.2.0-libgcc/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m pytest astropy/coordinates/tests/test_itrs_observed_transforms.py
  • SETUPTOOLS_USE_DISTUTILS=stdlib PYTHONPATH=. LD_LIBRARY_PATH=/nix/store/h4qhxh7vwmxgy6w05g0xsf6r1bfi9vga-gcc-15.2.0-lib/lib:/nix/store/dvb9i3s69j29md1hpx3gpaqqds6530df-gcc-15.2.0-libgcc/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m flake8 astropy/coordinates/builtin_frames/itrs_observed_transforms.py astropy/coordinates/tests/test_itrs_observed_transforms.py

Observed failure & reproduction

from astropy.coordinates import EarthLocation, AltAz
from astropy.time import Time
import astropy.units as u

loc = EarthLocation(lat=35*u.deg, lon=-111*u.deg, height=0*u.m)
obstime = Time('2024-01-01T00:00:00', scale='utc')

target_itrs = EarthLocation(lat=35*u.deg, lon=-111*u.deg, height=1000*u.m).get_itrs(obstime=obstime)

computed = target_itrs.transform_to(AltAz(obstime=obstime, location=loc, pressure=0*u.hPa))

assert abs((computed.alt - 90*u.deg).to_value(u.arcsec)) < 1, f"alt={computed.alt}"
Traceback (most recent call last):
  File "<stdin>", line 12, in <module>
AssertionError: alt=69.25429263945392 deg

Resolves #129.

@casey-brooks
casey-brooks requested a review from a team January 13, 2026 21:54
@casey-brooks

Copy link
Copy Markdown
Author

Local Validation

  • SETUPTOOLS_USE_DISTUTILS=stdlib PYTHONPATH=. LD_LIBRARY_PATH=/nix/store/h4qhxh7vwmxgy6w05g0xsf6r1bfi9vga-gcc-15.2.0-lib/lib:/nix/store/dvb9i3s69j29md1hpx3gpaqqds6530df-gcc-15.2.0-libgcc/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m pytest astropy/coordinates/tests/test_itrs_observed_transforms.py → 5 passed, 0 failed
  • SETUPTOOLS_USE_DISTUTILS=stdlib PYTHONPATH=. LD_LIBRARY_PATH=/nix/store/h4qhxh7vwmxgy6w05g0xsf6r1bfi9vga-gcc-15.2.0-lib/lib:/nix/store/dvb9i3s69j29md1hpx3gpaqqds6530df-gcc-15.2.0-libgcc/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m flake8 astropy/coordinates/builtin_frames/itrs_observed_transforms.py astropy/coordinates/tests/test_itrs_observed_transforms.py → no issues

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Direct ITRS↔observed transforms look great overall, but the AltAz/HADec→ITRS path still needs to emit the same unit-spherical warning called for in the spec, and the test suite should cover that behaviour.

Comment thread astropy/coordinates/builtin_frames/itrs_observed_transforms.py
Comment thread astropy/coordinates/tests/test_itrs_observed_transforms.py

@noa-lucent noa-lucent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for adding the observed→ITRS warning and coverage—the behaviour now matches the spec and the tests exercise both paths. I’m satisfied with the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants