Feature labels on top of map #1666

eirik-mortensen · 2020-10-15T06:43:22Z

Introduction

This pull request is based on the issue #1642: Add capability for non-inline grid labels to appear inside map

Rationale

As a satellite image provider I need to minimize the size of images and meet the customers requirements on how products shall be presented.

Implications

With this change all non-inline labels will be shown on top of the map and not outside. This occurs if x-, y- or both padding are negative. All labels that is shown outside the map is removed.

Input

top = 49.3457868  # north lat
left = -124.7844079  # west long
right = -66.9513812  # east long
bottom = 24.7433195  # south lat
plt.figure(figsize=(35, 35))
for i, proj in enumerate(TEST_PROJS, 1):
    if isinstance(proj, tuple):
       proj, kwargs = proj
    else:
       kwargs = {}
        ax = plt.subplot(7, 4, i, projection=proj(**kwargs))
        try:
            ax.set_extent([left, right, bottom, top], crs=ccrs.PlateCarree())
        except Exception:
            pass
        ax.set_title(proj, y=1.075)
        if ccrs.PROJ4_VERSION[:2] == (5, 0) and proj in (
            ccrs.Orthographic,
            ccrs.AlbersEqualArea,
            ccrs.Geostationary,
            ccrs.NearsidePerspective,
        ):
            # Above projections are broken, so skip labels.
            # Add gridlines anyway to minimize image differences.
            ax.gridlines()
        else:
            gl = ax.gridlines(draw_labels=True, clip_on=True)
            gl.xpadding = -5
            gl.ypadding = -5
        ax.coastlines(resolution="110m")
plt.subplots_adjust(wspace=0.35, hspace=0.35)

Code is the same as in the unit test.

Output

Note

I cant seem to figure out why a couple of the numbers in Mollweide, Sinusoidal and InterruptedGoodeHomolosine do intersect with the map bbox.
Unit test provided
All test passes

lib/cartopy/mpl/gridliner.py

lib/cartopy/tests/mpl/test_gridliner.py

stickler-ci · 2020-10-15T06:43:29Z

lib/cartopy/tests/mpl/test_gridliner.py

@@ -365,3 +363,39 @@ def test_gridliner_line_limits():
    for path in paths:
        assert (np.min(path.vertices, axis=0) >= (xlim[0], ylim[0])).all()
        assert (np.max(path.vertices, axis=0) <= (xlim[1], ylim[1])).all()
+
+@pytest.mark.natural_earth


E302 expected 2 blank lines, found 1

lib/cartopy/mpl/gridliner.py

lib/cartopy/tests/mpl/test_gridliner.py

stefraynaud · 2020-10-15T07:27:08Z

In think a special keyword like ìnsidemust be added toGridliner.initto activate this behavior of labels, and considerself.xpaddingandself.ypadding` as always positive.
I added this feature to my local version of PR #1537, which is finally a major revision of the labelling system that solves numerous issues.

eirik-mortensen · 2020-10-15T07:49:31Z

Ok. So you have implemented this functionality in your branch stefraynaud:bug_label_visibility? I also need the feature where labels outside of map is removed completely. Since i use bbox_inches="tight", pad_inches=0, to remove borders when saving images output.

What is your suggestion on how we proceed?

eirik-mortensen · 2020-10-16T06:30:17Z

@stefraynaud, I will just use my local solution until you have rolled out your PRs. You have a timeline for the next release?

lib/cartopy/mpl/gridliner.py

stickler-ci · 2020-10-16T07:42:45Z

lib/cartopy/tests/mpl/test_gridliner.py

+            gl.ypadding = -5
+        ax.coastlines(resolution="110m")
+    plt.subplots_adjust(wspace=0.35, hspace=0.35)
+


W391 blank line at end of file

stefraynaud · 2020-10-16T09:12:41Z

@Emsterr Except a few tests I must add, I mostly dépends on the CI success and on the review process.

stefraynaud · 2020-10-19T07:17:38Z

@Emsterr You should push forward this PR to make it merged as soon as possible.

eirik-mortensen · 2020-10-19T08:16:25Z

@stefraynaud sorry I'm a bit confused, what do you mean 'push forward'? isn't it ready to be merged?

greglucas · 2020-10-21T14:32:59Z

lib/cartopy/mpl/gridliner.py

-                    # Non-inline must not run through the outline.
-                    elif not outline_path.intersects_path(this_path):
+                    elif not outline_path.intersects_path(this_path,
+                                                          filled=is_filled):


What about adding the boolean tests directly into the argument here instead of creating another variable?

greglucas · 2020-10-21T14:34:01Z

lib/cartopy/tests/mpl/test_gridliner.py

+
+
+@pytest.mark.natural_earth
+@ImageTesting([grid_label_intersect_map_image],


Do we need another 1 MB image comparison test? Or can this be checked by comparing against the positions of the labels and whether they are visible or not?

Or maybe just a single smaller image would be fine.

Yes that could work. I'll make some new test tomorrow.

stickler-ci · 2020-12-22T12:31:24Z

lib/cartopy/mpl/geoaxes.py

                img[:, :, 0:3] = old_img
                # Put an alpha channel in if the image was masked.
                if not np.any(alpha):
                    alpha = 1
                img[:, :, 3] = np.ma.filled(alpha, fill_value=0) * \
-                    (~np.any(old_img.mask, axis=2))
+                               (~np.any(old_img.mask, axis=2))


E127 continuation line over-indented for visual indent

lib/cartopy/mpl/gridliner.py

EirikMortensen · 2020-12-22T12:50:16Z

@greglucas i did some changes based on stefraynaud feedback.

The user should only operate padding values as positive and adjust labels inside with the class input parameter.

However, in the class the _x/_ypadding is set to negative if labels_inside is true. The same goes for the initial value _x/_ypadding.

greglucas

I'm a little confused now. Why can't we allow negative values in the padding? What if I want x labels inside, but ylabels outside the plot? I guess it seems to me that if I want inside_labels, then I should add a negative padding. So, then in this code I assume it you could actually do the test
if xpadding < 0 then inside_x_labels = True and make all your adjustments later based on that? So, rather than adding a new keyword argument, we could just implicitly determine that based on whether the padding arguments are positive or negative?

eirik-mortensen · 2021-01-11T10:36:43Z

@greglucas i do agree with you. As you mention in your comment, I inital implemented this with the possibility to operate with negative input values, but as stefraynaud sugested (and had implemented in his solution) i changed it.

I will revert to the previous implementation and remove the labels_inside input variable.

As you mention, Regarding the x and y padding, What if I want x labels inside, but ylabels outside the plot?, should the padding top/bottom and left/right be adjusted as one, or is it a solution to seperate all of them. Example where you want bottom to be inside but not top.

x_top_padding, x_bottom_padding...

It would give more freedom, but, I think its a bit cumbersome and messy to seperate all sides.

Suggestions?

stefraynaud · 2021-01-11T17:30:11Z

I think that we code that is compute the effective padding must revisited to reach the requested behavior:
https://github.com/emsterr/cartopy/blob/e0a05d8bd4082474bcbcbf024a2d57312188edd2/lib/cartopy/mpl/gridliner.py#L736

dx = ypadding * np.cos(angle * np.pi / 180)
dy = xpadding * np.sin(angle * np.pi / 180)

Only the xpadding must be used as a distance for xlabels and only the ypadding must be used as a distance for ylabels.

# For xlabels
dx = xpadding * np.cos(angle * np.pi / 180)
dy = xpadding * np.sin(angle * np.pi / 180)

# For ylabels
dx = ypadding * np.cos(angle * np.pi / 180)
dy = ypadding * np.sin(angle * np.pi / 180)

I also have to take this into account in my PR.
My two cents.

eirik-mortensen · 2021-01-12T08:27:33Z

@stefraynaud, with the implementation i had where negative padding was possible, the behaviour was as expected.

What will differ with this implementation?

# For xlabels
dx = xpadding * np.cos(angle * np.pi / 180)
dy = xpadding * np.sin(angle * np.pi / 180)

# For ylabels
dx = ypadding * np.cos(angle * np.pi / 180)
dy = ypadding * np.sin(angle * np.pi / 180)

stefraynaud · 2021-01-12T08:39:00Z

@Emsterr I have trouble to find the implementation you had and I suspect that I misunderstood the meaning of x and y padding. So sorry for the noise and I will also update my PR accordingly.

eirik-mortensen · 2021-01-12T08:45:01Z

No worries, i squased my commits to the later implementation, so its not in the branch. I will implement the former solution again.

… labels on top of map

stickler-ci · 2021-01-13T14:22:48Z

lib/cartopy/tests/mpl/test_gridliner.py

+    assert gl.xpadding == 10 and gl.ypadding == 10
+    gl.xpadding = gl.ypadding = -5
+    assert gl.xpadding == -5 and gl.ypadding == -5
+


W391 blank line at end of file

… labels on top of map

eirik-mortensen · 2021-04-27T06:05:39Z

@stefraynaud God morning. Did you get a PR regarding negative padding into v0.19?

stefraynaud · 2021-04-27T07:23:28Z

Hi @Emsterr my PR includes it and I'm about to update the baseline images once my ssh connexion is allowed.
However, I'm not sure it's gonna be included in 0.19.

eirik-mortensen · 2021-04-27T10:36:30Z

@stefraynaud
Ok, from my perspective it would be good to get either yours or mine PR into v0.19 so i can remove my fork that is used by my software. Dont think i need to change to much if your version is merged.

As long as one of the PRs gets in i can take my shoulders down.

@greglucas, whats your thought on this?

smartlixx · 2021-04-27T11:45:41Z

V0.19 has been released. You should aim at V0.20.

eirik-mortensen · 2021-04-27T13:12:19Z

that's unfortunate. Hopefully it will take a shorter amount of time from v0.19 to v0.20 as from v0.18 to v0.19.

@greglucas Are there any plans on increasing the amount of releases in the near future?

greglucas · 2021-04-27T16:05:04Z

Yeah, I'd like to get to about a 6 month release schedule to more closely track other package's nominal release schedule as well. But, it all depends on volunteer hours submitting and reviewing PRs and we can't guarantee anything for the schedules.

QuLogic · 2021-09-10T02:51:48Z

Closed by #1537

QuLogic · 2021-09-10T02:55:56Z

OK, I'm not actually sure whether this really is replaced by #1537, but @stefraynaud included it in the list.

stefraynaud · 2021-09-13T08:30:47Z

@QuLogic I confirm: a negative padding should do the trick.

stickler-ci reviewed Oct 15, 2020

View reviewed changes

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

lib/cartopy/tests/mpl/test_gridliner.py Outdated Show resolved Hide resolved

stickler-ci reviewed Oct 16, 2020

View reviewed changes

greglucas reviewed Oct 21, 2020

View reviewed changes

greglucas added Component: Gridlines Type: Enhancement labels Oct 21, 2020

eirik-mortensen force-pushed the feature_labels_on_top_of_map branch from 3421c0e to 0ee38fe Compare December 22, 2020 12:31

stickler-ci reviewed Dec 22, 2020

View reviewed changes

eirik-mortensen force-pushed the feature_labels_on_top_of_map branch from 0ee38fe to e0a05d8 Compare December 22, 2020 12:40

greglucas reviewed Dec 24, 2020

View reviewed changes

eirik-mortensen added a commit to eirik-mortensen/cartopy that referenced this pull request Jan 13, 2021

PR SciTools#1666 - Negative padding values will now result in drawing…

175570c

… labels on top of map

eirik-mortensen force-pushed the feature_labels_on_top_of_map branch from e0a05d8 to 175570c Compare January 13, 2021 14:22

stickler-ci reviewed Jan 13, 2021

View reviewed changes

PR SciTools#1666 - Negative padding values will now result in drawing…

a21ec88

… labels on top of map

eirik-mortensen force-pushed the feature_labels_on_top_of_map branch from 175570c to a21ec88 Compare January 13, 2021 14:25

stefraynaud mentioned this pull request May 10, 2021

Labels visibility from side spines (fix #1530) #1537

Merged

QuLogic added this to the 0.20 milestone Sep 10, 2021

QuLogic closed this Sep 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature labels on top of map #1666

Feature labels on top of map #1666

eirik-mortensen commented Oct 15, 2020 •

edited

Loading

stickler-ci Oct 15, 2020

stefraynaud commented Oct 15, 2020

eirik-mortensen commented Oct 15, 2020

eirik-mortensen commented Oct 16, 2020

stickler-ci Oct 16, 2020

stefraynaud commented Oct 16, 2020

stefraynaud commented Oct 19, 2020

eirik-mortensen commented Oct 19, 2020

greglucas Oct 21, 2020

greglucas Oct 21, 2020

EirikMortensen Oct 21, 2020

stickler-ci Dec 22, 2020

EirikMortensen commented Dec 22, 2020 •

edited

Loading

greglucas left a comment

eirik-mortensen commented Jan 11, 2021

stefraynaud commented Jan 11, 2021

eirik-mortensen commented Jan 12, 2021

stefraynaud commented Jan 12, 2021

eirik-mortensen commented Jan 12, 2021

stickler-ci Jan 13, 2021

eirik-mortensen commented Apr 27, 2021

stefraynaud commented Apr 27, 2021

eirik-mortensen commented Apr 27, 2021

smartlixx commented Apr 27, 2021

eirik-mortensen commented Apr 27, 2021

greglucas commented Apr 27, 2021

QuLogic commented Sep 10, 2021

QuLogic commented Sep 10, 2021

stefraynaud commented Sep 13, 2021



		@pytest.mark.natural_earth
		@ImageTesting([grid_label_intersect_map_image],

Feature labels on top of map #1666

Feature labels on top of map #1666

Conversation

eirik-mortensen commented Oct 15, 2020 • edited Loading

Introduction

Rationale

Implications

Input

Output

Note

stickler-ci Oct 15, 2020

Choose a reason for hiding this comment

stefraynaud commented Oct 15, 2020

eirik-mortensen commented Oct 15, 2020

eirik-mortensen commented Oct 16, 2020

stickler-ci Oct 16, 2020

Choose a reason for hiding this comment

stefraynaud commented Oct 16, 2020

stefraynaud commented Oct 19, 2020

eirik-mortensen commented Oct 19, 2020

greglucas Oct 21, 2020

Choose a reason for hiding this comment

greglucas Oct 21, 2020

Choose a reason for hiding this comment

EirikMortensen Oct 21, 2020

Choose a reason for hiding this comment

stickler-ci Dec 22, 2020

Choose a reason for hiding this comment

EirikMortensen commented Dec 22, 2020 • edited Loading

greglucas left a comment

Choose a reason for hiding this comment

eirik-mortensen commented Jan 11, 2021

stefraynaud commented Jan 11, 2021

eirik-mortensen commented Jan 12, 2021

stefraynaud commented Jan 12, 2021

eirik-mortensen commented Jan 12, 2021

stickler-ci Jan 13, 2021

Choose a reason for hiding this comment

eirik-mortensen commented Apr 27, 2021

stefraynaud commented Apr 27, 2021

eirik-mortensen commented Apr 27, 2021

smartlixx commented Apr 27, 2021

eirik-mortensen commented Apr 27, 2021

greglucas commented Apr 27, 2021

QuLogic commented Sep 10, 2021

QuLogic commented Sep 10, 2021

stefraynaud commented Sep 13, 2021

eirik-mortensen commented Oct 15, 2020 •

edited

Loading

EirikMortensen commented Dec 22, 2020 •

edited

Loading