-
Notifications
You must be signed in to change notification settings - Fork 225
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
Reorganize the list of data files for caching #3171
Conversation
pygmt/helpers/caching.py
Outdated
@@ -100,4 +89,5 @@ def cache_data(): | |||
"@tut_ship.xyz", | |||
"@usgs_quakes_22.txt", | |||
] | |||
which(fname=datasets, download="a") | |||
|
|||
which(fname=datasets + caches + tiles, download="a") |
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.
Can we continue to use just a single list?
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.
We can. I just feel that having three separate lists makes it easier to organize the file list.
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.
Well, codecov is warning that there's missing code coverage for the extra two lists 🙂
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.
That's a good point to push me to merge them into a single list.
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.
Done in 72fcaa1.
pygmt/helpers/caching.py
Outdated
@@ -100,4 +89,5 @@ def cache_data(): | |||
"@tut_ship.xyz", | |||
"@usgs_quakes_22.txt", | |||
] | |||
which(fname=datasets, download="a") | |||
|
|||
which(fname=datasets + caches + tiles, download="a") |
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.
Well, codecov is warning that there's missing code coverage for the extra two lists 🙂
Due to a likely upstream bug, we can't request multiple tiled grids in a singlewhich
call (xref: #3170). We have to callwhich
multiple times and download a single tiled grid in each call, to make the "Cache data" workflow pass again.I also take this opportunity to address the request in #3144.Closes #3144.
Edit: With the workaround in PR #3174, now it's OK to download multiple tiled grids. So, this PR only reorganize the list of data files for caching to make it easy to maintain.