New govcan_dl_resources() methods #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Some issue on window for a specific test, I'm investigating (kind of harder without a Windows machine though), |
|
I had a lot of trouble with windows, turn out part of the problem was with the virtual environment.... see actions/runner-images#712, the second solution solves this! |
|
@KevCaz To avoid all these |
|
Yep, that's what I ended up doing! |
|
This looks absolutely awesome. Will review this on Monday. |
VLucet
reviewed
Nov 30, 2020
| @@ -20,7 +20,7 @@ print.ckan_package_stack <- function(x, ...) { | |||
| } else { | |||
| cat(" Packages: \n") | |||
| cli::cat_line() | |||
| purrr::map(x[1:dim(x)], print_ckan_package_custom) | |||
| purrr::map(x[seq_len(dim(x))], print_ckan_package_custom) | |||
There was a problem hiding this comment.
Good point, I always forget this is best practice to use seq_len
VLucet
approved these changes
Nov 30, 2020
|
Also, I agree about what you said for downloads and html files. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @VLucet ,
I took some time to rewrite
govcan_dl_resources(), in a nutshell :ckan_package_stack,ckan_package,ckan_resource,ckan_resource_stackandcharacter.ìd_as_filename) to use resource id as file name, this was needed as sometimes different resources have the same names (...:):).tibble) is returned to identify which files were downloaded (see example below indetails) and to gather all URLs.Note that documents such as html or wms are not downloaded (and for the moment I honestly think it is for the best). Also, because of the difficulties with "session" in
ckan_fetch(), I think it makes more sense to just store files locally, so I always usestore = "disk"but this cam always be changed in a future release.Let me know what do you think but I think with a little more work on this, you will be closed to a nice first release to the CRAN.
I can (of course) give you a hand for the doc and to add more tests.