cartopy feature download #4304

bjlittle · 2021-09-01T10:41:27Z

🚀 Pull Request

Description

This PR fixes the broken ABF fileformat documentation URL link.

Closes #4302

Consult Iris pull request check list

rcomer

LGTM. Just waiting for the CI to do its thing.

pp-mo

Makes sense.
I wondered whether the link should live in docs/src/common_links.inc,
but I think that is unhelpful because it is needed in code-docstrings, not just docs (rst).

rcomer · 2021-09-01T11:01:30Z

Looks like the Natural Earth server is having issues again 😞

bjlittle · 2021-09-02T20:37:23Z

Well... this PR took an unexpected diversion 🤣

Apart from various link barfs, we were also hitting issues during CI where cartopy downloads of certain resources were failing when running tests.

After a bit of investigation, I discovered that the cirrus-ci cache for cartopy wasn't fully populated (as expected) with all the shapefiles that we require during testing. This was the result of a race condition between parallel jobs, and unluckily for us, a CI job that doesn't download all the required Natural Earth resources pushed an incomplete cache first i.e., the partial cache was repeatedly forcing cartopy to download the missing resources everytime during testing, hence exposing us to service outages.

There is a fix now merged in cartopy, see PR SciTools/cartopy#1833 (nice one @rcomer ❤️), but this won't be available any time soon i.e., v0.19.0.post1+, whenever that is.

As a compromise, I've taken the approach to extend and wrap the cartopy/tools/feature_download.py command-line tool within iris to download cartopy assets that populate the cache, but using the more stable AWS endpoint.

This is actually a pretty handy little general utility, and whatsmore, as with cartopy it won't be bundled with iris when it is packaged.

I know that populating the cartopy cache is a pain-point for many users, particularly when pre-caching for offline usage, or when attempting to develop behind a firewall with no internet access e.g., on a secure HPC, so this may somewhat help ease the burden i.e., you can use the utility to download a cartopy cache, then lift and shift it to your installation, or be confident that you have all the resource you need before working offline.

rcomer · 2021-09-02T20:40:08Z

@bjlittle maybe update the title? I’m not sure I’m qualified to review this anymore.

.cirrus.yml

rcomer · 2021-09-03T08:14:56Z

This is actually a pretty handy little general utility, and whatsmore, as with cartopy it won't be bundled with iris when it is packaged.

I know that populating the cartopy cache is a pain-point for many users, particularly when pre-caching for offline usage, or when attempting to develop behind a firewall with no internet access e.g., on a secure HPC, so this may somewhat help ease the burden i.e., you can use the utility to download a cartopy cache, then lift and shift it to your installation, or be confident that you have all the resource you need before working offline.

I'm confused here: if the script isn't bundled with Iris, how do the users take advantage of it?

bjlittle · 2021-09-03T08:20:41Z

@rcomer Glad you asked 😄

I've followed the same pattern as cartopy, as a starting point i.e., cartopy/tools/feature_download.py is not bundled within cartopy as a package. So really, it's a development tool.

If we're serious about packaging this with iris, then we could easily make an entry-point for it, then it would indeed be accessible to users via the command line on install 🥳

I didn't want to do that work up front, only for people to object... I rather discuss it, have agreement, then do it.

bjlittle · 2021-09-03T08:20:58Z

@SciTools/iris-devs So since we're on topic... what would you/we prefer?

I have an opinion on this, but I like to hear the wisdom of the crowd first...

rcomer · 2021-09-03T08:37:58Z

@bjlittle my opinion is to punt that question to a new Issue: the scope on this PR has ~~creeped~~ swerved far enough, and fixing the CI should be the first priority!

trexfeathers · 2021-09-03T08:42:39Z

@bjlittle my opinion is to punt that question to a new Issue: the scope on this PR has creeped swerved far enough, and fixing the CI should be the first priority!

That's way better than what I was about to write. PR's that solve multiple problems are an anti-pattern, and as @rcomer eludes, this isn't exactly a small add-on.

bjlittle · 2021-09-03T08:45:23Z

Totally happy to follow this up with a GH discussion 👍

Anyone fancy reviewing and merging?

I'm keen to unblock CI

bjlittle · 2021-09-03T10:53:15Z

Note that SciTools/cartopy#1602 renames tools/feature_download.py to be tools/cartopy_feature_download.py and also makes it a pucker command line script (finally) 🥳

That PR was merged in May 2021 and didn't make the cut for the April release of cartopy 0.19.0.post1... but it will be available in the next release of cartopy, as will SciTools/cartopy#1833

pp-mo · 2021-09-03T10:53:50Z

@rcomer @trexfeathers
I'm good with this as it is. I reviewed the code and its CLI and I think it works OK.
I do also agree that including this as an installed script is another step, and we don't want to get into that yet
- assuming that is what everyone else meant by "punt that question to a new Issue" / "PR's that solve multiple problems" ??

Going to merge unless someone objects...

bjlittle · 2021-09-03T10:55:49Z

Once this PR is merge, I'll make an associated issue to discuss follow-up work.

This is all in a state of flux, and we may insist on a minimum pin for cartopy within iris moving forwards to make our lives easier 🤔 ... but that's to decide at a later date.

jamesp

Suggest we pause on this and consider putting changes into cartopy instead of iris

rcomer · 2021-09-03T12:55:45Z

Oh look 👀 conda-forge/cartopy-feedstock#116

bjlittle · 2021-09-03T13:14:04Z

That's only a fix to 0.19.0.post1, and not older versions of cartopy

rcomer · 2021-09-03T13:14:37Z

@bjlittle but it would get our CI moving?

.cirrus.yml

bjlittle · 2021-09-03T22:43:17Z

@jamesp ... and finally, this is good to go 🤞

* main: [pre-commit.ci] pre-commit autoupdate (SciTools#4299) Updated environment lockfiles (SciTools#4298) cartopy feature download (SciTools#4304) reset whatsnew latest (SciTools#4288) Updated environment lockfiles (SciTools#4289) Update cube.py (SciTools#4287) Missing whatsnew entries for 3.1 release. (SciTools#4283) Update CF standard name table to v77 (SciTools#4282) Added AtmosphereSigmaFactory (SciTools#4052) Updated environment lockfiles (SciTools#4281)

* Fix broken ABF link * add cartopy downloader utility * update netcdf4-python link * explicitly specify features to download for ci * added whatsnew entry * update utility help * utility tidy * tidy utility * copy with cartopy script rename * update whatsnew to clarify as dev tool * push fix to cartopy * use cartopy master

* Finalise whatsnew and version string. * cartopy feature download (#4304) * Fix broken ABF link * add cartopy downloader utility * update netcdf4-python link * explicitly specify features to download for ci * added whatsnew entry * update utility help * utility tidy * tidy utility * copy with cartopy script rename * update whatsnew to clarify as dev tool * push fix to cartopy * use cartopy master * Fix typo * Update copyright.rst * Update conf.py * Review changes. Co-authored-by: Bill Little <bill.james.little@gmail.com> Co-authored-by: Bill Little <bill.little@metoffice.gov.uk>

* main: (71 commits) Skip TestConstrainedLoad if data missing (SciTools#4319) Add 'Good First Issue' label to reasons an issue doesn't go stale (SciTools#4317) Gallery: simplify quiver example (SciTools#4120) Improve styling in a minor way in docs (SciTools#4314) bump version (SciTools#4310) Made clear we only test on Linux. (SciTools#4309) Updated environment lockfiles (SciTools#4308) Include Discussions in Getting Involved. (SciTools#4307) Fixed text to show as link. (SciTools#4305) [pre-commit.ci] pre-commit autoupdate (SciTools#4299) Updated environment lockfiles (SciTools#4298) cartopy feature download (SciTools#4304) Mesh Loading (AVD-1813) (SciTools#4262) reset whatsnew latest (SciTools#4288) Updated environment lockfiles (SciTools#4289) Update cube.py (SciTools#4287) Integrated whatsnew for v3.1 release (rc0) (SciTools#4285) Version changes and final whatsnew tweaks for 3v1rc0. (SciTools#4284) Missing whatsnew entries for 3.1 release. (SciTools#4283) Update CF standard name table to v77 (SciTools#4282) ...

* main: (94 commits) added support for make html-noapi and html-quick for the docs build (SciTools#4333) Refactor some netcdf save code (SciTools#4301) [pre-commit.ci] pre-commit autoupdate (SciTools#4329) Update to loading docs to cover absence of 'or' for constraints (SciTools#4321) update latest.rst.template (SciTools#4323) stable cartopy feature download (SciTools#4328) Skip TestConstrainedLoad if data missing (SciTools#4319) Add 'Good First Issue' label to reasons an issue doesn't go stale (SciTools#4317) Gallery: simplify quiver example (SciTools#4120) Improve styling in a minor way in docs (SciTools#4314) bump version (SciTools#4310) Made clear we only test on Linux. (SciTools#4309) Updated environment lockfiles (SciTools#4308) Include Discussions in Getting Involved. (SciTools#4307) Fixed text to show as link. (SciTools#4305) [pre-commit.ci] pre-commit autoupdate (SciTools#4299) Updated environment lockfiles (SciTools#4298) cartopy feature download (SciTools#4304) Mesh Loading (AVD-1813) (SciTools#4262) reset whatsnew latest (SciTools#4288) ...

Fix broken ABF link

509634f

bjlittle added New: Pull Request Type: Documentation labels Sep 1, 2021

rcomer self-assigned this Sep 1, 2021

rcomer approved these changes Sep 1, 2021

View reviewed changes

pp-mo approved these changes Sep 1, 2021

View reviewed changes

bjlittle added 2 commits September 2, 2021 21:20

add cartopy downloader utility

c611e0d

update netcdf4-python link

5e72e2b

rcomer removed their assignment Sep 2, 2021

bjlittle commented Sep 2, 2021

View reviewed changes

.cirrus.yml Outdated Show resolved Hide resolved

bjlittle commented Sep 2, 2021

View reviewed changes

.cirrus.yml Show resolved Hide resolved

explicitly specify features to download for ci

9d05745

bjlittle changed the title ~~Fix broken ABF link~~ cartopy feature download Sep 2, 2021

added whatsnew entry

ddcb95f

bjlittle force-pushed the fix-abf-link branch from 60dae57 to ddcb95f Compare September 2, 2021 21:24

update utility help

ac6697c

bjlittle added the New: Feature label Sep 2, 2021

utility tidy

81481ed

bjlittle added 2 commits September 3, 2021 10:39

tidy utility

6c5ebfc

copy with cartopy script rename

f4af47b

update whatsnew to clarify as dev tool

f120bc3

jamesp requested changes Sep 3, 2021

View reviewed changes

bjlittle marked this pull request as draft September 3, 2021 12:35

bjlittle force-pushed the fix-abf-link branch from c151bd2 to 33655a0 Compare September 3, 2021 13:43

push fix to cartopy

9e1fbc8

bjlittle force-pushed the fix-abf-link branch from b9761d2 to 9e1fbc8 Compare September 3, 2021 13:55

bjlittle commented Sep 3, 2021

View reviewed changes

.cirrus.yml Outdated Show resolved Hide resolved

bjlittle marked this pull request as ready for review September 3, 2021 15:03

use cartopy master

be51d8d

trexfeathers approved these changes Sep 6, 2021

View reviewed changes

jamesp approved these changes Sep 6, 2021

View reviewed changes

jamesp merged commit f34305c into SciTools:main Sep 6, 2021

bjlittle mentioned this pull request Sep 6, 2021

Update CI cartopy_cache cartopy URL #4306

Closed

bjlittle deleted the fix-abf-link branch September 8, 2021 08:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cartopy feature download #4304

cartopy feature download #4304

bjlittle commented Sep 1, 2021

rcomer left a comment

pp-mo left a comment

rcomer commented Sep 1, 2021

bjlittle commented Sep 2, 2021 •

edited

Loading

rcomer commented Sep 2, 2021

rcomer commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading

rcomer commented Sep 3, 2021

trexfeathers commented Sep 3, 2021

bjlittle commented Sep 3, 2021

bjlittle commented Sep 3, 2021

pp-mo commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading

jamesp left a comment

rcomer commented Sep 3, 2021

bjlittle commented Sep 3, 2021

rcomer commented Sep 3, 2021

bjlittle commented Sep 3, 2021

cartopy feature download #4304

cartopy feature download #4304

Conversation

bjlittle commented Sep 1, 2021

🚀 Pull Request

Description

rcomer left a comment

Choose a reason for hiding this comment

pp-mo left a comment

Choose a reason for hiding this comment

rcomer commented Sep 1, 2021

bjlittle commented Sep 2, 2021 • edited Loading

rcomer commented Sep 2, 2021

rcomer commented Sep 3, 2021 • edited Loading

bjlittle commented Sep 3, 2021 • edited Loading

bjlittle commented Sep 3, 2021 • edited Loading

rcomer commented Sep 3, 2021

trexfeathers commented Sep 3, 2021

bjlittle commented Sep 3, 2021

bjlittle commented Sep 3, 2021

pp-mo commented Sep 3, 2021 • edited Loading

bjlittle commented Sep 3, 2021 • edited Loading

jamesp left a comment

Choose a reason for hiding this comment

rcomer commented Sep 3, 2021

bjlittle commented Sep 3, 2021

rcomer commented Sep 3, 2021

bjlittle commented Sep 3, 2021

bjlittle commented Sep 2, 2021 •

edited

Loading

rcomer commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading

pp-mo commented Sep 3, 2021 •

edited

Loading

bjlittle commented Sep 3, 2021 •

edited

Loading