Changes for mdsumner review (#78)

* Add rev roles

* Discuss colocated compute

* Clarify downloading files without processing

* Assign downloads to objects

* Better check inputs to query function

* Add examples of stretching images

* Document merge option

* Highlight using st_make_grid()

DESCRIPTION

@@ -4,6 +4,10 @@ Version: 0.2.1.9000
 Authors@R: c(
     person("Michael", "Mahoney", , "[email protected]", role = c("aut", "cre"),
            comment = c(ORCID = "0000-0003-2402-304X")),
+        person("Felipe", "Carvalho", role = "rev",
+           comment = "Felipe reviewed the package (v. 0.3.0) for rOpenSci, see <https://github.com/ropensci/software-review/issues/636>"),
+    person("Michael", "Sumner", role = "rev",
+           comment = "Michael reviewed the package (v. 0.3.0) for rOpenSci, see <https://github.com/ropensci/software-review/issues/636>"),
     person("Permian Global", role = c("cph", "fnd"))
   )
 Description: Downloads spatial data from spatiotemporal asset catalogs 

R/download.R

@@ -80,6 +80,12 @@ rsi_download_rasters <- function(items,
     gdalwarp_options <- set_gdalwarp_extent(gdalwarp_options, aoi, NULL)
   }
 
+  # Which loop gets parallelized is determined based on which has more steps,
+  # working from the assumption that all downloads take about as long
+  #
+  # so if we aren't merging, or if there's more assets than tiles, we'll
+  # parallelize the outside loop that walks over assets (which, unless the user 
+  # has set up wild nested futures, turns the inside one into a serial process)
   asset_iterator <- ifelse(
     merge || (n_tiles_out < ncol(download_locations)),
     function(...) future.apply::future_lapply(..., future.seed = TRUE),

R/get_stac_data.R

@@ -132,7 +132,10 @@
 #' @param composite_function Character of length 1: The name of a
 #' function used to combine downloaded images into a single composite
 #' (i.e., to aggregate pixel values from multiple images into a single value).
-#' Must be one of of "sum", "mean", "median", "min", "max".
+#' 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", 
+#' "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).
 #' @inheritParams rstac::stac_search
@@ -184,6 +187,11 @@
 #'   output_filename = tempfile(fileext = ".tif")
 #' )
 #' 
+#' landsat_image |> 
+#'   terra::rast() |>
+#'   terra::stretch() |>
+#'   terra::plotRGB()
+#' 
 #' # The `get_*_imagery()` functions will download 
 #' # all available "data" assets by default
 #' # (usually including measured values, and excluding derived bands)
@@ -214,6 +222,26 @@
 #'   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,
+#' # consider tiling your AOI and requesting each tile separately:
+#' aoi <- sf::st_make_grid(aoi, n = 2)
+#' tiles <- lapply(
+#'   seq_along(aoi),
+#'   function(i) {
+#'     get_landsat_imagery(
+#'       aoi[i],
+#'       start_date = "2022-06-01",
+#'       end_date = "2022-08-30",
+#'       output_filename = tempfile(fileext = ".tif")
+#'     )
+#'   }
+#' )
+#' # You'll get a list of tiles that you can then composite or 
+#' # work with however you wish:
+#' unlist(tiles)
 #'
 #' @export
 get_stac_data <- function(aoi,

R/query_and_sign.R

@@ -10,9 +10,10 @@
 #' available for your AOI, or to perform cloud filtering prior to downloading
 #' assets.
 #'
-#' @param bbox An sfc object representing the spatial bounding box of your area
-#' of interest. This must be in EPSG:4326 coordinates (and, if this function is
-#' called from within `get_stac_data()`, it will be)
+#' @param bbox A `bbox` or `sfc` object, from the sf package, representing the 
+#' spatial bounding box of your area of interest. This must be in EPSG:4326 
+#' coordinates (and, if this function is called from within `get_stac_data()`, 
+#' it will be) or else it will be automatically reprojected.
 #' @inheritParams get_stac_data
 #' @param start_date,end_date Character strings of length 1 representing the
 #' boundaries of your temporal range of interest, in RFC-3339 format. Set either
@@ -52,6 +53,26 @@ rsi_query_api <- function(bbox,
     datetime <- NULL
   }
 
+  if (!(inherits(bbox, "bbox") | inherits(bbox, "sfc"))) {
+    rlang::abort(
+      "`bbox` must be either an sfc or a bbox object from the sf package.",
+      class = "rsi_bbox_wrong_class"
+    )
+  }
+
+  if (!(sf::st_crs(bbox) == sf::st_crs("EPSG:4326"))) {
+    rlang::warn(
+      "Reprojecting `bbox` to EPSG:4326.",
+      class = "rsi_reprojecting_bbox"
+    )
+    if (inherits(bbox, "bbox")) {
+      bbox <- sf::st_as_sfc(bbox)
+    }
+    bbox <- sf::st_transform(bbox, 4326)
+  }
+
+  if (inherits(bbox, "sfc")) bbox <- sf::st_bbox(bbox)
+
   items <- rstac::stac_search(
     rstac::stac(stac_source),
     collections = collection,

README.Rmd

@@ -34,6 +34,8 @@ The goal of rsi is to address several **r**epeated **s**patial **i**nfelicities,
 + A method for downloading STAC data -- excuse me, **r**etriving **S**TAC **i**nformation -- from any STAC server, with additional helpers for downloading Landsat, Sentinel-1, and Sentinel-2 data from free and public STAC servers providing **r**apid **s**atellite **i**magery, 
 + A **r**aster **s**tack **i**ntegration method for combining multiple rasters containing distinct data sets into a single raster stack.
 
+The functions in rsi are designed around letting you use the tools you're familiar with to process raster data using compute that you control -- whether that means grabbing imagery with your laptop to add some context to a map, or grabbing tranches of data to a virtual server hosted near your data provider for lightning fast downloads. The outputs from rsi functions are standard objects -- usually the file paths of raster files saved to your hard drive -- meaning it's easy to incorporate rsi into broader spatial data processing workflows.
+
 ## Installation
 
 You can install rsi via:
@@ -87,7 +89,7 @@ landsat_image <- get_stac_data(
 terra::plot(terra::rast(landsat_image))
 ```
 
-For these common use cases, however, rsi also provides helper functions which
+For common data sets, rsi also provides helper functions which
 provide most of these arguments for you. For instance, that `get_stac_data()`
 call could be as simple as:
 
@@ -101,6 +103,15 @@ landsat_image <- get_landsat_imagery(
 terra::plot(terra::rast(landsat_image))
 ```
 
+Note that we've been plotting each band individually so far by calling `terra::plot()`. We could also use `terra::plotRGB()` (after `terra::stretch()`ing the band values) to see what this mosaic of images would look like to the human eye:
+
+```{r}
+landsat_image |> 
+  terra::rast(lyrs = c("R", "G", "B")) |> 
+  terra::stretch() |>
+  terra::plotRGB()
+```
+
 By default, these functions download data from Microsoft's Planetary Computer
 API, using a number of configuration options set in `rsi_band_mapping` objects
 provided by the package. You can see these default configuration options by 

README.md

@@ -8,8 +8,8 @@
 [![R-CMD-check](https://github.com/Permian-Global-Research/rsi/actions/workflows/R-CMD-check.yaml/badge.svg)](https://github.com/Permian-Global-Research/rsi/actions/workflows/R-CMD-check.yaml)
 [![Codecov test
 coverage](https://codecov.io/gh/Permian-Global-Research/rsi/branch/main/graph/badge.svg)](https://app.codecov.io/gh/Permian-Global-Research/rsi?branch=main)
-[![License: Apache
-2.0](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://opensource.org/license/apache-2-0)
+[![License:
+Apache 2.0](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://opensource.org/license/apache-2-0)
 [![Lifecycle:
 maturing](https://img.shields.io/badge/lifecycle-maturing-blue.svg)](https://lifecycle.r-lib.org/articles/stages.html#maturing)
 [![Project Status: Active – The project has reached a stable, usable
@@ -27,17 +27,29 @@ The goal of rsi is to address several **r**epeated **s**patial
 and help avoid **r**epetitive **s**tress **i**njuries. Specifically, rsi
 provides:
 
-- An interface to the **R**some – excuse me, [*Awesome* Spectral Indices
-  project](https://github.com/awesome-spectral-indices/awesome-spectral-indices),
-  providing the list of indices directly in R as a friendly tibble,
-- A method for efficiently *calculating* those awesome spectral indices
-  using local rasters, enabling **r**apid **s**pectral **i**nference,
-- A method for downloading STAC data – excuse me, **r**etriving **S**TAC
-  **i**nformation – from any STAC server, with additional helpers for
-  downloading Landsat, Sentinel-1, and Sentinel-2 data from free and
-  public STAC servers providing **r**apid **s**atellite **i**magery,
-- A **r**aster **s**tack **i**ntegration method for combining multiple
-  rasters containing distinct data sets into a single raster stack.
+  - An interface to the **R**some – excuse me, [*Awesome* Spectral
+    Indices
+    project](https://github.com/awesome-spectral-indices/awesome-spectral-indices),
+    providing the list of indices directly in R as a friendly tibble,
+  - A method for efficiently *calculating* those awesome spectral
+    indices using local rasters, enabling **r**apid **s**pectral
+    **i**nference,
+  - A method for downloading STAC data – excuse me, **r**etriving
+    **S**TAC **i**nformation – from any STAC server, with additional
+    helpers for downloading Landsat, Sentinel-1, and Sentinel-2 data
+    from free and public STAC servers providing **r**apid **s**atellite
+    **i**magery,
+  - A **r**aster **s**tack **i**ntegration method for combining multiple
+    rasters containing distinct data sets into a single raster stack.
+
+The functions in rsi are designed around letting you use the tools
+you’re familiar with to process raster data using compute that you
+control – whether that means grabbing imagery with your laptop to add
+some context to a map, or grabbing tranches of data to a virtual server
+hosted near your data provider for lightning fast downloads. The outputs
+from rsi functions are standard objects – usually the file paths of
+raster files saved to your hard drive – meaning it’s easy to incorporate
+rsi into broader spatial data processing workflows.
 
 ## Installation
 
@@ -124,9 +136,9 @@ terra::plot(terra::rast(landsat_image))
 
 <img src="man/figures/README-unnamed-chunk-3-1.png" width="100%" />
 
-For these common use cases, however, rsi also provides helper functions
-which provide most of these arguments for you. For instance, that
-`get_stac_data()` call could be as simple as:
+For common data sets, rsi also provides helper functions which provide
+most of these arguments for you. For instance, that `get_stac_data()`
+call could be as simple as:
 
 ``` r
 landsat_image <- get_landsat_imagery(
@@ -140,6 +152,20 @@ terra::plot(terra::rast(landsat_image))
 
 <img src="man/figures/README-unnamed-chunk-4-1.png" width="100%" />
 
+Note that we’ve been plotting each band individually so far by calling
+`terra::plot()`. We could also use `terra::plotRGB()` (after
+`terra::stretch()`ing the band values) to see what this mosaic of images
+would look like to the human eye:
+
+``` r
+landsat_image |> 
+  terra::rast(lyrs = c("R", "G", "B")) |> 
+  terra::stretch() |>
+  terra::plotRGB()
+```
+
+<img src="man/figures/README-unnamed-chunk-5-1.png" width="100%" />
+
 By default, these functions download data from Microsoft’s Planetary
 Computer API, using a number of configuration options set in
 `rsi_band_mapping` objects provided by the package. You can see these
@@ -176,7 +202,7 @@ indices <- calculate_indices(
 terra::plot(terra::rast(indices))
 ```
 
-<img src="man/figures/README-unnamed-chunk-6-1.png" width="100%" />
+<img src="man/figures/README-unnamed-chunk-7-1.png" width="100%" />
 
 And last but not least, rsi includes a utility for efficiently combining
 rasters containing different data about the same location into a
@@ -194,14 +220,14 @@ raster_stack <- stack_rasters(
 terra::plot(terra::rast(raster_stack))
 ```
 
-<img src="man/figures/README-unnamed-chunk-7-1.png" width="100%" />
+<img src="man/figures/README-unnamed-chunk-8-1.png" width="100%" />
 
 This can be extremely useful as a way to create predictor bricks and
 other multi-band rasters from various data sources.
 
 ## Contributing
 
-We love contributions! See our [contribution
+We love contributions\! See our [contribution
 guide](https://github.com/Permian-Global-Research/rsi/blob/main/.github/CONTRIBUTING.md)
 for pointers on how to make your contribution as easy to accept as
 possible – in particular, consider [opening an

tests/testthat/test-query_and_sign.R

@@ -0,0 +1,30 @@
+test_that("non-4326 CRS warns", {
+  nc <- sf::read_sf(
+    system.file("shape/nc.shp", package = "sf")
+  )
+  expect_warning(
+    rsi_query_api(
+      sf::st_as_sfc(sf::st_bbox(nc)),
+      stac_source = "https://planetarycomputer.microsoft.com/api/stac/v1/",
+      collection = "landsat-c2-l2",
+      start_date = "2023-08-01",
+      end_date = "2023-09-01",
+      limit = 10
+    ),
+    class = "rsi_reprojecting_bbox"
+  )
+})
+
+test_that("unaccepted bbox objects error well", {
+  expect_error(
+    rsi_query_api(
+      "not a bbox",
+      stac_source = "https://planetarycomputer.microsoft.com/api/stac/v1/",
+      collection = "landsat-c2-l2",
+      start_date = "2023-08-01",
+      end_date = "2023-09-01",
+      limit = 10
+    ),
+    class = "rsi_bbox_wrong_class"
+  )
+})