Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed rgdal from data.land and data.remote #3229

Merged
merged 8 commits into from
Nov 2, 2023
Merged

removed rgdal from data.land and data.remote #3229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b24df97
rgdal to sf in gis.functions
istfer Nov 1, 2023
87559d7
removed rgdal call from data.remote as well
istfer Nov 1, 2023
c97edb1
attempt to change raster with terra
istfer Nov 1, 2023
1114f56
add warning
istfer Nov 1, 2023
bfb03a8
remove library call to raster
istfer Nov 1, 2023
2031590
fix typo
istfer Nov 1, 2023
8263c71
remove library and require calls from data.remote
istfer Nov 2, 2023
9137628
CI updates
istfer Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion modules/data.land/DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ Suggests:
PEcAn.settings,
redland,
raster,
rgdal,
RPostgreSQL,
testthat (>= 1.0.2)
License: BSD_3_clause + file LICENSE
Expand Down
8 changes: 4 additions & 4 deletions modules/data.land/R/gis.functions.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,13 @@ shp2kml <- function(dir, ext, kmz = FALSE, proj4 = NULL, color = NULL, NameField
# Read in shapefile(s) & get coordinates/projection info shp.file <-
# readShapeSpatial(file.path(dir,i),verbose=TRUE) coordinates(test) <- ~X+Y

layers <- rgdal::ogrListLayers(file.path(dir, i))
info <- rgdal::ogrInfo(file.path(dir, i), layers)
layers <- sf::st_layers(file.path(dir, i))
# shp.file <- readOGR(file.path(dir,i),layer=layers) # no need to read in file

# Display vector info to the console
print("")
print(paste0("Input layers: ", layers))
print(paste0("Input projection info: ", info$p4s))
print(paste0("Input layers: ", layers$name))
print(paste0("Input projection info: ", layers$crs[[1]]$input))
print("")

# Write out kml/kmz using plotKML package if (is.null(color)){ color <- 'grey70' }
Expand Down Expand Up @@ -133,6 +132,7 @@ get.attributes <- function(file, coords) {

#library(fields)
#require(rgdal)
# note that OGR support is now provided by the sf and terra packages among others

# print('NOT IMPLEMENTED YET') subset_layer(file,coords)
} # get.attributes
Expand Down
4 changes: 2 additions & 2 deletions modules/data.remote/DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ Imports:
PEcAn.utils,
purrr,
XML,
raster,
sp,
MODISTools (>= 1.1.0),
reticulate,
PEcAn.logger,
magrittr,
PEcAn.remote,
stringr (>= 1.1.0),
terra,
doParallel,
parallel,
foreach
Expand All @@ -36,7 +36,7 @@ Suggests:
dplyr,
ggplot2,
lubridate,
rgdal,
raster,
reshape,
testthat (>= 1.0.2),
tibble
Expand Down
29 changes: 15 additions & 14 deletions modules/data.remote/R/NLCD.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ download.NLCD <- function(outdir, year = 2011, con = NULL) {

## before downloading, check if the file already exists on this host
if (!is.null(con)) {
library(PEcAn.DB)

chk <- dbfile.check(type = "Input", id = input.id, con = con)
if (nrow(chk) > 0) {
machines <- db.query(paste("SELECT * from machines where id in (",
Expand Down Expand Up @@ -68,11 +68,9 @@ download.NLCD <- function(outdir, year = 2011, con = NULL) {
##'
##' @description Based on codes from Christy Rollinson and from Max Joseph (http://mbjoseph.github.io/2014/11/08/nlcd.html)
extract_NLCD <- function(buffer, coords, data_dir = NULL, con = NULL, year = 2011) {
library(raster)
require(rgdal)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if raster's dependencies on rgdal have been fixed? If not, this fix could be pretty short lived. I know in other cases we're trying to migrate from raster to terra, but that's a harder fix than the current PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I don't know much about raster, but checking terra some lines can be changed, e.g.:

nlcd <- raster::raster(filename)

to

nlcd <- terra::rast(filename)

and

sites <- sp::spTransform(sites, raster::crs(nlcd))

to

sites <- sp::spTransform(sites, terra::crs(nlcd, proj=TRUE))

then probably instead of raster::extract, terra::extract could be used, but I'm not familiar with this function. Is there someone who can quickly check if such changes would work as intended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@istfer Given that this function isn't actively being used in any automated workflow at the moment (i.e. there's no one else who has things set up for a quick and easy test), I'd be OK with you making the obvious changes and leaving a note or warning that they haven't been rigorously tested.

FYI the closest analog is @JoshuaPloshay work on PR #3211 which uses terra to extract a number of other data products.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, I made an attempt to replace with terra functions (I found an nlcd image to test it with) and put a warning

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raster no longer imports rgdal. raster has replaced rgdal imports with terra so you do not need to change this (but probably good to do it eventually). If a packages is on CRAN still, it no longer uses the retired r-spatial packages. You can also check if you've gotten rid of all the rgdal dependencies with pak::local_deps_explain("rgdal", "modules/data.land")



if (!is.null(con)) {
library(PEcAn.DB)

if (year == 2001) {
input.id <- 1000000482
} else if (year == 2011) {
Expand Down Expand Up @@ -104,21 +102,24 @@ extract_NLCD <- function(buffer, coords, data_dir = NULL, con = NULL, year = 201
print(paste("File not found:", filename))
return(NULL)
}
nlcd <- raster(filename)

# WARNING: the following extraction previously used raster and sp package functions
# this new implementation with terra functions has not been thoroughly tested
nlcd <- terra::rast(filename)

# transform points
sites <- SpatialPoints(coords = coords, proj4string = CRS("+proj=longlat +datum=WGS84"))
sites <- spTransform(sites, crs(nlcd))
sites <- terra::vect(coords, geom=c("long", "lat"), crs="+proj=longlat +datum=WGS84")
sites <- terra::buffer(sites, width=buffer)

# extract
sum.raw <- table(extract(nlcd, sites, buffer = buffer))
sum.raw <- table(terra::extract(nlcd, sites))
summ <- prop.table(sum.raw)
mydf <- data.frame(cover = names(summ), percent = as.vector(summ), count = as.vector(sum.raw))
mydf <- data.frame(cover.name = colnames(summ), percent = as.vector(summ), count = as.vector(sum.raw))
mydf <- mydf[mydf$count!=0,]

# land cover number to name conversions
cover.table <- nlcd@data@attributes[[1]]
cover.names <- cover.table[as.numeric(as.character(mydf$cover)) + 1, grep("Land", names(cover.table))]
mydf$cover.name <- cover.names
# land cover name to number conversions
nlcd_levels <- terra::levels(nlcd)[[1]]
mydf$cover <- nlcd_levels$value[nlcd_levels$`Land Cover Class` %in% mydf$cover.name]

return(mydf)
} # extract_NLCD
2 changes: 1 addition & 1 deletion modules/data.remote/R/download.thredds.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ download.thredds.AGB <- function(outdir = NULL, site_ids, run_parallel = FALSE,
} else {
ncores <- parallel::detectCores() -1
}
require(doParallel)

PEcAn.logger::logger.info(paste0("Running in parallel with: ", ncores))
cl = parallel::makeCluster(ncores)
doParallel::registerDoParallel(cl)
Expand Down
2 changes: 0 additions & 2 deletions modules/data.remote/tests/Rcheck_reference.log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ Author field differs from that derived from Authors@R
* checking loading without being on the library search path ... OK
* checking use of S3 registration ... OK
* checking dependencies in R code ... WARNING
'library' or 'require' calls not declared from:
‘PEcAn.DB’ ‘doParallel’ ‘raster’
'library' or 'require' calls in package code:
‘PEcAn.DB’ ‘doParallel’ ‘raster’ ‘rgdal’
Please use :: or requireNamespace() instead.
Expand Down