Some feedback

[sckott]

• Export explicitly what you want users to have available in the package, instead of using exportPattern("^[^\\.]") in the NAMESPACE file, using devtools::document() e.g, to generate that automatically is easiest
• Use @importFrom statements to import only the functions you need from dependencies instead of importing everything. E.g., I often do this in a package level man file, eg., https://github.com/ropensci/rgbif/blob/master/R/rgbif-package.r#L28-L32
• Related to last point, good to have a package level manual file, so when users try ?helminthR or ?helminthR-package they get a man page instead of an error that it's not found
• When I ran check, I ran into a variety of things:
  • The examples take a long time to run. I'd suggest having examples that don't take that long to run, if possible.
  • Given that this package interacts with web resources, I'd wrap all examples that make web requests in \dontrun{}. You can still run examples e.g., devtools::run_examples(run = FALSE)
  • I get: Malformed Title field: should not end in a period.
  • Looks like you use ggmap in the listlocations fxn. Definitely list that in imports/suggests as appropriate instead of loading within a function
  • I get: listLocations: no visible global function definition for ‘geocode’
• I'd recommend using a README.Rmd and generate your README.md from that
• I'd recommend making the Title in the DESCRIPTION file shorter, and put longer form description in the Description field in that file.
• I don't see where you use devtools, but it's listed as in import?
• The listlocations() function:
  • use of geocode() could perhaps be replaced with something from geonames package? geocode() seems to be very slow, though if you keep it, and even if you don't, caching the results would be good, so that a user doesn't have to run listlocations() every time for a new function call.
  • If the geocoding part is going to take a long time, I'd suggest doing that as a separate function call, so listlocations would just get the data from NHM, and if a user wants to get lat/long data, call a sep. function.
• I think rvest is great, but for using within a package to be distributed to others, I'd recommend using httr to request data, then xml2 to parse the html/xml. Using httr gives you proper controls over web requests, and gives nice error messages when things go wrong.

[taddallas]

• Export explicitly what you want users to have available in the package, instead of using exportPattern("^[^\\.]") in the NAMESPACE file, using devtools::document() e.g, to generate that automatically is easiest

This has been changed.

• Use @importFrom statements to import only the functions you need from dependencies instead of importing everything. E.g., I often do this in a package level man file, eg., https://github.com/ropensci/rgbif/blob/master/R/rgbif-package.r#L28-L32

This has been changed

• Related to last point, good to have a package level manual file, so when users try ?helminthR or ?helminthR-package they get a man page instead of an error that it's not found

This has been changed.

• When I ran check, I ran into a variety of things:
• The examples take a long time to run. I'd suggest having examples that don't take that long to run, if possible.
• Given that this package interacts with web resources, I'd wrap all examples that make web requests in \dontrun{}. You can still run examples e.g., devtools::run_examples(run = FALSE)
• I get: Malformed Title field: should not end in a period.
• Looks like you use ggmap in the listlocations fxn. Definitely list that in imports/suggests as appropriate instead of loading within a function
• I get: listLocations: no visible global function definition for ‘geocode’

I believe I've addressed all of the above.

• I'd recommend using a README.Rmd and generate your README.md from that

What are the benefits of writing my README as an Rmd instead of a md?

• I'd recommend making the Title in the DESCRIPTION file shorter, and put longer form description in the Description field in that file.

This has been changed.

• I don't see where you use devtools, but it's listed as in import?

I have no idea, but I also listed testthat as an import, which I don't think needs to be imported, right?

• The listlocations() function:
• use of geocode() could perhaps be replaced with something from geonames package? geocode() seems to be very slow, though if you keep it, and even if you don't, caching the results would be good, so that a user doesn't have to run listlocations() every time for a new function call.
• If the geocoding part is going to take a long time, I'd suggest doing that as a separate function call, so listlocations would just get the data from NHM, and if a user wants to get lat/long data, call a sep. function.

Caching the results of listLocation() sounds like a good idea. Have I done this correctly? geocode seems to be a bit more suited to my needs than anything I've found in the geonames package.

• I think rvest is great, but for using within a package to be distributed to others, I'd recommend using httr to request data, then xml2 to parse the html/xml. Using httr gives you proper controls over web requests, and gives nice error messages when things go wrong.

haha. So I need to change lots of functions, since I used XML and rvest for most of the heavy lifting. I might put this off for a bit.

---

Some additional things I could use some advice on:

• Travis is unhappy, telling me I'm running on the legacy infrastructure and erroring out for the oldrel run with the following error:

The command "./pkg-build.sh bootstrap" failed and exited with 1 during .

• error thrown during R CMD check

• checking for unstated dependencies in ‘tests’ ... WARNING 'library' or 'require' call not declared from: ‘testthat’

• Errors about no visible binding for the cached location (in data folder) or for the .cleanData function (in r folder)

no visible binding for global variable ‘locations’ no visible global function definition for ‘.cleanData’

[sckott]

What are the benefits of writing my README as an Rmd instead of a md?

That way you can easily update the entire thing by doing e.g. knitr::knit("README.Rmd") rather than having to update code blocks manually. Some prefer to use the rmarkdown package

I have no idea, but I also listed testthat as an import, which I don't think needs to be imported, right?

Only if you have tests and use testthat, and it would go in Suggests, as it would be needed to run tests

---

your Additional feedback

Travis is unhappy, telling me I'm running on the legacy infrastructure and erroring out for the oldrel run with the following error:

Link to a travis build where this happens?

error thrown during R CMD check

here https://github.com/ropensci/helminthR/blob/master/DESCRIPTION#L10 move testthat to Suggests, not Imports

Errors about no visible binding for the cached location (in data folder) or for the .cleanData function (in r folder)

Likely you'll just need to add those names to globals, like this https://github.com/ropensci/taxize/blob/master/R/globals.R

[taddallas]

Sounds good. I will work on updating the README file sometime in the coming days. I think I've properly input all the Imports and Suggests now.

I think the Travis issue is because my travis.yml file has the flag sudo: required which is incompatible with Travis' new container structure. So it's not actually an error. Do you know of any workaround for this? I guess I should use language: r instead of language: c but that's sort of a side issue, right?

I've added you as an author (at least in the helminthR-package.R file). Do you have any objection? There's still the open issue of obtaining helminth parasite taxonomic data, which I was hoping you could help with. Catalogue of Life doesn't seem to have much on helminths.

[sckott]

yeah, language: r vs. language: c is just preference, language: r uses the new travis containerized builds, which is supposed to be faster, sometimes that does seem true, but if you use r-builder https://github.com/metacran/r-builder to build on multiple R versions, you have to use language: c

I've added you as an author (at least in the helminthR-package.R file). Do you have any objection?

Totally up to you. I don't think i've contributed any code https://github.com/ropensci/helminthR/graphs/contributors , would be more warranted if I do contribute :)

There's still the open issue of obtaining helminth parasite taxonomic data, which I was hoping you could help with.

Definitely, let me know

[taddallas]

I'm closing the issue, but plan to revisit the helminth taxonomic data (through WoRMS, I guess?) at a later point. I appreciate your help with the package, and hope that folks use it to get at some cool questions.