FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add a context manager for imposing the value of a coordinate frame attribute by Cadair · Pull Request #20254 · astropy/astropy · GitHub

Add a context manager for imposing the value of a coordinate frame attribute - #20254

Draft
Cadair wants to merge 4 commits into
astropy:mainfrom
Cadair:assume_frame_attribute_values
Draft

Add a context manager for imposing the value of a coordinate frame attribute#20254
Cadair wants to merge 4 commits into
astropy:mainfrom
Cadair:assume_frame_attribute_values

Conversation

Cadair commented Aug 18, 2026
edited
Loading

Copy link
Copy Markdown
Member

Description

This PR is primarily motivated by use in sunpy (I can't think of a good example with just astropy frames, but maybe someone knows one).

In the solar frames in sunpy if you have two images taken at close to but not exactly the same time, there's a very small observer shift between the two images. If you transform a coordinate from one image to another, the sunpy coordinate frame machinery has to do a full origin shift which is expensive and also valid for points on the sun, not off disk. This functionality would allow the user to opt-in to assuming one fixed observer location for both sides of the coordinate transformation (such as the average position, or one or the other).

I have opened a sunpy implementation of this here: sunpy/sunpy#8753 but it required reimplementing Attribute.__get__ from astropy, hence the upstreaming.

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

Cadair requested a review from ayshih August 18, 2026 16:18
Cadair force-pushed the assume_frame_attribute_values branch from 0effb9e to d1e2f20 Compare August 18, 2026 16:19
Cadair added this to the v8.1.0 milestone Aug 18, 2026

Copy link
Copy Markdown
Member

Can you clarify here whether 'assume' also means 'override' if present?

Cadair commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

Yes, the value provided in the context manager will be used before any other value

ayshih commented Aug 19, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

The verb in the name should not be "assume", because that reads like it acts akin to a new default value. The verb should instead be either "impose" (a la the impose_finite_difference_dt() context manager) or "override".

Comment thread astropy/coordinates/attributes.py Outdated
Cadair force-pushed the assume_frame_attribute_values branch from 374f191 to deb740c Compare August 19, 2026 16:16
Cadair changed the title Add a context manager for assuming the value of a coordinate frame attribute Add a context manager for imposing the value of a coordinate frame attribute Aug 19, 2026

ayshih commented Aug 19, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

An example of this PR's benefit. The angular separation between two 2D HCRS coordinates with different obstimes normally raises an error due to the origin shift.

>>> import astropy.units as u
>>> from astropy.coordinates import SkyCoord

>>> coord1 = SkyCoord(10*u.deg, 20*u.deg, frame='hcrs', obstime='2026-01-01 00:00:00')
>>> coord2 = SkyCoord(20*u.deg, 30*u.deg, frame='hcrs', obstime='2026-01-01 00:00:00.001')

>>> print(coord1.separation(coord2))
...
astropy.units.errors.UnitsError: The input HCRS coordinates do not have length units. This probably means you created coordinates with lat/lon but no distance.  Heliocentric<->ICRS transforms cannot function in this case because there is an origin shift.

Before this PR, one can use .replicate() to instantiate a new version of coord2 that has the same obstime as coord1.

>>> print(coord1.separation(coord2.replicate(obstime=coord1.obstime)))
13d28m54.20360928s

After this PR:

>>> with impose_frame_attributes(obstime=coord1.obstime):
...     print(coord1.separation(coord2))
13d28m54.20360928s

It's not a big advantage for this simple example, but imagine working with a whole collection of coordinates or working with implicit coordinates (e.g., reprojection).

Cadair and others added 2 commits August 19, 2026 18:37
Co-authored-by: Albert Y. Shih <ayshih@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL