From 5933e954712049f39d501f205f900514e4f7ce8a Mon Sep 17 00:00:00 2001 From: njtierney Date: Fri, 29 Nov 2024 14:07:01 +1100 Subject: [PATCH 1/3] some small tweaks to improve code coverage and add tests, ignore some other files --- .Rbuildignore | 1 + .covrignore | 1 + R/tar-terra-rast.R | 4 +--- R/tar-terra-sprc.R | 8 ++------ R/tar-terra-vect.R | 4 +--- R/tar_terra_tiles.R | 7 ++++++- R/utils.R | 17 +++++++++------ man/geotargets-options.Rd | 32 ++++++++++++++--------------- man/tar_stars.Rd | 26 +++++++++++------------ man/tar_terra_rast.Rd | 26 +++++++++++------------ man/tar_terra_sds.Rd | 2 +- tests/testthat/_snaps/tile-funs.md | 33 ++++++++++++++++++++++++++++++ tests/testthat/test-tile-funs.R | 11 ++++++++++ 13 files changed, 110 insertions(+), 62 deletions(-) create mode 100644 .covrignore create mode 100644 tests/testthat/_snaps/tile-funs.md create mode 100644 tests/testthat/test-tile-funs.R diff --git a/.Rbuildignore b/.Rbuildignore index 4920a2f..5a8a6e5 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -21,3 +21,4 @@ ^doc$ ^Meta$ ^codemeta\.json$ +^\.covrignore$ diff --git a/.covrignore b/.covrignore new file mode 100644 index 0000000..a9c420b --- /dev/null +++ b/.covrignore @@ -0,0 +1 @@ +R/release_bullets.R diff --git a/R/tar-terra-rast.R b/R/tar-terra-rast.R index cdc8c52..ae184b6 100644 --- a/R/tar-terra-rast.R +++ b/R/tar-terra-rast.R @@ -88,9 +88,7 @@ tar_terra_rast <- function(name, preserve_metadata <- rlang::arg_match0(preserve_metadata, c("drop", "zip")) # ensure that user-passed `resources` doesn't include `custom_format` - if ("custom_format" %in% names(resources)) { - cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_rast}") - } + check_user_resources(resources) name <- targets::tar_deparse_language(substitute(name)) diff --git a/R/tar-terra-sprc.R b/R/tar-terra-sprc.R index 9a09476..5fbf8cd 100644 --- a/R/tar-terra-sprc.R +++ b/R/tar-terra-sprc.R @@ -73,9 +73,7 @@ tar_terra_sprc <- function(name, cue = targets::tar_option_get("cue"), description = targets::tar_option_get("description")) { # ensure that user-passed `resources` doesn't include `custom_format` - if ("custom_format" %in% names(resources)) { - cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_sprc}") - } + check_user_resources(resources) gdal <- gdal %||% character(0) filetype <- filetype %||% "GTiff" @@ -196,9 +194,7 @@ tar_terra_sds <- function(name, cue = targets::tar_option_get("cue"), description = targets::tar_option_get("description")) { # ensure that user-passed `resources` doesn't include `custom_format` - if ("custom_format" %in% names(resources)) { - cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_sprc}") - } + check_user_resources(resources) gdal <- gdal %||% character(0) filetype <- filetype %||% "GTiff" diff --git a/R/tar-terra-vect.R b/R/tar-terra-vect.R index cee7932..426445a 100644 --- a/R/tar-terra-vect.R +++ b/R/tar-terra-vect.R @@ -89,9 +89,7 @@ tar_terra_vect <- function(name, } # ensure that user-passed `resources` doesn't include `custom_format` - if ("custom_format" %in% names(resources)) { - cli::cli_abort("{.val custom_format} cannot be supplied to targets created with {.fn tar_terra_vect}") - } + check_user_resources(resources) name <- targets::tar_deparse_language(substitute(name)) diff --git a/R/tar_terra_tiles.R b/R/tar_terra_tiles.R index f4623f3..51be0b4 100644 --- a/R/tar_terra_tiles.R +++ b/R/tar_terra_tiles.R @@ -350,7 +350,12 @@ tile_blocksize <- function(raster, n_blocks_row = 1, n_blocks_col = 1) { #' @rdname tile_helpers tile_n <- function(raster, n) { if (!rlang::is_integerish(n)) { - rlang::abort("`n` must be an integer.") + cli::cli_abort( + c( + "{.val {n}} must be an integer.", + "We see that {.val n} is: {n}" + ) + ) } sq <- sqrt(n) sq_round <- floor(sq) diff --git a/R/utils.R b/R/utils.R index 28f7ce3..8fbbab1 100644 --- a/R/utils.R +++ b/R/utils.R @@ -44,10 +44,15 @@ get_gdal_available_driver_list <- function(driver_type) { drv } -semicolon_split <- function(env_vars) { - strsplit(env_vars, ";")[[1]] -} - -semicolon_paste <- function(vec) { - paste0(vec, collapse = ";") +check_user_resources <- function(resources, + call = rlang::caller_env()){ + if ("custom_format" %in% names(resources)) { + cli::cli_abort( + message = c( + "{.val custom_format} cannot be supplied to targets created \\ + with {.fn tar_terra_rast}" + ), + call = call + ) + } } diff --git a/man/geotargets-options.Rd b/man/geotargets-options.Rd index dc16160..c35b159 100644 --- a/man/geotargets-options.Rd +++ b/man/geotargets-options.Rd @@ -63,22 +63,22 @@ These options can also be set using \code{options()}. For example, } \examples{ if (Sys.getenv("TAR_LONG_EXAMPLES") == "true") { - targets::tar_dir({ # tar_dir() runs code from a temporary directory. - library(geotargets) - op <- getOption("geotargets.gdal.raster.driver") - withr::defer(options("geotargets.gdal.raster.driver" = op)) - geotargets_option_set(gdal_raster_driver = "COG") - targets::tar_script({ - list( - geotargets::tar_terra_rast( - terra_rast_example, - system.file("ex/elev.tif", package = "terra") |> terra::rast() - ) - ) - }) - targets::tar_make() - x <- targets::tar_read(terra_rast_example) - }) + targets::tar_dir({ # tar_dir() runs code from a temporary directory. + library(geotargets) + op <- getOption("geotargets.gdal.raster.driver") + withr::defer(options("geotargets.gdal.raster.driver" = op)) + geotargets_option_set(gdal_raster_driver = "COG") + targets::tar_script({ + list( + geotargets::tar_terra_rast( + terra_rast_example, + system.file("ex/elev.tif", package = "terra") |> terra::rast() + ) + ) + }) + targets::tar_make() + x <- targets::tar_read(terra_rast_example) + }) } geotargets_option_get("gdal.raster.driver") diff --git a/man/tar_stars.Rd b/man/tar_stars.Rd index cf45feb..8d4823d 100644 --- a/man/tar_stars.Rd +++ b/man/tar_stars.Rd @@ -266,19 +266,19 @@ The \code{iteration} argument is unavailable because it is hard-coded to \examples{ \dontshow{if (rlang::is_installed("stars")) (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} if (Sys.getenv("TAR_LONG_EXAMPLES") == "true") { - targets::tar_dir({ # tar_dir() runs code from a temporary directory. - library(geotargets) - targets::tar_script({ - list( - geotargets::tar_stars( - stars_example, - stars::read_stars(system.file("tif", "olinda_dem_utm25s.tif", package = "stars")) - ) - ) - }) - targets::tar_make() - x <- targets::tar_read(stars_example) - }) + targets::tar_dir({ # tar_dir() runs code from a temporary directory. + library(geotargets) + targets::tar_script({ + list( + geotargets::tar_stars( + stars_example, + stars::read_stars(system.file("tif", "olinda_dem_utm25s.tif", package = "stars")) + ) + ) + }) + targets::tar_make() + x <- targets::tar_read(stars_example) + }) } \dontshow{\}) # examplesIf} } diff --git a/man/tar_terra_rast.Rd b/man/tar_terra_rast.Rd index 3b4b314..e46e3a7 100644 --- a/man/tar_terra_rast.Rd +++ b/man/tar_terra_rast.Rd @@ -236,19 +236,19 @@ The \code{iteration} argument is unavailable because it is hard-coded to } \examples{ if (Sys.getenv("TAR_LONG_EXAMPLES") == "true") { - targets::tar_dir({ # tar_dir() runs code from a temporary directory. - library(geotargets) - targets::tar_script({ - list( - geotargets::tar_terra_rast( - terra_rast_example, - system.file("ex/elev.tif", package = "terra") |> terra::rast() - ) - ) - }) - targets::tar_make() - x <- targets::tar_read(terra_rast_example) - }) + targets::tar_dir({ # tar_dir() runs code from a temporary directory. + library(geotargets) + targets::tar_script({ + list( + geotargets::tar_terra_rast( + terra_rast_example, + system.file("ex/elev.tif", package = "terra") |> terra::rast() + ) + ) + }) + targets::tar_make() + x <- targets::tar_read(terra_rast_example) + }) } } \seealso{ diff --git a/man/tar_terra_sds.Rd b/man/tar_terra_sds.Rd index fec2e03..746a28f 100644 --- a/man/tar_terra_sds.Rd +++ b/man/tar_terra_sds.Rd @@ -223,7 +223,7 @@ if (Sys.getenv("TAR_LONG_EXAMPLES") == "true") { library(geotargets) targets::tar_script({ elev_scale <- function(z = 1) { - terra::rast(system.file("ex", "elev.tif", package = "terra")) * z + terra::rast(system.file("ex", "elev.tif", package = "terra")) * z } list( tar_terra_sds( diff --git a/tests/testthat/_snaps/tile-funs.md b/tests/testthat/_snaps/tile-funs.md new file mode 100644 index 0000000..f6b2870 --- /dev/null +++ b/tests/testthat/_snaps/tile-funs.md @@ -0,0 +1,33 @@ +# tile_n fails with non integer + + Code + tile_n(r, n = 3.14) + Condition + Error in `tile_n()`: + ! 3.14 must be an integer. + We see that "n" is: 3.14 + +--- + + Code + tile_n(r, n = 4) + Message + creating 2 * 2 = 4 tile extents + Output + [[1]] + xmin xmax ymin ymax + 5.741667 6.141667 49.816667 50.191667 + + [[2]] + xmin xmax ymin ymax + 6.133333 6.533333 49.816667 50.191667 + + [[3]] + xmin xmax ymin ymax + 5.741667 6.141667 49.441667 49.816667 + + [[4]] + xmin xmax ymin ymax + 6.133333 6.533333 49.441667 49.816667 + + diff --git a/tests/testthat/test-tile-funs.R b/tests/testthat/test-tile-funs.R new file mode 100644 index 0000000..f1d80c2 --- /dev/null +++ b/tests/testthat/test-tile-funs.R @@ -0,0 +1,11 @@ +test_that("tile_n fails with non integer", { + f <- system.file("ex/elev.tif", package = "terra") + r <- terra::rast(f) + expect_snapshot( + error = TRUE, + tile_n(r, n = 3.14) + ) + expect_snapshot( + tile_n(r, n = 4) + ) +}) From 4b55430c32ca74590b18017e484e1a5262879142 Mon Sep 17 00:00:00 2001 From: njtierney Date: Fri, 29 Nov 2024 14:16:03 +1100 Subject: [PATCH 2/3] better local tests for check user resources --- R/utils.R | 4 +++- tests/testthat/_snaps/check-user-resources.md | 10 ++++++++++ tests/testthat/test-check-user-resources.R | 9 +++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/_snaps/check-user-resources.md create mode 100644 tests/testthat/test-check-user-resources.R diff --git a/R/utils.R b/R/utils.R index 8fbbab1..1699c8b 100644 --- a/R/utils.R +++ b/R/utils.R @@ -50,7 +50,9 @@ check_user_resources <- function(resources, cli::cli_abort( message = c( "{.val custom_format} cannot be supplied to targets created \\ - with {.fn tar_terra_rast}" + with {.fn tar_terra_rast}", + "We see in {.code names(resources)}:", + "{.val {names(resources)}}" ), call = call ) diff --git a/tests/testthat/_snaps/check-user-resources.md b/tests/testthat/_snaps/check-user-resources.md new file mode 100644 index 0000000..692e0d7 --- /dev/null +++ b/tests/testthat/_snaps/check-user-resources.md @@ -0,0 +1,10 @@ +# check_user_resources works + + Code + check_user_resources(resources = c(custom_format = 1)) + Condition + Error: + ! "custom_format" cannot be supplied to targets created with `tar_terra_rast()` + We see in `names(resources)`: + "custom_format" + diff --git a/tests/testthat/test-check-user-resources.R b/tests/testthat/test-check-user-resources.R new file mode 100644 index 0000000..54f6a84 --- /dev/null +++ b/tests/testthat/test-check-user-resources.R @@ -0,0 +1,9 @@ +test_that("check_user_resources works", { + expect_snapshot( + error = TRUE, + check_user_resources(resources = c("custom_format" = 1)) + ) + expect_silent( + check_user_resources(resources = c("anything else" = 1)) + ) +}) From 32952a3c24b64f90a77d25b7d3c975e0f6582d69 Mon Sep 17 00:00:00 2001 From: njtierney Date: Fri, 29 Nov 2024 14:23:05 +1100 Subject: [PATCH 3/3] skip this test on CI, as there are some slight variations on machines --- tests/testthat/test-tile-funs.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-tile-funs.R b/tests/testthat/test-tile-funs.R index f1d80c2..5e88f90 100644 --- a/tests/testthat/test-tile-funs.R +++ b/tests/testthat/test-tile-funs.R @@ -5,6 +5,7 @@ test_that("tile_n fails with non integer", { error = TRUE, tile_n(r, n = 3.14) ) + skip_on_ci() expect_snapshot( tile_n(r, n = 4) )