Add inline grid meridian/parallel labeling #1089

ahuang11 · 2018-07-15T20:33:54Z

Work in progress after the newest merge; I know I haven't fully updated the tests! (Ignore the red s, I was using that as a reference)

lib/cartopy/mpl/gridliner.py

ahuang11 · 2018-07-15T20:39:20Z

Maybe shouldn't remove that _assert_can_draw_ticks; instead, add an additional keyword arg to ax.gridlines(draw_labels_inline=True)~

lib/cartopy/mpl/gridliner.py

ahuang11 · 2018-07-16T06:03:34Z

Noticed some projections might be a bit off too
Half of the temperatures in US seems to be wrong:

England is all dark red in one and light red in the other

lib/cartopy/mpl/gridliner.py

QuLogic

In general, I like the idea, but it needs a little bit of clarification in some places, and it definitely needs tests.

lib/cartopy/mpl/geoaxes.py

lib/cartopy/mpl/gridliner.py

QuLogic · 2018-10-28T06:02:07Z

lib/cartopy/mpl/gridliner.py

+            alpha = 1
+        else:
+            label_transform = cartopy.crs.PlateCarree()
+            alpha = 0.7


I'm not sure if we want to be changing transparency. Best would be if the line were cut under the label, like contour lines, but this is somewhat more difficult to do. But maybe using the contour machinery would help.

Will have to revisit.

lib/cartopy/mpl/gridliner.py

ahuang11

Thanks all for the review! I'll have to revisit some of the comments sometime later, so if anyone is interested in making their own PR based on this one, feel free!

lib/cartopy/mpl/gridliner.py

ahuang11 · 2018-10-29T04:59:47Z

lib/cartopy/mpl/gridliner.py

+            alpha = 1
+        else:
+            label_transform = cartopy.crs.PlateCarree()
+            alpha = 0.7


Will have to revisit.

lib/cartopy/mpl/gridliner.py

kaedonkers · 2018-11-05T13:51:13Z

Thank you for the contribution @ahuang11! Could you please also sign a CLA agreement, following the guidance in the SciTools documentation.

corinnebosley · 2018-12-04T11:35:00Z

Hi @ahuang11, this PR is really valuable and I'd like to move it forward if you're still in a position to work on it.

If you could fill out this CLA form and return it to us at [email protected] then we can get this PR moving again, and I'd be very happy.

docs/source/conf.py

lib/cartopy/tests/mpl/test_feature_artist.py

lib/cartopy/tests/mpl/test_gridliner.py

ahuang11 · 2018-12-05T06:09:36Z

Hmm I screwed up the pull... Can anyone advise on how to rebase / remove all the commits from others?

Edit: Nevermind; I think I fixed it.

…th v/h_align, better auto prevention of overlap and cramping, added more outer grid line projections

…e grid labels

…ding documentation)

ahuang11 · 2018-12-20T04:11:45Z

Thanks for the commands @stefraynaud; that seemed to work. I might have been using conda-forge earlier so it wasn't working.

corinnebosley · 2019-02-05T10:58:56Z

Hi @ahuang11, I've merged #1117 now, so if you could rebase this when you get time then we'll try and get things moving again.

Conflicts: lib/cartopy/mpl/geoaxes.py lib/cartopy/mpl/gridliner.py lib/cartopy/tests/mpl/baseline_images/mpl/test_gridliner/gridliner_labels.png lib/cartopy/tests/mpl/baseline_images/mpl/test_gridliner/gridliner_labels_1.5.png lib/cartopy/tests/mpl/test_gridliner.py

stickler-ci · 2019-02-07T06:23:56Z

lib/cartopy/mpl/gridliner.py

+            lq = 25
+            uq = 75
+        midpoints = (self._round(np.percentile(lim, lq), cent),
+                       self._round(np.percentile(lim, uq), cent))


E127 continuation line over-indented for visual indent

stickler-ci · 2019-02-07T06:23:57Z

lib/cartopy/mpl/gridliner.py

@@ -504,55 +526,125 @@ def _draw_gridliner(self, nx=None, ny=None, background_patch=None,

                    # Loop on head and tail and plot label by extrapolation
                    for tail, head in zip(tails, heads):
-                        for pt0, pt1 in [tail, head]:
+                        for i ,(pt0, pt1) in enumerate([tail, head]):


E203 whitespace before ','
E231 missing whitespace after ','

stickler-ci · 2019-02-07T06:23:57Z

lib/cartopy/mpl/gridliner.py

        return kw, angle, loc

-    def _segment_angle_to_text_specs(self, angle):
+    def _text_angle_to_specs_(self, angle, lonlat):
+        """Get appropriate kwargs for a rotated label from its angle in degrees"""


E501 line too long (82 > 79 characters)

stickler-ci · 2019-02-07T06:23:57Z

lib/cartopy/mpl/gridliner.py

+            angle -= 360
+
+        if ((self.x_inline and lonlat == 'lon') or
+            (self.y_inline and lonlat == 'lat')):


E129 visually indented line with same indent as next logical line

stickler-ci · 2019-02-07T06:23:57Z

lib/cartopy/mpl/gridliner.py

-                                       x=dx, y=dy, units='dots')
-        kw.update(transform=transform)
+        if ((self.x_inline and lonlat == 'lon') or
+            (self.y_inline and lonlat == 'lat')):


E129 visually indented line with same indent as next logical line

stickler-ci · 2019-02-07T06:23:59Z

lib/cartopy/tests/mpl/test_gridliner.py

    ax = plt.subplot(3, 2, 4, projection=crs_pc)
+>>>>>>> 45d0479abed27f73c5bb8bac35036772a3faa773


E225 missing whitespace around operator

stickler-ci · 2019-02-07T06:23:59Z

lib/cartopy/tests/mpl/test_gridliner.py

@@ -186,5 +240,46 @@ def test_grid_labels():
    gl.xlabel_style = gl.ylabel_style = {'size': 9}

    # Increase margins between plots to stop them bumping into one another.
+<<<<<<< HEAD


E227 missing whitespace around bitwise or shift operator
E225 missing whitespace around operator

lib/cartopy/tests/mpl/test_gridliner.py

stickler-ci · 2019-02-07T06:23:59Z

lib/cartopy/tests/mpl/test_gridliner.py

+        ax.set_title(proj, y=1.08)
+        ax.gridlines(draw_labels=True, auto_inline=True, clip_on=True)
+        ax.coastlines()
+=======


E305 expected 2 blank lines after class or function definition, found 0
E225 missing whitespace around operator

stickler-ci · 2019-02-07T06:23:59Z

lib/cartopy/tests/mpl/test_gridliner.py

    plt.subplots_adjust(wspace=0.25, hspace=0.25, top=.98, left=.07,
                        bottom=0.02, right=0.93)
+>>>>>>> 45d0479abed27f73c5bb8bac35036772a3faa773


E225 missing whitespace around operator

ahuang11 · 2019-02-07T06:27:03Z

Hi @ahuang11, I've merged #1117 now, so if you could rebase this when you get time then we'll try and get things moving again.

Okay halfway there~

lib/cartopy/mpl/geoaxes.py

stefraynaud · 2019-02-07T13:14:42Z

That's really nice with the addition of inline labels!

corinnebosley · 2019-02-21T13:18:13Z

Hi @ahuang11, I'm still looking into the travis errors that are occurring at build time here, but it's bothering me that the SciTools CLA-checker isn't recognising you any more, despite the fact that I remember CLAing you myself.

I think what's happened is that one of my more knowledgeable and capable colleagues has moved elsewhere in the business, but not before making a whole load of modifications to SciTools which has broken a lot of links in chains.

You should be able to re-add yourself to the CLA list by filling in this form: https://docs.google.com/forms/d/e/1FAIpQLSfd0tdE-DcJOXh8ej_7T93IizwJFYBFyRWYQOi2A8QRaKwykA/viewform

Please ping me once you've done that, I'll need to think about what to do next.

ahuang11 · 2019-02-22T02:21:52Z

@corinnebosley Done.

corinnebosley · 2019-02-22T08:58:39Z

Thanks @ahuang11, unfortunately it looks like the form isn't working. I'll continue to investigate both issues and tell you when I've found something.

corinnebosley · 2019-02-22T10:36:24Z

@ahuang11 My mistake, apparently the form is working and you are now on two whole lists (I'm not sure why we now have two or where they live, but you are on them both).

I will continue to look into the test fails. I'm still pretty confused. I'll keep you posted.

xylar · 2019-04-24T06:35:58Z

Any headway here? This would be such a great capability to have and it's one of the few missing features keeping my group from switching from basemap to cartopy for all our plotting.

corinnebosley · 2019-04-29T14:52:55Z

@ahuang11 @xylar I'm so sorry for dragging this on; we're very short-staffed at the moment but I'm trying to squeeze this in where I can.

I am a bit concerned that this test failure has several causes, but it may be possible to get it passing by adding the new image to the cartopy image repository. Or by running idiff and accepting the image differences.

Before we do that though (and by 'we' I think I mean 'you' (@ahuang11)), I need to really dig into this error to make sure this is the right action to take. It's possible that the tests that pass might be supposed to fail and actually our tests just aren't working; in which case it would be wrong to just add an image to the repo.

I will post updates when I know more.

xylar · 2019-04-29T17:06:30Z

@corinnebosley, thanks so much for the update. I know we're all volunteers here and I appreciate this work and the time you all put into it.

ahuang11 · 2019-04-30T03:19:45Z

Okay, I think this time, the test isn't failing because the images are different, but rather it's failing to build cartopy.

    In file included from lib/cartopy/trace.cpp:655:0:
    /home/travis/miniconda/envs/test-environment/include/proj_api.h:37:2: error: #error 'To use the proj_api.h you must define the macro ACCEPT_USE_OF_DEPRECATED_PROJ_API_H'
     #error 'To use the proj_api.h you must define the macro ACCEPT_USE_OF_DEPRECATED_PROJ_API_H'
      ^
    error: command 'gcc' failed with exit status 1
    ----------------------------------------
ERROR: Command "/home/travis/miniconda/envs/test-environment/bin/python -u -c 'import setuptools, tokenize;__file__='"'"'/tmp/pip-req-build-x7g9t3dn/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-lrfw1y7w/install-record.txt --single-version-externally-managed --compile" failed with error code 1 in /tmp/pip-req-build-x7g9t3dn/
The command "pip install --no-deps ." failed and exited with 1 during .

corinnebosley · 2019-05-02T10:53:16Z

@ahuang11 You're quite right, the cartopy build itself is now failing due to updates to proj4. My colleague is looking into this and either she or I will put up a fix when we understand what cython's going to do with it a little better.

For now though, all your tests are now passing (thanks for that!) and I want to give this another review before going ahead.

corinnebosley · 2019-05-02T12:18:59Z

Hey @ahuang11!

I'm mostly very happy, but I'm slightly concerned by the images produced by your inline grid label machine for the USA. What worries me is the difference in text angles for labels which look like they should be at the same angle, shown on the Geostationary and Orthographic projections.

I know it's even more work, but could you please add some tests to check that the text angle is being calculated correctly?

It principle, it doesn't really matter if some plots just look ugly when constructed in a certain way (i.e. with inline grid labels); you just wouldn't make your plot with that argument. However, if there genuinely is something being calculated incorrectly that's a different matter and I shouldn't merge it yet.

A few basic tests for the numbers themselves as opposed to images should do it I reckon.

ahuang11 · 2019-05-03T04:35:22Z

If I'm understanding correctly, you're saying that the 80W should be the same angle?

I have checked out the current master branch of Cartopy and was able to replicate it because those aren't actually "inline" grid labels for Geostationary and Orthographic projections for the USA--those are labels from #1117

I have the default set to auto_inline=True which by default doesn't do an inline grid label for Orthographic / Geostationary (it has to be in _X_INLINE_PROJS or _POLAR_PROJS https://github.com/SciTools/cartopy/pull/1089/files#diff-ce4cd585b732c7d15594d639a4054cfdR42)

This is what it would look like if it's forced to be inline (the angle will be set to 0).

Unfortunately, I did not write the angling logic and I don't have much of a clue about it (not sure if it's actually being calculated correctly or not).

corinnebosley · 2019-05-03T08:29:11Z

@ahuang11 I see. On that basis (the basis that I merged #1117 and was happy with the logic and testing), LGTM!

corinnebosley · 2019-05-03T08:29:36Z

Thanks @ahuang11, nice work!

milenaveneziani · 2019-09-27T18:13:17Z

Hi all, thanks for putting this in! Do you have a rough idea of when the next release will be, with this feature in? Thanks!

dopplershift · 2019-09-27T19:23:22Z

This will be in the next release, 0.18. I'm hoping start pushing on that in a week or so, but I'm hesitant to give a firm timeline on when the release will be out.

SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Jul 15, 2018

stickler-ci reviewed Jul 15, 2018

View reviewed changes

stickler-ci reviewed Jul 16, 2018

View reviewed changes

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

stickler-ci reviewed Jul 16, 2018

View reviewed changes

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

dopplershift reviewed Aug 10, 2018

View reviewed changes

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

stefraynaud mentioned this pull request Sep 20, 2018

Add support for all projections to lon/lat labelling #1117

Merged

kaedonkers reviewed Sep 28, 2018

View reviewed changes

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

aymeric-spiga mentioned this pull request Oct 11, 2018

use of cartopy aymeric-spiga/planetoplot#13

Open

QuLogic reviewed Oct 28, 2018

View reviewed changes

QuLogic added Type: Enhancement Experience-needed: medium Component: Gridlines labels Oct 28, 2018

QuLogic added this to the Medium term milestone Oct 28, 2018

stickler-ci reviewed Oct 29, 2018

View reviewed changes

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

lib/cartopy/mpl/gridliner.py Outdated Show resolved Hide resolved

ahuang11 commented Oct 29, 2018

View reviewed changes

xylar mentioned this pull request Nov 21, 2018

Switch from basemap to cartopy MPAS-Dev/MPAS-Analysis#490

Closed

3 tasks

stickler-ci reviewed Dec 5, 2018

View reviewed changes

ahuang11 added 8 commits December 4, 2018 22:17

Add inline labels for all projections

b54f0e3

Add inline arg, differentiate between meridian and parallel labels wi…

4cb4d01

…th v/h_align, better auto prevention of overlap and cramping, added more outer grid line projections

Fix flake8

7a7807a

Skip -180 in rotated pole to prevent overlaying

010eccc

Fix stickler

052c5f1

Add auto keyword that will decide whether to use out of line or inlin…

4ff3998

…e grid labels

Revise based on feedback (changing kwarg order, fixing poor logic, ad…

a788c76

…ding documentation)

Rename _SUPPORTED_PROJS

caed560

corinnebosley modified the milestones: Medium term, 0.18 Dec 18, 2018

Fix 1.5 tests

365ddea

stickler-ci reviewed Feb 7, 2019

View reviewed changes

stefraynaud reviewed Feb 7, 2019

View reviewed changes

lib/cartopy/mpl/geoaxes.py Show resolved Hide resolved

ahuang11 added 2 commits February 7, 2019 22:06

Update tests, fix some bugs, stickler

c2578f1

Fix test

2880180

Fix tests maybe

9779616

corinnebosley merged commit 1942c7b into SciTools:master May 3, 2019

friedrichknuth mentioned this pull request May 14, 2020

Label lat lon gridlines parallel to intersection angle at plot boundary #1560

Closed

		ax = plt.subplot(3, 2, 4, projection=crs_pc)
		>>>>>>> 45d0479abed27f73c5bb8bac35036772a3faa773

Add inline grid meridian/parallel labeling #1089

Add inline grid meridian/parallel labeling #1089

Conversation

ahuang11 commented Jul 15, 2018 • edited Loading

ahuang11 commented Jul 15, 2018 • edited Loading

ahuang11 commented Jul 16, 2018

QuLogic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahuang11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaedonkers commented Nov 5, 2018

corinnebosley commented Dec 4, 2018

ahuang11 commented Dec 5, 2018 • edited Loading

ahuang11 commented Dec 20, 2018

corinnebosley commented Feb 5, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahuang11 commented Feb 7, 2019

stefraynaud commented Feb 7, 2019

corinnebosley commented Feb 21, 2019

ahuang11 commented Feb 22, 2019

corinnebosley commented Feb 22, 2019

corinnebosley commented Feb 22, 2019

xylar commented Apr 24, 2019

corinnebosley commented Apr 29, 2019

xylar commented Apr 29, 2019

ahuang11 commented Apr 30, 2019 • edited Loading

corinnebosley commented May 2, 2019

corinnebosley commented May 2, 2019

ahuang11 commented May 3, 2019 • edited Loading

corinnebosley commented May 3, 2019

corinnebosley commented May 3, 2019

milenaveneziani commented Sep 27, 2019

dopplershift commented Sep 27, 2019

ahuang11 commented Jul 15, 2018 •

edited

Loading

ahuang11 commented Jul 15, 2018 •

edited

Loading

ahuang11 commented Dec 5, 2018 •

edited

Loading

ahuang11 commented Apr 30, 2019 •

edited

Loading

ahuang11 commented May 3, 2019 •

edited

Loading