-
Notifications
You must be signed in to change notification settings - Fork 416
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
Plotting Shapely objects in declarative interface #1973
Conversation
fe0ec6a
to
336f135
Compare
Linter doesn't like the modulo operators for some reason. Is that something I need to fix, or is it a false positive on the linter's part? Also, codecov is being finicky on the conda tests for some reason. Other than that, this is ready for review. |
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.
Here are some preliminary thoughts. Also, it would be good to bury the shapely
imports within the class or a function. Otherwise, this becomes an unconditional dependency (we only optionally depend on Cartopy).
Also, the modulus operator from flake8 is a false positive bug in pep8-3101 (gforcada/flake8-pep3101#23). Seeing as this is the standpoint:
We can stop using that plugin and I'll look for an f-string plugin that doesn't do this instead. |
b960bb8
to
5c96ec9
Compare
I don't understand why the |
This avoids installing CartoPy build dependencies unless we actually need CartoPy.
It's failing because we manually install Shapely as part of Cartopy's build dependencies, even when we don't need Cartopy. I've pushed a commit to this PR to fix that. Be sure to pull those down before a future rebase (assuming this works properly). |
src/metpy/plots/declarative.py
Outdated
class PlotGeometry(HasTraits): | ||
"""Plot collections of Shapely objects and customize their appearance.""" | ||
|
||
import shapely.geometry as shp |
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.
This will always be executed when the class body is run. I think the only way around this is by putting this inside the appropriate methods.
tests/plots/test_declarative.py
Outdated
@@ -11,14 +11,16 @@ | |||
import numpy as np | |||
import pandas as pd | |||
import pytest | |||
from shapely.geometry import (LineString, MultiLineString, MultiPoint, MultiPolygon, Point, |
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.
These will need to move inside the test functions that need them so that our tests can continue working without Cartopy installed.
Introduces support for plotting Shapely objects in the declarative plotting interface. This new feature is implemented in a new class
PlotGeometry
. Similar to other classes in the declarative interface, users create a newPlotGeometry
object, adjust the attributes to their needs, and add it to aMapPanel
/PanelContainer
to plot.The primary use case is for plotting data from geoJSON and shapefile formats, which can be read with geopandas. For that reason, the class is designed plot objects from the 'geometry' column from geopandas dataframes, but any collection of Shapely objects can be plotted.
Users can add labels to plotted geometry, as well as adjust the colors of geometry and labels.
Checklist