From a157976a363c01dafdc79f9d9e2f8e04369d7d9d Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 11:14:06 -0700 Subject: [PATCH 01/10] add test that resources get passed --- tests/testthat/test-tar-terra.R | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/testthat/test-tar-terra.R b/tests/testthat/test-tar-terra.R index 802debf..061f6f6 100644 --- a/tests/testthat/test-tar-terra.R +++ b/tests/testthat/test-tar-terra.R @@ -87,3 +87,24 @@ targets::tar_test("tar_terra_vect() works with multiple workers (tests marshalin expect_s4_class(targets::tar_read(vect1), "SpatVector") }) +targets::tar_test("user resources are passed correctly", { + persistent <- crew_controller_local(name = "persistent") + transient <- crew_controller_local(name = "transient", tasks_max = 1L) + targets::tar_option_set( + controller = crew_controller_group(persistent, transient), + resources = tar_resources( + crew = tar_resources_crew(controller = "transient") + )) + testthat::expect_equal( + tar_terra_rast(x, 1)$settings$resources$crew, + tar_resources_crew(controller = "transient") + ) + testthat::expect_equal( + tar_terra_rast( + x, 1, + resources = tar_resources(crew = tar_resources_crew(controller = "persistent")) + )$settings$resources$crew, + tar_resources_crew(controller = "persistent") + ) +}) + From 822ce696bbd5d6e94b7861174d14b37fa11d97ad Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 11:35:37 -0700 Subject: [PATCH 02/10] use modifyList to combine user supplied resources with those set by tar_terra* --- R/tar-terra-rast.R | 24 +++++++++++++++--------- R/tar-terra-sprc.R | 21 +++++++++++++-------- R/tar-terra-vect.R | 22 ++++++++++++++-------- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/R/tar-terra-rast.R b/R/tar-terra-rast.R index 2449873..edc0eed 100644 --- a/R/tar-terra-rast.R +++ b/R/tar-terra-rast.R @@ -57,6 +57,11 @@ tar_terra_rast <- function(name, check_pkg_installed("terra") + #ensure that user-passed `resources` doesn't include `custom_format` + if ("custom_format" %in% names(resources)) { + stop("`custom_format` cannot be supplied to targets created with `tar_terra_rast()`") + } + name <- targets::tar_deparse_language(substitute(name)) envir <- targets::tar_option_get("envir") @@ -103,17 +108,18 @@ tar_terra_rast <- function(name, garbage_collection = garbage_collection, deployment = deployment, priority = priority, - resources = targets::tar_resources( - custom_format = targets::tar_resources_custom_format( - #these envvars are used in write function of format - envvars = c( - "GEOTARGETS_GDAL_RASTER_DRIVER" = filetype, - "GEOTARGETS_GDAL_RASTER_CREATION_OPTIONS" = ( - paste0(gdal, collapse = ";") + resources = modifyList( + targets::tar_resources( + custom_format = targets::tar_resources_custom_format( + #these envvars are used in write function of format + envvars = c( + "GEOTARGETS_GDAL_RASTER_DRIVER" = filetype, + "GEOTARGETS_GDAL_RASTER_CREATION_OPTIONS" = ( + paste0(gdal, collapse = ";") ) ) - ) - ), + ) + ), resources), storage = storage, retrieval = retrieval, cue = cue diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index c5068b5..f6ceb76 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -62,6 +62,10 @@ tar_terra_sprc <- function(name, retrieval = targets::tar_option_get("retrieval"), cue = targets::tar_option_get("cue")) { check_pkg_installed("terra") + #ensure that user-passed `resources` doesn't include `custom_format` + if ("custom_format" %in% names(resources)) { + stop("`custom_format` cannot be supplied to targets created with `tar_terra_rast()`") + } gdal <- gdal %||% character(0) filetype <- filetype %||% "GTiff" @@ -124,16 +128,17 @@ tar_terra_sprc <- function(name, deployment = deployment, priority = priority, resources = targets::tar_resources( - custom_format = targets::tar_resources_custom_format( - #these envvars are used in write function of format - envvars = c( - "GEOTARGETS_GDAL_RASTER_DRIVER" = filetype, - "GEOTARGETS_GDAL_RASTER_CREATION_OPTIONS" = ( - paste0(gdal, collapse = ";") + custom_format = modifyList( + targets::tar_resources_custom_format( + #these envvars are used in write function of format + envvars = c( + "GEOTARGETS_GDAL_RASTER_DRIVER" = filetype, + "GEOTARGETS_GDAL_RASTER_CREATION_OPTIONS" = ( + paste0(gdal, collapse = ";") ) + ) ) - ) - ), + ), resources), storage = storage, retrieval = retrieval, cue = cue diff --git a/R/tar-terra-vect.R b/R/tar-terra-vect.R index 9801e0c..7837729 100644 --- a/R/tar-terra-vect.R +++ b/R/tar-terra-vect.R @@ -66,6 +66,11 @@ tar_terra_vect <- function(name, check_pkg_installed("terra") + #ensure that user-passed `resources` doesn't include `custom_format` + if ("custom_format" %in% names(resources)) { + stop("`custom_format` cannot be supplied to targets created with `tar_terra_vect()`") + } + name <- targets::tar_deparse_language(substitute(name)) envir <- targets::tar_option_get("envir") @@ -103,16 +108,17 @@ tar_terra_vect <- function(name, deployment = deployment, priority = priority, resources = targets::tar_resources( - custom_format = targets::tar_resources_custom_format( - #these envvars are used in write function of format - envvars = c( - "GEOTARGETS_GDAL_VECTOR_DRIVER" = filetype, - "GEOTARGETS_GDAL_VECTOR_CREATION_OPTIONS" = ( - paste0(gdal, collapse = ";") + custom_format = modifyList( + targets::tar_resources_custom_format( + #these envvars are used in write function of format + envvars = c( + "GEOTARGETS_GDAL_VECTOR_DRIVER" = filetype, + "GEOTARGETS_GDAL_VECTOR_CREATION_OPTIONS" = ( + paste0(gdal, collapse = ";") ) + ) ) - ) - ), + ), resources), storage = storage, retrieval = retrieval, cue = cue From 59a1925b6e648bc4b31de457de0dce570dc259e9 Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 11:42:13 -0700 Subject: [PATCH 03/10] namespace functions --- R/tar-terra-rast.R | 2 +- R/tar-terra-sprc.R | 2 +- R/tar-terra-vect.R | 2 +- tests/testthat/test-tar-terra.R | 7 ++++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/R/tar-terra-rast.R b/R/tar-terra-rast.R index edc0eed..09e4642 100644 --- a/R/tar-terra-rast.R +++ b/R/tar-terra-rast.R @@ -108,7 +108,7 @@ tar_terra_rast <- function(name, garbage_collection = garbage_collection, deployment = deployment, priority = priority, - resources = modifyList( + resources = utils::modifyList( targets::tar_resources( custom_format = targets::tar_resources_custom_format( #these envvars are used in write function of format diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index f6ceb76..6d2ee0f 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -128,7 +128,7 @@ tar_terra_sprc <- function(name, deployment = deployment, priority = priority, resources = targets::tar_resources( - custom_format = modifyList( + custom_format = utils::modifyList( targets::tar_resources_custom_format( #these envvars are used in write function of format envvars = c( diff --git a/R/tar-terra-vect.R b/R/tar-terra-vect.R index 7837729..6d9b14e 100644 --- a/R/tar-terra-vect.R +++ b/R/tar-terra-vect.R @@ -108,7 +108,7 @@ tar_terra_vect <- function(name, deployment = deployment, priority = priority, resources = targets::tar_resources( - custom_format = modifyList( + custom_format = utils::modifyList( targets::tar_resources_custom_format( #these envvars are used in write function of format envvars = c( diff --git a/tests/testthat/test-tar-terra.R b/tests/testthat/test-tar-terra.R index 061f6f6..cde4406 100644 --- a/tests/testthat/test-tar-terra.R +++ b/tests/testthat/test-tar-terra.R @@ -88,10 +88,11 @@ targets::tar_test("tar_terra_vect() works with multiple workers (tests marshalin }) targets::tar_test("user resources are passed correctly", { - persistent <- crew_controller_local(name = "persistent") - transient <- crew_controller_local(name = "transient", tasks_max = 1L) + library(crew) + persistent <- crew::crew_controller_local(name = "persistent") + transient <- crew::crew_controller_local(name = "transient", tasks_max = 1L) targets::tar_option_set( - controller = crew_controller_group(persistent, transient), + controller = crew::crew_controller_group(persistent, transient), resources = tar_resources( crew = tar_resources_crew(controller = "transient") )) From 845c0dc7cdc71bcaeaa8802a19e2ef90965909ab Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 11:59:22 -0700 Subject: [PATCH 04/10] add tests for vect and sprc --- tests/testthat/test-tar-terra.R | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/testthat/test-tar-terra.R b/tests/testthat/test-tar-terra.R index cde4406..7328906 100644 --- a/tests/testthat/test-tar-terra.R +++ b/tests/testthat/test-tar-terra.R @@ -1,4 +1,5 @@ # test_that() #Included to make RStudio recognize this file as a test +library(targets) targets::tar_test("tar_terra_rast() works", { # geotargets::geotargets_option_set(gdal_raster_creation_options = c("COMPRESS=DEFLATE", "TFW=YES")) targets::tar_script({ @@ -107,5 +108,19 @@ targets::tar_test("user resources are passed correctly", { )$settings$resources$crew, tar_resources_crew(controller = "persistent") ) + testthat::expect_equal( + tar_terra_vect( + x, 1, + resources = tar_resources(crew = tar_resources_crew(controller = "persistent")) + )$settings$resources$crew, + tar_resources_crew(controller = "persistent") + ) + testthat::expect_equal( + tar_terra_sprc( + x, 1, + resources = tar_resources(crew = tar_resources_crew(controller = "persistent")) + )$settings$resources$crew, + tar_resources_crew(controller = "persistent") + ) }) From e9219a0aafd129d33e761b2f79e0a66af6350665 Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 12:05:21 -0700 Subject: [PATCH 05/10] fix modifyList location --- R/tar-terra-sprc.R | 6 +++--- R/tar-terra-vect.R | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index 6d2ee0f..6be2212 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -127,9 +127,9 @@ tar_terra_sprc <- function(name, garbage_collection = garbage_collection, deployment = deployment, priority = priority, - resources = targets::tar_resources( - custom_format = utils::modifyList( - targets::tar_resources_custom_format( + resources = utils::modifyList( + targets::tar_resources( + custom_format = targets::tar_resources_custom_format( #these envvars are used in write function of format envvars = c( "GEOTARGETS_GDAL_RASTER_DRIVER" = filetype, diff --git a/R/tar-terra-vect.R b/R/tar-terra-vect.R index 6d9b14e..6d6bcf3 100644 --- a/R/tar-terra-vect.R +++ b/R/tar-terra-vect.R @@ -107,9 +107,9 @@ tar_terra_vect <- function(name, garbage_collection = garbage_collection, deployment = deployment, priority = priority, - resources = targets::tar_resources( - custom_format = utils::modifyList( - targets::tar_resources_custom_format( + resources = utils::modifyList( + targets::tar_resources( + custom_format = targets::tar_resources_custom_format( #these envvars are used in write function of format envvars = c( "GEOTARGETS_GDAL_VECTOR_DRIVER" = filetype, From e4ca967d05fddd0880bfbf1dc5c99d003df95460 Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 12:05:30 -0700 Subject: [PATCH 06/10] isolate changes to options in tests --- tests/testthat/test-geotargets-option.R | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/testthat/test-geotargets-option.R b/tests/testthat/test-geotargets-option.R index fef5fff..10bdaca 100644 --- a/tests/testthat/test-geotargets-option.R +++ b/tests/testthat/test-geotargets-option.R @@ -18,7 +18,11 @@ targets::tar_test("geotargets_options_get() retrieves options in correct priorit ) }) + test_that("geotargets_option_set() works", { + op <- getOption("geotargets.gdal.raster.driver") + withr::defer(options("geotargets.gdal.raster.driver" = op)) + geotargets_option_set(gdal_raster_driver = "COG") expect_equal(getOption("geotargets.gdal.raster.driver"), "COG") expect_equal(geotargets_option_get("gdal.raster.driver"), "COG") @@ -26,6 +30,11 @@ test_that("geotargets_option_set() works", { }) test_that("options aren't reset with multiple calls to geotargets_option_set()", { + op_rast <- getOption("geotargets.gdal.raster.driver") + withr::defer(options("geotargets.gdal.raster.driver" = op_rast)) + op_vect <- getOption("geotargets.gdal.vector.driver") + withr::defer(options("geotargets.gdal.vector.driver" = op_vect)) + geotargets_option_set(gdal_raster_driver = "GPKG") geotargets_option_set(gdal_vector_driver = "GPKG") expect_equal(geotargets_option_get("gdal_vector_driver"), "GPKG") From 8a76cb5ef1e1ccef94dc19489398d1aab9185d50 Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Thu, 9 May 2024 13:05:25 -0700 Subject: [PATCH 07/10] fix error msg --- R/tar-terra-sprc.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index 6be2212..9f0c87a 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -64,7 +64,7 @@ tar_terra_sprc <- function(name, check_pkg_installed("terra") #ensure that user-passed `resources` doesn't include `custom_format` if ("custom_format" %in% names(resources)) { - stop("`custom_format` cannot be supplied to targets created with `tar_terra_rast()`") + stop("`custom_format` cannot be supplied to targets created with `tar_terra_sprc()`") } gdal <- gdal %||% character(0) From 078cd6c2f9ebadf745b728ebc75372b29ce855fd Mon Sep 17 00:00:00 2001 From: "Eric R. Scott" Date: Fri, 10 May 2024 09:05:42 -0700 Subject: [PATCH 08/10] Update R/tar-terra-rast.R Co-authored-by: Nicholas Tierney --- R/tar-terra-rast.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/tar-terra-rast.R b/R/tar-terra-rast.R index 09e4642..cd0faac 100644 --- a/R/tar-terra-rast.R +++ b/R/tar-terra-rast.R @@ -59,7 +59,7 @@ tar_terra_rast <- function(name, #ensure that user-passed `resources` doesn't include `custom_format` if ("custom_format" %in% names(resources)) { - stop("`custom_format` cannot be supplied to targets created with `tar_terra_rast()`") + cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_rast}") } name <- targets::tar_deparse_language(substitute(name)) From e7c54d003ea0084f34030296639481426a20962b Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Fri, 10 May 2024 09:09:34 -0700 Subject: [PATCH 09/10] use cli --- R/tar-terra-sprc.R | 2 +- R/tar-terra-vect.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index 9f0c87a..520c962 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -64,7 +64,7 @@ tar_terra_sprc <- function(name, check_pkg_installed("terra") #ensure that user-passed `resources` doesn't include `custom_format` if ("custom_format" %in% names(resources)) { - stop("`custom_format` cannot be supplied to targets created with `tar_terra_sprc()`") + cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_sprc}") } gdal <- gdal %||% character(0) diff --git a/R/tar-terra-vect.R b/R/tar-terra-vect.R index 6d6bcf3..0302d29 100644 --- a/R/tar-terra-vect.R +++ b/R/tar-terra-vect.R @@ -68,7 +68,7 @@ tar_terra_vect <- function(name, #ensure that user-passed `resources` doesn't include `custom_format` if ("custom_format" %in% names(resources)) { - stop("`custom_format` cannot be supplied to targets created with `tar_terra_vect()`") + cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_vect}") } name <- targets::tar_deparse_language(substitute(name)) From 51ca40bdcfa906b709d3a31ac1aa48253451025c Mon Sep 17 00:00:00 2001 From: Eric Scott Date: Fri, 10 May 2024 09:09:48 -0700 Subject: [PATCH 10/10] update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index d88cc57..04c3f66 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,3 +4,4 @@ * Created `tar_terra_sprc()` that creates a `SpatRasterCollection` object. * `geotargets_options_get()` and `geotargets_options_set()` can be used to set and get options specific to `geotargets`. * `geotargets` now requires `targets` version 1.7.0 or higher +* fixed a bug where `resources` supplied to `tar_terra_*()` were being ignored (#66)