diff --git a/DESCRIPTION b/DESCRIPTION index adeff0b..9f9de12 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: drift Title: Detecting Riparian and Inland Floodplain Transitions -Version: 0.20.0 +Version: 0.21.0 Date: 2026-09-29 Authors@R: c( person("Allan", "Irvine", , "al@newgraphenvironment.com", role = c("aut", "cre"), diff --git a/NEWS.md b/NEWS.md index 027bfb4..0749063 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,11 @@ +# drift 0.21.0 + +- **A misspelt `resampling` no longer returns a nearest-neighbour raster without a word (#96).** `dft_stac_cube()`, `dft_stac_composite()` and `dft_stac_fetch()` passed `resampling` straight to `gdalcubes::cube_view()`, which reads a value it does not know as `near` with no error. So `resampling = "bilinaer"` returned and cached a nearest-neighbour cube. This is the same silent fallback #92 fixed for `aggregation`. + - **Refused up front:** all three now refuse any `resampling` outside `near`, `bilinear`, `cubic`, `cubicspline`, `lanczos`, `average`, `mode`, `max`, `min`, `med`, `q1` and `q3`. Each of the twelve was measured to come back unchanged from `cube_view()`. Case is ignored and the value is hashed as given, so no existing cache key moves. The check runs before any network call, and again at both `cube_view()` call sites. + - **`mean` and `median` are refused too, which can break a call that used to work.** gdalcubes honours them, but renames them to `average` and `med`. drift keeps one spelling per method, and the error names the one to use. Switching to it changes the cache key, so the first call re-streams once. + - **Old caches:** a file cached under a `resampling` gdalcubes did not know (a typo, or `sum`, `rms`, `gauss`, `nearest`, `none`) holds nearest-neighbour data, and since that value is now refused, nothing reads it again. It sits in the current cache scheme beside good files and cannot be told apart from them, so only `dft_cache_clear()`, which clears everything by default, reclaims it. + - `aggregation` is now also checked where `dft_stac_fetch()` builds each `cube_view()`, tiled or not, as it already was in `dft_stac_cube()` and `dft_stac_composite()`. + # drift 0.20.0 - **`dft_stac_composite(aggregation = "count")` returned reflectance, not a count, and now returns clear-day counts (#92).** diff --git a/R/dft_stac_composite.R b/R/dft_stac_composite.R index 5217e16..2ef0da5 100644 --- a/R/dft_stac_composite.R +++ b/R/dft_stac_composite.R @@ -93,7 +93,12 @@ #' refused. gdalcubes reads a value it does not know as no aggregation at all #' and returns reflectance with no error, which is how `"count"` behaved in #' drift 0.18.0 to 0.19.2, so drift passes it only values measured to work. -#' @param resampling Character. Spatial resampling (default `"bilinear"`). +#' @param resampling Character. Spatial resampling (default `"bilinear"`): +#' one of `"near"`, `"bilinear"`, `"cubic"`, `"cubicspline"`, +#' `"lanczos"`, `"average"`, `"mode"`, `"max"`, `"min"`, `"med"`, `"q1"` or +#' `"q3"`. Anything else is refused: gdalcubes reads a value it does not know as +#' `"near"`, with no error, so drift passes it only values measured to work. +#' `"mean"` and `"median"` are refused too; use `"average"` and `"med"`. #' @param clip Logical. Clip the output to the AOI polygon (default `FALSE`). #' Reference imagery is read around a place, not only inside it, so the #' default keeps the whole AOI bounding box. `TRUE` uses the same rule as @@ -185,6 +190,7 @@ dft_stac_composite <- function(aoi, } aggregation <- aggregation_check(aggregation, c(.cube_view_aggregations, "count")) + resampling <- resampling_check(resampling) # matched without case, like every aggregation; the count family is new, so # normalising it moves no existing key is_count <- identical(tolower(aggregation), "count") diff --git a/R/dft_stac_cube.R b/R/dft_stac_cube.R index 6ce3b6f..7063af7 100644 --- a/R/dft_stac_cube.R +++ b/R/dft_stac_cube.R @@ -40,7 +40,12 @@ #' `"max"`, `"first"` or `"last"`. Anything else is refused: gdalcubes reads a #' value it does not know as no aggregation, with no error, so drift passes it #' only values measured to work. -#' @param resampling Character. Spatial resampling (default `"bilinear"`). +#' @param resampling Character. Spatial resampling (default `"bilinear"`): +#' one of `"near"`, `"bilinear"`, `"cubic"`, `"cubicspline"`, +#' `"lanczos"`, `"average"`, `"mode"`, `"max"`, `"min"`, `"med"`, `"q1"` or +#' `"q3"`. Anything else is refused: gdalcubes reads a value it does not know as +#' `"near"`, with no error, so drift passes it only values measured to work. +#' `"mean"` and `"median"` are refused too; use `"average"` and `"med"`. #' @param clip Logical. When `TRUE` (default), clip the returned stack to the AOI #' polygon with `terra::mask()`, so #' [dft_rast_break()] / [dft_rast_trend()] reduce only in-polygon pixels. The @@ -167,6 +172,7 @@ dft_stac_cube <- function(aoi, sign_fn = rstac::sign_planetary_computer()) { check_gdalcubes("to fetch STAC cubes") aggregation <- aggregation_check(aggregation) + resampling <- resampling_check(resampling) # gdalcubes worker count and GDAL /vsicurl tuning, both restored on exit so # the caller's session is untouched (see stac_cube_session()). @@ -309,6 +315,20 @@ dft_stac_cube <- function(aoi, # an index or a class code. "none" is the silent fallback itself. .cube_view_aggregations <- c("min", "max", "mean", "median", "first", "last") +# The resamplings drift passes to gdalcubes::cube_view(). gdalcubes 0.7.5 +# lower-cases the string and maps anything it does not know to "near" without +# error, so a typo returns a nearest-neighbour cube (#96). Measured by +# round-tripping each value through cube_view()$resampling: the twelve below come +# back unchanged. "mean" and "median" are honoured too, but come back as +# "average" and "med"; drift refuses them (naming the spelling to use) so each +# method has one documented spelling and the behaviour test stays a round trip. +# "nearest" reaches "near" only through the fallback, the same as a typo. sum, rms +# and gauss (GDAL names) fall back as well. +.cube_view_resamplings <- c("near", "bilinear", "cubic", "cubicspline", + "lanczos", "average", "mode", "max", "min", "med", + "q1", "q3") +.cube_view_resampling_aliases <- c(mean = "average", median = "med") + #' Refuse an `aggregation` outside `allowed`, before any network call #' #' Shared by [dft_stac_fetch()], [dft_stac_cube()] and [dft_stac_composite()], @@ -319,20 +339,49 @@ dft_stac_cube <- function(aoi, #' mixed-case caller, silently re-streaming a cube that is already cached. #' @noRd aggregation_check <- function(aggregation, allowed = .cube_view_aggregations) { - if (!is.character(aggregation) || length(aggregation) != 1L || - is.na(aggregation) || !tolower(aggregation) %in% allowed) { + cube_view_choice_check(aggregation, "aggregation", allowed, + fallback = "none", class = "drift_bad_aggregation") +} + +#' Refuse a `resampling` gdalcubes would read as `"near"`, before any network call +#' +#' The `resampling` sibling of `aggregation_check()`, with the same callers and +#' the same as-given return (#96). +#' @noRd +resampling_check <- function(resampling) { + cube_view_choice_check(resampling, "resampling", .cube_view_resamplings, + fallback = "near", class = "drift_bad_resampling", + aliases = .cube_view_resampling_aliases) +} + +#' Refuse a `cube_view()` string argument outside `allowed` +#' +#' gdalcubes reads a value it does not know as `fallback` with no error, so +#' anything outside the measured set is an error here. Case is ignored, as +#' gdalcubes ignores it, and `x` is returned as given. `aliases` names values +#' gdalcubes does honour under another spelling: those are refused too, but the +#' message says what they mean rather than calling them unknown. +#' @noRd +cube_view_choice_check <- function(x, arg, allowed, fallback, class, + aliases = character(0), + call = rlang::caller_env()) { + is_string <- is.character(x) && length(x) == 1L && !is.na(x) + if (!is_string || !tolower(x) %in% allowed) { + alias <- if (is_string) unname(aliases[tolower(x)]) else NA_character_ cli::cli_abort(c( - "{.arg aggregation} must be one of {.or {.val {allowed}}}.", - "x" = if (is.character(aggregation) && length(aggregation) == 1L && - !is.na(aggregation)) { - "Got {.val {aggregation}}. gdalcubes reads a value it does not know as \\ - {.val none} without an error, so drift passes only these." + "{.arg {arg}} must be one of {.or {.val {allowed}}}.", + "x" = if (!is.na(alias)) { + "Got {.val {x}}, which gdalcubes reads as {.val {alias}}. drift takes \\ + one spelling per method: use {.val {alias}}." + } else if (is_string) { + "Got {.val {x}}. gdalcubes reads a value it does not know as \\ + {.val {fallback}} without an error, so drift passes only these." } else { - "Got {.obj_type_friendly {aggregation}}." + "Got {.obj_type_friendly {x}}." } - ), class = "drift_bad_aggregation") + ), class = class, call = call) } - aggregation + x } @@ -572,8 +621,9 @@ stac_cube_assemble <- function(fetched, cfg, aoi_target, target_crs, t0, t1, mask_values, offset, offset_before, pixel_fn, tile_size = NULL) { # the last point before cube_view(): an aggregation it does not honour is - # read as "none" and returns plausible pixels (#92) + # read as "none" and returns plausible pixels (#92), a resampling as "near" (#96) aggregation_check(aggregation) + resampling_check(resampling) mask_asset <- cfg$roles$mask features <- fetched$features is_pre <- fetched$is_pre diff --git a/R/dft_stac_fetch.R b/R/dft_stac_fetch.R index 9a6db78..f4797de 100644 --- a/R/dft_stac_fetch.R +++ b/R/dft_stac_fetch.R @@ -52,8 +52,13 @@ #' `"last"`, `"median"`, `"mean"`, `"min"` or `"max"`; anything else is #' refused: gdalcubes reads a value it does not know as no aggregation, with #' no error, so drift passes it only values measured to work. -#' @param resampling Character. Spatial resampling method (default `"near"` -#' for categorical data). +#' @param resampling Character. Spatial resampling method (default `"near"`, +#' right for categorical data; `"mode"`, the most common class, also suits it): +#' one of `"near"`, `"bilinear"`, `"cubic"`, `"cubicspline"`, +#' `"lanczos"`, `"average"`, `"mode"`, `"max"`, `"min"`, `"med"`, `"q1"` or +#' `"q3"`. Anything else is refused: gdalcubes reads a value it does not know as +#' `"near"`, with no error, so drift passes it only values measured to work. +#' `"mean"` and `"median"` are refused too; use `"average"` and `"med"`. #' @param tile_size Numeric or `NULL` (default). Edge length, in CRS units #' (metres for the default UTM CRS), of the download-tiling grid. When `NULL`, #' one cube is streamed over the whole AOI bounding box (the download scales @@ -103,6 +108,7 @@ dft_stac_fetch <- function(aoi, sign_fn = rstac::sign_planetary_computer()) { check_gdalcubes("to fetch STAC rasters") aggregation <- aggregation_check(aggregation) + resampling <- resampling_check(resampling) # Normalize tile_size ONCE so the path gate (is.null) and the cache key derive # from the same snapped scalar. When tiling, tune GDAL for the many extra @@ -608,6 +614,10 @@ tile_grid <- function(aoi_target, tile_size, res) { #' @noRd fetch_extent_to <- function(col, ext, t0, t1, target_crs, res, dt, aggregation, resampling, out_nc) { + # the last point before cube_view(), which reads an unknown aggregation as + # "none" (#92) and an unknown resampling as "near" (#96) without an error + aggregation_check(aggregation) + resampling_check(resampling) v <- gdalcubes::cube_view( srs = target_crs, extent = list( diff --git a/inst/notes/gdalcubes-pc-gotchas.md b/inst/notes/gdalcubes-pc-gotchas.md index 90672a4..bdecf87 100644 --- a/inst/notes/gdalcubes-pc-gotchas.md +++ b/inst/notes/gdalcubes-pc-gotchas.md @@ -217,7 +217,11 @@ Measured on gdalcubes 0.7.5, rstac 1.0.1 and terra 1.9.50, with the BULK floodpl - **`cube_view()` does not refuse an aggregation it does not know. It reads it as `"none"`.** The R wrapper checks only that the value is one string. The C++ side lower-cases it, and maps anything unrecognised to `AGG_NONE`, which copies every image in. That copy includes NaNs, so a masked item can blank a clear one. This is how `aggregation = "count"` came back as plausible reflectance. - Measured by round-tripping values through `cube_view()$aggregation`. `min`, `max`, `mean`, `median`, `first` and `last` survive, and so do the undocumented `count_values` and `count_images`. `count`, `sum` and `""` become `none`. - drift passes only the first six (`aggregation_check()`). `count_*` count **items**, so overlapping MGRS tiles double-count one acquisition, and every drift caller would read the result as reflectance, an index or a class code. - - `resampling` has the same fallback, to `near` (`bilinaer -> near`), in #96. +- **`resampling` has the same fallback, to `near` (#96, 2026-10-01).** Measured the same way through `cube_view()$resampling` on 0.7.5: + - Twelve values come back unchanged: `near`, `bilinear`, `cubic`, `cubicspline`, `lanczos`, `average`, `mode`, `max`, `min`, `med`, `q1` and `q3`. Case is ignored (`Q1 -> q1`). + - Two are honoured under another name: `mean -> average`, `median -> med`. + - Everything else becomes `near`, including GDAL's own `sum`, `rms` and `gauss`, plus `nearest`, `none`, `""` and any typo. + - drift passes only the twelve (`resampling_check()`). It refuses the two aliases, naming the spelling to use, so each method has one spelling and the pin test stays a plain round trip. - **Count clear days, not items: `dt = "P1D"`, then `reduce_time(cube, "count(B04)", names = ...)`.** Masked pixels are NaN before aggregation, and every aggregation tried (first, max, median) skips NaN within a day. So two same-day tiles give one clear day, and a cloudy tile does not blank a clear one. The string reducer is C++, so the R-callback closure trap above does not apply. - `reduce_time()` passes a single-time-step cube through unchanged. A count over a one-day window is therefore the input, not a count. - **A zero-clear pixel is 0 or NaN depending on chunk layout.** An all-NaN chunk stays empty (NaN), while a pixel in a chunk that holds a clear value somewhere reads 0. Measured on a 128 x 128 fixture: 12,544 zeros at 256 px chunks, against 4,352 zeros plus 8,192 NaN at 64 px. Chunk size follows `gdalcubes_options(parallel =)`, so the raw output depends on a setting drift documents as cost-only. diff --git a/man/dft_stac_composite.Rd b/man/dft_stac_composite.Rd index 90b8607..4de84e8 100644 --- a/man/dft_stac_composite.Rd +++ b/man/dft_stac_composite.Rd @@ -56,7 +56,12 @@ refused. gdalcubes reads a value it does not know as no aggregation at all and returns reflectance with no error, which is how \code{"count"} behaved in drift 0.18.0 to 0.19.2, so drift passes it only values measured to work.} -\item{resampling}{Character. Spatial resampling (default \code{"bilinear"}).} +\item{resampling}{Character. Spatial resampling (default \code{"bilinear"}): +one of \code{"near"}, \code{"bilinear"}, \code{"cubic"}, \code{"cubicspline"}, +\code{"lanczos"}, \code{"average"}, \code{"mode"}, \code{"max"}, \code{"min"}, \code{"med"}, \code{"q1"} or +\code{"q3"}. Anything else is refused: gdalcubes reads a value it does not know as +\code{"near"}, with no error, so drift passes it only values measured to work. +\code{"mean"} and \code{"median"} are refused too; use \code{"average"} and \code{"med"}.} \item{clip}{Logical. Clip the output to the AOI polygon (default \code{FALSE}). Reference imagery is read around a place, not only inside it, so the diff --git a/man/dft_stac_cube.Rd b/man/dft_stac_cube.Rd index 325b671..8527467 100644 --- a/man/dft_stac_cube.Rd +++ b/man/dft_stac_cube.Rd @@ -52,7 +52,12 @@ must agree with.} value it does not know as no aggregation, with no error, so drift passes it only values measured to work.} -\item{resampling}{Character. Spatial resampling (default \code{"bilinear"}).} +\item{resampling}{Character. Spatial resampling (default \code{"bilinear"}): +one of \code{"near"}, \code{"bilinear"}, \code{"cubic"}, \code{"cubicspline"}, +\code{"lanczos"}, \code{"average"}, \code{"mode"}, \code{"max"}, \code{"min"}, \code{"med"}, \code{"q1"} or +\code{"q3"}. Anything else is refused: gdalcubes reads a value it does not know as +\code{"near"}, with no error, so drift passes it only values measured to work. +\code{"mean"} and \code{"median"} are refused too; use \code{"average"} and \code{"med"}.} \item{clip}{Logical. When \code{TRUE} (default), clip the returned stack to the AOI polygon with \code{terra::mask()}, so diff --git a/man/dft_stac_fetch.Rd b/man/dft_stac_fetch.Rd index e5aad4f..c577d34 100644 --- a/man/dft_stac_fetch.Rd +++ b/man/dft_stac_fetch.Rd @@ -51,8 +51,13 @@ When \code{NULL}, auto-detected from the AOI centroid's UTM zone.} refused: gdalcubes reads a value it does not know as no aggregation, with no error, so drift passes it only values measured to work.} -\item{resampling}{Character. Spatial resampling method (default \code{"near"} -for categorical data).} +\item{resampling}{Character. Spatial resampling method (default \code{"near"}, +right for categorical data; \code{"mode"}, the most common class, also suits it): +one of \code{"near"}, \code{"bilinear"}, \code{"cubic"}, \code{"cubicspline"}, +\code{"lanczos"}, \code{"average"}, \code{"mode"}, \code{"max"}, \code{"min"}, \code{"med"}, \code{"q1"} or +\code{"q3"}. Anything else is refused: gdalcubes reads a value it does not know as +\code{"near"}, with no error, so drift passes it only values measured to work. +\code{"mean"} and \code{"median"} are refused too; use \code{"average"} and \code{"med"}.} \item{tile_size}{Numeric or \code{NULL} (default). Edge length, in CRS units (metres for the default UTM CRS), of the download-tiling grid. When \code{NULL}, diff --git a/planning/archive/2026-10-issue-96-resampling-check/README.md b/planning/archive/2026-10-issue-96-resampling-check/README.md new file mode 100644 index 0000000..9724b1a --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/README.md @@ -0,0 +1,15 @@ +## Outcome + +`resampling` went straight into `gdalcubes::cube_view()`, which reads a value it does not know as `"near"` with no error, so `resampling = "bilinaer"` returned and cached a nearest-neighbour cube. `resampling_check()` now refuses anything outside the measured set at the three entry points (`dft_stac_cube()`, `dft_stac_fetch()`, `dft_stac_composite()`) and at both `cube_view()` call sites (`stac_cube_assemble()`, `fetch_extent_to()`, which also gained the #92 `aggregation_check()` it lacked). Both checks share `cube_view_choice_check()`. The honoured aliases `mean`/`median` are refused with the spelling to use. The plan review moved the release from 0.20.1 to 0.21.0, because refusing those two breaks calls that used to give correct output. Learned: an assertion meant to pin the fallback (`"near"`) could not fail, because the same string is the first member of the allowed set printed in the headline (code-check round 1). And a mutation probe that summed testthat's `failed` column alone reported three killed mutants as survivors, because an uncaught error is counted under `error`. + +## Measurement + +gdalcubes 0.7.5, `cube_view(...)$resampling` round trip, 2026-10-01: +- Twelve fixed points: `near bilinear cubic cubicspline lanczos average mode max min med q1 q3`. +- Two aliases: `mean -> average`, `median -> med`. +- `near` fallback for `sum rms gauss none first nearest nearest_neighbor cubic_spline "" bilinaer foo`. +- Case is ignored. + +Mutation table: 11 of 11 killed (`findings.md`), covering each of the five call sites, a widened or narrowed set, a lower-cased return, the alias hint, and a wrong fallback name. `devtools::test()`: FAIL 0, PASS 1508, SKIP 16. BULK scale test not run: the change is argument validation before any read, so it has no memory or runtime surface. + +Closed by: PR (see `/gh-pr-push`) diff --git a/planning/archive/2026-10-issue-96-resampling-check/findings.md b/planning/archive/2026-10-issue-96-resampling-check/findings.md new file mode 100644 index 0000000..e827e33 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/findings.md @@ -0,0 +1,68 @@ +# Findings — resampling typo silently becomes "near" (#96) + +## Issue context + +## Problem + +`resampling` goes straight into `gdalcubes::cube_view()`. The mechanism is the same one that #92 hit for `aggregation`: gdalcubes 0.7.5 reads a value it does not know as a default and raises no error. For `resampling` that default is `"near"`. + +Measured by round-tripping values through `cube_view(...)$resampling`: + +``` +bilinear -> bilinear +bilinaer -> near +foo -> near +Bilinear -> bilinear # case-insensitive +cubic -> cubic +``` + +So `dft_stac_cube(aoi, resampling = "bilinaer")` returns a nearest-neighbour cube and caches it under the misspelt key, with no warning. The three callers are `stac_cube_assemble()` (`R/dft_stac_cube.R`, used by the cube and the composite) and `fetch_extent_to()` (`R/dft_stac_fetch.R`). + +## Ask + +Validate `resampling` the same way #92 validates `aggregation`: + +- Build the allowed set by measuring the `cube_view()` round trip, not from `?cube_view`. +- Match case-insensitively. +- Run the check before any network call. +- Pin the set with a behaviour test. + +Found by the `/code-check` round-3 review on #92. + + +## Round-trip measurement (gdalcubes 0.7.5, 2026-10-01) + +`cube_view(...)$resampling`: + +- fixed points: near bilinear cubic cubicspline lanczos average mode max min med q1 q3 +- aliases: mean -> average, median -> med +- fallback to near: bilinaer foo sum rms gauss none first nearest nearest_neighbor cubic_spline +- case-insensitive: Bilinear -> bilinear, MODE -> mode, Q1 -> q1 + +## Plan review (Plan agent, 2026-10-01) — folded in + +No blockers. Acted on: (1) `mean`/`median` error claimed gdalcubes "does not know" them — it renames them; added an `aliases` hint ("reads as "average" ... use "average""). (2) The checks inside `stac_cube_assemble()` and `fetch_extent_to()` had no test; added direct calls. (3) `fetch_extent_to()` never had #92's `aggregation_check()`; added beside `resampling_check()`. (4) Refusing `mean`/`median`, which used to give correct output, is a behaviour change: release as 0.21.0, not 0.20.1. (5) Comment rationale corrected: case variants already give one output two keys, so the reason for refusing aliases is one documented spelling per method, not key uniqueness. + +Refactor regression caught by hand: with the shared helper, the error header read `cube_view_choice_check(...)`; `call = rlang::caller_env()` restores `aggregation_check()` / `resampling_check()`. + +## Mutation table (scratch copy, 2026-10-01) + +| mutant | red | +|---|---| +| M0 unmutated | 0 | +| M1 drop check at top of `dft_stac_cube()` | 1 | +| M2 drop check at top of `dft_stac_fetch()` | 1 | +| M3 drop check at top of `dft_stac_composite()` | 1 | +| M4 drop check in `stac_cube_assemble()` | 1 | +| M5 drop resampling check in `fetch_extent_to()` | 1 | +| M6 drop aggregation check in `fetch_extent_to()` | 1 | +| M7 widen set with `nearest` | 4 | +| M8 lower-case the return | 5 | +| M9 drop the alias hint | 1 | +| M10 narrow set (drop `q3`) | 2 | + +## Errors Encountered + +| Error | Resolution | +|-------|------------| +| First mutation probe reported M1-M3 surviving | probe summed `failed` only; testthat records an uncaught error in `error`. Summed both: all killed | diff --git a/planning/archive/2026-10-issue-96-resampling-check/progress.md b/planning/archive/2026-10-issue-96-resampling-check/progress.md new file mode 100644 index 0000000..32887ee --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/progress.md @@ -0,0 +1,10 @@ +# Progress — resampling typo silently becomes "near" (#96) + +## Session 2026-10-01 + +- Plan-mode exploration — phases approved by user +- Created branch `96-resampling-typo-silently-becomes-near-in` off main +- Scaffolded PWF baseline from issue #96 with approved phases +- Next: start Phase 1 +- Phase 1: `resampling_check()` + shared `cube_view_choice_check()`; checks at all three entry points and both `cube_view()` call sites. Plan review folded in (alias hint, call-site tests, `aggregation_check()` in `fetch_extent_to()`, version 0.21.0). Mutation table 11/11 killed (findings.md). `/code-check` rounds: 1 finding (vacuous `"near"` assertion, fixed), 2 clean, 3 clean. `devtools::test()`: FAIL 0, PASS 1508, SKIP 16. +- Phase 2: @param docs, gotchas note, NEWS 0.21.0. Docs review (1 round, fact-check against code + fresh round trip): clean; widened the old-caches bullet on its note. diff --git a/planning/archive/2026-10-issue-96-resampling-check/review-docs-round1.md b/planning/archive/2026-10-issue-96-resampling-check/review-docs-round1.md new file mode 100644 index 0000000..22bdc01 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/review-docs-round1.md @@ -0,0 +1,55 @@ +# Review — docs/NEWS/notes for #96, round 1 + +Reviewer scope: staged diff `diff_p2.patch` (NEWS.md, `@param resampling` in +dft_stac_cube.R / dft_stac_fetch.R / dft_stac_composite.R, man/*.Rd, +inst/notes/gdalcubes-pc-gotchas.md). Every claim checked against the code or a +fresh measurement, not against the other documents. + +## Clean + +No factual defect, stale number or Rd breakage found. + +## What was verified + +- **Round trip, gdalcubes 0.7.5** (`cube_view(srs = "EPSG:32609", ..., resampling = r)$resampling`, + run fresh): `near bilinear cubic cubicspline lanczos average mode max min med q1 q3` + all come back unchanged; `mean -> average`, `median -> med` (and `Mean`, `MEDIAN` + likewise); `sum rms gauss nearest none "" bilinaer -> near`; `Q1 -> q1`, + `MODE -> mode`, `Bilinear -> bilinear`. Every value the NEWS entry, the roxygen and + the notes name matches what was measured. +- **Allowed set**: `.cube_view_resamplings` is exactly the twelve listed, in the same + order, in all three `@param` blocks, the NEWS bullet and the notes. +- **Aliases**: `resampling_check("mean")` / `("median")` / `("Mean")` refuse with + class `drift_bad_resampling` and name `"average"` / `"med"`, as NEWS and the + roxygen say. +- **Hashed as given / no key moves**: `cube_view_choice_check()` returns `x` + unchanged; all three entry points assign `resampling <- resampling_check(resampling)`; + `stac_cache_key()`, `stac_cube_cache_key()` and `stac_composite_cache_key()` hash + that value. So no accepted value's key moves, and `mean -> average` does change the + key (re-stream once), as NEWS says. +- **"Before any network call, and again at both `cube_view()` call sites"**: entry + checks sit after only `check_gdalcubes()` (and, in composite, the static + `dft_stac_config()` lookup, which makes no request). There are exactly two + `gdalcubes::cube_view()` calls in R/ — `stac_cube_assemble()` and + `fetch_extent_to()` — and both now call `resampling_check()`. +- **"`aggregation` is now also checked where `dft_stac_fetch()` builds each + `cube_view()`, tiled or not, as it already was in cube/composite"**: at d6bbaed + `aggregation_check()` was in `stac_cube_assemble()` (shared by cube and composite) + but not in `fetch_extent_to()`; `fetch_extent_to()` is used by both the untiled and + tiled fetch branches. Claim holds. +- **Old caches / `dft_cache_clear()`**: the cache scheme dir is unchanged in this + release, keys are opaque hashes, and `dft_cache_clear()` defaults to + `scheme = "all"`. Claim holds. +- **Rd sync**: `devtools::document()` in a copy of the staged tree reproduces + `man/` byte-for-byte (`/usr/bin/diff -r` clean). `tools::checkRd()` reports only + the pre-existing non-ASCII notes (standalone checkRd without the package encoding), + nothing from this diff. +- **Notes "pin test stays a plain round trip"**: tests/testthat/test-dft_stac_cube.R + L832-850 round-trips each allowed value and each alias through `cube_view()`. +- DESCRIPTION is still 0.20.0; the 0.21.0 NEWS header is the accepted deliberate bump. + +## Observations, not defects + +- NEWS "Old caches" speaks of a *misspelt* `resampling`; files cached under `sum`, + `rms`, `gauss`, `nearest` or `none` are in the same state (near data, now + unreachable). The wording is loose rather than wrong. diff --git a/planning/archive/2026-10-issue-96-resampling-check/review-round1.md b/planning/archive/2026-10-issue-96-resampling-check/review-round1.md new file mode 100644 index 0000000..4196ca8 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/review-round1.md @@ -0,0 +1,15 @@ +# Review round 1 — #96 resampling_check (staged diff) + +## Findings + +- **[fragile]** tests/testthat/test-dft_stac_cube.R, in "resampling_check refuses anything outside the set..." — `expect_error(drift:::resampling_check("bilinaer"), "near")` cannot fail. The comment says it pins "the fallback", but the headline already interpolates the allowed set, and the set's first member is `"near"`: `` `resampling` must be one of "near", "bilinear", ... ``. If the `x` bullet were deleted, or its `{.val {fallback}}` rendered empty or named the wrong fallback (for example `"none"`, a copy-paste from the aggregation branch), the assertion would still match. This is the code-check rule "An assertion that matches an interpolated value cannot see the claim around it". To pin the claim, match the rendered sentence instead, e.g. `"does not know as \"near\""`, which only the bullet contains. The aggregation counterpart (`expect_error(aggregation_check("bilinear"), "none")`) is fine: `"none"` does not appear in the aggregation set. + +## Checked and found sound + +- Every `gdalcubes::cube_view()` call site in `R/` (two: `stac_cube_assemble()` and `fetch_extent_to()`) now has `resampling_check()`, and so do all three exported entry points. Each entry point calls it before any network access or cache-key use. +- The value is returned as given. Cache keys are unchanged for callers that already pass valid values, and no shipped caller (R/, vignettes/, data-raw/, tests/) passes a now-refused value: the defaults are `"near"` and `"bilinear"`. +- Alias lookup `unname(aliases[tolower(x)])` behaves correctly for `""`, for unmatched names and for the empty `aliases = character(0)` on the aggregation path: single-bracket indexing does not partial-match and returns NA. Non-string inputs skip the lookup. +- The cli messages render correctly at `cli.condition_width = Inf` for a typo, for each alias (including mixed case), for a numeric value, for NA, and for the aggregation path. The `{.val {fallback}}` placed after a line-fold renders; it is not swallowed. +- The error `call` is the internal helper's frame (`resampling_check()` / `aggregation_check()`), which is the same as before the refactor for aggregation, so this is no regression. +- Restore-the-bug reasoning: removing any one call-site check makes a test go red. The entry-point tests would hit the `stop("reached the network")` mocks, and the internal call-site test would hit an unclassed gdalcubes or argument error. +- I ran the tests in a copy (`NOT_CRAN=true`, `load_all`). test-dft_stac_cube.R, test-dft_stac_composite.R and test-dft_stac_fetch.R all pass, with 0 failures; only the network and missing-package tests were skipped. gdalcubes is 0.7.5. diff --git a/planning/archive/2026-10-issue-96-resampling-check/review-round2.md b/planning/archive/2026-10-issue-96-resampling-check/review-round2.md new file mode 100644 index 0000000..1e89806 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/review-round2.md @@ -0,0 +1,46 @@ +# Code-check round 2 — #96 (staged diff) + +## Clean + +No issues found. + +## What was checked, and how + +Worked in a copy (`rsync` of the tree to the scratchpad); nothing in the repo was edited except this file. + +- **The round-1 fix holds and is not vacuous.** `expect_error(resampling_check("bilinaer"), "does not know as \"near\"")`. + Mutated the copy's `resampling_check()` to `fallback = "nearX"` and ran `test_file()`. The #96 block went red + (an error, because testthat 3e re-raises an error that does not match the regexp; it is still red). The closing quote in + the pattern is what makes it discriminate, so `"near"` inside the allowed list cannot satisfy it. +- **Same class swept across every new assertion.** Each message was rendered through the real code path under + `local_reproducible_output()`: + - `"cubicspline"` appears only in the headline's allowed list. That is the claim it pins. + - `"bilinaer"` appears only in the `Got` bullet. + - `reads as "average"` and `use "med"` cannot be matched by `"average"` or `"med"` in the allowed list, because their + prefixes are not there. + - `expect_no_match(..., "does not know")` on `"mean"` is a real negative. The alias bullet replaces the fallback bullet. + - `aggregation_check("bilinear")` with `"none"`: "none" appears nowhere else in that message. The allowed list is + min, max, mean, median, first and last, and "one of" is not "none". +- **The cli fold before `{.val {fallback}}` renders.** The output was "...does not know as "near" without an error" and + "...as "none"...". The interpolation was not swallowed. +- **Call-site tests are non-vacuous by construction.** `stac_cube_assemble()` and `fetch_extent_to()` get placeholder + args such as `cfg = list()` and `target_crs = NULL`. Without the check, each would fail later with an unclassed + error, and `expect_error(class = ...)` would go red. The `aggregation = "count"` case in `fetch_extent_to()` exercises + the newly added `aggregation_check()`. +- **The composite count case** refuses at the top-level `resampling_check()`. That happens before the cache, before + `stac_cube_items()` (mocked to `stop()`) and before the `"first"` substitution. The comment gives the reason refusing + matters, not the mechanism, so it is not false. +- **The count path through the new `fetch_extent_to()` / `stac_cube_assemble()` checks.** The composite passes + `"first"` downstream for a count. `dft_stac_fetch()` defaults to `"first"` / `"near"`, and no source config injects + an aggregation or resampling. So the added last-line checks cannot refuse a value the entry points accepted. +- **Case.** gdalcubes 0.7.5 lower-cases both arguments, measured on Bilinear, Q1, MED, CubicSpline, LANCZOS, Mode, + AVERAGE, Median, FIRST and Mean. So the case-insensitive acceptance matches what gdalcubes honours. Returning the + value as given keeps cache keys stable, which is the accepted tradeoff. +- **The alias lookup.** `aliases[tolower(x)]` on a named atomic vector matches exactly and returns `NA` for a miss, + including with `character(0)` and `""`. It is reached only when `is_string`, so `NA`, `NULL`, `character(0)`, + length-2 and numeric inputs cannot index it. +- **`call = rlang::caller_env()`** is evaluated in `cube_view_choice_check()`'s frame, so it names + `resampling_check()` / `aggregation_check()`. That is the same frame the pre-refactor `aggregation_check()` reported. + There is no regression. +- **Test results.** Every #96 block passed: cube 37/13/11/3 expectations and composite 7, with 0 failed and 0 errors. + They do not skip on CRAN. The one `skip_if_not_installed("gdalcubes")` sits in the blocks that need it. diff --git a/planning/archive/2026-10-issue-96-resampling-check/review-round3.md b/planning/archive/2026-10-issue-96-resampling-check/review-round3.md new file mode 100644 index 0000000..b441860 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/review-round3.md @@ -0,0 +1,70 @@ +# Code-check round 3: #96 (staged diff) + +## Clean + +No issues found. + +## The mechanism behind the round-1 defect + +**One token, two sources in one rendered artifact.** A check reads a string, and that string occurs in the +output for more than one reason. In round 1, `"near"` was both the fallback named in the `x` bullet and the +first member of the allowed set interpolated into the headline. So a regex that was meant to pin the bullet's +claim was satisfied by the headline instead. This is code-check.md's "An assertion that matches an interpolated +value cannot see the claim around it". The general form is a check and its target reading the same value from +different places, or reading it differently. + +## Where that mechanism reaches in this diff, checked one by one + +Messages were rendered through the real code under `local_reproducible_output()` in a scratch copy, and each +assertion pattern was matched against them with `gregexpr`: + +| assertion | occurrences in the rendered message | where it matches | +|---|---|---| +| `"cubicspline"` (typo) | 1 | headline only, and the set is the claim it pins | +| `"bilinaer"` | 1 | the `Got` bullet only | +| `does not know as "near"` | 1 | the bullet only; the closing quote excludes the headline | +| `reads as "average"` (mean) | 1 | the alias bullet | +| `use "med"` (Median) | 1 | the alias bullet | +| `does not know` (mean, negative) | 0 | the alias bullet replaces the fallback bullet | +| `"none"` (aggregation) | 1 | the bullet; the aggregation set does not contain it | + +The test-dft_stac_composite.R and call-site tests assert only `class =`, which this mechanism cannot reach. + +**Non-test side: two lists that must agree, and a check and its consumer reading one value.** + +- **The allowed set and what gdalcubes honours.** These are pinned by the round-trip test. On gdalcubes 0.7.5 + all 12 values come back unchanged and `bilinaer` comes back as `near`. The test hard-codes its own copy of the + set (`expect_setequal`), so it does not read the set it is checking. +- **The aliases and the set.** The alias values (`average`, `med`) are members of the set, and the alias names + are not. The second half matters: if `mean` or `median` were in the set, the alias bullet could never be + reached. +- **The aliases and gdalcubes.** Probed: `mean` maps to `average` and `median` to `med`, so the bullet's claim + is true today. No test pins this. If gdalcubes stopped honouring either alias, the hint "gdalcubes reads as + ..." would become false. The value would still be refused, so this cannot produce a wrong cube. That makes + it a message-accuracy gap, not a defect, and it is not reported as a finding. +- **Case.** The validator uses `tolower(x) %in% allowed` and gdalcubes lower-cases the value itself, so the two + agree. The cache keys (`stac_cache_key`, `stac_cube_cache_key`, `stac_composite_cache_key`) hash the value as + given, which is the accepted tradeoff and unchanged from before. The composite's `is_count` uses `tolower` + too, so it agrees with the validator. +- **NA, length, type and whitespace.** `NA`, `NA_character_`, length 0, length 2, numeric, `NULL` and factor + inputs are all refused before the alias lookup is reached. `" near"` is refused. That is the safe direction, + even though gdalcubes would read it as near. +- **The alias lookup on the aggregation path.** `character(0)[name]` returns `NA`, so the fallback bullet is + used. +- **Every `gdalcubes::cube_view()` in `R/`.** There are two: `stac_cube_assemble()` and `fetch_extent_to()`. + Both now carry both checks, and all three exported entry points check before the cache or network. The + composite count path passes `"first"` downstream, so the new `aggregation_check()` inside `fetch_extent_to()` + and `stac_cube_assemble()` cannot refuse a value the entry point accepted. No source config injects a + resampling or aggregation. No shipped caller in R/, tests/, vignettes/ or inst/ passes a value that is now + refused; data-raw calls gdalcubes directly with `near`/`bilinear`. +- **A pre-existing wording point, not introduced here.** In the composite's aggregation message, the allowed + list includes `"count"` and the bullet says "drift passes only these". drift does not pass `count` to + gdalcubes; it substitutes `first`. This is cosmetic and dates from #92. + +## Test run (scratch copy, NOT_CRAN=true, load_all, gdalcubes 0.7.5) + +| file | failed | error | skipped | passed | +|---|---|---|---|---| +| test-dft_stac_cube.R | 0 | 0 | 3 | 209 | +| test-dft_stac_composite.R | 0 | 0 | 3 | 110 | +| test-dft_stac_fetch.R | 0 | 0 | 4 | 154 | diff --git a/planning/archive/2026-10-issue-96-resampling-check/task_plan.md b/planning/archive/2026-10-issue-96-resampling-check/task_plan.md new file mode 100644 index 0000000..b492d83 --- /dev/null +++ b/planning/archive/2026-10-issue-96-resampling-check/task_plan.md @@ -0,0 +1,68 @@ +# Task: resampling typo silently becomes "near" in dft_stac_cube / dft_stac_composite / dft_stac_fetch (#96) + +## Problem + +`resampling` goes straight into `gdalcubes::cube_view()`. The mechanism is the same one that #92 hit for `aggregation`: gdalcubes 0.7.5 reads a value it does not know as a default and raises no error. For `resampling` that default is `"near"`. + +Measured by round-tripping values through `cube_view(...)$resampling`: + +``` +bilinear -> bilinear +bilinaer -> near +foo -> near +Bilinear -> bilinear # case-insensitive +cubic -> cubic +``` + +So `dft_stac_cube(aoi, resampling = "bilinaer")` returns a nearest-neighbour cube and caches it under the misspelt key, with no warning. The three callers are `stac_cube_assemble()` (`R/dft_stac_cube.R`, used by the cube and the composite) and `fetch_extent_to()` (`R/dft_stac_fetch.R`). + +## Measured set (gdalcubes 0.7.5, `cube_view(...)$resampling` round trip, today) + +Fixed points (survive unchanged) — **the allowed set, 12 values**: +`near bilinear cubic cubicspline lanczos average mode max min med q1 q3` + +Fall back to `near`: `bilinaer foo sum rms gauss none first nearest nearest_neighbor cubic_spline ""` +Case-insensitive: `Bilinear -> bilinear`, `MODE -> mode`, `Q1 -> q1`. + +**Aliases (decision, recommendation taken):** `mean -> average` and `median -> med` are honoured but *renamed*. Allow only the fixed points and refuse the aliases, naming the canonical spelling in the set. Reason: one spelling per method, so `"mean"` and `"average"` cannot cache the same output under two keys, and the pin test stays `honoured(r) == r`. `nearest` is refused too — it lands on `near` only via the fallback. + +## Phase 1: Validate `resampling` (tests first) + +- [x] Tests in `tests/testthat/test-dft_stac_cube.R`, beside the #92 blocks (~L758): + - `resampling_check()` accepts the 12; returns mixed case **as given** (`"Bilinear"`, `"Q1"`) — cache keys hash it, lower-casing would move them + - refuses `"bilinaer" "foo" "nearest" "sum" "rms" "none" "mean" "median" "" NA_character_ NA c("near","bilinear") character(0) 1 NULL` with class `drift_bad_resampling`; message names the set and the refused value + - behaviour pin: every `.cube_view_resamplings` member round-trips through `cube_view()` unchanged, and `"bilinaer"` comes back `"near"` (so the test fails if gdalcubes ever starts refusing, or the set drifts) + - before-network: `dft_stac_cube`, `dft_stac_fetch` (io-lulc), `dft_stac_composite` with bad `resampling` error with `drift_bad_resampling` under the existing `stac_cube_items`/`stac_items_paged` → `stop("reached the network")` mocks, and the cache dir stays empty (composite case in `test-dft_stac_composite.R` ~L300) +- [x] `R/dft_stac_cube.R`: add `.cube_view_resamplings` (comment carrying the measurement + alias/nearest rationale) and `resampling_check()`. Factor the body of `aggregation_check()` into one private helper (`cube_view_choice_check(x, arg, allowed, fallback, class)`) that both call, so the two messages and the as-given return cannot drift apart; `aggregation_check()`'s signature and existing tests unchanged +- [x] Call sites, each beside the existing `aggregation_check()`: + - top of `dft_stac_cube()`, `dft_stac_fetch()`, `dft_stac_composite()` (before any network call) + - `stac_cube_assemble()` — last point before `cube_view()` (covers cube + composite) + - `fetch_extent_to()` (`R/dft_stac_fetch.R:609`) — last point before `cube_view()` for fetch +- [x] `devtools::test()` green; restore-the-bug check: drop the call in `dft_stac_cube()` and confirm the before-network test goes red + +## Phase 2: Docs and NEWS + +- [x] `@param resampling` in `dft_stac_cube.R`, `dft_stac_fetch.R`, `dft_stac_composite.R`: list the set, say anything else is refused because gdalcubes reads it as `"near"`; keep fetch's "`near` for categorical" note; `devtools::document()` +- [x] `inst/notes/gdalcubes-pc-gotchas.md` L220: replace the "#96" forward pointer with the measured set, the aliases and the decision +- [x] `NEWS.md` new `# drift 0.21.0` entry (version moved from 0.20.1 on plan review: refusing `mean`/`median` is a behaviour change); 0.20.0 sentence pointing at #96 left as history +- [x] lintr clean on touched files (vignette and `dup` lints predate this branch) + +## Phase 3: Release + +- [x] Version bump to 0.21.0 in `DESCRIPTION` as the final branch commit ("Release v0.21.0 (#96)") + +## Not doing + +- BULK scale test: no raster computation changes, only argument validation before any read — no memory/runtime surface. Will say so in the PR body. +- No cache-key change: values are hashed as given, defaults (`"bilinear"`, `"near"`) unaffected; existing key-pin tests cover this. + +## Verification + +`devtools::test()` (pins above), `devtools::document()`, `pkgdown::check_pkgdown()` (no new exports, sanity), manual: `dft_stac_cube(aoi, resampling = "bilinaer")` errors immediately with no network. + +## Validation + +- [x] Tests pass +- [x] `/code-check` clean on each commit +- [x] PWF checkboxes match landed work +- [x] `/planning-archive` on completion diff --git a/tests/testthat/test-dft_stac_composite.R b/tests/testthat/test-dft_stac_composite.R index 5295292..cd1b5fd 100644 --- a/tests/testthat/test-dft_stac_composite.R +++ b/tests/testthat/test-dft_stac_composite.R @@ -310,6 +310,24 @@ test_that("dft_stac_composite refuses an aggregation outside the set drift passe expect_length(list.files(cache, recursive = TRUE), 0L) }) +test_that("dft_stac_composite refuses a resampling gdalcubes would read as near (#96)", { + skip_if_not_installed("gdalcubes") + testthat::local_mocked_bindings( + stac_cube_items = function(...) stop("reached the network") + ) + cache <- withr::local_tempdir() + for (b in list("bilinaer", "median", NA_character_, c("near", "bilinear"), 1)) { + expect_error(dft_stac_composite(aoi_pkg(), years = 2021, resampling = b, + cache_dir = cache), + class = "drift_bad_resampling") + } + # a count refuses it too: the count path also reaches cube_view() + expect_error(dft_stac_composite(aoi_pkg(), years = 2021, aggregation = "count", + resampling = "bilinaer", cache_dir = cache), + class = "drift_bad_resampling") + expect_length(list.files(cache, recursive = TRUE), 0L) +}) + # A local, dated Sentinel-2-shaped collection: one B04 and one SCL GeoTIFF per # item, on an n x n grid at 10 m in EPSG:32609. `items` is a list of # list(id, date, scl), `scl` a function of cell x/y returning SCL classes. Separate diff --git a/tests/testthat/test-dft_stac_cube.R b/tests/testthat/test-dft_stac_cube.R index 5f96d7f..5921b7d 100644 --- a/tests/testthat/test-dft_stac_cube.R +++ b/tests/testthat/test-dft_stac_cube.R @@ -793,6 +793,114 @@ test_that("every allowed aggregation is one gdalcubes::cube_view actually honour expect_identical(honoured("count"), "none") }) +test_that("resampling_check refuses anything outside the set drift passes to cube_view (#96)", { + allowed <- c("near", "bilinear", "cubic", "cubicspline", "lanczos", "average", + "mode", "max", "min", "med", "q1", "q3") + expect_setequal(drift:::.cube_view_resamplings, allowed) + for (ok in allowed) expect_identical(drift:::resampling_check(ok), ok) + # case-insensitive, as gdalcubes is, and returned as given: callers hash it + # into cache keys, so lower-casing it would move a mixed-case caller's key + expect_identical(drift:::resampling_check("Bilinear"), "Bilinear") + expect_identical(drift:::resampling_check("Q1"), "Q1") + # "mean" and "median" are honoured under another name, "nearest" only by the + # fallback: one spelling per method, so one output never caches under two keys + bad <- list("bilinaer", "foo", "nearest", "sum", "rms", "none", "mean", + "median", "", NA_character_, NA, c("near", "bilinear"), + character(0), 1, NULL) + for (b in bad) { + expect_error(drift:::resampling_check(b), class = "drift_bad_resampling") + } + # the message names the valid set, the value it refused, and the fallback + expect_error(drift:::resampling_check("bilinaer"), "cubicspline") + expect_error(drift:::resampling_check("bilinaer"), "bilinaer") + expect_error(drift:::resampling_check("bilinaer"), "does not know as \"near\"") + # an alias gdalcubes honours is refused with the spelling to use, not called + # unknown; matched without case like everything else + expect_error(drift:::resampling_check("mean"), "reads as \"average\"", + class = "drift_bad_resampling") + expect_error(drift:::resampling_check("Median"), "use \"med\"", + class = "drift_bad_resampling") + expect_no_match( + tryCatch(drift:::resampling_check("mean"), error = conditionMessage), + "does not know" + ) + # aggregation keeps its own class and fallback through the shared helper + expect_error(drift:::aggregation_check("bilinear"), "none", + class = "drift_bad_aggregation") +}) + +test_that("every allowed resampling is one gdalcubes::cube_view actually honours", { + # Behaviour, not documentation: gdalcubes maps an unknown resampling to + # "near" silently, so each allowed value must survive the round trip, and the + # #96 typo must not. + skip_if_not_installed("gdalcubes") + honoured <- function(r) { + gdalcubes::cube_view( + srs = "EPSG:32609", + extent = list(left = 0, right = 40, bottom = 0, top = 40, + t0 = "2021-07-01", t1 = "2021-07-31"), + dx = 10, dy = 10, dt = "P1M", resampling = r + )$resampling + } + for (r in drift:::.cube_view_resamplings) expect_identical(honoured(r), r) + expect_identical(honoured("bilinaer"), "near") + # the alias hint claims gdalcubes reads these as another member of the set + for (a in names(drift:::.cube_view_resampling_aliases)) { + expect_identical(honoured(a), drift:::.cube_view_resampling_aliases[[a]]) + } +}) + +test_that("dft_stac_cube and dft_stac_fetch refuse a bad resampling before any network call (#96)", { + skip_if_not_installed("gdalcubes") + aoi <- sf::st_read( + system.file("extdata", "example_aoi.gpkg", package = "drift"), + quiet = TRUE + ) + cache <- withr::local_tempdir() + testthat::local_mocked_bindings( + stac_cube_items = function(...) stop("reached the network"), + stac_items_paged = function(...) stop("reached the network") + ) + for (b in list("bilinaer", "nearest", NA_character_, c("near", "bilinear"), 1)) { + expect_error(dft_stac_cube(aoi, resampling = b, cache_dir = cache), + class = "drift_bad_resampling") + expect_error(dft_stac_fetch(aoi, source = "io-lulc", years = 2017, + resampling = b, cache_dir = cache), + class = "drift_bad_resampling") + } + expect_length(list.files(cache, recursive = TRUE), 0L) +}) + +test_that("the two cube_view() call sites refuse a bad value themselves (#96)", { + # The last line of defence for a future caller that skips the exported entry + # points' checks. Both checks run before their other arguments are touched, so + # placeholders suffice. + expect_error( + drift:::stac_cube_assemble( + fetched = list(), cfg = list(), aoi_target = NULL, target_crs = NULL, + t0 = NULL, t1 = NULL, res = 10, dt = "P1M", aggregation = "median", + resampling = "bilinaer", band_assets = NULL, mask_values = NULL, + offset = 0, offset_before = 0, pixel_fn = identity + ), + class = "drift_bad_resampling" + ) + for (args in list( + list(aggregation = "first", resampling = "bilinaer", + class = "drift_bad_resampling"), + list(aggregation = "count", resampling = "near", + class = "drift_bad_aggregation") + )) { + expect_error( + drift:::fetch_extent_to( + col = NULL, ext = list(), t0 = NULL, t1 = NULL, target_crs = NULL, + res = 10, dt = "P1Y", aggregation = args$aggregation, + resampling = args$resampling, out_nc = tempfile() + ), + class = args$class + ) + } +}) + test_that("dft_stac_cube and dft_stac_fetch refuse a bad aggregation before any network call (#92)", { skip_if_not_installed("gdalcubes") aoi <- sf::st_read(