umr-amap / aRchi

R package aRchi. Tree architecture from terrestrial laser scanning (TLS) data
22 stars 6 forks source link

Backward compatibility with lidR v4.0.0 #3

Closed Jean-Romain closed 2 years ago

Jean-Romain commented 3 years ago

Has you know I'm working on lidR v4 which will no longer be based on old R spatial packages (sp). One consequence is that LAS objects will no longer have a slot @proj4string.

Your package stores a serialized LAS object in Tree1_aRchi. This object will no longer be valid in v4 because it has a proj4string instead of a crs. You cannot yet modify your aRchi object because lidR v4 is not released and I won't be able to release lidR v4 because there is a backward incompatibility that triggers a warning with R CMD check

This PR provides a workaround and allows get_pointcloud to return a LASv4 if lidR v4 is installed and a LASv3 (original object stored in Tree1_aRchi) if lidR v3 is installed.

I don't know why git labelled so much code as "changed". It is probably a matter of file encoding.The only important part is get_pointcloud.R L25-45. I also changed aRchi version if DESCRIPTION

# Transparent backward compatibility with lidR v3 and lidR v4
# If the aRchi stores a LASv3 object it is converted
# into LASv4 but only if the lidR package installed is v4. Otherwise the
# LASv3 is preserved.
 if (is(aRchi@pointcloud, "LAS"))
 {
    if (!methods::.hasSlot(aRchi@pointcloud, "crs"))
    {
        pts <- aRchi@pointcloud@data
        phb <- aRchi@pointcloud@header@PHB
        vlr <- aRchi@pointcloud@header@VLR
        crs <- aRchi@pointcloud@proj4string
        return(suppressWarnings(lidR::LAS(pts, c(phb, vlr), proj4string = crs, check = FALSE)))
    }
}

Please update aRchi on CRAN before the release of lidR v4. I do no have schedules but I won't release lidR v4 before 2022 so, it is not in a hurry. Actually I discourage you to do it has soon as possible because lidR v4 is still in development so I may detect another problem latter.

If you want to get an eye on llidR v4 the current state is available in the branch v4. See also NEWS

Cheers

Jean-Romain commented 2 years ago

You can forget this PR. I found a way within lidR to be backward compatible.