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

added missing shapefile group to download script #1894

Merged
merged 4 commits into from
Sep 29, 2021
Merged

added missing shapefile group to download script #1894

merged 4 commits into from
Sep 29, 2021

Conversation

georgemccabe
Copy link
Contributor

@georgemccabe georgemccabe commented Sep 28, 2021

Rationale

Using cartopy.features.STATES with matplotlib causes an error because the shapefile group that is used is not available. Adding these missing groups to the download script will allow it to be used to obtain the files needed to run.

The group that is used to draw state/province boundaries is admin_1_states_provinces_lakes:

STATES = NaturalEarthFeature(
'cultural', 'admin_1_states_provinces_lakes',
auto_scaler, edgecolor='black', facecolor='none')

The files are available in the new location on the web:

https://naturalearth.s3.amazonaws.com/50m_cultural/ne_50m_admin_1_states_provinces_lakes.zip
https://naturalearth.s3.amazonaws.com/110m_cultural/ne_110m_admin_1_states_provinces_lakes.zip

Implications

Additional files will be downloaded when a user runs the script specifying the 'cultural-extra' group. However, these files are very small.

I tested the changes by obtaining the script from my branch and running it, then rerunning the use case that was failing. I confirmed that the use case now runs without error.

@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 Sep 28, 2021
@georgemccabe
Copy link
Contributor Author

georgemccabe commented Sep 28, 2021

Here is an example I used to recreate this error. It is using cartopy==0.18.0 and matplotlib==3.3.0.

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

fig = plt.figure(figsize=(10,10))
proj = ccrs.PlateCarree(central_longitude=60)
ax1 = fig.add_subplot(5,2,1,projection=proj)
ax1.add_feature(cfeature.STATES)
plt.savefig('test.png',format='png',dpi=400, bbox_inches='tight')

The commands work fine if the line that adds the feature cfeature.STATES is removed.

I downloaded the cartopy_feature_download.py script from the master branch of SciTools/cartopy and ran with the 'cultural-extra' argument. I reran the commands and still received the error. Next I downloaded the script from my fork, ran it again to get the missing shapefiles, and reran the commands. This time there were no errors.

Copy link
Contributor

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Changes look good, thanks for the contribution. Just waiting on the CLA to be signed.

…of admin_1_states_provinces_lakes

Co-authored-by: Greg Lucas <greg.m.lucas@gmail.com>
@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 29, 2021
@georgemccabe
Copy link
Contributor Author

Changes look good, thanks for the contribution. Just waiting on the CLA to be signed.

Thanks! I signed the CLA. I committed the suggestion from @greglucas to obtain all scales for the new shapefiles group instead of listing out the 2 that I needed. This seems like a cleaner solution. I tested the change locally and it obtained the 10m scale as well.

@greglucas greglucas merged commit 7669afa into SciTools:master Sep 29, 2021
@QuLogic QuLogic added this to the 0.20.1 milestone Sep 29, 2021
QuLogic pushed a commit to QuLogic/cartopy that referenced this pull request Oct 7, 2021
added missing shapefile group to download script
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.

5 participants