do.extract function incorrectly calls spatial.dat object, not dat argument

[hrj21]

Hi all, me again (only because I love your package so much ;) ).

The do.extract() function seems to internally look for the object spatial.dat in the environment, rather than honouring the dat argument to the function. This is fine if we name our spatial object spatial.dat as in the tutorial, but I don't think this is intended behaviour. I've copied the function code below. The following lines seem problematic and should have dat instead of spatial.dat I think (or just change the name of the argument to spatial.dat):

• roi.stack <- spatial.dat[[roi]]$RASTERS
• roi.poly <- spatial.dat[[roi]]$MASKS[[mask]]$polygons
• other.polys <- names(spatial.dat[[roi]]$MASKS)
• ply <- spatial.dat[[roi]]$MASKS[[ply.name]]$polygons
• spatial.dat[[roi]]$DATA[[name]] <- roi.dat
• return(spatial.dat)

function (dat, mask, name = "CellData", fun = "mean") 
{
    require("rgeos")
    require("sp")
    require("rgdal")
    require("exactextractr")
    require("data.table")
    rois <- names(spatial.dat)
    for (roi in rois) {
        message(paste0("Processing ", roi))
        roi.stack <- spatial.dat[[roi]]$RASTERS
        roi.poly <- spatial.dat[[roi]]$MASKS[[mask]]$polygons
        raster.names <- names(roi.stack)
        ply.df <- as.data.frame(roi.poly)
        ply.df
        ply.centroids <- gCentroid(roi.poly, byid = TRUE)
        ply.centroids.df <- as.data.frame(ply.centroids)
        ply.centroids.df
        ply.centroids.df <- cbind(ply.centroids.df, as.data.frame(area(roi.poly)))
        names(ply.centroids.df)[3] <- "Area"
        for (i in raster.names) {
            message(paste0("... ", i))
            temp.dat <- roi.stack[[i]]
            res <- exactextractr::exact_extract(temp.dat, st_as_sf(roi.poly), 
                fun = fun)
            res <- as.data.table(res)
            names(res) <- i
            ply.centroids.df <- cbind(ply.centroids.df, res)
        }
        ID <- c(1:nrow(ply.centroids.df))
        roi.dat <- cbind(ID, ply.centroids.df)
        roi.dat <- as.data.table(roi.dat)
        other.polys <- names(spatial.dat[[roi]]$MASKS)
        other.polys <- other.polys[!other.polys %in% mask]
        cols <- c("x", "y", "ID")
        roi.dat.xyid <- roi.dat[, ..cols]
        names(roi.dat.xyid) <- c("Longitude", "Latitude", 
            "Names")
        roi.dat.xyid
        Longitude <- roi.dat.xyid$Longitude
        Latitude <- roi.dat.xyid$Latitude
        coordinates(roi.dat.xyid) <- ~Longitude + Latitude
        if (length(other.polys) != 0) {
            for (i in c(1:(length(other.polys)))) {
                ply.name <- other.polys[[i]]
                message(paste0("... occurance in ", ply.name))
                ply <- spatial.dat[[roi]]$MASKS[[ply.name]]$polygons
                proj4string(roi.dat.xyid) <- proj4string(ply)
                over.res <- over(roi.dat.xyid, ply)
                over.res <- as.data.table(over.res)
                roi.dat <- cbind(roi.dat, over.res)
            }
        }
        spatial.dat[[roi]]$DATA[[name]] <- roi.dat
    }
    return(spatial.dat)
}

[SamGG]

Just a simple reminder about library vs require https://yihui.org/en/2014/07/library-vs-require/

[ghar1821]

@hrj21 Thanks for spotting that. I've fixed the function up and pushed the changes to development branch.

@SamGG Thanks for the recommendation. We will need to revisit the way in which the dependencies are loaded and installed by the user. It may be an idea to simply install and load them up as users are using the relevant functions. Something like what is recommended here: https://statsandr.com/blog/an-efficient-way-to-install-and-load-r-packages/

[SamGG]

@ghar1821 Thanks for the link. The "More efficient way" is sufficient to me as it does not rely on other packages. While using installed.packages() sounds great for finding out packages to install, I don't think it is useful to load and test required packages. For example, https://github.com/ImmuneDynamics/Spectre/blob/742ebc4bc09ce69b970eceb78291bdbf5229d20d/R/read.files.R#L36 should be as simple as

library(Spectre)

or

if (!require('Spectre')) stop('Spectre is required but not installed') 

@hrj21 Sorry for diverging from your issue.

[hrj21]

@hrj21 Thanks for spotting that. I've fixed the function up and pushed the changes to development branch.

@SamGG Thanks for the recommendation. We will need to revisit the way in which the dependencies are loaded and installed by the user. It may be an idea to simply install and load them up as users are using the relevant functions. Something like what is recommended here: https://statsandr.com/blog/an-efficient-way-to-install-and-load-r-packages/

I can confirm this now works 👯‍♂️