-
Notifications
You must be signed in to change notification settings - Fork 124
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
Add shapes argument to geo_im_from_array() in climada.util.plot #805
Conversation
shapes : bool, optional | ||
Overlay Earth's countries coastlines to matplotlib.pyplot axis. | ||
The default is True |
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.
I would call this coastlines
instead of shapes
. Much clearer.
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.
Since other functioms like geo_bin_from_array()
, geo_scatter_from_array()
, etc, they all name them as shapes for turning on/off the plotting of coastline, I would suggest either having it as shapes
in geo_im_from_array()
, or renaming all relevant shapes
as coastlines
, to make it more consistent.
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.
Ok, let it be shapes
then. It is a very bad name, but consistency of code is probably more important.
Can you please update the And could you post a picture of a plot without the |
Perfect. Then, please just update the CHANGELOG.md file. |
@yklpaul0902 : many thanks for the contribution! 😃 The failing compatibility test is unrelated and due to a (hopefully) temporary unavailability of the ECMWF ftp server. Hence: ready to merge. |
Changes proposed in this PR:
This PR fixes geo_im_from_array#
PR Author Checklist
develop
)PR Reviewer Checklist