From 53a2b4b48a56a8921a987199a50a2061028c183e Mon Sep 17 00:00:00 2001 From: njtierney Date: Fri, 10 Jan 2025 17:47:48 +1100 Subject: [PATCH 1/4] add more tests - add checks and tests for integerish for tile_grid and tile_blocksize - add a first pass test for tile_n returning the number of tiles being output --- R/tar_terra_tiles.R | 13 +++--- R/utils.R | 75 ++++++++++++++++++++------------- tests/testthat/test-tile-funs.R | 54 ++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 37 deletions(-) diff --git a/R/tar_terra_tiles.R b/R/tar_terra_tiles.R index 51be0b4..05172dc 100644 --- a/R/tar_terra_tiles.R +++ b/R/tar_terra_tiles.R @@ -312,6 +312,8 @@ set_window <- function(raster, window) { #' ) #' } tile_grid <- function(raster, ncol, nrow) { + check_is_integerish(ncol) + check_is_integerish(nrow) template <- terra::rast( x = terra::ext(raster), ncol = ncol, @@ -333,6 +335,8 @@ tile_grid <- function(raster, ncol, nrow) { #' @export #' @rdname tile_helpers tile_blocksize <- function(raster, n_blocks_row = 1, n_blocks_col = 1) { + check_is_integerish(n_blocks_row) + check_is_integerish(n_blocks_col) tile_ext <- terra::getTileExtents( raster, @@ -349,14 +353,7 @@ tile_blocksize <- function(raster, n_blocks_row = 1, n_blocks_col = 1) { #' @export #' @rdname tile_helpers tile_n <- function(raster, n) { - if (!rlang::is_integerish(n)) { - cli::cli_abort( - c( - "{.val {n}} must be an integer.", - "We see that {.val n} is: {n}" - ) - ) - } + check_is_integerish(n) sq <- sqrt(n) sq_round <- floor(sq) quotient <- n/sq_round diff --git a/R/utils.R b/R/utils.R index 1699c8b..fb08a76 100644 --- a/R/utils.R +++ b/R/utils.R @@ -1,47 +1,47 @@ # helper for adding "geotargets." when options missing that. geotargets_repair_option_name <- function(option_name) { - if (!startsWith(option_name, "geotargets.")) { - option_name <- paste0("geotargets.", option_name) - } - gsub("_", ".", tolower(option_name)) + if (!startsWith(option_name, "geotargets.")) { + option_name <- paste0("geotargets.", option_name) + } + gsub("_", ".", tolower(option_name)) } check_pkg_installed <- function(pkg, call = rlang::caller_env()) { - if (!requireNamespace(pkg, quietly = TRUE)) { - cli::cli_abort( - message = "package {.pkg {pkg}} is required", - call = call - ) - } + if (!requireNamespace(pkg, quietly = TRUE)) { + cli::cli_abort( + message = "package {.pkg {pkg}} is required", + call = call + ) + } } gdal_version <- function() { - numeric_version(terra::gdal(lib = "gdal")) + numeric_version(terra::gdal(lib = "gdal")) } check_gdal_shz <- function(min_version = "3.1", call = rlang::caller_env()) { - if (gdal_version() < numeric_version(min_version)) { - cli::cli_abort( - message = c( - "Must have {.pkg GDAL} version {.val {min_version}} or greater", - "i" = "To write a {.val .shz} file requires GDAL version \\ + if (gdal_version() < numeric_version(min_version)) { + cli::cli_abort( + message = c( + "Must have {.pkg GDAL} version {.val {min_version}} or greater", + "i" = "To write a {.val .shz} file requires GDAL version \\ {.val {min_version}} or greater" - ), - call = call - ) - } + ), + call = call + ) + } } get_gdal_available_driver_list <- function(driver_type) { - # get list of drivers available for writing depending on what the user's GDAL supports - drv <- terra::gdal(drivers = TRUE) - if (utils::packageVersion("terra") > "1.7-74") { - drv <- drv[drv[[driver_type]] & grepl("write", drv$can), ] - } else { - drv <- drv[drv$type == driver_type & grepl("write", drv$can), ] - } - drv + # get list of drivers available for writing depending on what the user's GDAL supports + drv <- terra::gdal(drivers = TRUE) + if (utils::packageVersion("terra") > "1.7-74") { + drv <- drv[drv[[driver_type]] & grepl("write", drv$can), ] + } else { + drv <- drv[drv$type == driver_type & grepl("write", drv$can), ] + } + drv } check_user_resources <- function(resources, @@ -53,7 +53,24 @@ check_user_resources <- function(resources, with {.fn tar_terra_rast}", "We see in {.code names(resources)}:", "{.val {names(resources)}}" - ), + ), + call = call + ) + } +} + +check_is_integerish <- function(x, + call = rlang::caller_env(), + arg = rlang::caller_arg(x)){ + + not_integerish <- !rlang::is_integerish(x) + + if (not_integerish) { + cli::cli_abort( + c( + "{.arg {arg}} must be an integer.", + "We see that {.arg {arg}} is: {.val {x}}" + ), call = call ) } diff --git a/tests/testthat/test-tile-funs.R b/tests/testthat/test-tile-funs.R index 5e88f90..83bb1dd 100644 --- a/tests/testthat/test-tile-funs.R +++ b/tests/testthat/test-tile-funs.R @@ -10,3 +10,57 @@ test_that("tile_n fails with non integer", { tile_n(r, n = 4) ) }) + +# expected number of tiles are being output +test_that("tile_n gives the appropriate number of outputs", { + f <- system.file("ex/elev.tif", package = "terra") + r <- terra::rast(f) + skip_on_ci() + tiled_1 <- tile_n(r, n = 1) + tiled_2 <- tile_n(r, n = 2) + tiled_3 <- tile_n(r, n = 3) + tiled_4 <- tile_n(r, n = 4) + tiled_5 <- tile_n(r, n = 5) + + expect_length(tiled_1, 1) + expect_length(tiled_2, 2) + expect_length(tiled_3, 3) + expect_length(tiled_4, 4) + expect_length(tiled_5, 6) +}) + +# sum of number of cells is equal to number of cells in input raster + +# In `test-tile-funs.R` add tests for `tile_blocksize` and `tile_grid`. +# Additional tests could include that the expected number of tiles are being +# output and the sum of the number of cells is equal to the number of cells +# in the input raster. + +test_that("tile_grid fails with non integer", { + f <- system.file("ex/elev.tif", package = "terra") + r <- terra::rast(f) + expect_snapshot( + error = TRUE, + tile_grid(r, + ncol = 1.5, + nrow = 2.5) + ) + expect_snapshot( + error = TRUE, + tile_grid(r, + ncol = 1, + nrow = 2.5) + ) + expect_snapshot( + error = TRUE, + tile_grid(r, + ncol = 1.5, + nrow = 2) + ) + skip_on_ci() + expect_snapshot( + tile_grid(r, + ncol = 2, + nrow = 3) + ) +}) From bb1bbebb7fe9a247273259ce048999b495296a7e Mon Sep 17 00:00:00 2001 From: njtierney Date: Thu, 16 Jan 2025 15:30:11 +1100 Subject: [PATCH 2/4] update snapshot tests for tile funs based on new changes to checking functions --- tests/testthat/_snaps/tile-funs.md | 61 +++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/tests/testthat/_snaps/tile-funs.md b/tests/testthat/_snaps/tile-funs.md index 7ed7039..f38646e 100644 --- a/tests/testthat/_snaps/tile-funs.md +++ b/tests/testthat/_snaps/tile-funs.md @@ -4,8 +4,8 @@ tile_n(r, n = 3.14) Condition Error in `tile_n()`: - ! 3.14 must be an integer. - We see that "n" is: 3.14 + ! `n` must be an integer. + We see that `n` is: 3.14 --- @@ -31,3 +31,60 @@ 6.141667 6.533333 49.441667 49.816667 +# tile_grid fails with non integer + + Code + tile_grid(r, ncol = 1.5, nrow = 2.5) + Condition + Error in `tile_grid()`: + ! `ncol` must be an integer. + We see that `ncol` is: 1.5 + +--- + + Code + tile_grid(r, ncol = 1, nrow = 2.5) + Condition + Error in `tile_grid()`: + ! `nrow` must be an integer. + We see that `nrow` is: 2.5 + +--- + + Code + tile_grid(r, ncol = 1.5, nrow = 2) + Condition + Error in `tile_grid()`: + ! `ncol` must be an integer. + We see that `ncol` is: 1.5 + +--- + + Code + tile_grid(r, ncol = 2, nrow = 3) + Output + [[1]] + xmin xmax ymin ymax + 5.741667 6.141667 49.941667 50.191667 + + [[2]] + xmin xmax ymin ymax + 6.141667 6.533333 49.941667 50.191667 + + [[3]] + xmin xmax ymin ymax + 5.741667 6.141667 49.691667 49.941667 + + [[4]] + xmin xmax ymin ymax + 6.141667 6.533333 49.691667 49.941667 + + [[5]] + xmin xmax ymin ymax + 5.741667 6.141667 49.441667 49.691667 + + [[6]] + xmin xmax ymin ymax + 6.141667 6.533333 49.441667 49.691667 + + From f23e6bdf07283d2fef13393e7c873bd0f00639e9 Mon Sep 17 00:00:00 2001 From: njtierney Date: Thu, 16 Jan 2025 15:31:37 +1100 Subject: [PATCH 3/4] add tests for sprc and sds to check that values are carried through --- tests/testthat/_snaps/tar-terra-sprc.md | 68 +++++++++++++++++++++++++ tests/testthat/test-tar-terra-sprc.R | 33 +++++++++++- 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/tests/testthat/_snaps/tar-terra-sprc.md b/tests/testthat/_snaps/tar-terra-sprc.md index f29f87e..15f6bd5 100644 --- a/tests/testthat/_snaps/tar-terra-sprc.md +++ b/tests/testthat/_snaps/tar-terra-sprc.md @@ -12,6 +12,40 @@ crs (first) : lon/lat WGS 84 (EPSG:4326) names : raster_elevs, raster_elevs +--- + + Code + x[1] + Output + class : SpatRaster + dimensions : 90, 95, 1 (nrow, ncol, nlyr) + resolution : 0.008333333, 0.008333333 (x, y) + extent : 5.741667, 6.533333, 49.44167, 50.19167 (xmin, xmax, ymin, ymax) + coord. ref. : lon/lat WGS 84 (EPSG:4326) + source : raster_elevs + name : elevation + min value : 141 + max value : 547 + unit : m + time (days) : 2025-01-15 + +--- + + Code + x[2] + Output + class : SpatRaster + dimensions : 115, 114, 1 (nrow, ncol, nlyr) + resolution : 683.4048, 683.4048 (x, y) + extent : 1480982, 1558890, 5478149, 5556741 (xmin, xmax, ymin, ymax) + coord. ref. : +proj=igh +lon_0=0 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs + source : raster_elevs + name : elevation + min value : 282.8344 + max value : 1087.3800 + unit : m + time (days) : 2025-01-15 + # tar_terra_sds() works Code @@ -27,3 +61,37 @@ source(s) : raster_elevs names : raster_elevs, raster_elevs +--- + + Code + x[1] + Output + class : SpatRaster + dimensions : 90, 95, 1 (nrow, ncol, nlyr) + resolution : 0.008333333, 0.008333333 (x, y) + extent : 5.741667, 6.533333, 49.44167, 50.19167 (xmin, xmax, ymin, ymax) + coord. ref. : lon/lat WGS 84 (EPSG:4326) + source : raster_elevs + name : elevation + min value : 141 + max value : 547 + unit : m + time (days) : 2025-01-15 + +--- + + Code + x[2] + Output + class : SpatRaster + dimensions : 90, 95, 1 (nrow, ncol, nlyr) + resolution : 0.008333333, 0.008333333 (x, y) + extent : 5.741667, 6.533333, 49.44167, 50.19167 (xmin, xmax, ymin, ymax) + coord. ref. : lon/lat WGS 84 (EPSG:4326) + source : raster_elevs + name : elevation + min value : 282 + max value : 1094 + unit : m + time (days) : 2025-01-15 + diff --git a/tests/testthat/test-tar-terra-sprc.R b/tests/testthat/test-tar-terra-sprc.R index 2debae9..4be6490 100644 --- a/tests/testthat/test-tar-terra-sprc.R +++ b/tests/testthat/test-tar-terra-sprc.R @@ -6,7 +6,7 @@ targets::tar_test("tar_terra_sprc() works", { ) targets::tar_script({ elev_scale <- function(z = 1, projection = "EPSG:4326") { - terra::project( + rast_elev_scale <- terra::project( terra::rast( system.file( "ex", @@ -16,6 +16,16 @@ targets::tar_test("tar_terra_sprc() works", { ) * z, projection ) + # test to ensure metadata comes along. + # e.g., layer names, units, time, variables + # The snapshots for these tests do not include those metadata + # so this may be as easy as assigning some of these attributes in the + # test SPRC/SDS and adding those fields in the snapshot. + terra::units(rast_elev_scale) <- "m" + terra::varnames(rast_elev_scale) <- "elev" + terra::longnames(rast_elev_scale) <- "really-long-name" + terra::time(rast_elev_scale) <- as.Date("2025-01-15") + rast_elev_scale } list( geotargets::tar_terra_sprc( @@ -33,6 +43,8 @@ targets::tar_test("tar_terra_sprc() works", { x <- targets::tar_read(raster_elevs) expect_s4_class(x, "SpatRasterCollection") expect_snapshot(x) + expect_snapshot(x[1]) + expect_snapshot(x[2]) }) targets::tar_test("tar_terra_sds() works", { @@ -42,13 +54,24 @@ targets::tar_test("tar_terra_sds() works", { ) targets::tar_script({ elev_scale <- function(z = 1) { - terra::rast( + rast_elev_scale <- terra::rast( system.file( "ex", "elev.tif", package = "terra" ) ) * z + + # test to ensure metadata comes along. + # e.g., layer names, units, time, variables + # The snapshots for these tests do not include those metadata + # so this may be as easy as assigning some of these attributes in the + # test SPRC/SDS and adding those fields in the snapshot. + terra::units(rast_elev_scale) <- "m" + terra::varnames(rast_elev_scale) <- "elev" + terra::longnames(rast_elev_scale) <- "really-long-name" + terra::time(rast_elev_scale) <- as.Date("2025-01-15") + rast_elev_scale } list( geotargets::tar_terra_sds( @@ -65,6 +88,12 @@ targets::tar_test("tar_terra_sds() works", { x <- targets::tar_read(raster_elevs) expect_s4_class(x, "SpatRasterDataset") expect_snapshot(x) + expect_snapshot(x[1]) + expect_equal(terra::units(x[1]), "m") + expect_equal(terra::time(x[1]), as.Date("2025-01-15")) + expect_snapshot(x[2]) + expect_equal(terra::units(x[2]), "m") + expect_equal(terra::time(x[2]), as.Date("2025-01-15")) }) # difficult to test for this warning from tar_terra_sprc() because it doesn't end From 9027153322dc480531e1c0566a4a2d90e0fad234 Mon Sep 17 00:00:00 2001 From: njtierney Date: Fri, 17 Jan 2025 10:39:26 +1100 Subject: [PATCH 4/4] remove varnames and longnames from tests --- tests/testthat/test-tar-terra-sprc.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testthat/test-tar-terra-sprc.R b/tests/testthat/test-tar-terra-sprc.R index 4be6490..960f040 100644 --- a/tests/testthat/test-tar-terra-sprc.R +++ b/tests/testthat/test-tar-terra-sprc.R @@ -22,8 +22,6 @@ targets::tar_test("tar_terra_sprc() works", { # so this may be as easy as assigning some of these attributes in the # test SPRC/SDS and adding those fields in the snapshot. terra::units(rast_elev_scale) <- "m" - terra::varnames(rast_elev_scale) <- "elev" - terra::longnames(rast_elev_scale) <- "really-long-name" terra::time(rast_elev_scale) <- as.Date("2025-01-15") rast_elev_scale }