Quick patch to bypass scale validation in new Cartopy

[jthielen]

Description Of Changes

Quick patch of the __init__ method of MetPyMapFeature to fix the breakage caused by the scale validation added in Cartopy 0.18 to NaturalEarthFeature. Not a long term fix, since it assumes an exact set of attributes for NaturalEarthFeature and particular behavior of NaturalEarthFeature's parent class Feature. A more robust fix will be discussed for 1.0 in #1368 / #1370.

[lgtm-com]

This pull request introduces 1 alert when merging a4aeb54 into c3df3bc - view on LGTM.com

new alerts:

• 1 for First argument to super() is not enclosing class

[kgoebber]

Investigating the test failures has led to the following:

• The failure of test_barb_projection' and test_arrow_projectionis due to a change in the defaults of addinggridlines`. The test specifies the xlocs but not the ylocs of the gridlines. My suggestion is to update the baseline images for these tests and put in set ylocs.
• The failure of test_colorfill, test_colorfill_horiz_colorbar, and test_colorfill_no_colorbar is due to the Adaptive_Scaler choosing a different resolution of data. Don't know what to suggest here, but I think updating the baseline will be what is needed, even for a 1.0. I don't think the adaptive scaler was working properly before.
• The rest of the declarative failures is related to it no longer clipping to the axes. Interestingly the barbs are clipping fine, but not the symbols or parameters. Also, the clipping test for the station plot itself is not failing. I have not yet been able to pin down the root cause of the clipping not occurring within the declarative syntax. Will keep looking/searching for potential solutions.

[kgoebber]

Okay, as best as I can figure out at this point, it seems to be something with the Cartopy 0.18 update related to GeoSpine. This change in Cartopy deals with switching to a new way to handle outline_patch stuff. In this section of code there is a default clip_on gets set to False and even though someone sets it differently, it doesn't appear to pass through.

I also drafted some test code and if you don't set a projection, the clip_on setting works, once you add a projection then it no longer clips. Testing this same code on Cartopy 0.17 works for both instances. So it appears to be Cartopy, but not absolutely sure what or where this is being caused.

import cartopy.crs as ccrs
import matplotlib.pyplot as plt
from metpy.plots import StationPlot
import numpy as np

lat = np.linspace(40, 30, 11)
lon = np.linspace(-90, -80, 11)

lon, lat = np.meshgrid(lon, lat)

T = np.ones(lon.shape)*25

# Create the figure and an axes set to the projection.
fig = plt.figure(figsize=(8, 8))
ax = fig.add_subplot(1, 1, 1)
#ax.set_extent([-88, -86, 32, 38])
ax.set_xlim(-88, -84)
stationplot = StationPlot(ax, lon.ravel(), lat.ravel(), fontsize=10, clip_on=True)
stationplot.plot_parameter('C', T.ravel())

Produces...

And now adding a projection and making it a GeoAxes...

# Create the figure and an axes set to the projection.
fig = plt.figure(figsize=(8, 8))
ax = fig.add_subplot(1, 1, 1, projection=ccrs.PlateCarree())
#ax.set_extent([-88, -86, 32, 38])
ax.set_xlim(-88, -84)
stationplot = StationPlot(ax, lon.ravel(), lat.ravel(), fontsize=10, clip_on=True)
stationplot.plot_parameter('C', T.ravel())

Produces...

[kgoebber]

Okay, I've continued digging and not found much, but here is a summary of what I have found/tried...

Clearly we are able to clip appropriately when not using a Geo-referenced Axes, so MetPy and Matplotlib are working okay together. It also seems like clipping happens okay with geo-referenced axes when there is a wrapped Matplotlib function in Cartopy (read: contour, contourf, barbs, etc.), but not for the scattertext function that is in MetPy working on TextCollections from Matplotlib. I just can't seem to wrap my head around the code to know where and how it falls apart on clipping the text.

[jthielen]

@kgoebber Thank you for digging into this and investigating! It's a lot more than I've been able to do.

At this point, does the "fix" have to come down to pinning v0.12.2 to Cartopy <=0.17? I've had this issue come up a few times with supporting people, so it would be great to have at least some form of a resolution ASAP.

[kgoebber]

This fix will allow things to work with Cartopy 0.18 - the only user issue would be if they are wanting to plot surface observations - they won't clip properly, which is really a different issue from this one. So if we are okay committing over CI failures (which are all just image issues within this PR; some resulting from this change, others unrelated (e.g., clipping issues)), then this works just fine with Cartopy 0.18.

[kgoebber]

Overcomes import hurdles when using Cartopy 0.18. This quick fix gets the job done for that. There are still other Cartopy 0.18 issues. We should update the test images for the ones that change as the result of the updated adaptive scaler and gridline issues.

[kgoebber]

I found a work around for the clipping! The clipbox attribute was not being set correctly (or unset?), but the bbox that was created in _mpl.py can be used to set that attribute in the MetPy code.

Adding the following code at line 105 in _mpl.py located in src/metpy/plots gets to the same effect. There may be a better place or better method to use, but this does work.

# Update the clipbox based on bbox
text_obj.clipbox = self.bbox

[dopplershift]

Good find @kgoebber. So the chain is that, as noted in SciTools/cartopy#1526, after SciTools/cartopy#1422 the default patch is a bit more complicated; most importantly it is not an instance of Rectangle. This is important because noted in matplotlib/matplotlib#8270, Text is not clipped properly by a clip path, only by a clip box; and it turns out that Artist.set_clip_path has a special path to use a clip box when the background patch is a Rectangle.

breathes

So I think the clipping issue is actually a bug in matplotlib that has been made a problem for us by changes in CartoPy. I think we can do something like @kgoebber notes above until the bug is fixed in matplotlib.

[jthielen]

Closing in favor of @dopplershift's #1372.