Smart formatting of tick labels in WCSAxes #7215

astrofrog · 2018-02-19T23:05:15Z

This fixes the formatting of tick labels when used with a right ascension axis, in which case the hh:mm:ss format should be used (not dd:mm:ss). This is done by making it so that AngleFormatterLocator takes a format_unit (like ScalarFormatterLocator). This also fixes the use of set_format_unit with angle coordinates.

This is a bordeline bug/enhancement case, but I'd argue that the fact that set_format_unit didn't work correctly for angle coordinates previously was a bug.

Here's what an RA/Dec plot now looks like by default:

Remaining to-dos:

Improve auto-determination of ticks when using hh:mm:ss as above (which shows the default spacing is not ideal)
Update existing image tests
Add new image tests, including with LaTeX tick labels

Fixes #7209

astropy-bot · 2018-02-19T23:05:31Z

Hi there @astrofrog 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I noticed the following issue with this pull request:

Changelog entry section (v3.1) inconsistent with milestone (v3.0.2)

Would it be possible to fix this? Thanks!

If there are any issues with this message, please report them here.

Cadair · 2018-02-20T09:20:25Z

Does it make any sense to add u.arcsec as a format unit? I am not quite sure I am following all the code in here, but for all the sunpy plots being in u.arcsec is the same as formatting the axes with s.s.

astrofrog · 2018-02-20T09:48:15Z

@Cadair - with this PR, you can use ax.coords[0].set_format_unit(u.arcsec) and this will then automatically use decimal formatting but will choose the number of decimal places automatically (unlike e.g. s.ss which would pick two).

Cadair · 2018-02-20T09:50:39Z

Nice!

astrofrog · 2018-02-20T09:51:20Z

At least that's the theory ;) Feel free to try it out!

lpsinger

How would this code work with plots that have axes that are angle offsets rather than absolute angles?

lpsinger · 2018-02-20T16:31:33Z

astropy/visualization/wcsaxes/formatter_locator.py

+        return self._format_unit
+
+    @format_unit.setter
+    def format_unit(self, unit):


Could the setter also take a string like 'hourangle' or 'deg'?

Yep, I've updated this and set_format_unit to accept anything that can initialize Unit

Looks good to me!

lpsinger · 2018-02-20T16:50:45Z

astropy/visualization/wcsaxes/tests/test_images.py

@@ -109,6 +109,9 @@ def test_contour_overlay(self):
        ax.set_xlim(0., 720.)
        ax.set_ylim(0., 720.)

+        # For backward-compatibility with previous reference images


Maybe explain in this comment what change required this?

astrofrog · 2018-02-20T19:16:11Z

@lpsinger:

How would this code work with plots that have axes that are angle offsets rather than absolute angles?

This code will only enable hh:mm:ss notation if CTYPE[:4] is RA-- which I think likely won't be the case for offset coordinates? Basically the only output that is changed with this PR is when CTYPE[:4] is RA--``, and the rest should be the same I think? Do you have examples of files with offset coordinates?

bsipocz · 2018-02-20T19:59:37Z

@astrofrog - If this is an enhancement rather than a bugfix, ultimately it should be 3.1 rather than 3.0.1 isn't it?

astrofrog · 2018-02-21T09:55:33Z

@bsipocz - I think it's a borderline case - see my comment at the top of this PR:

This is a bordeline bug/enhancement case, but I'd argue that the fact that set_format_unit didn't work correctly for angle coordinates previously was a bug.

I'm happy to re-milestone this to 3.1 though as I don't feel strongly about it.

bsipocz · 2018-02-21T12:37:35Z

@astrofrog - I'm just playing the gatekeeper role with no real preference, and I'm actually happy to backport borderline cases if anyone makes a decision about them.

larrybradley

Thanks, @astrofrog. LGTM except for a couple minor comments.

It's borderline, but let's leave this as 3.0.1.

larrybradley · 2018-02-21T14:40:34Z

astropy/visualization/wcsaxes/formatter_locator.py


        if not isinstance(values, u.Quantity) and values is not None:
            raise TypeError("values should be a Quantities array")

        if len(values) > 0:
+


Why the blank line?

larrybradley · 2018-02-21T14:41:22Z

astropy/visualization/wcsaxes/coordinate_helpers.py

+        ----------
+        value : float
+            The value to format
+        format : { 'auto' | 'ascii' | 'latex' }, optional


nitpick: shouldn't this be a comma-separated list (that's what numpydoc uses) instead of pipe-separated?

larrybradley · 2018-02-26T18:30:45Z

LGTM. Thanks, @astrofrog. This will need to be rebased once #7234 is merged.

…ault. This also generalizes the AngleFormatterLocator so that it can take default units as well as having a way to specify to use decimal formatting by default.

…lay units

…ializer

…tting

… take string units

drdavella · 2018-03-12T16:35:05Z

@larrybradley I just rebased this in the hopes that it can be merged in the near future.

astrofrog · 2018-03-12T17:03:47Z

There are real test failures due to @Cadair's commit :) I need to investigate and fix them, I'll try and take a look now.

astrofrog · 2018-03-12T17:06:04Z

astropy/visualization/wcsaxes/formatter_locator.py

+                fmt = None
+
+                decimal = False
+                sep = 'fromunit'


@Cadair - can you explain this change? Why set decimal to False here and then change sep when it was set to None on line 354?

astrofrog · 2018-03-21T17:55:07Z

Since this PR was functional and approved prior to @Cadair's commit, I've removed that commit and will add it (and fix the failures) in a separate PR.

astrofrog · 2018-03-27T17:06:33Z

It looks like I moved this to 3.1 in the last commit before merging, but the astropy bot didn't have time to catch the inconsistency in the milestone. I'm going to change the milestone to 3.1 as I don't really feel strongly that this really should be backported and these are non-trivial changes.

astrofrog added Enhancement Work in progress visualization.wcsaxes labels Feb 19, 2018

astrofrog force-pushed the smart-format-wcsaxes branch from 5c9d547 to 0741eef Compare February 20, 2018 08:24

astrofrog added Ready-for-final-review and removed Work in progress labels Feb 20, 2018

astrofrog added this to the v3.0.1 milestone Feb 20, 2018

astrofrog requested review from lpsinger and larrybradley February 20, 2018 13:57

lpsinger reviewed Feb 20, 2018

View reviewed changes

astrofrog removed the Enhancement label Feb 21, 2018

larrybradley reviewed Feb 21, 2018

View reviewed changes

larrybradley added 💤 merge-when-ci-passes Do not use: We have auto-merge option now. and removed Ready-for-final-review labels Feb 26, 2018

larrybradley approved these changes Feb 26, 2018

View reviewed changes

lpsinger approved these changes Feb 26, 2018

View reviewed changes

astrofrog force-pushed the smart-format-wcsaxes branch from 8a7eccf to a9966bb Compare March 6, 2018 10:08

drdavella force-pushed the smart-format-wcsaxes branch from a9966bb to 1f2075e Compare March 12, 2018 16:34

astrofrog added 2 commits March 12, 2018 12:34

Make it so that WCS coordinates with RA are formatted in hours by def…

e7745b0

…ault. This also generalizes the AngleFormatterLocator so that it can take default units as well as having a way to specify to use decimal formatting by default.

Fix LaTeX formatting and distinguish between intrinsic units and disp…

715876e

…lay units

astrofrog added 11 commits March 12, 2018 12:34

Rename option to display_unit

2dd3919

Rename to format_unit

fd00563

Fix determination of interval and logic in AngleFormatterLocator init…

7d09b36

…ializer

Fix backward-compatibility with previous reference images

2bcac5f

Fix tests

042af74

Added unit tests for labels including hms formatting and LaTeX typese…

ad27f13

…tting

CircleCI: update version of docker images

7376619

Rename plain= option to format=

508100b

Added changelog entry

eb8e3ef

Expand comments, and allow set_format_unit and format_unit to…

a001ac1

… take string units

Minor formatting fixes

32f6038

astrofrog commented Mar 12, 2018

View reviewed changes

bsipocz modified the milestones: v3.0.1, v3.0.2 Mar 12, 2018

astrofrog mentioned this pull request Mar 21, 2018

Right ascension tick label formatting should default to hh:mm:ss glue-viz/glue#1432

Closed

astrofrog force-pushed the smart-format-wcsaxes branch from 1f2075e to 32f6038 Compare March 21, 2018 17:54

Moved changelog entry

7355dcf

astrofrog merged commit 99de28b into astropy:master Mar 21, 2018

This was referenced Mar 21, 2018

Improve formatting of decimal labels in WCSAxes #7318

Merged

Have x-axis in image viewers default to hh:mm:ss when that axis is RA spacetelescope/cubeviz#269

Closed

astrofrog modified the milestones: v3.0.2, v3.1 Mar 27, 2018

bsipocz mentioned this pull request Oct 5, 2018

Use raw strings for latex formatting in test #7788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smart formatting of tick labels in WCSAxes #7215

Smart formatting of tick labels in WCSAxes #7215

astrofrog commented Feb 19, 2018 •

edited

Loading

astropy-bot bot commented Feb 19, 2018 •

edited

Loading

Cadair commented Feb 20, 2018

astrofrog commented Feb 20, 2018

Cadair commented Feb 20, 2018

astrofrog commented Feb 20, 2018

lpsinger left a comment

lpsinger Feb 20, 2018

astrofrog Feb 25, 2018

lpsinger Feb 26, 2018

lpsinger Feb 20, 2018

astrofrog Feb 25, 2018

astrofrog commented Feb 20, 2018

bsipocz commented Feb 20, 2018

astrofrog commented Feb 21, 2018

bsipocz commented Feb 21, 2018

larrybradley left a comment •

edited

Loading

larrybradley Feb 21, 2018

astrofrog Feb 25, 2018

larrybradley Feb 21, 2018

astrofrog Feb 25, 2018

larrybradley commented Feb 26, 2018

drdavella commented Mar 12, 2018

astrofrog commented Mar 12, 2018

astrofrog Mar 12, 2018

astrofrog commented Mar 21, 2018

astrofrog commented Mar 27, 2018

Smart formatting of tick labels in WCSAxes #7215

Smart formatting of tick labels in WCSAxes #7215

Conversation

astrofrog commented Feb 19, 2018 • edited Loading

astropy-bot bot commented Feb 19, 2018 • edited Loading

Cadair commented Feb 20, 2018

astrofrog commented Feb 20, 2018

Cadair commented Feb 20, 2018

astrofrog commented Feb 20, 2018

lpsinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astrofrog commented Feb 20, 2018

bsipocz commented Feb 20, 2018

astrofrog commented Feb 21, 2018

bsipocz commented Feb 21, 2018

larrybradley left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larrybradley commented Feb 26, 2018

drdavella commented Mar 12, 2018

astrofrog commented Mar 12, 2018

Choose a reason for hiding this comment

astrofrog commented Mar 21, 2018

astrofrog commented Mar 27, 2018

astrofrog commented Feb 19, 2018 •

edited

Loading

astropy-bot bot commented Feb 19, 2018 •

edited

Loading

larrybradley left a comment •

edited

Loading