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

Jump to bottom

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 2 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
1 change: 0 additions & 1 deletion modules/data.remote/DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ Suggests:
dplyr,
ggplot2,
lubridate,
rgdal,
reshape,
testthat (>= 1.0.2),
tibble
Expand Down
3 changes: 1 addition & 2 deletions modules/data.remote/R/NLCD.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ 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) {
Expand Down
Loading