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

enforce stable aws endpoint for cartopy_feature_download.py #1837

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Sep 3, 2021

This PR is related to #1833 and the ability to download cartopy feature resources from the now preferred stable AWS endpoint.

The now renamed tools/cartopy_feature_download.py (#1602) on master only requires a one line change to ensure that this script works with almost all older versions of cartopy, see line +142. Thus removing the need to backport this fix. Clearly there is community appetite for this to happen given #1834.

We have a live use case on SciTools/iris, see here, where we want the benefits of using #1833 to download and populate a cartopy cache for our CI. As it happens, the fix proposed here works with an installed cartopy v0.18.

Initially, we worked around the issue by crafting the change solely within iris, but there is a bigger win for the cartopy community here rather than limiting the win within iris e.g., downloading this PR version of cartopy_feature_download.py will work for older immutable production versions of cartopy to populate the cache and avoid downloading resources on demand.

The benefits of this PR is also not dependant on conda-forge/cartopy-feedstock#116, which only helps users of cartopy_feature_download.py with cartopy v0.19.0.post1 and onwards.

Closes #1834

tools/cartopy_feature_download.py Outdated Show resolved Hide resolved
tools/cartopy_feature_download.py Outdated Show resolved Hide resolved
config['pre_existing_data_dir'] = args.output
config['data_dir'] = args.output
target_dir = pathlib.Path(args.output).expanduser().resolve()
target_dir.mkdir(parents=True, exist_ok=True)
Copy link
Member Author

Choose a reason for hiding this comment

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

The exist_ok flag was added in Python 3.5, so I assume it's okay to use here...surely 😜

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you'll just get an error if you want to try with an earlier Python version ;)

Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

Just tried locally with all the various options and an earlier version of Cartopy and it looks good to me. Thanks for the quick update @bjlittle!

@greglucas greglucas merged commit d9b7b64 into SciTools:master Sep 3, 2021
@bjlittle
Copy link
Member Author

bjlittle commented Sep 3, 2021

@greglucas awesome, thanks 👍

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.

Backport NaturalEarth url change in release 0.18 and 0.19 ?
4 participants