-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add new gather_images
function
#33
Conversation
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.
Thanks! this is a good idea. A few requests for change:
- Please use markdown docs, see https://roxygen2.r-lib.org/articles/markdown.html
- I don't use package
parallel
much. If someone doesn't want to run things in parallel, is that possible with the function as is? - for the utility functions added, I don't think they each need their own file given their simplicity, please put them in the file
zzz.R
OR put them both in a file likegather_images_utils.R
or similar - various comments on the code
DESCRIPTION
Outdated
@@ -4,7 +4,7 @@ Description: Work with 'Phylopic' web service (<http://phylopic.org/api/>) | |||
to get 'silhouette' images of 'organisms', search names, and more. | |||
Includes functions for adding 'silhouettes' to both base plots and | |||
'ggplot2' plots. | |||
Version: 0.3.2.91 | |||
Version: 0.3.3 |
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.
keep the dev version on please, e.g., 0.3.3.91
NEWS.md
Outdated
@@ -1,3 +1,10 @@ | |||
Version: 0.3.3 |
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.
I probably don't document this, but I try to push only even number versions to CRAN - change to 0.3.4 please
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.
This seems to conflict with your previous request. Could you clarify?
#33 (comment)
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.
Makes sense to have the version in the NEWS file be the next to go to cran whereas the version in the DESCRIPTION file i like to use the dev version until the cran push
I'm afraid I don't have time atm to learn this alternative syntax, but feel free to modify to your preference after merging.
The default is
I suppose this is a matter of coding preference, I always put separate functions in separate file to make them easy to find.
Addressing these now. |
Thanks, looks good |
Thanks for all your great work @sckott ! |
& thx for your contribution |
Description
There are multiple images per species, and not all image UIDs are valid.
This function makes searching for a valid image for each species much easier.
Searches for the best name match per \code{species} and
iteratively tries image UIDs until a viable image is found.
Parallelisation also implemented via
parallel::mclapply
to help speed up these queries.Example
Unit tests added in: tests/testthat/test-gather_images.R