ropensci / osmapiR

An R interface to OpenStreetMap API for fetching and saving data from/to the OpenStreetMap database
https://docs.ropensci.org/osmapiR/
GNU General Public License v3.0
20 stars 4 forks source link

Remove explicit doc links to osmdata pkg #13

Closed mpadge closed 8 months ago

mpadge commented 8 months ago

Hi @jmaspons, congratulations on submitting this package to rOpenSci! It'll be a great addition to the OSM ecosystem there. I was trying to debug why checks for this package didn't appear, and discovered that the following lines refer to functions from osmdata, yet that package is neither imported nor suggested here:

https://github.com/jmaspons/osmapiR/blob/6589fc37d48e187ee6b4106d4d6bcccb2a5c68c2/R/osm_get_objects.R#L28-L31

https://github.com/jmaspons/osmapiR/blob/6589fc37d48e187ee6b4106d4d6bcccb2a5c68c2/R/osmapi_elements.R#L679-L682

https://github.com/jmaspons/osmapiR/blob/6589fc37d48e187ee6b4106d4d6bcccb2a5c68c2/R/osmapi_elements.R#L906-L909

https://github.com/jmaspons/osmapiR/blob/6589fc37d48e187ee6b4106d4d6bcccb2a5c68c2/R/osmapi_miscellaneous.R#L222-L224

These become errors because the rOpenSci systems runs roxygen2::roxygenise() or the equivalent devtools::document(), while those are not called in standard rcmdcheck, which is why those checks all pass. roxygen2 has to then parse the function name from the package, which requires the package to be installed. My preferred way around this is to replace, for example,

The function [osmdata::opq()]

with

The \pkg{osmdata} function opq()

The \pkg{...} tag is resolved by roxygen simply by examining available.packages(), regardless of whether or not they are installed, and so does not error. The function name is then just plain code that requires no interpretation. When you've fixed those, feel free to ping the bot to run checks again.

mpadge commented 8 months ago

And BTW, installing the pkgcheck-action in this repo would have helped diagnose that. I'm sure it will help you progress through review if you have that workflow running here too.

jmaspons commented 8 months ago

Thanks, fixed! I will have a look to the pkgcheck-action