Fix plotting of quantities in WCSAxes by astrofrog · Pull Request #20779 · astropy/astropy

astrofrog · 2026-11-05T10:05:25Z

Description

We never really supported passing quantities to e.g. ax.scatter or ax.plot but as described in #20777, quantities were being silently ignored. I realized that we could actually make this work by leveraging the ability for Matplotlib to have hooks for unit conversion on the axes.

With this PR, several things happen:

Axes have a unit converter set on them so that quantities are always converted to the native units of the WCS. Note that technically speaking there is not necessarily a 1-to-1 mapping from xaxis/yaxis to world coordinates, but it doesn't matter because the idea is that the xaxis/yaxis converters really mean converters that apply to the first and second arguments of e.g. scatter and plot.
We no longer hard-code the input units for transforms returned by get_transform as being in degrees when passing a celestial frame, so we properly recognize e.g. arcsec input
The SphericalCircle and Quadrangle patches have been updated to also recognize unit converters on the axes, as they don't go through the unit conversion machinery automatically
Bare values passed with a celestial Frame transform are assumed to be in the WCS units instead of always degrees
A quantity in units which is incompatible with the WCS will now error (rather than silently being accepted before)

Things that haven't changed:

Bare values with no transform are still pixels
Quantities in u.pix are still pixels
plot_coord, scatter_coord and coordinate grid overlays are unchanged

This needs to be carefully tested with sunpy, ndcube, and dkist at least - @hayesla @ricardogando

@alasdairwilson - you might want to check this since it changes a little the behavior after #20600. The following:

  wcs = WCS(naxis=2, preserve_units=True)
  wcs.wcs.ctype = ["RA---TAN", "DEC--TAN"]
  wcs.wcs.cunit = ["arcsec", "arcsec"]
  ...
  ax = fig.add_subplot(projection=wcs)

  ax.plot(10.1, 20.1, "o", transform=ax.get_transform("icrs"))

will now assume that the bare values are in arcsec, not in degrees (basically bare values, when using a celestial transform, are assumed to be in the WCS units.

Because this is not a trivial fix and might break some corner cases, we should not backport it (nor is it easy to go through deprecation)

Fixes #20777

AI Disclosure

If AI tools were used to develop this pull request, describe the tools including specific model and version, how they were used, and what content is AI generated. Otherwise enter "N/A".

Assisted by Claude Opus 5.5

I certify that I am human and that I take full responsibility for this pull request including all interactions with reviewers.

Merge method

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-11-05T10:05:39Z

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.

alasdairwilson

So this is good...really good.

It breaks loads of our stuff but in a good way I think... we will have to work around it but like I said this is so much better. ❤️

alasdairwilson · 2026-11-05T13:50:18Z

I think the general strategy on our side to have code that works both pre and post this PR being released.

is to change where we are doing stuff like:

transform = axes.get_transform(frame)
Tx = coord.spherical.lon.to_value(u.deg)
Ty = coord.spherical.lat.to_value(u.deg)

to

transform = axes.get_transform("world")
Tx = coord.spherical.lon.to_value(axes.wcs.world_axis_units[0])
Ty = coord.spherical.lat.to_value(axes.wcs.world_axis_units[1])

so if we are on pre #20779 astropy then we are fine because wcs units are assumed and the unit is dropped and post is of course fine because we are sending a Quantity anyway.

Because this is not a trivial fix and might break some corner cases, we should not backport it (nor is it easy to go through deprecation)

Yeah... part of the problem when we were discussing #20777 yesterday was that we used code that assumed degrees and it was fine because it WAS degrees and we realised when writing sunpy/sunpy#8819 and related branches in that stack PR that astropy was now assuming wcs units....

We can fix all our internal code in the manner I described above, the problem is, as you point out, that we taught people by example to do this conversion to u.deg and we can't undo that teaching so those people were gonna be bitten by our 8.1 release anyway. So the good news is then that even though more user code will break due to this...it hasn't made the user-facing problem much worse for us, they were already going to have problems if they were passing degrees to maps that are now in arcsec and at least this way if they update astropy and as long as they are passing the values as a Quantity, they might not even notice.

mhvk

Very nice! I wondered a bit about astropy.visualization.quantity_support, but now see that you are using some of its functionality internally. So, in a way, you just turn it on by default, which makes sense.

Inline a small comment, and a small query.

mhvk · 2026-11-07T01:33:17Z

+    def convert(val, unit, axis):
+        # Pixel coordinates are plotted with the default transform, which does
+        # not use the world units
+        if isinstance(val, u.Quantity) and val.unit == u.pix:


Probably very unlikely to happen in practice, but someone might put in a quantity in "kpix", so perhaps it is worth doing val.unit.is_equivalent(u.pix) and val.to_value(u.pix) instead?

mhvk · 2026-11-07T01:36:05Z

+        return MplQuantityConverter.convert(val, unit, axis)
+
+
+# A single instance, since Matplotlib refuses to replace an explicitly set


Should that be considered a bug in matplotlib? Worth reporting?

Is it why you cannot use MplQuantityConverter more directly?

I don't think this is a bug, I think it's just by design

…orms to the world units of the WCS.

…it converter.

…ount check in WCSAxes.

…c validation.

…it to the angular units of the axes.

…erting them to the world units.

…s on WCSAxes.

…es and clarify why the unit converter is a single instance.

astrofrog · 2026-11-13T11:27:36Z

@alasdairwilson @Cadair - as we are a week away from feature freeze, I'd like us to make a decision on this. I was wondering if you could confirm if sunpy is happy with this and we can go ahead or whether it will be too disruptive?

astrofrog requested a review from larrybradley as a code owner November 5, 2026 10:05

github-actions Bot added Docs visualization visualization.wcsaxes labels Nov 5, 2026

astrofrog added the API change PRs and issues that change an existing API, possibly requiring a deprecation period label Nov 5, 2026

astrofrog requested a review from Cadair November 5, 2026 10:22

alasdairwilson reviewed Nov 5, 2026

View reviewed changes

pllim added this to the v8.1.0 milestone Nov 5, 2026

mhvk reviewed Nov 7, 2026

View reviewed changes

astrofrog added 9 commits November 13, 2026 11:24

Convert Quantity values plotted on WCSAxes with world or frame transf…

f215ba6

…orms to the world units of the WCS.

Only set the world unit on the x axis of 1D WCSAxes.

60050b9

Reuse the Quantity conversion from quantity_support in the WCSAxes un…

756bfb2

…it converter.

Explain the converter singleton, the clear guard and the world unit c…

20a7d0e

…ount check in WCSAxes.

Document the parameters of the private polygon base class for numpydo…

77841ab

…c validation.

Always convert SphericalCircle and Quadrangle vertices from vertex_un…

e4a54da

…it to the angular units of the axes.

Plot pixel quantities on WCSAxes as pixel coordinates instead of conv…

8305dc8

…erting them to the world units.

Add a changelog entry and a what's new section for plotting quantitie…

a356c75

…s on WCSAxes.

Plot quantities in prefixed pixel units on WCSAxes as pixel coordinat…

e7dbb8a

…es and clarify why the unit converter is a single instance.

astrofrog force-pushed the wcsaxes-quantity-units branch from 8af16f7 to e7dbb8a Compare November 13, 2026 11:24

Navigation Menu

Repository navigation

Fix plotting of quantities in WCSAxes - #20779

Fix plotting of quantities in WCSAxes#20779
astrofrog wants to merge 9 commits into
astropy:mainfrom
astrofrog:wcsaxes-quantity-units

astrofrog commented Nov 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

alasdairwilson left a comment

Uh oh!

alasdairwilson commented Nov 5, 2026

Uh oh!

mhvk left a comment

Uh oh!

mhvk Nov 7, 2026

Uh oh!

astrofrog Nov 13, 2026

Uh oh!

mhvk Nov 7, 2026

Uh oh!

astrofrog Nov 13, 2026

Uh oh!

astrofrog commented Nov 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return MplQuantityConverter.convert(val, unit, axis)


		# A single instance, since Matplotlib refuses to replace an explicitly set

Conversation

astrofrog commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

AI Disclosure

Merge method

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

alasdairwilson left a comment

Choose a reason for hiding this comment

Uh oh!

alasdairwilson commented Nov 5, 2026

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

mhvk Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

astrofrog commented Nov 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

astrofrog commented Nov 5, 2026 •

edited

Loading