fixes terra plot problem

[rhijmans]

The current version of {lidR} "freezes" terra::plot. That is, if you first install current-terra and then current-lidR; and then install devel-terra plot<SpatRaster> does not work anymore. In that scenario (as in a CRAN update of terra), after loading the lidR package, the terra::plot method used is the current-terra method; which is now provided by lidR. That creates problems because it depends on calls to internal functions that have changed. I discovered this in reverse dependency checking of {rcaiman}. That package has, in its examples, calls to plotRGB and plot with SpatRaster arguments. This now stops me from submitting terra to CRAN.

The problem goes away when installing the lidR package again (from source, not when installing from the CRAN binary, on windows; the binary version would need to be rebuilt first).

I am not sure why this happens; it seems to be a NAMSPACE problem, perhaps related to mixing S3 and S4 methods. The changes I made in the PR do fix the problem, but there may be other, better, paths to fix this.

I first changed the one S3 plot method to an S4 method and then ran roxygen2::roxygenise. I then added if (!isGeneric("plot")) to setGeneric("plot", function(x, y, ...) (not sure if that is needed); and I edited the NAMESPACE (I do not know how to do this via roxygen.

With these changes, the below works as expected

# install CRAN-raster
# install (dev-) LidR
# install dev-raster

library(terra)
b <- rast(system.file("ex/logo.tif", package="terra"))   
plotRGB(b)
library(lidR)
plotRGB(b)

But without these changes you get

library(lidR)
plotRGB(b)
#Error in .local(x, y, ...) : 
#  (min(y) > 0) & (max(y) <= nlyr(x)) is not TRUE

And that is not surprising given that the plot<SpatRaster,numeric> method (called by plotRGB) changes when loading lidR:

library(terra)
#terra 1.7.25
head(getMethod("plot", c("SpatRaster", "numeric")), 25)

#1  new("MethodDefinition", .Data = function (x, y = 1, ...)              
#2  {                                                                     
#3      .local <- function (x, y = 1, col, type = NULL, mar = NULL,       
#4          legend = TRUE, axes = TRUE, plg = list(), pax = list(),       
#5          maxcell = 5e+05, smooth = FALSE, range = NULL, levels = NULL, 
#6          all_levels = FALSE, breaks = NULL, breakby = "eqint",         
#7          fun = NULL, colNA = NULL, alpha = NULL, sort = FALSE,         
#8          decreasing = FALSE, grid = FALSE, ext = NULL, reset = FALSE,  
#9          add = FALSE, buffer = FALSE, background = NULL, box = axes,   
#10         clip = TRUE, ...)                                             
#11     {                                                                 
#12         y <- round(y)                                                 
#13         hasRGB <- FALSE                                               
#14         if (has.RGB(x) && ((is.null(type) && (y[1] < 0)))) {          
#15             type <- "rgb"                                             
#16             legend <- FALSE                                           
#17             if (is.null(mar)) {                                       
#18                 mar <- 0                                              
#19                 axes <- FALSE                                         
#20             }                                                         
#21             hasRGB <- TRUE                                            
#22             y <- RGB(x)                                               
#23         }                                                             
#24         stopifnot((min(y) > 0) & (max(y) <= nlyr(x)))                 
#25         if ((!hasRGB) && (length(y) > 1)) {                           

library(lidR)
#lidR 4.0.3 using 4 threads. Help on <gis.stackexchange.com>. Bug report on <github.com/r-lidar/lidR>.

head(getMethod("plot", c("SpatRaster", "numeric")), 25)

#1  new("MethodDefinition", .Data = function (x, y = 1, ...)                          
#2  {                                                                                 
#3      .local <- function (x, y = 1, col, type, mar = NULL, legend = TRUE,           
#4          axes = TRUE, plg = list(), pax = list(), maxcell = 5e+05,                 
#5          smooth = FALSE, range = NULL, levels = NULL, all_levels = FALSE,          
#6          breaks = NULL, breakby = "eqint", fun = NULL, colNA = NULL,               
#7          alpha = NULL, sort = FALSE, decreasing = FALSE, grid = FALSE,             
#8          ext = NULL, reset = FALSE, add = FALSE, buffer = FALSE,                   
#9          background = NULL, box = axes, clip = TRUE, ...)                          
#10     {                                                                             
#11         y <- round(y)                                                             
#12         stopifnot((min(y) > 0) & (max(y) <= nlyr(x)))                             
#13         if (length(y) > 1) {                                                      
#14             x <- x[[y]]                                                           
#15             if (inherits(alpha, "SpatRaster")) {                                  
#16                 if (nlyr(alpha) > 1) {                                            
#17                   alpha <- alpha[[y]]                                             
#18                 }                                                                 
#19             }                                                                     
#20             plot(x, col = col, type = type, mar = mar, legend = legend,           
#21                 axes = axes, plg = plg, pax = pax, maxcell = 2 *                  
#22                   maxcell/length(y), smooth = smooth, range = range,              
#23                 levels = levels, all_levels = all_levels, breaks = breaks,        
#24                 breakby = breakby, fun = fun, colNA = colNA,                      
#25                 alpha = alpha, grid = grid, sort = sort, decreasing = decreasing, 

[Jean-Romain]

And that is not surprising given that the plot<SpatRaster,missing> method (called by plotRGB) changes when loading lidR:

How is it possible that loading lidR changes the code of the plot function in terra ??

I have been able to reproduce your error once or two after installing terra-devel. Now I'm no longer able to reproduce (I did not change anything). The issue is not constantly reproducible on my side.

[rhijmans]

The issue is not constantly reproducible on my side.

It should be if you install in this order: CRAN-terra, CRAN-lidR, devel-terra.

How is it possible that loading lidR changes the code of the plot function in terra ??

It is possible if, during INSTALL, lidR imports plot<SpatRaster,missing> and then ... (re-exports it??) I am not sure what triggers the problem here. But I have seen something like this before where a package made a generic out of a raster function and then implemented the generic for one of their classes.

[Jean-Romain]

I wrote a script that install automatically cran-terra -> source-lidR -> devel-terra in a R vanilla session. I'm now able to reproduce consistently. But it takes a solid 10 minutes to run (compiling terra twice + lidR once)

[rhijmans]

You can avoid the compilation on Windows if you use the binary versions from CRAN and R-Universe. I sent terra to CRAN but somehow their revdep check did not find this (it did find something else that I did not find). I wonder if they rebuild the packages before checking.

[Jean-Romain]

I do not have Windows. I will work on it tomorrow.