This is needed for the modelling workflow proposed by Jaime. Today, we discussed two ways how it could be added:
First way: A new function extract_all_ids(cropped_subc_layer, ...) { ... }
Pro: Simple, understandable.
Contra: A whole new function for very few lines of code, as we'd more or less just "wrap" the terra.unique() function.
Second way: Extend the existing extract_ids() function (https://github.com/glowabio/hydrographr/blob/main/R/extract_ids.R#L70). Instead of a data table containing points for which you want the ids, just pass the string 'ALL'. The logic is: The first argument is currently the one determining for which points we want the ids, so having a simple way of saying all of them might be understandable.
Pro: Just adding few lines of code to existing function.
Contra: Might be harder to understand? Extracting ids for points/coordinates is maybe not the same as getting all ids for a layer.
Also, arguments lat and lon are no longer needed, so explaining which arguments are needed for what functionality could be confusing.
extract_ids <- function(data, lon, lat, id = NULL, basin_layer = NULL,
subc_layer = NULL, quiet = TRUE) {
# Check if input data is of type data.frame,
# data.table or tibble
if (!is(data, "data.frame"))
if (data == 'ALL') {
terra.unique(...) # blablabla
} else {
stop("data: Has to be of class 'data.frame' or the string 'ALL'.")
...
This is needed for the modelling workflow proposed by Jaime. Today, we discussed two ways how it could be added:
First way: A new function
extract_all_ids(cropped_subc_layer, ...) { ... }
terra.unique()
function.Second way: Extend the existing
extract_ids()
function (https://github.com/glowabio/hydrographr/blob/main/R/extract_ids.R#L70). Instead of a data table containing points for which you want the ids, just pass the string'ALL'
. The logic is: The first argument is currently the one determining for which points we want the ids, so having a simple way of saying all of them might be understandable.lat
andlon
are no longer needed, so explaining which arguments are needed for what functionality could be confusing.Any opinions?