Fix hardcoded degree units in fitswcs APE 14 world to pixel by nabobalis · Pull Request #19506 · astropy/astropy

nabobalis · 2026-03-31T00:14:10Z

Description

I started using preserve_units with astropy 7.2 for my WCS and I have a roundtrip issues for my solar raster data.

Changelog if this patch is ok

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.

github-actions · 2026-03-31T00:14:22Z

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 · 2026-03-31T15:13:00Z

It should be noted that this bug is only present in the high level API component:

In [25]: wcs.wcs_world2pix(*wcs.wcs_pix2world(0, 56, 0), 0)
Out[25]: [array(6.88338275e-15), array(56.)]

In [26]: wcs.world_to_pixel_values(*wcs.pixel_to_world_values(0, 56))
Out[26]: (array(6.88338275e-15), array(56.))

Your fix is in the wrong place though the bug must be in how wcs.WCS sets its world_axis_object_components property.

nabobalis · 2026-03-31T15:14:31Z

So where should I move it to and what is the actual fix I need to implement?

Cadair · 2026-03-31T15:18:35Z

From a cursory look, I'm pretty sure the bug is here:

astropy/astropy/wcs/wcsapi/fitswcs.py

Lines 449 to 450 in 449a4c9

    
           components[self.wcs.lng] = ("celestial", 0, "spherical.lon.degree") 
        
           components[self.wcs.lat] = ("celestial", 1, "spherical.lat.degree")

it's always accessing the component in degrees which it shouldn't be.

pllim · 2026-03-31T17:42:24Z

I started using preverse_units

Hopefully you meant preserve units, and not perverse units... 😹

nabobalis · 2026-03-31T18:25:33Z

I started using preverse_units

Hopefully you meant preserve units, and not perverse units... 😹

Woops!

astrofrog · 2026-04-02T14:21:41Z

    b=48.25 * u.deg,
    distance=0 * u.km,
-    radial_velocity=-(3.346e-3 / 2.725 * c).to(u.km / u.s),
+    radial_velocity=-(3.346e-3 / 2.725 * C_SI).to(u.km / u.s),


Why this change?

I am stupid.

astrofrog · 2026-04-02T14:22:44Z

                kwargs = {}
                kwargs["frame"] = celestial_frame
-                # Very occasionally (i.e. with TAB) wcs does not convert the units to degrees
+                # Very occasionally (i.e. with TAB) wcs does not convert the units


Does this comment need updating now that this is not the only way the units might not be degrees?

nabobalis · 2026-04-02T14:34:15Z

@@ -0,0 +1,3 @@
+def assert_celestial_component(component, index):


I don't really like this change but I had no better ideas to avoid me repeating this pattern in the unit tests.

Also since its now a callable, I am not checking its correct either.

nabobalis · 2026-04-02T15:52:46Z

That failure is:

FAILED ../../.tox/py311-test-oldestdeps-alldeps-cov-clocale/lib/python3.11/site-packages/astropy/io/fits/tests/test_core.py::TestFileFunctions::test_open_from_remote_url - TimeoutError: [Errno 110] Connection timed out

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

pllim · 2026-04-02T17:32:33Z

test_open_from_remote_url - TimeoutError

I don't see how this is your fault. Might just be a transient failure. 🤞

nabobalis · 2026-04-13T17:59:59Z

Sorry to ping, any chance of an update on this?

Edit: I forgot about some setting for copilot which I have now disabled.

Copilot

Pull request overview

This PR fixes FITS WCS (APE 14) world_to_pixel behavior when preserve_units=True, ensuring non-degree celestial units (e.g., arcsec, mas) are respected instead of being implicitly converted/hardcoded to degrees.

Changes:

Update FITSWCS celestial component extraction to convert SkyCoord lon/lat to the WCS’ configured units rather than degrees.
Refactor multiple tests to avoid hardcoding "spherical.*.degree" component paths and add a regression test for arcsec/mas round-tripping.
Add a WCS changelog bugfix fragment.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`astropy/wcs/wcsapi/fitswcs.py`	Adjusts celestial component extraction logic to preserve configured lon/lat units.
`astropy/wcs/wcsapi/tests/test_fitswcs.py`	Adds a regression test for non-degree units and updates component assertions to accept callables.
`astropy/wcs/wcsapi/wrappers/tests/test_sliced_wcs.py`	Updates assertions to avoid degree-string coupling, using a helper assertion instead.
`astropy/wcs/wcsapi/tests/helpers.py`	Introduces `assert_celestial_component` helper for reusable component checks.
`docs/changes/wcs/19506.bugfix.rst`	Documents the bugfix in the WCS changelog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+                components[self.wcs.lng] = (
+                    "celestial",
+                    0,
+                    lambda c: c.spherical.lon.to_value(lon_unit),
+                )


github-actions bot added the wcs.wcsapi label Mar 31, 2026

nabobalis commented Mar 31, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/tests/test_fitswcs.py

nabobalis marked this pull request as ready for review March 31, 2026 00:20

nabobalis requested a review from astrofrog as a code owner March 31, 2026 00:20

Cadair marked this pull request as draft March 31, 2026 15:19

Cadair added the backport-v7.2.x on-merge: backport to v7.2.x label Mar 31, 2026

Cadair added this to the v7.2.1 milestone Mar 31, 2026

nabobalis changed the title ~~Fix world_to_pixel for preserve_units celestial WCS axes~~ Fix unit change in fitswcs Mar 31, 2026

nabobalis changed the title ~~Fix unit change in fitswcs~~ Fix unit hardcode in fitswcs Mar 31, 2026

nabobalis force-pushed the main branch from 32686c1 to 42da5f3 Compare March 31, 2026 15:50

nabobalis commented Mar 31, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/fitswcs.py Outdated

nabobalis commented Mar 31, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/fitswcs.py Outdated

nabobalis force-pushed the main branch 3 times, most recently from c7b1810 to d202074 Compare March 31, 2026 17:09

pllim added the Bug label Mar 31, 2026

nabobalis force-pushed the main branch from ebca261 to 83935bf Compare March 31, 2026 18:23

nabobalis force-pushed the main branch 3 times, most recently from adb9d27 to 6a99e7c Compare March 31, 2026 18:38

nabobalis marked this pull request as ready for review April 1, 2026 17:31

Cadair approved these changes Apr 2, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/fitswcs.py Outdated

Cadair changed the title ~~Fix unit hardcode in fitswcs~~ Fix hardcoded degree units in fitswcs APE 14 world to pixel Apr 2, 2026

nabobalis force-pushed the main branch from 20c28ae to e96224f Compare April 2, 2026 14:09

Cadair reviewed Apr 2, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/fitswcs.py Outdated

nabobalis force-pushed the main branch 4 times, most recently from 31944b7 to 6a79e8a Compare April 2, 2026 14:23

astrofrog reviewed Apr 2, 2026

View reviewed changes

nabobalis commented Apr 2, 2026

View reviewed changes

nabobalis added 2 commits April 2, 2026 07:34

Remove hard coded unit with using lambda

daa6c64

Changelog

65390ae

nabobalis force-pushed the main branch from d8653bf to 65390ae Compare April 2, 2026 14:34

pllim reviewed Apr 2, 2026

View reviewed changes

Comment thread docs/changes/wcs/19506.bugfix.rst Outdated

Update docs/changes/wcs/19506.bugfix.rst

f71355e

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

nabobalis added 3 commits April 6, 2026 09:33

Merge branch 'main' into main

83749f5

Merge branch 'main' into main

fc73d8c

Merge branch 'main' into main

9e10a99

Copilot AI review requested due to automatic review settings April 13, 2026 17:58

Copilot started reviewing on behalf of nabobalis April 13, 2026 17:59 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Comment thread astropy/wcs/wcsapi/fitswcs.py

Comment on lines +450 to +454

components[self.wcs.lng] = (

"celestial",

0,

lambda c: c.spherical.lon.to_value(lon_unit),

)

		@@ -0,0 +1,3 @@
		def assert_celestial_component(component, index):

Uh oh!

Conversation

nabobalis commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions bot commented Mar 31, 2026

Uh oh!

Uh oh!

Cadair commented Mar 31, 2026

Uh oh!

nabobalis commented Mar 31, 2026

Uh oh!

Cadair commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pllim commented Mar 31, 2026

Uh oh!

nabobalis commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

astrofrog Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

nabobalis Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

nabobalis Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

nabobalis Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

nabobalis commented Apr 2, 2026

Uh oh!

Uh oh!

pllim commented Apr 2, 2026

Uh oh!

nabobalis commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nabobalis commented Mar 31, 2026 •

edited

Loading

Cadair commented Mar 31, 2026 •

edited

Loading

nabobalis commented Mar 31, 2026 •

edited

Loading

nabobalis commented Apr 13, 2026 •

edited

Loading