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.

Sign up for GitHub

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

Use geopandas.plot() for Centroids.plot() #896

Merged
merged 15 commits into from
Jul 9, 2024
Merged

Use geopandas.plot() for Centroids.plot() #896

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a973808
use geopandas for plot()
sarah-hlsn Jun 17, 2024
a3279d4
include admin-0 in plot function
sarah-hlsn Jun 17, 2024
24e27fb
copy and reproject gdf to DEF_CRS before plotting
sarah-hlsn Jun 18, 2024
124af45
clean up according to pylint
sarah-hlsn Jun 18, 2024
ac9b827
Merge remote-tracking branch 'origin/develop' into feature/refactor_c…
sarah-hlsn Jun 24, 2024
af8bad4
add axis argument, custom error message
sarah-hlsn Jun 24, 2024
22dddaa
Update climada/hazard/centroids/centr.py
sarah-hlsn Jun 25, 2024
2ed3c52
fix typo
sarah-hlsn Jun 25, 2024
838dd66
Merge remote-tracking branch 'origin/develop' into feature/refactor_c…
sarah-hlsn Jul 1, 2024
aa79235
include tests, delete unnecessary file
sarah-hlsn Jul 1, 2024
fc19727
remove *args from plotting method
sarah-hlsn Jul 5, 2024
11fe901
fix error
sarah-hlsn Jul 5, 2024
616f8b1
Merge remote-tracking branch 'origin/develop' into feature/refactor_c…
sarah-hlsn Jul 5, 2024
ca9bac8
update changelog
sarah-hlsn Jul 5, 2024
4ab1729
Merge remote-tracking branch 'origin/develop' into feature/refactor_c…
sarah-hlsn Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 19 additions & 28 deletions climada/hazard/centroids/centr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import h5py
import cartopy.crs as ccrs
import cartopy.feature as cfeature
import geopandas as gpd
import matplotlib.pyplot as plt
import numpy as np
Expand All @@ -38,7 +39,6 @@

from climada.util.constants import DEF_CRS
import climada.util.coordinates as u_coord
import climada.util.plot as u_plot

__all__ = ['Centroids']

Expand Down Expand Up @@ -477,45 +477,36 @@
(self.lat >= lat_min) & (self.lat <= lat_max)
)
return sel_cen

#TODO replace with nice GeoDataFrame util plot method.
def plot(self, axis=None, figsize=(9, 13), **kwargs):
"""Plot centroids scatter points over earth

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do I understand this correctly that now you cannot plot the centroids on an existing axis anymore? Was this a functionality that is not not needed or that would be difficult to implement using gdf.plot()?

Copy link
Member

Choose a reason for hiding this comment

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

That would be I think a suboptimal idea. Both the axis and the figures should be possible arguments for the method. Otherwise it is very difficult to change projections, figure sizes, plot several centroids in a multi-axis etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi both, I see your point and I briefly talke to Sam & Lukas about this as well. We thought since the main use of this function would be to get a quick overview of the centroids, this very simple version enables users to do that. Now that the centroids class is a gdf, any more fancy type of plotting can easily be done using the geopandas plotting function.
Of course it would be nice to be able to plot on an existing axis, but as far as I am aware, in order to create a cartopy axis instance, you have to pass the projection argument when creating the axis, and it is not possible to add it retroactively to an existing axis. So I am not sure how to make this "fool-proof" for axis objects created by the user independently outside the function. Regarding projections, we could make that an argument if you think it would add a lot. Let me know what you think :)

Copy link
Member

Choose a reason for hiding this comment

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

Good points! However, I see no reason not to allow a user to pass their cartopy axis or figure if they want to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we can definitely allow it, but how would you deal with the case that the user did not create a cartopy axis? write a check in the function and throw a warning? Or something else?

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that there is a need to check if it is written in the docstring.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mhm I think if we're writing the function flexibly in the spirit of user-friendliness shouldn't we also make any potential error clear without the person having to go look in the docstrings? But maybe the error will be clear anyways... I will adjust the method and see what happens

Copy link
Member

Choose a reason for hiding this comment

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

I agree. At the same time, we do not want to start checking for errors in using other packages (here geopandas).

Let's see what your test says :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The default error you would get when passing the wrong type of axis does not make it obvious what the issue is, so I added a custom error message to the function. The user can now plot the centroids on a custom axis. I also ensured that plotting with different projections on user-defined axes is possible. Ready for a new round of reviews :)

def plot(self, figsize=(9, 13), *args, **kwargs):

Check warning on line 481 in climada/hazard/centroids/centr.py

View check run for this annotation

Jenkins - WCR / Pylint

trailing-whitespace 

LOW:
Trailing whitespace
Raw output 
Used when there is whitespace between the end of a line and the newline.
"""Plot centroids geodataframe using geopandas and cartopy plotting functions.

Check warning on line 482 in climada/hazard/centroids/centr.py

View check run for this annotation

Jenkins - WCR / Pylint

keyword-arg-before-vararg 

NORMAL:
Keyword argument before variable positional arguments list in the definition of plot function
Raw output 
no description found

Parameters
----------
axis : matplotlib.axes._subplots.AxesSubplot, optional
axis to use
figsize: (float, float), optional
figure size for plt.subplots
The default is (9, 13)
args : optional
positional arguments for geopandas.GeoDataFrame.plot
kwargs : optional
arguments for scatter matplotlib function

keyword arguments for geopandas.GeoDataFrame.plot
Returns

Check warning on line 494 in climada/hazard/centroids/centr.py

View check run for this annotation

Jenkins - WCR / Pylint

trailing-whitespace 

LOW:
Trailing whitespace
Raw output 
Used when there is whitespace between the end of a line and the newline.
-------
axis : matplotlib.axes._subplots.AxesSubplot
ax : cartopy.mpl.geoaxes.GeoAxes instance
"""
pad = np.abs(u_coord.get_resolution(self.lat, self.lon)).min()

proj_data, _ = u_plot.get_transformation(self.crs)
proj_plot = proj_data
if isinstance(proj_data, ccrs.PlateCarree):
# use different projections for plot and data to shift the central lon in the plot
xmin, ymin, xmax, ymax = u_coord.latlon_bounds(self.lat, self.lon, buffer=pad)
proj_plot = ccrs.PlateCarree(central_longitude=0.5 * (xmin + xmax))
fig, axis = plt.subplots(figsize=figsize, subplot_kw={"projection": ccrs.PlateCarree()})
axis.add_feature(cfeature.BORDERS)

Check warning on line 499 in climada/hazard/centroids/centr.py

View check run for this annotation

Jenkins - WCR / Pylint

unused-variable 

NORMAL:
Unused variable 'fig'
Raw output 
Used when a variable is defined but not used.
axis.add_feature(cfeature.COASTLINE)
axis.gridlines(draw_labels=True, dms=True, x_inline=False, y_inline=False)

if self.gdf.crs != DEF_CRS:
copy_gdf = copy.deepcopy(self)
copy_gdf.to_default_crs()
copy_gdf.gdf.plot(ax=axis, *args, **kwargs)
else:
xmin, ymin, xmax, ymax = (self.lon.min() - pad, self.lat.min() - pad,
self.lon.max() + pad, self.lat.max() + pad)

if not axis:
_fig, axis, _fontsize = u_plot.make_map(proj=proj_plot, figsize=figsize)

axis.set_extent((xmin, xmax, ymin, ymax), crs=proj_data)
u_plot.add_shapes(axis)
axis.scatter(self.lon, self.lat, transform=proj_data, **kwargs)
plt.tight_layout()
self.gdf.plot(ax=axis, *args, **kwargs)
return axis

Check warning on line 509 in climada/hazard/centroids/centr.py

View check run for this annotation

Jenkins - WCR / Code Coverage

Not covered lines 

Lines 499-509 are not covered by tests

def set_region_id(self, level='country', overwrite=False):
"""Set region_id as country ISO numeric code attribute for every pixel or point.
Expand Down
108 changes: 108 additions & 0 deletions test_plot.ipynb
View file
Open in desktop

Large diffs are not rendered by default.