Issue81 catch errors (#89)

* Add further error handling for downloads * error when single point on geom with no extent is used to as aoi. * test error on single geom * update download function in band mapping * version bump * remove pointless x from error statement * band mapping updates * add suffix for dduplicated date times (relevant for s2 images). * Apply suggestions from code review --------- Co-authored-by: Michael Mahoney <[email protected]>
Permian-Global-Research · Jan 21, 2025 · e0f8133 · e0f8133
1 parent ed27ab3
commit e0f8133
Show file tree

Hide file tree

Showing 10 changed files with 95 additions and 39 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: rsi
 Title: Efficiently Retrieve and Process Satellite Imagery
-Version: 0.3.1.9000
+Version: 0.3.2
 Authors@R: c(
     person("Michael", "Mahoney", , "[email protected]", role = c("aut", "cre"),
            comment = c(ORCID = "0000-0003-2402-304X")),

diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,12 @@
 # rsi (development version)
 
+* Failed downloads and merges should now be handled a bit better. Thanks
+  to @h-a-graham for #89 and to @lucas-johnson for #81.
+
+* If `composite = NULL` and the resource has duplicated asset timestamps,
+  `get_stac_data()` will now generate unique filenames (rather than saving
+  multiple files to the same path). Thanks to @h-a-graham for #89 and #90.
+
 # rsi 0.3.1
 
 * A test that requires online resources is now skipped on CRAN. There are

diff --git a/R/download.R b/R/download.R
@@ -128,11 +128,31 @@ rsi_download_rasters <- function(items,
               )
             },
             error = function(e) {
-              rlang::warn(
-                glue::glue(
-                  "Failed to download {items$features[[which_item]]$id %||% 'UNKNOWN'} from {items$features[[which_item]]$properties$datetime %||% 'UNKNOWN'}" # nolint
+              # stop if failure occurs when merging.
+              if (merge) {
+                rlang::abort(
+                  glue::glue(
+                    "GDAL warp failed when attempting to merge ",
+                    "{length(unlist(feature_iter))} items"
+                  ),
+                  class = "rsi_download_warp_error", parent = e
                 )
+              }
+
+              expr1 <- items$features[[which_item]]$id %||% "UNKNOWN"
+              expr2 <- items$features[[which_item]]$properties$datetime %||% "UNKNOWN"
+
+              err_msg <- glue::glue(
+                "Failed to download {expr1} from {expr2}" # nolint
               )
+
+              # stop if failure occurs when there is only one item to download.
+              if (length(feature_iter) == 1) {
+                rlang::abort(err_msg, class = "rsi_download_warp_error", parent = e)
+              }
+
+              # warn if failure occurs when multiple items are being downloaded.
+              rlang::warn(err_msg, parent = e)
               download_locations[which_item, ] <<- NA
             }
           )

diff --git a/R/get_stac_data.R b/R/get_stac_data.R
@@ -134,7 +134,7 @@
 #' (i.e., to aggregate pixel values from multiple images into a single value).
 #' Options include "merge", which 'stamps' images on top of one another such that
 #' the "last" value downloaded for a pixel -- which isn't guaranteed to be the most
-#' recent one -- will be the only value used, or any of "sum", "mean", "median", 
+#' recent one -- will be the only value used, or any of "sum", "mean", "median",
 #' "min", or "max", which consider all values available at each pixel.
 #' Set to `NULL` to not composite
 #' (i.e., to rescale and save each individual file independently).
@@ -143,7 +143,7 @@
 #' argument of [sf::gdal_utils()]. The same set of options are used for all
 #' downloaded data and the final output images; this means that some common
 #' options (for instance, `PREDICTOR=3`) may cause errors if bands are of
-#' varying data types. The default values are provided by 
+#' varying data types. The default values are provided by
 #' [rsi_gdalwarp_options()].
 #' @param gdal_config_options Options passed to `gdalwarp` through the
 #' `config_options` argument of [sf::gdal_utils()]. The default values are
@@ -186,13 +186,13 @@
 #'   end_date = "2022-08-30",
 #'   output_filename = tempfile(fileext = ".tif")
 #' )
-#' 
-#' landsat_image |> 
+#'
+#' landsat_image |>
 #'   terra::rast() |>
 #'   terra::stretch() |>
 #'   terra::plotRGB()
-#' 
-#' # The `get_*_imagery()` functions will download 
+#'
+#' # The `get_*_imagery()` functions will download
 #' # all available "data" assets by default
 #' # (usually including measured values, and excluding derived bands)
 #' sentinel1_data <- get_sentinel1_imagery(
@@ -202,17 +202,17 @@
 #'   output_filename = tempfile(fileext = ".tif")
 #' )
 #' names(terra::rast(sentinel1_data))
-#' 
+#'
 #' # You can see what bands will be downloaded by a function
 #' # by inspecting the corresponding `band_mapping` object:
 #' sentinel2_band_mapping$planetary_computer_v1
-#' 
+#'
 #' # And you can add additional assets using `c()`:
 #' c(
 #'   sentinel2_band_mapping$planetary_computer_v1,
 #'   "scl"
 #' )
-#' 
+#'
 #' # Or subset the assets downloaded using `[` or `[[`:
 #' sentinel2_imagery <- get_sentinel2_imagery(
 #'   aoi,
@@ -222,7 +222,7 @@
 #'   output_filename = tempfile(fileext = ".tif")
 #' )
 #' names(terra::rast(sentinel2_imagery))
-#' 
+#'
 #' # If you're downloading data for a particularly large AOI,
 #' # and can't composite the resulting images or want to make
 #' # sure you can continue an interrupted download,
@@ -239,7 +239,7 @@
 #'     )
 #'   }
 #' )
-#' # You'll get a list of tiles that you can then composite or 
+#' # You'll get a list of tiles that you can then composite or
 #' # work with however you wish:
 #' unlist(tiles)
 #'
@@ -273,6 +273,17 @@ get_stac_data <- function(aoi,
     )
   }
 
+  aoi_bbox <- sf::st_bbox(aoi)
+  if (aoi_bbox$xmin == aoi_bbox$xmax | aoi_bbox$ymin == aoi_bbox$ymax) {
+    rlang::abort(
+      c(
+        "`aoi` has no extent.",
+        "i" = "This can occur when `aoi` is a single point geometry."
+      ),
+      class = "rsi_aoi_is_point"
+    )
+  }
+
   if (sf::st_is_longlat(aoi) && !(is.null(pixel_x_size) || is.null(pixel_y_size)) && all(c(pixel_x_size, pixel_y_size) %in% c(10, 30))) {
     rlang::warn(
       c(
@@ -360,13 +371,14 @@ get_stac_data <- function(aoi,
   if (is.null(asset_names)) {
     asset_names <- rstac::items_assets(items)
     if (length(asset_names) > 1) {
-      rlang::warn(c(
-        "`asset_names` was `NULL`, so rsi is attempting to download all assets in items in this collection.",
-        i = "This includes multiple assets, so rsi is attempting to download all of them using the same download function.",
-        i = "This might cause errors or not be what you want! Specify `asset_names` to fix this (and to silence this warning)."
-      ),
-      class = "rsi_missing_asset_names"
-    )
+      rlang::warn(
+        c(
+          "`asset_names` was `NULL`, so rsi is attempting to download all assets in items in this collection.",
+          i = "This includes multiple assets, so rsi is attempting to download all of them using the same download function.",
+          i = "This might cause errors or not be what you want! Specify `asset_names` to fix this (and to silence this warning)."
+        ),
+        class = "rsi_missing_asset_names"
+      )
     }
   }
   if (is.null(names(asset_names))) names(asset_names) <- asset_names
@@ -412,6 +424,9 @@ get_stac_data <- function(aoi,
   if (!is.null(stats::na.action(download_results))) {
     items$features[stats::na.action(download_results)] <- NULL
   }
+
+  temp_ras_files <- unlist(download_results)
+  on.exit(file.remove(temp_ras_files), add = TRUE)
   # mask
   if (!is.null(mask_band)) {
     download_results <- rsi_apply_masks(
@@ -440,10 +455,7 @@ get_stac_data <- function(aoi,
     lapply(download_results, rescale_band, scale_strings)
   }
 
-  on.exit(file.remove(unlist(download_results)), add = TRUE)
-
   if (drop_mask_band) items_urls[[mask_band]] <- NULL
-
   mapply(
     function(in_bands, vrt) {
       stack_rasters(
@@ -462,7 +474,8 @@ get_stac_data <- function(aoi,
     app <- tryCatch(rstac::items_datetime(items), error = function(e) NA)
     app <- gsub(":", "", app) # #29, #32
     if (any(is.na(app))) app <- NULL
-    app <- app %||% seq_along(download_results)
+    app <- app %||% as.character(seq_along(download_results))
+    app <- make.unique(app, sep = "_")
 
     output_filename <- paste0(
       tools::file_path_sans_ext(output_filename),
@@ -724,15 +737,17 @@ rsi_composite_bands <- function(download_locations,
           )
         )
       } else {
-        do.call(
-          terra::mosaic,
-          list(
-            x = terra::sprc(lapply(download_locations[[band_name]], terra::rast)),
-            fun = composite_function,
-            filename = out_file,
-            overwrite = TRUE
+        capture.output({
+          do.call(
+            terra::mosaic,
+            list(
+              x = terra::sprc(lapply(download_locations[[band_name]], terra::rast)),
+              fun = composite_function,
+              filename = out_file,
+              overwrite = TRUE
+            )
           )
-        )
+        })
       }
 
       out_file
@@ -845,11 +860,12 @@ get_rescaling_formula <- function(items, band_name, element) {
   )
 
   if (length(unique(elements)) != 1) {
-    rlang::warn(c(
-      glue::glue("Images in band {band_name} have different {element}s."),
-      i = "Returning images without rescaling."
-    ),
-    class = "rsi_multiple_scaling_formulas"
+    rlang::warn(
+      c(
+        glue::glue("Images in band {band_name} have different {element}s."),
+        i = "Returning images without rescaling."
+      ),
+      class = "rsi_multiple_scaling_formulas"
     )
     elements <- NA_real_
   }

diff --git a/data/alos_palsar_band_mapping.rda b/data/alos_palsar_band_mapping.rda
diff --git a/data/dem_band_mapping.rda b/data/dem_band_mapping.rda
diff --git a/data/landsat_band_mapping.rda b/data/landsat_band_mapping.rda
diff --git a/data/sentinel1_band_mapping.rda b/data/sentinel1_band_mapping.rda
diff --git a/data/sentinel2_band_mapping.rda b/data/sentinel2_band_mapping.rda
diff --git a/tests/testthat/test-get_stac_data.R b/tests/testthat/test-get_stac_data.R
@@ -386,3 +386,16 @@ test_that("Providing no asset names fires the expected warning", {
     )
   )
 })
+
+test_that("Providing a single point geometry throws an error", {
+  expect_error(
+    get_landsat_imagery(
+      aoi = sf::st_sfc(sf::st_point(c(-74.912131, 44.080410)), crs = 4326),
+      start_date = "2022-06-01",
+      end_date = "2022-08-01",
+      composite_function = NULL,
+      output_filename = tempfile(fileext = ".tif")
+    ),
+    class = "rsi_aoi_is_point"
+  )
+})