Remove rgeos/rgdal dependencies and update functions for sf/terra/stars

[silasprincipe]

rgeos and rgdal will be retired later this year (in fact, rgeos is already down). obistools have some dependencies on these packages that will make it non functional soon (see lines 40/41 of NAMESPACE).

Suggestions:

• [ ] Convert dependency on rgeos::gDistance to sf::st_distance (to check)
• [ ] Convert dependency on rgeos::readWKT to sf::st_read or terra::vect (to check which one is better suited)
• [ ] Convert anything depending on sp to modern sf
• [ ] Convert anything depending on raster to modern and faster terra

This may include a revision of obistools to check if any function deserves an update or deprecation.

[reblake]

I ran into this dependency problem today, as I'm trying to do the exercises in Module 6 of the Contributing and publishing datasets to OBIS Course in Ocean Teacher. Now I'm stuck, as the course instructs you to use obistools, but I can't install obistools because I can't install rgeos. Is there any way this issue could be elevated in priority so folks taking the course can continue on with it? I'm sure I'm not the only one.

[pieterprovoost]

Thanks, we'll fix this asap.

[pieterprovoost]

Tracking this in https://github.com/iobis/obistools/tree/fix-spatial/.

[pieterprovoost]

@silasprincipe Do you have some time to work on the raster issue at https://github.com/iobis/obistools/blob/master/tests/testthat/test_check_depth.R#L83? Alternatively we can remove the custom bathymetry functionality for now. @rubenpp7, do any of your tools use the bathymetry parameter in check_depth()?

[pieterprovoost]

If anyone wants to test, please try:

devtools::install_github("iobis/obistools@fix-spatial")

Note that outlier detection is currently not working, and the use of external bathymetry layers has been disabled.

The package still depends on sp and raster due to the Leaflet dependency. See:

pkgnet::CreatePackageReport(pkg_name = "obistools")

[silasprincipe]

Hi @pieterprovoost,

I restored the custom bathymetry to use only terra functions. One thing that I was wondering if it would not be good to warn the user that depth in OBIS is expressed always as positive numbers for below surface (that is, something is 5m below water, and not -5). Sometimes people use bathymetry in negative terms, and supplying an all negative bathymetry layer would result all points raising a warning.

Also did some work on the calculate_centroid function to use only terra and not a mix sf + terra. Note that for now I added one workaround for MULTIPOINT type, but I'm contacting terra maintainer to see if there is a more elegant way of solving that.

Passing all tests through devtools::test(), except this one:

test_that("calculate_centroid accross dateline works", {
  centr <- calculate_centroid("POLYGON ((179 -1, 179 1, -179 1, -179 -1, 179 -1))")
  expect_equal(abs(centr[1,1]), 180)
  expect_equal(centr[1,2], 0)
})

But the previous version (mix sf+terra) was also failing at this one. Not sure if this is something we should bother for now.

Dependency from terra and sp are just because of leaflet now, and I suppose they will soon remove those dependencies.

[rubenpp7]

Hey @pieterprovoost and @silasprincipe ,

Thanks a lot for working on this. I am not using the bathymetry parameter in check_depth

[silasprincipe]

@pieterprovoost performed tests on Windows today. Had to add mode = "wb" when downloading the land shape for check_onland. Except for that, all the rest is working (tests only fail on the already mentioned test). I think the branch is ready to go.

[reblake]

Thanks all for your prompt work on this! I was able to use devtools::install_github("iobis/obistools@fix-spatial") to install obistools and successfully do almost everything in the instructional video for this module of the course. The exception was the two mapping functions plot_map() and plot_map_leaflet(), which produced errors or weird images. I'm not sure if that's at all related to this issue though, and I've found a workaround using ggplot so it's not a blocker for me. Thanks again!

[silasprincipe]

@reblake thanks for the feedback! I tested here and both the plot_map() and plot_map_leaflet() functions are working properly. Can you provide a few more details about when specifically the functions are producing errors (and also, which are those errors)? If possible, send also the result from sessionInfo().

@pieterprovoost I did one more commit: I removed the full import of the terra package to avoid the startup conflict messages. terra (as its predecessor raster) have several conflicts with packages from the tidyverse. So, its better for us to never import it in full on obistools, and always use terra::fun()

[reblake]

@silasprincipe , I've put an example that should reproduce the error and weird plot from the mapping functions (it does on my machine) in this gist rather than clogging up this issue. I've also included my sessionInfo() and a screenshot of the output on my machine. Again, this isn't a blocker for me, and it's not related to this issue, so happy to make a new issue if it is needed. Thanks.

[pieterprovoost]

@reblake Regarding the gist, is it possible that longitude and latitude are switched?

[silasprincipe]

@pieterprovoost yes, this is the problem - I just answered you @reblake on your gist about that, check the solution there.

[reblake]

Thanks @silasprincipe. My manual plotting worked exactly correctly, so I didn't think this was the problem.
Any chance you could add an informative error message to the two plotting functions in obistools? That would be helpful in giving package users a place to start troubleshooting plotting issues.