iri-pycpt / pycpt

package joining pycpt stuff
4 stars 1 forks source link

Add option for not including country borders on maps #53

Open nhsavage opened 9 months ago

nhsavage commented 9 months ago

Many countries don't agree on international boundaries, and so often maps produced by RCOFs don't have any boundaries on at all. This is an important feature but at the moment, skill maps etc all use the standard Cartopy BORDERS feature. This plots the de facto boundaries as defined in Natural Earth: https://www.naturalearthdata.com/about/disputed-boundaries-policy/

To improve this, we would need to include a logical flag for "plot_borders" and pass it to all plotting routines and then instead of for example:

ax[i][j].add_feature(cartopy.feature.BORDERS)

have:

if plot_borders:
   ax[i][j].add_feature(cartopy.feature.BORDERS)

Even better would be to optionally allow the use of a specific country point of view (see natural earth boundaries above) but this would need even more development.

nhsavage commented 9 months ago

a first version of this has been created at https://github.com/nhsavage/pycpt/tree/i53_country_borders

I have tested it with the seasonal notebook and all code works and if plotting functions are run with plot_borders=False no borders are shown. I need to do more testing but before I proceed further I'd welcome comments on whether this is considered a good way forward here

aaron-kaplan commented 9 months ago

Your changes look reasonable.

We've also had requests for using custom borders instead of the NaturalEarth ones, so the ideal would probably be to have a configuration option that defaults to cartopy.feature.BORDERS but can be overridden with either a different value for custom borders, or None for no borders. But if you don't feel like taking it all the way, a pull request that just adds this boolean option would be welcome.

nhsavage commented 9 months ago

I don't have time at the moment to add all the code for custom shapefiles, and being able to turn off borders is fairly pressing for us, so I will

I can give the idea of multiple options (custom shapefile, country pov) more thought later. Ideally, users would just set this option in one place and it would be picked up by all the plotting routines, but I can't see a clean way to do that at the moment

aaron-kaplan commented 9 months ago

How about this: instead of a boolean argument plot_borders, could you call it borders, and have the default value be cartopy.feature.BORDERS? Then you can set it to None if you want to turn borders off, and in the future we can add the ability to use different features without changing the API.

nhsavage commented 9 months ago

that seems like a simple solution - I will try and implement that. That also makes it easy to use the country pov by using e.g. cartopy.feature.NaturalEarthFeature('cultural', 'admin_0_countries_egy', '10m') for Egypt (random example) although at the moment, that has a small bug in cartopy which makes it fail on first but not following runs https://github.com/SciTools/cartopy/issues/2319