r-spatial / mapview

Interactive viewing of spatial data in R
https://r-spatial.github.io/mapview/
GNU General Public License v3.0
515 stars 91 forks source link

raster dependency #418

Closed tim-salabim closed 2 years ago

tim-salabim commented 2 years ago

I got the following emails from CRAN team:

Dear maintainer,

Please see the problems shown on https://cran.r-project.org/web/checks/check_results_mapview.html.

Please correct before 2022-04-26 to safely retain your package on CRAN.

The CRAN Team

and

This is seen for checks with _R_CHECK_SUGGESTSONLY=true , including check --as-cran. Ultimately you are calling raster which only has rgdal in Suggests, so you need to declare your dependence (likely in others of your packages too).

The resulting error is:

Error in .requireRgdal() : package 'rgdal' is not available

.requireRgdal() is an unexported function of the raster package.

I can reproduce the error with

export _R_CHECK_SUGGESTS_ONLY_=true
R CMD build mapview --resave-data
R CMD check mapview_2.10.4.9007.tar.gz --as-cran

If I understand the CRAN suggestion correctly, I would either need to declare rgdal as a Suggests in my package (which seems to work - checks cleanly on my local machine) or Depend on raster (not tried). I am assuming that this will affect all packages that are using raster and at least have examples or tests run during R CMD check.

In the light of the propsed retirement of rgdal at the end of 2022(3?), it seems a bit odd to be forced to state it as a Suggests now.

How do we envision this to be addressed in the long run? @rhijmans @rsbivand @edzer

I have no problem with this rgdal Suggests as a hotfix, but it clearly only is a hotfix...

edzer commented 2 years ago

In the light of the propsed retirement of rgdal at the end of 2022(3?), it seems a bit odd to be forced to state it as a Suggests now.

It's probably the easiest, and won't be harmful. Most of the real work lies with raster, which will have to make itself independent from rgdal when calling (many of the) functions that use GDAL.

rsbivand commented 2 years ago

raster now Imports terra, so (as yet untried) using that package might work (but I don't see terra in mapview now).

rsbivand commented 2 years ago

Just cloned 9009 passes on released and devel r82118 (that is with rgdal in Suggests), with the env. variable given.

tim-salabim commented 2 years ago

raster now Imports terra, so (as yet untried) using that package might work (but I don't see terra in mapview now).

Yeah, providing methods for terra is still on my to-do list. But that wouldn't solve the raster issue as we'd still want to keep the raster methods (as we do for e.g. sp - for now).

tim-salabim commented 2 years ago

Just cloned 9009 passes on released and devel r82118 (that is with rgdal in Suggests), with the env. variable given.

Thanks, it also checks cleanly on win-builder (r-devel) now.

rsbivand commented 2 years ago

My suspicion is that raster may move towards replacing calls out to rgdal by calls to *'terra** at some point.

tim-salabim commented 2 years ago

Ok, I'll stick with rgdal in suggests for now and wait how raster things pan out. Thanks for all the input!

rhijmans commented 2 years ago

This is just to confirm that the plan for raster is to eventually replace functions from rgdal and rgeos with equivalent functions from terra. By making raster dependent on terra I have made one big step in that direction. But I am otherwise not in a hurry, because I prefer to work on terra; and because I am assuming that the "retirement" of rgdal and rgeos (or of their maintainer, really) will not immediately break things.

rsbivand commented 2 years ago

It is likely that retirement will mean archiving the three packages at the latest ultimo 2023. Creating masking sequel packages creates more maintenance, so unless sp methods are involved, especially sp::spTransform(), the best solution is hard retirement. We'll know more by August this year.