Fixes and Improvements to WCSAxes #9392

Cadair · 2019-10-17T16:01:37Z

This PR is a collection of WCSAxes related fixes, primarily focusing on APE 14 and 1D plotting.

It does the following things:

Fixes a bug in WCSAxes if slices is passed and SlicedLowLevelWCS is the wcs object. (Cadair@f7cc7b8#commitcomment-35522807)
Adds a get_format_unit method to CoordianateHelper
Raises an error in SlicedLowLevelWCS if the slice is too long.
Fixes WCSAxes no longer has an entry for every world dimension in CoordinatesMap #9370
Puts the most rapidly varying world axis on the bottom spine when plotting a 1D plot.
Automatically add labels with the coordinate type and unit to plots

astropy/wcs/utils.py

astrofrog

So does changing back to including all coordinates in ax.coords not break any existing tests?!

Otherwise just a few comments below, but overall I'm 👍 on this

astropy/visualization/wcsaxes/tests/test_wcsapi.py

astropy/visualization/wcsaxes/coordinate_helpers.py

Cadair · 2019-10-22T14:50:53Z

Given the addition of the auto spine selection for 1D plots, I have made a change here which by default plots the physical type (and for scalar axes unit) as an axis label. This primarily so that it is easy to identify which spatial axis is which, but is also obviously more generally useful.

This is a reasonably large change to the default behaviour, so I would be happy to add it only to the 1D plots if people think that's a good idea. @astrofrog @larrybradley

astrofrog

This all looks great (just a couple of comments below). Regarding the auto-labelling, I think this would be a good improvement to make. I know it would 'break' some existing plots where people don't want axis labels, but I'm thinking that's a pretty narrow use case as probably the majority of people will be using it with labels and won't see the difference?

However I think it's very important we document this well - both in the wcsaxes docs and in the what's new, so could you add both of these to this PR?

@larrybradley - do you agree about enabling the auto-labelling?

astropy/visualization/wcsaxes/coordinate_helpers.py

astropy/wcs/utils.py

larrybradley

I think the auto labeling is fine. For those who may not want axis labels, can we add an simple example/docs showing how to turn off the automatic labels?

Cadair · 2019-10-23T14:26:26Z

New reference images are in astropy/astropy-data#76 also I wrote a script to generate them: astropy/astropy-tools#141

I will add a section to the narrative docs about disabling autolabel

Cadair · 2019-10-23T14:56:37Z

Small doc section added, let me know if you think it should be expanded.

bsipocz · 2019-10-23T18:30:10Z

@Cadair - please update to use the new images, I've just merged your PR in astropy-data.

Cadair · 2019-10-24T08:51:20Z

According to @astrofrog in chat yesterday I put them in the wrong dir. I will issue a new data PR to move them.

Cadair · 2019-10-24T10:23:10Z

astropy/astropy-data#77

… correlated with which world axes

If we had a split coupled SLLWCS it broke

…t WCS In addition to this, the generation of the coord_map is handled correctly if a SlicedLowLevelWCS object and the slices argument are specified, where previously this would cause errors because of incorrect mapping between WCSAxes sliced WCS and the input WCS.

This means that for 1D plots the world axis which has the highest derivative is plotted on the bottom axes by default.

Cadair · 2019-10-25T11:13:32Z

slumps back into a chair

I think this is now ready for final review.

astrofrog

Looks good overall, thanks! A few small things/issues related to the reference images:

the reference image for test_changed_axis_units and test_minor_ticks looks wrong - the label for dec is on the wrong side
the test_noncelestial_angular reference image has confusing labels - both are the same
Is there a y label missing for test_direct_init or is it just out of the figure bounds? Maybe you can just turn off auto-labelling for that one too?
I wonder if we should turn off the auto-labelling for elliptical frames, the test_elliptical_frame example shows that it's maybe not ideal?
can you edit the test_axisbelow test to turn off autolabelling to limit a bit how many reference images are changed? (that one is a particularly prolific test)
there are reference images added for test_axes_off that are empty
for test_tick_params the unit bracket is empty so maybe not worth showing that bracket when unit is empty? also maybe disable auto-labelling for that test since it's really about the ticks
for test_update_clip_path_change_wcsandtest_update_clip_path_rectangular`` same comment, unit is empty but actually maybe just turn off auto-labelling
for test_update_clip_path_nonrectangular keep the auto-labelling but this one can be the test for when the unit is empty?

Cadair · 2019-10-25T21:19:20Z

I have updated the data PR, although i haven't modified the figure for #9411

astrofrog

This looks good to me, so approving - however I will deal with merging the PR for the reference images tomorrow, which is needed for the CI to pass here. I'll do it over the weekend to avoid disrupting other PRs.

Cadair · 2019-10-27T12:59:56Z

Thanks everyone!!

astropy-bot bot added visualization visualization.wcsaxes wcs wcs.wcsapi labels Oct 17, 2019

Cadair force-pushed the wcsaxes_ndcube_fixes branch from de051a6 to 9799c4e Compare October 21, 2019 14:10

Cadair commented Oct 21, 2019

View reviewed changes

astropy/wcs/utils.py Outdated Show resolved Hide resolved

Cadair force-pushed the wcsaxes_ndcube_fixes branch 2 times, most recently from 48192ff to 4aacd74 Compare October 21, 2019 16:39

Cadair marked this pull request as ready for review October 21, 2019 16:59

astrofrog requested changes Oct 21, 2019

View reviewed changes

astropy/visualization/wcsaxes/tests/test_wcsapi.py Show resolved Hide resolved

astropy/visualization/wcsaxes/tests/test_wcsapi.py Outdated Show resolved Hide resolved

astropy/visualization/wcsaxes/coordinate_helpers.py Show resolved Hide resolved

bsipocz added this to the v4.0 milestone Oct 21, 2019

Cadair force-pushed the wcsaxes_ndcube_fixes branch from 337bece to 3f5d9f7 Compare October 22, 2019 14:49

astrofrog requested changes Oct 22, 2019

View reviewed changes

astropy/visualization/wcsaxes/coordinate_helpers.py Outdated Show resolved Hide resolved

astropy/wcs/utils.py Show resolved Hide resolved

larrybradley reviewed Oct 23, 2019

View reviewed changes

Cadair mentioned this pull request Oct 23, 2019

Add a new set of images for 4.0 astropy/astropy-data#76

Merged

astrofrog and others added 9 commits October 25, 2019 11:52

Added initial version of a function to help find which pixel axes are…

0960e06

… correlated with which world axes

Clean up local_partial_pixel_derivatives

3afbcf7

Raise an error if the length of the slice is too long

b7c4702

Add a failing wcsaxes test when slicing a SlicedLowLevelWCS object

3c96e01

Fix world_keep when supplied a SlicedLowLevelWCS object

9ba43ed

If we had a split coupled SLLWCS it broke

Add a CoordinateHelper.get_format_unit

08d3841

Add a failing test for drawing most varying axis on the bottom (1D)

7b34e0f

Order 1D world axes by relative derivatives

04c6c65

This means that for 1D plots the world axis which has the highest derivative is plotted on the bottom axes by default.

Cadair added 3 commits October 25, 2019 12:03

Fix coord map repr

4cdbace

Determine which axes get ticks using transform_wcs and world_map

1c0546b

Fix invert_xy with and without multi-dim

06f4e3c

Cadair force-pushed the wcsaxes_ndcube_fixes branch from fe04e38 to 06f4e3c Compare October 25, 2019 11:11

Cadair added a commit to Cadair/astropy-data that referenced this pull request Oct 25, 2019

Update figures for astropy/astropy#9392 and astropy/astropy#9411

52607cc

Cadair mentioned this pull request Oct 25, 2019

more wcsaxes figure changes astropy/astropy-data#78

Merged

Flake

82ebbaa

bsipocz added the Ready-for-final-review label Oct 25, 2019

bsipocz requested review from larrybradley and astrofrog October 25, 2019 16:27

astrofrog requested changes Oct 25, 2019

View reviewed changes

Cadair added 4 commits October 25, 2019 19:53

Put axislabels in the right place

cc5476a

Don't truncate CTYPE

51da947

Address comments

b607134

update repr

5325d89

Cadair added a commit to Cadair/astropy-data that referenced this pull request Oct 25, 2019

Update figures for astropy/astropy#9392 and astropy/astropy#9411

64930c4

Cadair mentioned this pull request Oct 25, 2019

Only draw the axislabel on an spine if there are tick labels to draw #9481

Open

astrofrog approved these changes Oct 25, 2019

View reviewed changes

Fix underline

f05c96e

astrofrog pushed a commit to Cadair/astropy-data that referenced this pull request Oct 26, 2019

Update figures for astropy/astropy#9392

53f3f8b

astrofrog added 2 commits October 26, 2019 22:07

Disable axis labels in all subplots of test_tick_params

c0895ef

Updated changelog entry

64862f4

astrofrog added the 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Oct 26, 2019

This was referenced Oct 26, 2019

WCS values don't appear on axes #7180

Open

Update ERFA leap seconds using new IERS LeapSecond class. #9365

Merged

mhvk merged commit c5d03e3 into astropy:master Oct 26, 2019

Cadair deleted the wcsaxes_ndcube_fixes branch October 27, 2019 13:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes and Improvements to WCSAxes #9392

Fixes and Improvements to WCSAxes #9392

Cadair commented Oct 17, 2019 •

edited

astrofrog left a comment

Cadair commented Oct 22, 2019

astrofrog left a comment

larrybradley left a comment

Cadair commented Oct 23, 2019

Cadair commented Oct 23, 2019

bsipocz commented Oct 23, 2019

Cadair commented Oct 24, 2019

Cadair commented Oct 24, 2019

Cadair commented Oct 25, 2019

astrofrog left a comment

Cadair commented Oct 25, 2019

astrofrog left a comment

Cadair commented Oct 27, 2019

Fixes and Improvements to WCSAxes #9392

Fixes and Improvements to WCSAxes #9392

Conversation

Cadair commented Oct 17, 2019 • edited

astrofrog left a comment

Choose a reason for hiding this comment

Cadair commented Oct 22, 2019

astrofrog left a comment

Choose a reason for hiding this comment

larrybradley left a comment

Choose a reason for hiding this comment

Cadair commented Oct 23, 2019

Cadair commented Oct 23, 2019

bsipocz commented Oct 23, 2019

Cadair commented Oct 24, 2019

Cadair commented Oct 24, 2019

Cadair commented Oct 25, 2019

astrofrog left a comment

Choose a reason for hiding this comment

Cadair commented Oct 25, 2019

astrofrog left a comment

Choose a reason for hiding this comment

Cadair commented Oct 27, 2019

Cadair commented Oct 17, 2019 •

edited