-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update docs and dependencies #302
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.
Actually, moving sp
to Suggests - which is a really great idea!! - then needs each function which actually uses it to first have:
requireNamespace ("sp")
Those 2 functions are
Line 16 in 85cd695
osm_elevation <- function (dat, elev_file) { |
and
Line 63 in 85cd695
trim_osmdata_sfp <- function (dat, bb_poly, exclude = TRUE) { |
The latter has an example of how it'd done here - initial lines at the very start of the function. Thanks
@jmaspons Once you address the above, i'll happily approve and merge. I also wanted to arrange a video chat with you, mostly so that i can express my gratitude for all of your work. I don't have contact details for you, so can you please either email me, or you should be able to book a meeting at cal.com / markpadgham. Thanks. |
Co-authored-by: mark padgham <mark.padgham@email.com>
Hopfully solved! I'll contact you, @mpadge . Let's find some day for next week |
Perhaps it requires some more work to move |
Sorry, that's my fault. Just need to delete this line: Line 430 in 85cd695
That was just there for some historical reason I can no longer remember, but is no longer needed. |
Thanks @jmaspons. One more thing that generally needs to be done is to update the NEWS file. I'll merge for now and do that straight away, but we should both try to remember to always update NEWS with each PR 😀 |
Added missing
osmdata_*
functions,silicate
as a suggested dependency andsp
moved from imported to suggested