Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -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"),
Expand Down
8 changes: 8 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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).**
Expand Down
8 changes: 7 additions & 1 deletion R/dft_stac_composite.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
74 changes: 62 additions & 12 deletions R/dft_stac_cube.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()).
Expand Down Expand Up @@ -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()],
Expand All @@ -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
}


Expand Down Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions R/dft_stac_fetch.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion inst/notes/gdalcubes-pc-gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion man/dft_stac_composite.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion man/dft_stac_cube.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions man/dft_stac_fetch.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions planning/archive/2026-10-issue-96-resampling-check/README.md
Original file line number Diff line number Diff line change
@@ -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`)
Loading
Loading