Skip to content
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

Legends for feature artists #1500

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

poplarShift
Copy link

@poplarShift poplarShift commented Mar 29, 2020

Fixes #334.

Enables legends for FeatureArtists by implementing a legend handler and registering it with matplotlib.

This necessitated factoring out some of the code in the FeatureArtists' draw method to make it accessible to the new HandlerFeature legend handler.

As for tests, I haven't looked into how to test legend handling in matplotlib. Can do that though if you think this PR is useful!

Example:

import matplotlib.pyplot as plt
import cartopy.crs as ccrs
import cartopy.feature as cfeature

ax = plt.axes(projection=ccrs.PlateCarree())
ax.set_extent([80, 170, -45, 30])

states_provinces = cfeature.NaturalEarthFeature(
    category='cultural',
    name='admin_1_states_provinces_lines',
    scale='50m',
    facecolor='none')

land = ax.add_feature(cfeature.LAND, facecolor='wheat', edgecolor='black')
prov = ax.add_feature(states_provinces, edgecolor='gray', ls=':')
plt.legend(handles=[land, prov], labels=['Land', 'Province borders'], loc='lower left')

PS:
cb3de4f draws all legend artists as Polygons, which looks as below, will also respect facecolor options and is much simpler even for LineStrings but may look non-intuitive when facecolor=none.

image

cb3de4f adds another if/else clause based on facecolor to decide between Rectangle and Line2D for the legend artist, which looks like

image

PPS:
I have a vague feeling that the legend artists could have been based off of HandlerPathCollection since that is what the FeatureArtist is drawing but I could not find a way to make the right submethods available to the handler since the actual artist is not based off PathCollection.

lib/cartopy/mpl/feature_artist.py Outdated Show resolved Hide resolved
lib/cartopy/mpl/feature_artist.py Show resolved Hide resolved
lib/cartopy/mpl/feature_artist.py Show resolved Hide resolved
lib/cartopy/mpl/feature_artist.py Outdated Show resolved Hide resolved
lib/cartopy/mpl/feature_artist.py Show resolved Hide resolved
@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Mar 29, 2020
@poplarShift
Copy link
Author

poplarShift commented Apr 30, 2020

I've also submitted the CLA now.

@dopplershift dopplershift added this to the 0.19 milestone Jul 14, 2020
@QuLogic QuLogic modified the milestones: 0.19, 0.20 Apr 22, 2021

def get_stylised_paths(self, geoms, feature_crs, projection, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this private.

c.set_clip_path(ax.patch)
c.set_figure(ax.figure)
c.draw(renderer)
def get_geometry(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this too.

Comment on lines +232 to +233
warnings.warn('''Unable to determine extent.
Defaulting to global.''')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning should definitely not have a newline and a bunch of whitespace in it. Use plain strings, not a multiline string.

for style in stylised_paths.keys():
style = dict(style)
facecolor = style.get('facecolor', 'none')
if facecolor not in ('none', 'never') or type(geom) is Polygon:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use isinstance

width=width, height=height,
**style
)
elif type(geom) in (LineString, LinearRing):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use isinstance

@dopplershift dopplershift modified the milestones: 0.20, 0.21 Sep 17, 2021
@QuLogic QuLogic modified the milestones: 0.21, 0.22 Sep 11, 2022
@SciTools-assistant SciTools-assistant removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Sep 11, 2022
@dopplershift dopplershift modified the milestones: 0.22, Next Release Aug 4, 2023
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement LegendHandler for FeatureArtist
6 participants