[FIX] _transform_plot_args was converting to degrees instead of coord_unit

[nabobalis]

Discovered this when trying to plot a coordinate using a gWCS.

I am unsure how to unit test this.

[Cadair]

Thanks @nabobalis !

Given you can't create a WCS with coord_type longitude and a coord_unit of anything other than deg with astropy I am not sure you can easily test this without importing gwcs into the tests.

If you could add a changelog that would be good though.

[nabobalis]

I added a simple changelog, let me know if it's sufficient.

[WilliamJamieson]

Thanks @nabobalis !

Given you can't create a WCS with coord_type longitude and a coord_unit of anything other than deg with astropy I am not sure you can easily test this without importing gwcs into the tests.

If you could add a changelog that would be good though.

@nabobalis would it be possible for you to contribute a test for this to GWCS?

[nabobalis]

Thanks @nabobalis !
Given you can't create a WCS with coord_type longitude and a coord_unit of anything other than deg with astropy I am not sure you can easily test this without importing gwcs into the tests.
If you could add a changelog that would be good though.

@nabobalis would it be possible for you to contribute a test for this to GWCS?

Don't see why not. Point me at a repo and I will try.

[Cadair]

I would focus on the user facing affects of this:

``WCSAxes.plot_coord`` and ``plot_scatter`` now work correctly for APE 14 compliant WCSes where the units are not always converted to degrees.

[astrofrog]

Given you can't create a WCS with coord_type longitude and a coord_unit of anything other than deg with astropy I am not sure you can easily test this without importing gwcs into the tests.

Couldn't we make a small fake APE 14 WCS to replicate this bug?

[Cadair]

Couldn't we make a small fake APE 14 WCS to replicate this bug?

I am just thinking about what it would need to do, I am not sure how easy it would be to test the behaviour without an image test, so I think you would have to wrap a astropy.wcs.WCS object and do the unit conversion in pixel_to_world_values and world_to_pixel_values?

[WilliamJamieson]

Don't see why not. Point me at a repo and I will try.

See https://github.com/spacetelescope/gwcs

[pllim]

We don't even declare gwcs to be installed in setup.cfg, so adding a GWCS object in test would be... controversial.

Is this good to go as is?

[nabobalis]

Is this good to go as is?

I was told it was, but I imagine one of the WCSAXES maintainers needs to approve?

[pllim]

I guess depends on how badly @astrofrog wants you to make a fake WCS for a test here. Adding a test to GWCS downstream should be tracked over at GWCS repo separately.

Regardless, please squash the commits. Thanks!

[nabobalis]

Regardless, please squash the commits. Thanks!

Done.

[astrofrog]

I have pushed a commit with a regression test that uses a custom APE 14 WCS as I'd rather make sure we test for that here and not just in GWCS. We'll need to update the hashes once the CI has run. The reference figure looks like this:

and it didn't work properly before this PR.

[pllim]

I don't think we should auto backport this because we bumped the minversion of matplotlib on main but not the backport branches. That would mess with the hash stuff. If you really want to backport, please manually do it and use the approrpriate matplotlib versions being tested over there to generate the hashes.

[pllim]

Thanks, all!

[astrofrog]

I'm happy to backport manually but in future could you leave the backport labels if a PR is deemed to need backporting even if it will require a manual backport? Otherwise may lose track of it and also it's useful to have the bot instructions. This one won't be very difficult to backport.

[pllim]

OK, my bad, let's see how far the bot can go then.

@meeseeksdev backport to v5.0.x

@meeseeksdev backport to v5.2.x

[astrofrog]

Thanks will backport later once at desk!

[nabobalis]

Thank you to @astrofrog and @pllim for getting this over the line.