-
Notifications
You must be signed in to change notification settings - Fork 371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make GridLiner into an Artist #2249
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,9 @@ | |
import warnings | ||
|
||
import matplotlib | ||
import matplotlib.artist | ||
import matplotlib.collections as mcollections | ||
import matplotlib.text | ||
import matplotlib.ticker as mticker | ||
import matplotlib.transforms as mtrans | ||
import numpy as np | ||
|
@@ -101,11 +103,7 @@ def _north_south_formatted(latitude, num_format='g'): | |
_north_south_formatted(v)) | ||
|
||
|
||
class Gridliner: | ||
# NOTE: In future, one of these objects will be add-able to a GeoAxes (and | ||
# maybe even a plain old mpl axes) and it will call the "_draw_gridliner" | ||
# method on draw. This will enable automatic gridline resolution | ||
# determination on zoom/pan. | ||
class Gridliner(matplotlib.artist.Artist): | ||
def __init__(self, axes, crs, draw_labels=False, xlocator=None, | ||
ylocator=None, collection_kwargs=None, | ||
xformatter=None, yformatter=None, dms=False, | ||
|
@@ -115,7 +113,7 @@ def __init__(self, axes, crs, draw_labels=False, xlocator=None, | |
xpadding=5, ypadding=5, offset_angle=25, | ||
auto_update=False, formatter_kwargs=None): | ||
""" | ||
Object used by :meth:`cartopy.mpl.geoaxes.GeoAxes.gridlines` | ||
Artist used by :meth:`cartopy.mpl.geoaxes.GeoAxes.gridlines` | ||
to add gridlines and tick labels to a map. | ||
|
||
Parameters | ||
|
@@ -234,7 +232,13 @@ def __init__(self, axes, crs, draw_labels=False, xlocator=None, | |
used for the map, meridians and parallels can cross both the X axis and | ||
the Y axis. | ||
""" | ||
self.axes = axes | ||
super().__init__() | ||
|
||
# We do not want the labels clipped to axes. | ||
self.set_clip_on(False) | ||
# Backcompat: the LineCollection was previously added directly to the | ||
# axes, having a default zorder of 2. | ||
self.set_zorder(2) | ||
|
||
#: The :class:`~matplotlib.ticker.Locator` to use for the x | ||
#: gridlines and labels. | ||
|
@@ -332,10 +336,10 @@ def __init__(self, axes, crs, draw_labels=False, xlocator=None, | |
raise ValueError(f"Invalid draw_labels argument: {value}") | ||
|
||
if auto_inline: | ||
if isinstance(self.axes.projection, _X_INLINE_PROJS): | ||
if isinstance(axes.projection, _X_INLINE_PROJS): | ||
self.x_inline = True | ||
self.y_inline = False | ||
elif isinstance(self.axes.projection, _POLAR_PROJS): | ||
elif isinstance(axes.projection, _POLAR_PROJS): | ||
self.x_inline = False | ||
self.y_inline = True | ||
else: | ||
|
@@ -399,7 +403,7 @@ def __init__(self, axes, crs, draw_labels=False, xlocator=None, | |
#: Control the rotation of labels. | ||
if rotate_labels is None: | ||
rotate_labels = ( | ||
self.axes.projection.__class__ in _ROTATE_LABEL_PROJS) | ||
axes.projection.__class__ in _ROTATE_LABEL_PROJS) | ||
if not isinstance(rotate_labels, (bool, float, int)): | ||
raise ValueError("Invalid rotate_labels argument") | ||
self.rotate_labels = rotate_labels | ||
|
@@ -436,10 +440,6 @@ def __init__(self, axes, crs, draw_labels=False, xlocator=None, | |
self._drawn = False | ||
self._auto_update = auto_update | ||
|
||
# Check visibility of labels at each draw event | ||
# (or once drawn, only at resize event ?) | ||
self.axes.figure.canvas.mpl_connect('draw_event', self._draw_event) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why this was here. I think it's telling Matplotlib to call the method on each draw, but that was also happening by calling it within If I just remove this line from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you're saying intuitively makes sense. |
||
|
||
@property | ||
def xlabels_top(self): | ||
warnings.warn('The .xlabels_top attribute is deprecated. Please ' | ||
|
@@ -488,9 +488,6 @@ def ylabels_right(self, value): | |
'use .right_labels to toggle visibility instead.') | ||
self.right_labels = value | ||
|
||
def _draw_event(self, event): | ||
self._draw_gridliner(renderer=event.renderer) | ||
|
||
def has_labels(self): | ||
return len(self._labels) != 0 | ||
|
||
|
@@ -629,13 +626,9 @@ def _draw_gridliner(self, nx=None, ny=None, renderer=None): | |
return | ||
self._drawn = True | ||
|
||
# Clear lists of artists | ||
for lines in [*self.xline_artists, *self.yline_artists]: | ||
lines.remove() | ||
# Clear lists of child artists | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if we could get rid of this as well and just update the artists that are already there instead. Something to look into the future I think. |
||
self.xline_artists.clear() | ||
self.yline_artists.clear() | ||
for label in self._labels: | ||
label.artist.remove() | ||
self._labels.clear() | ||
|
||
# Inits | ||
|
@@ -673,6 +666,7 @@ def _draw_gridliner(self, nx=None, ny=None, renderer=None): | |
if not any(x in collection_kwargs for x in ['lw', 'linewidth']): | ||
collection_kwargs.setdefault('linewidth', | ||
matplotlib.rcParams['grid.linewidth']) | ||
collection_kwargs.setdefault('clip_path', self.axes.patch) | ||
|
||
# Meridians | ||
lat_min, lat_max = lat_lim | ||
|
@@ -696,7 +690,6 @@ def _draw_gridliner(self, nx=None, ny=None, renderer=None): | |
lon_lc = mcollections.LineCollection(lon_lines, | ||
**collection_kwargs) | ||
self.xline_artists.append(lon_lc) | ||
self.axes.add_collection(lon_lc, autolim=False) | ||
|
||
# Parallels | ||
lon_min, lon_max = lon_lim | ||
|
@@ -711,7 +704,6 @@ def _draw_gridliner(self, nx=None, ny=None, renderer=None): | |
lat_lc = mcollections.LineCollection(lat_lines, | ||
**collection_kwargs) | ||
self.yline_artists.append(lat_lc) | ||
self.axes.add_collection(lat_lc, autolim=False) | ||
|
||
################# | ||
# Label drawing # | ||
|
@@ -925,7 +917,9 @@ def update_artist(artist, renderer): | |
|
||
# Add text to the plot | ||
text = formatter(tick_value) | ||
artist = self.axes.text(x, y, text, **kw) | ||
artist = matplotlib.text.Text(x, y, text, **kw) | ||
artist.set_figure(self.axes.figure) | ||
artist.axes = self.axes | ||
|
||
# Update loc from spine overlapping now that we have a bbox | ||
# of the label. | ||
|
@@ -1239,6 +1233,26 @@ def _axes_domain(self, nx=None, ny=None): | |
|
||
return lon_range, lat_range | ||
|
||
def get_visible_children(self): | ||
r"""Return a list of the visible child `.Artist`\s.""" | ||
all_children = (self.xline_artists + self.yline_artists | ||
+ self.label_artists) | ||
return [c for c in all_children if c.get_visible()] | ||
|
||
def get_tightbbox(self, renderer=None): | ||
self._draw_gridliner(renderer=renderer) | ||
bboxes = [c.get_tightbbox(renderer=renderer) | ||
for c in self.get_visible_children()] | ||
if bboxes: | ||
return mtrans.Bbox.union(bboxes) | ||
else: | ||
return mtrans.Bbox.null() | ||
|
||
def draw(self, renderer=None): | ||
self._draw_gridliner(renderer=renderer) | ||
for c in self.get_visible_children(): | ||
c.draw(renderer=renderer) | ||
|
||
|
||
class Label: | ||
"""Helper class to manage the attributes for a single label""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,9 @@ | |
# See COPYING and COPYING.LESSER in the root of the repository for full | ||
# licensing details. | ||
|
||
import io | ||
from unittest import mock | ||
|
||
import matplotlib.pyplot as plt | ||
import matplotlib.ticker as mticker | ||
import numpy as np | ||
|
@@ -13,7 +16,8 @@ | |
import cartopy.crs as ccrs | ||
from cartopy.mpl.geoaxes import GeoAxes | ||
from cartopy.mpl.gridliner import (LATITUDE_FORMATTER, LONGITUDE_FORMATTER, | ||
classic_formatter, classic_locator) | ||
Gridliner, classic_formatter, | ||
classic_locator) | ||
from cartopy.mpl.ticker import LongitudeFormatter, LongitudeLocator | ||
|
||
|
||
|
@@ -241,9 +245,14 @@ def test_grid_labels_tight(): | |
fig.tight_layout() | ||
|
||
# Ensure gridliners were drawn | ||
num_gridliners_drawn = 0 | ||
for ax in fig.axes: | ||
for gl in ax._gridliners: | ||
assert hasattr(gl, '_drawn') and gl._drawn | ||
for artist in ax.artists: | ||
if isinstance(artist, Gridliner) and getattr(artist, '_drawn', | ||
False): | ||
num_gridliners_drawn += 1 | ||
|
||
assert num_gridliners_drawn == 4 | ||
|
||
return fig | ||
|
||
|
@@ -432,3 +441,82 @@ def test_gridliner_formatter_kwargs(): | |
fig.canvas.draw() | ||
labels = [a.get_text() for a in gl.bottom_label_artists if a.get_visible()] | ||
assert labels == ['75°O', '70°O', '65°O', '60°O', '55°O', '50°O', '45°O'] | ||
|
||
|
||
def test_gridliner_count_draws(): | ||
fig = plt.figure() | ||
ax = fig.add_subplot(1, 1, 1, projection=ccrs.PlateCarree()) | ||
ax.set_global() | ||
gl = ax.gridlines() | ||
|
||
with mock.patch.object(gl, '_draw_gridliner', return_value=None) as mocked: | ||
ax.get_tightbbox(renderer=None) | ||
mocked.assert_called_once() | ||
|
||
with mock.patch.object(gl, '_draw_gridliner', return_value=None) as mocked: | ||
fig.draw_without_rendering() | ||
mocked.assert_called_once() | ||
|
||
|
||
@pytest.mark.mpl_image_compare( | ||
baseline_dir='baseline_images/mpl/test_mpl_integration', | ||
filename='simple_global.png') | ||
def test_gridliner_remove(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you test against an already existing image? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can try - can you specify a different directory in the image test decorator? Or should I just create a symlink from the gridliner baseline image directory to the target image? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea... I was just pointing you at an empty figure to test against :) FWIW, I think your current version with the asserts is fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm. I can report that the decorator takes a baseline_dir keyword, but it doesn't appear to have any effect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, it's possible if we modify |
||
fig = plt.figure() | ||
ax = fig.add_subplot(1, 1, 1, projection=ccrs.PlateCarree()) | ||
ax.set_global() | ||
ax.coastlines() | ||
gl = ax.gridlines(draw_labels=True) | ||
fig.draw_without_rendering() # Generate child artists | ||
gl.remove() | ||
|
||
assert gl not in ax.artists | ||
assert not ax.collections | ||
|
||
return fig | ||
|
||
|
||
def test_gridliner_save_tight_bbox(): | ||
# Smoke test for save with auto_update=True and bbox_inches=Tight (gh2246). | ||
fig = plt.figure() | ||
ax = fig.add_subplot(1, 1, 1, projection=ccrs.PlateCarree()) | ||
ax.set_global() | ||
ax.gridlines(draw_labels=True, auto_update=True) | ||
fig.savefig(io.BytesIO(), bbox_inches='tight') | ||
|
||
|
||
@pytest.mark.mpl_image_compare(filename='gridliner_labels_title_adjust.png', | ||
tolerance=grid_label_tol) | ||
def test_gridliner_title_adjust(): | ||
# Test that title do not overlap labels | ||
projs = [ccrs.Mercator(), ccrs.AlbersEqualArea(), ccrs.LambertConformal(), | ||
ccrs.Orthographic()] | ||
|
||
# Turn on automatic title placement (this is default in mpl rcParams but | ||
# not in these tests). | ||
plt.rcParams['axes.titley'] = None | ||
|
||
fig = plt.figure(layout='constrained') | ||
fig.get_layout_engine().set(h_pad=1/8) | ||
for n, proj in enumerate(projs, 1): | ||
ax = fig.add_subplot(2, 2, n, projection=proj) | ||
ax.coastlines() | ||
ax.gridlines(draw_labels=True) | ||
ax.set_title(proj.__class__.__name__) | ||
|
||
return fig | ||
|
||
|
||
def test_gridliner_title_noadjust(): | ||
fig = plt.figure() | ||
ax = fig.add_subplot(1, 1, 1, projection=ccrs.PlateCarree()) | ||
ax.set_global() | ||
ax.set_title('foo') | ||
ax.gridlines(draw_labels=['left', 'right'], ylocs=[-60, 0, 60]) | ||
fig.draw_without_rendering() | ||
pos = ax.title.get_position() | ||
|
||
# Title position shouldn't change when a label is on the top boundary. | ||
ax.set_extent([-180, 180, -60, 60]) | ||
fig.draw_without_rendering() | ||
assert ax.title.get_position() == pos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to set the clipping differently for the lines vs the text? Allow the text to be outside, but the lines to be clipped?
Edit: I looked at the code below and I think you are accounting for this after all!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the lines need clipping to the axes patch, which happens by default within
add_collection
https://github.com/matplotlib/matplotlib/blob/ff552f45f2d30c76120fe0355306afac9a9dd2da/lib/matplotlib/axes/_base.py#L2255-L2256
I got a lot of image test failures before I realised that!