From 6c4f2990ae1029696b45e61f2ec23594a5610f94 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 15:47:02 +0100 Subject: [PATCH 01/17] Explicit arguments to orderly_location_pull_packet --- R/location.R | 48 +++++++++++++---------------- R/root.R | 3 +- R/util.R | 5 +++ man/orderly_location_pull_packet.Rd | 34 +++++++++++--------- tests/testthat/test-location.R | 28 +++++------------ 5 files changed, 57 insertions(+), 61 deletions(-) diff --git a/R/location.R b/R/location.R index af93b619..5181fd73 100644 --- a/R/location.R +++ b/R/location.R @@ -323,18 +323,6 @@ orderly_location_pull_metadata <- function(location = NULL, root = NULL) { ##' ##' @title Pull one or more packets from a location ##' -##' @param ... Arguments passed through to -##' [orderly2::orderly_search]. In the special case where the first -##' argument is a character vector of ids *and* there are no named -##' dot arguments, then we interpret this argument as a vector of -##' ids directly. Be careful here, your query may pull a lot of data -##' - in particular, passing `NULL` will match everything that every -##' remote has! -##' -##' @param options Options passed to [orderly2::orderly_search]. -##' The option `allow_remote` must be `TRUE` as otherwise no packet could -##' possibly be pulled, so an error is thrown if this is FALSE. -##' ##' @param recursive If non-NULL, a logical, indicating if we should ##' recursively pull all packets that are referenced by the packets ##' specified in `id`. This might copy a lot of data! If `NULL`, @@ -342,27 +330,35 @@ orderly_location_pull_metadata <- function(location = NULL, root = NULL) { ##' `require_complete_tree`. ##' ##' @inheritParams orderly_metadata +##' @inheritParams orderly_search +##' @inheritParams orderly_search_options ##' ##' @return Invisibly, the ids of packets that were pulled ##' @export -orderly_location_pull_packet <- function(..., options = NULL, recursive = NULL, +orderly_location_pull_packet <- function(expr, + name = NULL, + location = NULL, + pull_metadata = FALSE, + recursive = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - options <- as_orderly_search_options(options, list(allow_remote = TRUE)) - if (!options$allow_remote) { - cli::cli_abort( - "If specifying 'options', 'allow_remote' must be TRUE", - i = "If FALSE, then we can't find a packet you don't already have :)") - } - if (dots_is_literal_id(...)) { - ids <- ..1 + options <- orderly_search_options(location = location, + allow_remote = TRUE, + pull_metadata = pull_metadata) + + if (is.character(expr) && is.null(name) && all(grepl(re_id, expr))) { + ids <- expr } else { - ids <- orderly_search(..., options = options, root = root) + ## TODO: we may drop options here + ids <- orderly_search(expr, name = name, + options = options, + # location = location, pull_metadata = pull_metadata, + root = root) } - if (length(ids) == 0) { - if (options$allow_remote && !options$pull_metadata) { - pull_arg <- gsub(" ", "\u00a0", "options = list(pull_metadata = TRUE)") + if (length(ids) == 0 || (length(ids) == 1 && is.na(ids))) { + if (pull_metadata) { + pull_arg <- cli_nbsp("pull_metadata = TRUE") hint <- c(i = paste("Did you forget to pull metadata? You can do this", "by using the argument '{pull_arg}' in the call", "to 'orderly_location_pull_packet()', or", @@ -377,7 +373,7 @@ orderly_location_pull_packet <- function(..., options = NULL, recursive = NULL, hint)) } - plan <- location_build_pull_plan(ids, options$locations, recursive, root, + plan <- location_build_pull_plan(ids, location, recursive, root, call = environment()) if (plan$info$n_extra > 0) { diff --git a/R/root.R b/R/root.R index d90be649..8e5c589a 100644 --- a/R/root.R +++ b/R/root.R @@ -82,7 +82,8 @@ orderly_init <- function(root = ".", ".vscode", ".Rhistory", ".RData", "*.Rproj", ".Rproj.user") contents <- dir(root, all.files = TRUE, no.. = TRUE) - m <- vapply(glob2rx(allowed), grepl, logical(length(contents)), contents) + m <- vapply(utils::glob2rx(allowed), grepl, logical(length(contents)), + contents) if (!is.matrix(m)) { # exactly one file to compare m <- rbind(m) } diff --git a/R/util.R b/R/util.R index 85753bdd..486bec8f 100644 --- a/R/util.R +++ b/R/util.R @@ -714,3 +714,8 @@ fill_missing_names <- function(x) { } x } + + +cli_nbsp <- function(x) { + gsub(" ", "\u00a0", x, fixed = TRUE) +} diff --git a/man/orderly_location_pull_packet.Rd b/man/orderly_location_pull_packet.Rd index 00b9448f..2033d17d 100644 --- a/man/orderly_location_pull_packet.Rd +++ b/man/orderly_location_pull_packet.Rd @@ -5,26 +5,32 @@ \title{Pull one or more packets from a location} \usage{ orderly_location_pull_packet( - ..., - options = NULL, + expr, + name = NULL, + location = NULL, + pull_metadata = FALSE, recursive = NULL, root = NULL ) } \arguments{ -\item{...}{Arguments passed through to -\link{orderly_search}. In the special case where the first -argument is a character vector of ids \emph{and} there are no named -dot arguments, then we interpret this argument as a vector of -ids directly. Be careful here, your query may pull a lot of data -\itemize{ -\item in particular, passing \code{NULL} will match everything that every -remote has! -}} +\item{expr}{The query expression. A \code{NULL} expression matches everything.} -\item{options}{Options passed to \link{orderly_search}. -The option \code{allow_remote} must be \code{TRUE} as otherwise no packet could -possibly be pulled, so an error is thrown if this is FALSE.} +\item{name}{Optionally, the name of the packet to scope the query on. This +will be intersected with \code{scope} arg and is a shorthand way of running +\code{scope = list(name = "name")}} + +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} \item{recursive}{If non-NULL, a logical, indicating if we should recursively pull all packets that are referenced by the packets diff --git a/tests/testthat/test-location.R b/tests/testthat/test-location.R index b0a0c531..97d7828b 100644 --- a/tests/testthat/test-location.R +++ b/tests/testthat/test-location.R @@ -407,8 +407,11 @@ test_that("can error where a query returns no packets", { id <- create_random_packet(root$src) orderly_location_add_path("src", path = root$src$path, root = root$dst) - err <- expect_error( - orderly_location_pull_packet(name = "data", root = root$dst), + expect_error( + orderly_location_pull_packet(NULL, name = "data", root = root$dst), + "No packets found in query, so cannot pull anything") + expect_error( + orderly_location_pull_packet("latest", name = "data", root = root$dst), "No packets found in query, so cannot pull anything") }) @@ -706,7 +709,8 @@ test_that("can pull from multiple locations with multiple files", { ids_b <- create_random_packet(root$b$path, n_files = 2) orderly_location_pull_metadata(root = root$dst) - suppressMessages(orderly_location_pull_packet(name = "data", root = root$dst)) + suppressMessages( + orderly_location_pull_packet(NULL, name = "data", root = root$dst)) ## It has pulled both packets, and correct number of files expect_setequal( @@ -943,28 +947,12 @@ test_that("can pull packets as a result of a query", { orderly_location_pull_packet( "parameter:i < 3", name = "data", - options = list(pull_metadata = TRUE, allow_remote = TRUE), + pull_metadata = TRUE, root = root$dst$path)) expect_setequal(ids_moved, ids[1:2]) }) -test_that("pull packet sets allow_remote to TRUE if not given", { - root <- list() - for (name in c("src", "dst")) { - root[[name]] <- create_temporary_root() - } - - id <- create_random_packet(root$src) - orderly_location_add_path("src", path = root$src$path, root = root$dst) - orderly_location_pull_metadata(root = root$dst) - expect_error( - orderly_location_pull_packet(NULL, options = list(allow_remote = FALSE), - root = root$dst), - "If specifying 'options', 'allow_remote' must be TRUE") -}) - - test_that("handle metadata where the hash does not match reported", { here <- create_temporary_root() there <- create_temporary_root() From 7033fc47539422fe923efd0993e787df98f14eab Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 15:52:20 +0100 Subject: [PATCH 02/17] Explicit arguments to orderly_metadata_extract --- R/location.R | 2 +- R/outpack_tools.R | 26 +++++++++++++-------- R/query.R | 7 ++++++ man/orderly_metadata_extract.Rd | 41 ++++++++++++++++++++++++++++----- vignettes/collaboration.Rmd | 2 +- 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/R/location.R b/R/location.R index 5181fd73..2ced409d 100644 --- a/R/location.R +++ b/R/location.R @@ -346,7 +346,7 @@ orderly_location_pull_packet <- function(expr, allow_remote = TRUE, pull_metadata = pull_metadata) - if (is.character(expr) && is.null(name) && all(grepl(re_id, expr))) { + if (expr_is_literal_id(expr, name)) { ids <- expr } else { ## TODO: we may drop options here diff --git a/R/outpack_tools.R b/R/outpack_tools.R index 7a2ae3f1..91a723ef 100644 --- a/R/outpack_tools.R +++ b/R/outpack_tools.R @@ -182,27 +182,33 @@ ##' ##' @title Extract metadata from orderly2 packets ##' -##' @param ... Arguments passed through to -##' [orderly2::orderly_search]. In the special case where the first -##' argument is a character vector of ids *and* there are no named -##' dot arguments, then we interpret this argument as a vector of -##' ids directly. -##' ##' @param extract A character vector of columns to extract, possibly ##' named. See Details for the format. ##' ##' @inheritParams orderly_metadata +##' @inheritParams orderly_search +##' @inheritParams orderly_search_options ##' ##' @return A `data.frame`, the columns of which vary based on the ##' names of `extract`; see Details for more information. ##' ##' @export -orderly_metadata_extract <- function(..., extract = NULL, root = NULL) { +orderly_metadata_extract <- function(expr = NULL, name = NULL, location = NULL, + allow_remote = NULL, pull_metadata = FALSE, + extract = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - if (dots_is_literal_id(...)) { - ids <- ..1 + + options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) + if (expr_is_literal_id(expr, name)) { + ids <- expr } else { - ids <- orderly_search(..., root = root) + ## TODO: we may drop options here + ids <- orderly_search(expr, name = name, + options = options, + # location = location, pull_metadata = pull_metadata, + root = root) } extract <- parse_extract(extract, environment()) diff --git a/R/query.R b/R/query.R index 1be14753..811ab934 100644 --- a/R/query.R +++ b/R/query.R @@ -55,6 +55,13 @@ dots_is_literal_id <- function(...) { } +expr_is_literal_id <- function(expr, ...) { + all(vlapply(list(...), is.null)) && + is.character(expr) && + all(grepl(re_id, expr)) +} + + as_orderly_query <- function(expr, name = NULL, scope = NULL, subquery = NULL, arg = "expr", call = parent.frame()) { if (missing(expr)) { diff --git a/man/orderly_metadata_extract.Rd b/man/orderly_metadata_extract.Rd index 31f9f8ca..1c807698 100644 --- a/man/orderly_metadata_extract.Rd +++ b/man/orderly_metadata_extract.Rd @@ -4,14 +4,43 @@ \alias{orderly_metadata_extract} \title{Extract metadata from orderly2 packets} \usage{ -orderly_metadata_extract(..., extract = NULL, root = NULL) +orderly_metadata_extract( + expr = NULL, + name = NULL, + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, + extract = NULL, + root = NULL +) } \arguments{ -\item{...}{Arguments passed through to -\link{orderly_search}. In the special case where the first -argument is a character vector of ids \emph{and} there are no named -dot arguments, then we interpret this argument as a vector of -ids directly.} +\item{expr}{The query expression. A \code{NULL} expression matches everything.} + +\item{name}{Optionally, the name of the packet to scope the query on. This +will be intersected with \code{scope} arg and is a shorthand way of running +\code{scope = list(name = "name")}} + +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} \item{extract}{A character vector of columns to extract, possibly named. See Details for the format.} diff --git a/vignettes/collaboration.Rmd b/vignettes/collaboration.Rmd index 66600f90..6b105481 100644 --- a/vignettes/collaboration.Rmd +++ b/vignettes/collaboration.Rmd @@ -176,7 +176,7 @@ Bob can now query for packets available on the server: ```{r, as = "bob", orderly_root = path_bob} orderly2::orderly_metadata_extract( name = "data", - options = list(allow_remote = TRUE, pull_metadata = TRUE)) + allow_remote = TRUE, pull_metadata = TRUE) ``` Having seen there is a new "data" packet here, he can pull this down locally (TODO: mrc-4414 makes this nicer): From 72afca72269aad3023de2b3a870527a4fcc50e62 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 16:18:02 +0100 Subject: [PATCH 03/17] Explicit arguments to orderly_copy_files --- R/metadata.R | 17 ++++++++-- R/outpack_helpers.R | 47 ++++++++++++++++----------- R/outpack_packet.R | 4 ++- man/orderly_copy_files.Rd | 43 +++++++++++++++++++----- tests/testthat/test-outpack-helpers.R | 8 ++--- 5 files changed, 84 insertions(+), 35 deletions(-) diff --git a/R/metadata.R b/R/metadata.R index e6fb2d39..6784eb9f 100644 --- a/R/metadata.R +++ b/R/metadata.R @@ -286,6 +286,10 @@ orderly_dependency <- function(name, query, files) { subquery <- NULL query <- orderly_query(query, name = name, subquery = subquery) search_options <- as_orderly_search_options(ctx$search_options) + ## TODO: this separation of codepaths here is quite weird. We + ## should do the copy here and have the outpack function probably + ## just do the metadata update. The logic is otherwise fine I + ## think. if (ctx$is_active) { res <- outpack_packet_use_dependency(ctx$packet, query, files, search_options = search_options, @@ -293,9 +297,16 @@ orderly_dependency <- function(name, query, files) { overwrite = TRUE) } else { res <- orderly_copy_files( - query, files = files, dest = ctx$path, overwrite = TRUE, - parameters = ctx$parameters, options = search_options, - envir = ctx$envir, root = ctx$root) + query, + files = files, + dest = ctx$path, + overwrite = TRUE, + parameters = ctx$parameters, + location = search_options$location, + allow_remote = search_options$allow_remote, + pull_metadata = search_options$pull_metadata, + envir = ctx$envir, + root = ctx$root) } cli::cli_alert_info( diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 31c14447..9bbad63b 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -72,43 +72,53 @@ ##' typically what you want, but set to `FALSE` if you would prefer ##' that an error be thrown if the destination file already exists. ##' -##' @param ... Additional arguments passed through to [orderly_search] -##' ##' @inheritParams orderly_search +##' @inheritParams orderly_search_options ##' @inheritParams orderly_metadata ##' ##' @return Nothing, invisibly. Primarily called for its side effect ##' of copying files from a packet into the directory `dest` ##' ##' @export -orderly_copy_files <- function(..., files, dest, overwrite = TRUE, - envir = parent.frame(), options = NULL, +orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, + name = NULL, location = NULL, + allow_remote = NULL, pull_metadata = FALSE, + parameters = NULL, + envir = parent.frame(), root = NULL) { root <- root_open(root, require_orderly = FALSE) + options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) ## Validate files and dest early; it gives a better error where this ## was not provided with names. files <- validate_file_from_to(files, envir) assert_scalar_character(dest, call = environment()) - if (dots_is_literal_id(...)) { - id <- ..1 + if (expr_is_literal_id(expr, name)) { + id <- expr if (length(id) != 1) { - cli::cli_abort(sprintf( - "Expected a length 1 value for first argument if id (not %d)", - length(id))) + cli::cli_abort( + "Expected a length 1 value for 'expr' if id (not {length(id)})", + arg = expr) } } else { - id <- orderly_search(..., options = options, envir = envir, root = root) + ## TODO: we may drop options here + id <- orderly_search(expr, name = name, + options = options, + parameters = parameters, # TODO, bind these earlier? + # location = location, pull_metadata = pull_metadata, + root = root) if (length(id) > 1) { - cli::cli_abort(c( - sprintf("Query returned %d results, expected a single result", - length(id)), - i = "Did you forget latest()?")) + cli::cli_abort( + c("Query returned {length(id)} results, expected a single result", + i = "Did you forget latest()?")) } if (length(id) == 0 || is.na(id)) { - explanation <- orderly_query_explain(..., options = options, - envir = envir, root = root) + explanation <- orderly_query_explain( + expr, name = name, parameters = parameters, options = options, + envir = envir, root = root) cli::cli_abort( c("Query returned 0 results", i = "See 'rlang::last_error()$explanation' for details"), @@ -135,11 +145,10 @@ orderly_copy_files <- function(..., files, dest, overwrite = TRUE, c("Unable to copy files, due to {reason} packet {id}", i = "Consider '{cmd}' to remove this packet from consideration"), parent = e) - } else if (!as_orderly_search_options(options)$allow_remote) { + } else if (!options$allow_remote) { cli::cli_abort( c("Unable to copy files, as they are not available locally", - i = paste("To fetch from a location, try again with", - "options = list(allow_remote = TRUE)")), + i = "To fetch from a location, try again with allow_remote = TRUE"), parent = e) } copy_files_from_remote(id, plan$there, plan$here, dest, overwrite, root, diff --git a/R/outpack_packet.R b/R/outpack_packet.R index a65ecfee..791c73df 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -194,7 +194,9 @@ outpack_packet_use_dependency <- function(packet, query, files, } result <- orderly_copy_files(id, files = files, dest = packet$path, - options = search_options, + location = search_options$location, + allow_remote = search_options$allow_remote, + pull_metadata = search_options$pull_metadata, overwrite = overwrite, envir = envir, root = packet$root) diff --git a/man/orderly_copy_files.Rd b/man/orderly_copy_files.Rd index beb7279c..6a366f97 100644 --- a/man/orderly_copy_files.Rd +++ b/man/orderly_copy_files.Rd @@ -5,17 +5,21 @@ \title{Copy files from a packet} \usage{ orderly_copy_files( - ..., + expr, files, dest, overwrite = TRUE, + name = NULL, + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, + parameters = NULL, envir = parent.frame(), - options = NULL, root = NULL ) } \arguments{ -\item{...}{Additional arguments passed through to \link{orderly_search}} +\item{expr}{The query expression. A \code{NULL} expression matches everything.} \item{files}{Files to copy from the other packet. This can be (1) a character vector, in which case files are copied over without @@ -51,16 +55,39 @@ the destination filename by doing \verb{$\{x\}}.} typically what you want, but set to \code{FALSE} if you would prefer that an error be thrown if the destination file already exists.} +\item{name}{Optionally, the name of the packet to scope the query on. This +will be intersected with \code{scope} arg and is a shorthand way of running +\code{scope = list(name = "name")}} + +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} + +\item{parameters}{Optionally, a named list of parameters to substitute +into the query (using the \verb{this:} prefix)} + \item{envir}{Optionally, an environment to substitute into the query (using the \verb{environment:} prefix). The default here is to use the calling environment, but you can explicitly pass this in if you want to control where this lookup happens.} -\item{options}{Optionally, a \link{orderly_search_options} -object for controlling how the search is performed, and which -packets should be considered in scope. If not provided, default -options are used (i.e., \code{orderly2::orderly_search_options()})} - \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function does not require that the directory is diff --git a/tests/testthat/test-outpack-helpers.R b/tests/testthat/test-outpack-helpers.R index b6f7b5ec..999c0e93 100644 --- a/tests/testthat/test-outpack-helpers.R +++ b/tests/testthat/test-outpack-helpers.R @@ -32,7 +32,7 @@ test_that("can copy files from location, using store", { suppressMessages( orderly_copy_files(id, files = c("data.rds" = "data.rds"), dest = tmp, - options = list(allow_remote = TRUE), root = here)) + allow_remote = TRUE, root = here)) expect_equal(dir(tmp), "data.rds") meta <- orderly_metadata(id, root = there) @@ -57,7 +57,7 @@ test_that("can copy files from location, using archive", { suppressMessages( orderly_copy_files(id, files = c("data.rds" = "data.rds"), dest = tmp, - options = list(allow_remote = TRUE), root = here)) + allow_remote = TRUE, root = here)) expect_equal(dir(tmp), "data.rds") meta <- orderly_metadata(id, there) @@ -90,12 +90,12 @@ test_that("require a single id for search", { ids <- replicate(3, outpack_id()) expect_error( orderly_copy_files(ids, files = c(here = "there"), dest = dst, root = root), - "Expected a length 1 value for first argument if id (not 3)", + "Expected a length 1 value for 'expr' if id (not 3)", fixed = TRUE) expect_error( orderly_copy_files(character(), files = c(here = "there"), dest = dst, root = root), - "Expected a length 1 value for first argument if id (not 0)", + "Expected a length 1 value for 'expr' if id (not 0)", fixed = TRUE) }) From 48f6fb3198d43937343d60e4405ff1caa0a4734b Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 17:00:19 +0100 Subject: [PATCH 04/17] Explicit arguments to orderly_validate_archive --- R/validate.R | 19 +++++++++---------- man/orderly_validate_archive.Rd | 15 +++++++++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/R/validate.R b/R/validate.R index c1ff459c..3eb14d78 100644 --- a/R/validate.R +++ b/R/validate.R @@ -29,21 +29,19 @@ ##' ##' @title Validate unpacked packets. ##' -##' @param ... Either arguments that a search can be constructed from -##' (useful options here include `name = "something"`), a character -##' vector of ids, or leave empty to validate everything. -##' ##' @param action The action to take on finding an invalid packet. See ##' Details. ##' ##' @inheritParams orderly_metadata +##' @inheritParams orderly_search +##' @inheritParams orderly_search_options ##' ##' @return Invisibly, a character vector of repaired (or invalid) ##' packets. ##' ##' @export -orderly_validate_archive <- function(..., action = "inform", - root = NULL) { +orderly_validate_archive <- function(expr = NULL, name = NULL, + action = "inform", root = NULL) { root <- root_open(root, require_orderly = FALSE) action <- match_value(action, c("inform", "orphan", "delete", "repair"), call = environment()) @@ -52,11 +50,12 @@ orderly_validate_archive <- function(..., action = "inform", cli::cli_abort("You have no archive to validate") } - if (dots_is_literal_id(...)) { - ids <- ..1 + if (expr_is_literal_id(expr, name)) { + ids <- expr } else { - options <- orderly_search_options(location = local) - ids <- orderly_search(..., options = options, root = root) + ## TODO: we may drop options here + options <- orderly_search_options(location = "local", allow_remote = FALSE) + ids <- orderly_search(expr, name = name, options = options, root = root) } cache <- new.env(parent = emptyenv()) diff --git a/man/orderly_validate_archive.Rd b/man/orderly_validate_archive.Rd index 614e5f75..d7eefb79 100644 --- a/man/orderly_validate_archive.Rd +++ b/man/orderly_validate_archive.Rd @@ -4,12 +4,19 @@ \alias{orderly_validate_archive} \title{Validate unpacked packets.} \usage{ -orderly_validate_archive(..., action = "inform", root = NULL) +orderly_validate_archive( + expr = NULL, + name = NULL, + action = "inform", + root = NULL +) } \arguments{ -\item{...}{Either arguments that a search can be constructed from -(useful options here include \code{name = "something"}), a character -vector of ids, or leave empty to validate everything.} +\item{expr}{The query expression. A \code{NULL} expression matches everything.} + +\item{name}{Optionally, the name of the packet to scope the query on. This +will be intersected with \code{scope} arg and is a shorthand way of running +\code{scope = list(name = "name")}} \item{action}{The action to take on finding an invalid packet. See Details.} From 724b0d66be7259af96269dce342e83b36f151fb4 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 17:15:53 +0100 Subject: [PATCH 05/17] Explicit arguments to orderly_query_explain --- R/outpack_helpers.R | 4 +++- R/outpack_packet.R | 4 +++- R/query_explain.R | 10 ++++++++-- man/orderly_query_explain.Rd | 28 +++++++++++++++++++++++----- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 9bbad63b..2ff72597 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -117,7 +117,9 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, } if (length(id) == 0 || is.na(id)) { explanation <- orderly_query_explain( - expr, name = name, parameters = parameters, options = options, + expr, name = name, parameters = parameters, + location = options$location, + allow_remote = options$allow_remote, envir = envir, root = root) cli::cli_abort( c("Query returned 0 results", diff --git a/R/outpack_packet.R b/R/outpack_packet.R index 791c73df..69d0fa98 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -179,7 +179,9 @@ outpack_packet_use_dependency <- function(packet, query, files, if (is.na(id)) { explanation <- orderly_query_explain( query, parameters = packet$parameters, envir = envir, - options = search_options, root = packet$root) + location = search_options$location, + allow_remote = search_options$allow_remote, + root = packet$root) cli::cli_abort( c("Failed to find packet for query '{format(query)}'", i = "See 'rlang::last_error()$explanation' for details"), diff --git a/R/query_explain.R b/R/query_explain.R index 9dff38ba..f20526f8 100644 --- a/R/query_explain.R +++ b/R/query_explain.R @@ -6,6 +6,7 @@ ##' @title Explain a query ##' ##' @inheritParams orderly_search +##' @inheritParams orderly_search_options ##' ##' @return An object of class `orderly_query_explain`, which can be ##' inspected (contents subject to change) and which has a print @@ -16,10 +17,15 @@ orderly_query_explain <- function(expr, name = NULL, scope = NULL, subquery = NULL, parameters = NULL, envir = parent.frame(), - options = NULL, root = NULL) { + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, + root = NULL) { root <- root_open(root, require_orderly = FALSE) query <- as_orderly_query(expr, name, scope, subquery) - options <- as_orderly_search_options(options) + options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) found <- orderly_search(query, parameters = parameters, envir = envir, options = options, root = root) query_simplified <- query_simplify(query) diff --git a/man/orderly_query_explain.Rd b/man/orderly_query_explain.Rd index f75161fc..f89902cf 100644 --- a/man/orderly_query_explain.Rd +++ b/man/orderly_query_explain.Rd @@ -11,7 +11,9 @@ orderly_query_explain( subquery = NULL, parameters = NULL, envir = parent.frame(), - options = NULL, + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, root = NULL ) } @@ -36,10 +38,26 @@ query (using the \verb{environment:} prefix). The default here is to use the calling environment, but you can explicitly pass this in if you want to control where this lookup happens.} -\item{options}{Optionally, a \link{orderly_search_options} -object for controlling how the search is performed, and which -packets should be considered in scope. If not provided, default -options are used (i.e., \code{orderly2::orderly_search_options()})} +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working From af68695ab2d94b0ee59ec7c8f4af80bdad7a1465 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 17:26:28 +0100 Subject: [PATCH 06/17] Explicit arguments to orderly_interactive_set_search_options --- R/interactive.R | 11 ++++--- man/orderly_interactive_set_search_options.Rd | 29 ++++++++++++++++--- tests/testthat/test-run.R | 5 ++-- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/R/interactive.R b/R/interactive.R index e30a814b..2d362a6e 100644 --- a/R/interactive.R +++ b/R/interactive.R @@ -75,13 +75,16 @@ detect_orderly_interactive_path <- function( ##' ##' @title Set search options for interactive use ##' -##' @param options Optional control over locations, when used -##' with [orderly2::orderly_dependency]; see of Details section of -##' [orderly2::orderly_run]. +##' @inheritParams orderly_search_options ##' ##' @return Nothing, called for its side effects ##' @export -orderly_interactive_set_search_options <- function(options = NULL) { +orderly_interactive_set_search_options <- function(location = NULL, + allow_remote = NULL, + pull_metadata = FALSE) { + options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) .interactive$search_options <- options } diff --git a/man/orderly_interactive_set_search_options.Rd b/man/orderly_interactive_set_search_options.Rd index 621e4c51..3b9414d0 100644 --- a/man/orderly_interactive_set_search_options.Rd +++ b/man/orderly_interactive_set_search_options.Rd @@ -4,12 +4,33 @@ \alias{orderly_interactive_set_search_options} \title{Set search options for interactive use} \usage{ -orderly_interactive_set_search_options(options = NULL) +orderly_interactive_set_search_options( + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE +) } \arguments{ -\item{options}{Optional control over locations, when used -with \link{orderly_dependency}; see of Details section of -\link{orderly_run}.} +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} } \value{ Nothing, called for its side effects diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index 9bb22d2c..a3875945 100644 --- a/tests/testthat/test-run.R +++ b/tests/testthat/test-run.R @@ -820,8 +820,9 @@ test_that("can select location when querying dependencies interactively", { ids[["us"]] <- orderly_run_quietly("data", envir = envir1, root = path[["us"]]) - orderly_interactive_set_search_options(list(location = "prod")) - expect_equal(.interactive$search_options, list(location = "prod")) + orderly_interactive_set_search_options(location = "prod") + expect_equal(.interactive$search_options, + orderly_search_options(location = "prod")) envir2 <- new.env() path_src <- file.path(path[["us"]], "src", "depends") From a6fa9e046b76d7ba5dedc171633758ba3e4b1179 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 17:55:00 +0100 Subject: [PATCH 07/17] Explicit arguments to orderly_search --- R/location.R | 9 +++++---- R/outpack_helpers.R | 10 ++++++---- R/outpack_packet.R | 4 +++- R/outpack_tools.R | 9 +++++---- R/query_explain.R | 14 +++++++++++--- R/query_search.R | 13 ++++++------- R/validate.R | 7 ++++++- man/orderly_search.Rd | 28 +++++++++++++++++++++++----- tests/testthat/test-query-search.R | 27 +++++++++++---------------- 9 files changed, 76 insertions(+), 45 deletions(-) diff --git a/R/location.R b/R/location.R index 2ced409d..f735d93a 100644 --- a/R/location.R +++ b/R/location.R @@ -349,10 +349,11 @@ orderly_location_pull_packet <- function(expr, if (expr_is_literal_id(expr, name)) { ids <- expr } else { - ## TODO: we may drop options here - ids <- orderly_search(expr, name = name, - options = options, - # location = location, pull_metadata = pull_metadata, + ids <- orderly_search(expr, + name = name, + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = options$pull_metadata, root = root) } diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 2ff72597..63875794 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -105,10 +105,12 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, } } else { ## TODO: we may drop options here - id <- orderly_search(expr, name = name, - options = options, - parameters = parameters, # TODO, bind these earlier? - # location = location, pull_metadata = pull_metadata, + id <- orderly_search(expr, + name = name, + parameters = parameters, + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = options$pull_metadata, root = root) if (length(id) > 1) { cli::cli_abort( diff --git a/R/outpack_packet.R b/R/outpack_packet.R index 69d0fa98..a252c887 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -174,7 +174,9 @@ outpack_packet_use_dependency <- function(packet, query, files, id <- orderly_search(query, parameters = packet$parameters, envir = envir, - options = search_options, + location = search_options$location, + allow_remote = search_options$allow_remote, + pull_metadata = search_options$pull_metadata, root = packet$root) if (is.na(id)) { explanation <- orderly_query_explain( diff --git a/R/outpack_tools.R b/R/outpack_tools.R index 91a723ef..1bfef626 100644 --- a/R/outpack_tools.R +++ b/R/outpack_tools.R @@ -204,10 +204,11 @@ orderly_metadata_extract <- function(expr = NULL, name = NULL, location = NULL, if (expr_is_literal_id(expr, name)) { ids <- expr } else { - ## TODO: we may drop options here - ids <- orderly_search(expr, name = name, - options = options, - # location = location, pull_metadata = pull_metadata, + ids <- orderly_search(expr, + name = name, + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = options$pull_metadata, root = root) } extract <- parse_extract(extract, environment()) diff --git a/R/query_explain.R b/R/query_explain.R index f20526f8..5b7071b6 100644 --- a/R/query_explain.R +++ b/R/query_explain.R @@ -27,7 +27,10 @@ orderly_query_explain <- function(expr, name = NULL, scope = NULL, allow_remote = allow_remote, pull_metadata = pull_metadata) found <- orderly_search(query, parameters = parameters, envir = envir, - options = options, root = root) + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = options$pull_metadata, + root = root) query_simplified <- query_simplify(query) ret <- list(found = found, n = length(stats::na.omit(found)), # latest() returns NA @@ -36,8 +39,13 @@ orderly_query_explain <- function(expr, name = NULL, scope = NULL, for (name in names(query_simplified$parts)) { expr <- query_simplified$parts[[name]] - found <- orderly_search(expr, parameters = parameters, envir = envir, - options = options, root = root) + found <- orderly_search(expr, + parameters = parameters, + envir = envir, + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = FALSE, # not again. + root = root) ret$parts[[name]] <- list( name = name, str = deparse_query(expr, NULL, NULL), diff --git a/R/query_search.R b/R/query_search.R index 809c5c79..6cd94d4c 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -15,13 +15,9 @@ ##' use the calling environment, but you can explicitly pass this in ##' if you want to control where this lookup happens. ##' -##' @param options Optionally, a [orderly2::orderly_search_options] -##' object for controlling how the search is performed, and which -##' packets should be considered in scope. If not provided, default -##' options are used (i.e., `orderly2::orderly_search_options()`) -##' ##' @inheritParams orderly_metadata ##' @inheritParams orderly_query +##' @inheritParams orderly_search_options ##' ##' @return A character vector of matching ids. In the case of no ##' match from a query returning a single value (e.g., `latest(...)` @@ -31,10 +27,13 @@ ##' @export orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL, parameters = NULL, envir = parent.frame(), - options = NULL, root = NULL) { + location = NULL, allow_remote = NULL, + pull_metadata = FALSE, root = NULL) { root <- root_open(root, require_orderly = FALSE) query <- as_orderly_query(expr, name, scope, subquery) - options <- as_orderly_search_options(options) + options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) validate_parameters(parameters, environment()) orderly_query_eval(query, parameters, envir, options, root, call = environment()) diff --git a/R/validate.R b/R/validate.R index 3eb14d78..44dd127b 100644 --- a/R/validate.R +++ b/R/validate.R @@ -55,7 +55,12 @@ orderly_validate_archive <- function(expr = NULL, name = NULL, } else { ## TODO: we may drop options here options <- orderly_search_options(location = "local", allow_remote = FALSE) - ids <- orderly_search(expr, name = name, options = options, root = root) + ids <- orderly_search(expr, + name = name, + location = options$location, + allow_remote = options$allow_remote, + pull_metadata = options$pull_metadata, + root = root) } cache <- new.env(parent = emptyenv()) diff --git a/man/orderly_search.Rd b/man/orderly_search.Rd index dbb81e74..60948ba3 100644 --- a/man/orderly_search.Rd +++ b/man/orderly_search.Rd @@ -11,7 +11,9 @@ orderly_search( subquery = NULL, parameters = NULL, envir = parent.frame(), - options = NULL, + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, root = NULL ) } @@ -36,10 +38,26 @@ query (using the \verb{environment:} prefix). The default here is to use the calling environment, but you can explicitly pass this in if you want to control where this lookup happens.} -\item{options}{Optionally, a \link{orderly_search_options} -object for controlling how the search is performed, and which -packets should be considered in scope. If not provided, default -options are used (i.e., \code{orderly2::orderly_search_options()})} +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index bcd2e442..51d8d7ab 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -248,18 +248,13 @@ test_that("Can filter query to packets that are locally available (unpacked)", { } orderly_location_pull_metadata(root = root$a) - options_local <- orderly_search_options(location = c("x", "y"), - allow_remote = FALSE) - options_remote <- orderly_search_options(location = c("x", "y"), - allow_remote = TRUE) - expect_equal( - orderly_search(quote(name == "data"), options = options_remote, + orderly_search(quote(name == "data"), location = c("x", "y"), root = root$a), c(ids$x, ids$y)) expect_equal( - orderly_search(quote(name == "data"), options = options_local, - root = root$a), + orderly_search(quote(name == "data"), location = c("x", "y"), + allow_remote = FALSE, root = root$a), character()) for (i in ids$x) { @@ -267,12 +262,12 @@ test_that("Can filter query to packets that are locally available (unpacked)", { } expect_equal( - orderly_search(quote(name == "data"), options = options_remote, + orderly_search(quote(name == "data"), location = c("x", "y"), root = root$a), c(ids$x, ids$y)) expect_equal( - orderly_search(quote(name == "data"), options = options_local, - root = root$a), + orderly_search(quote(name == "data"), location = c("x", "y"), + allow_remote = FALSE, root = root$a), ids$x) }) @@ -294,12 +289,12 @@ test_that("scope and allow_local can be used together to filter query", { options_remote <- orderly_search_options(allow_remote = TRUE) expect_equal( - orderly_search(quote(latest(parameter:p == 1)), options = options_remote, + orderly_search(quote(latest(parameter:p == 1)), allow_remote = TRUE, scope = quote(name == "x"), root = root$dst), x2) expect_equal( - orderly_search(quote(latest(parameter:p == 1)), options = options_local, + orderly_search(quote(latest(parameter:p == 1)), allow_remote = FALSE,, scope = quote(name == "x"), root = root$dst), NA_character_) @@ -309,12 +304,12 @@ test_that("scope and allow_local can be used together to filter query", { } expect_equal( - orderly_search(quote(latest(parameter:p == 1)), options = options_remote, + orderly_search(quote(latest(parameter:p == 1)), allow_remote = TRUE, scope = quote(name == "x"), root = root$dst), x2) expect_equal( - orderly_search(quote(latest(parameter:p == 1)), options = options_local, + orderly_search(quote(latest(parameter:p == 1)), allow_remote = FALSE, scope = quote(name == "x"), root = root$dst), x1) @@ -904,7 +899,7 @@ test_that("allow search before query", { character(0)) expect_equal( orderly_search(quote(name == "data"), root = root$a, - options = list(pull_metadata = TRUE, allow_remote = TRUE)), + pull_metadata = TRUE, allow_remote = TRUE), ids) expect_setequal(names(root$a$index$data()$metadata), ids) }) From ba91c6c38c30af1b6823895855b2d0dc796d84cf Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 18:00:06 +0100 Subject: [PATCH 08/17] Explicit arguments to orderly_run --- R/run.R | 16 +++++++++------- man/orderly_run.Rd | 32 +++++++++++++++++++++++++------- tests/testthat/test-run.R | 4 ++-- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/R/run.R b/R/run.R index 3bf15c40..fdd87c86 100644 --- a/R/run.R +++ b/R/run.R @@ -58,8 +58,7 @@ ##' @section Equivalence to the old `use_draft` option: ##' ##' The above location handling generalises orderly (v1)'s old -##' `use_draft` option, in terms of the `location` argument to -##' orderly2::orderly_search_options`: +##' `use_draft` option, in terms of the new `location` argument: ##' ##' * `use_draft = TRUE` is `location = "local"` ##' * `use_draft = FALSE` is `location = c(...)` where you should provide @@ -72,7 +71,7 @@ ##' as they currently exist on production right now with the options: ##' ##' ``` -##' location = "production", pull_metadata = TRUE, require_unpacked = FALSE +##' location = "production", pull_metadata = TRUE ##' ``` ##' ##' which updates your current metadata from production, then runs @@ -127,9 +126,7 @@ ##' @param echo Optional logical to control printing output from ##' `source()` to the console. ##' -##' @param search_options Optional control over locations, when used -##' with [orderly2::orderly_dependency]; converted into a -##' [orderly2::orderly_search_options] object, see Details. +##' @inheritParams orderly_search ##' ##' @param root The path to the root directory, or `NULL` (the ##' default) to search for one from the current working @@ -153,7 +150,8 @@ ##' # and we can query the metadata: ##' orderly2::orderly_metadata_extract(name = "data", root = path) orderly_run <- function(name, parameters = NULL, envir = NULL, echo = TRUE, - search_options = NULL, root = NULL) { + location = NULL, allow_remote = NULL, + pull_metadata = FALSE, root = NULL) { env_root_src <- Sys.getenv("ORDERLY_SRC_ROOT", NA_character_) root <- root_open(root, require_orderly = is.na(env_root_src), call = environment()) @@ -175,6 +173,10 @@ orderly_run <- function(name, parameters = NULL, envir = NULL, echo = TRUE, parameters <- check_parameters(parameters, dat$parameters, environment()) orderly_validate(dat, src) + search_options <- orderly_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) + id <- outpack_id() path <- file.path(root_src, "draft", name, id) fs::dir_create(path) diff --git a/man/orderly_run.Rd b/man/orderly_run.Rd index deec4b08..ad381a45 100644 --- a/man/orderly_run.Rd +++ b/man/orderly_run.Rd @@ -9,7 +9,9 @@ orderly_run( parameters = NULL, envir = NULL, echo = TRUE, - search_options = NULL, + location = NULL, + allow_remote = NULL, + pull_metadata = FALSE, root = NULL ) } @@ -29,9 +31,26 @@ may not always be what is wanted.} \item{echo}{Optional logical to control printing output from \code{source()} to the console.} -\item{search_options}{Optional control over locations, when used -with \link{orderly_dependency}; converted into a -\link{orderly_search_options} object, see Details.} +\item{location}{Optional vector of locations to pull from. We +might in future expand this to allow wildcards or exceptions.} + +\item{allow_remote}{Logical, indicating if we should allow packets +to be found that are not currently unpacked (i.e., are known +only to a location that we have metadata from). If this is +\code{TRUE}, then in conjunction with \link{orderly_dependency} +you might pull a large quantity of data. The default \code{NULL} is +\code{TRUE} if remote locations are listed explicitly as a character +vector in the \code{location} argument, or if you have specified +\code{pull_metadata = TRUE}, otherwise \code{FALSE}.} + +\item{pull_metadata}{Logical, indicating if we should pull +metadata immediately before the search. If \code{location} is given, +then we will pass this through to +\link{orderly_location_pull_metadata} to filter locations +to update. If pulling many packets in sequence, you \emph{will} want +to update this option to \code{FALSE} after the first pull, otherwise +it will update the metadata between every packet, which will be +needlessly slow.} \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working @@ -107,8 +126,7 @@ locations are pulled from. The above location handling generalises orderly (v1)'s old -\code{use_draft} option, in terms of the \code{location} argument to -orderly2::orderly_search_options`: +\code{use_draft} option, in terms of the new \code{location} argument: \itemize{ \item \code{use_draft = TRUE} is \code{location = "local"} \item \code{use_draft = FALSE} is \code{location = c(...)} where you should provide @@ -121,7 +139,7 @@ all locations \emph{except} local default behaviour). In addition, you could resolve dependencies as they currently exist on production right now with the options: -\if{html}{\out{
}}\preformatted{location = "production", pull_metadata = TRUE, require_unpacked = FALSE +\if{html}{\out{
}}\preformatted{location = "production", pull_metadata = TRUE }\if{html}{\out{
}} which updates your current metadata from production, then runs diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index a3875945..fcd3169d 100644 --- a/tests/testthat/test-run.R +++ b/tests/testthat/test-run.R @@ -781,7 +781,7 @@ test_that("Can select location when querying dependencies for a report", { ## Filter to only allow prod: id2 <- orderly_run_quietly("depends", - search_options = list(location = "prod"), + location = "prod", root = path[["us"]], envir = new.env()) expect_equal(orderly_metadata(id2, path[["us"]])$depends$packet, @@ -789,7 +789,7 @@ test_that("Can select location when querying dependencies for a report", { ## Allow any location: id3 <- orderly_run_quietly("depends", - search_options = list(location = c("prod", "dev")), + location = c("prod", "dev"), root = path[["us"]], envir = new.env()) expect_equal(orderly_metadata(id3, path[["us"]])$depends$packet, From 9e0ecbbf52ae55be8cd8f03de8ae772d06501e48 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 18 Oct 2024 18:01:26 +0100 Subject: [PATCH 09/17] Drop lint --- tests/testthat/test-query-search.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index 51d8d7ab..d43c16d4 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -294,7 +294,7 @@ test_that("scope and allow_local can be used together to filter query", { root = root$dst), x2) expect_equal( - orderly_search(quote(latest(parameter:p == 1)), allow_remote = FALSE,, + orderly_search(quote(latest(parameter:p == 1)), allow_remote = FALSE, scope = quote(name == "x"), root = root$dst), NA_character_) From c9edec38f866f76b7a0b133d7dc98dba6933ec0f Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 09:37:55 +0100 Subject: [PATCH 10/17] Drop as_orderly_search_options --- R/metadata.R | 3 ++- R/outpack_packet.R | 3 ++- R/query_search.R | 35 ---------------------------- tests/testthat/test-outpack-packet.R | 9 ++++--- tests/testthat/test-query-search.R | 33 -------------------------- 5 files changed, 8 insertions(+), 75 deletions(-) diff --git a/R/metadata.R b/R/metadata.R index 6784eb9f..6c171b25 100644 --- a/R/metadata.R +++ b/R/metadata.R @@ -285,7 +285,8 @@ orderly_dependency <- function(name, query, files) { ctx <- orderly_context(rlang::caller_env()) subquery <- NULL query <- orderly_query(query, name = name, subquery = subquery) - search_options <- as_orderly_search_options(ctx$search_options) + search_options <- ctx$search_options %||% orderly_search_options() + ## TODO: this separation of codepaths here is quite weird. We ## should do the copy here and have the outpack function probably ## just do the metadata update. The logic is otherwise fine I diff --git a/R/outpack_packet.R b/R/outpack_packet.R index a252c887..f378692a 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -162,7 +162,8 @@ outpack_packet_use_dependency <- function(packet, query, files, overwrite = TRUE) { packet <- check_current_packet(packet) query <- as_orderly_query(query, arg = "query") - search_options <- as_orderly_search_options(search_options) + search_options <- search_options %||% orderly_search_options() + assert_is(search_options, "orderly_search_options") if (!query$info$single) { stop(paste( diff --git a/R/query_search.R b/R/query_search.R index 6cd94d4c..e0e5ea57 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -98,41 +98,6 @@ orderly_search_options <- function(location = NULL, } -as_orderly_search_options <- function(x, defaults = list(), - name = deparse(substitute(x))) { - if (!is.name(name)) { - name <- "options" - } - if (inherits(x, "orderly_search_options")) { - return(x) - } - if (is.null(x)) { - if (length(defaults) == 0) { - return(orderly_search_options()) - } - x <- list() - } - if (!is.list(x)) { - stop(sprintf( - "Expected '%s' to be an 'orderly_search_options' or a list of options", - name), - call. = FALSE) - } - err <- setdiff(names(x), names(formals(orderly_search_options))) - if (length(err) > 0) { - stop(sprintf("Invalid option passed to 'orderly_search_options': %s", - paste(squote(err), collapse = ", ")), - call. = FALSE) - } - for (i in names(defaults)) { - if (is.null(x[[i]])) { - x[[i]] <- defaults[[i]] - } - } - do.call(orderly_search_options, x) -} - - orderly_query_eval <- function(query, parameters, envir, options, root, call = NULL) { assert_is(query, "orderly_query", call = call) diff --git a/tests/testthat/test-outpack-packet.R b/tests/testthat/test-outpack-packet.R index 6354c757..4c5bd228 100644 --- a/tests/testthat/test-outpack-packet.R +++ b/tests/testthat/test-outpack-packet.R @@ -689,7 +689,7 @@ test_that("can pull in dependency from specific location", { p <- outpack_packet_start_quietly(path_src, "example", root = root$a) query <- quote(latest(name == "data" && parameter:p > 2)) - options <- list(location = c("x", "y"), allow_remote = FALSE) + options <- orderly_search_options(location = c("x", "y"), allow_remote = FALSE) expect_error( outpack_packet_use_dependency(p, query, c("data.rds" = "data.rds"), search_options = options), @@ -740,10 +740,10 @@ test_that("can pull in dependency when not found, if requested", { expect_equal(nrow(root$a$index$data()$location), 0) expect_equal(length(root$a$index$data()$unpacked), 0) + options <- orderly_search_options(pull_metadata = TRUE, allow_remote = TRUE) suppressMessages( outpack_packet_use_dependency(p_a, query, c("data.rds" = "data.rds"), - search_options = list(pull_metadata = TRUE, - allow_remote = TRUE))) + search_options = options)) expect_length(root$a$index$data()$metadata, 3) expect_equal(nrow(root$a$index$data()$location), 3) @@ -754,8 +754,7 @@ test_that("can pull in dependency when not found, if requested", { p_b <- outpack_packet_start_quietly(path_src_b, "example", root = root$b$path) suppressMessages( outpack_packet_use_dependency(p_b, query, c("data.rds" = "data.rds"), - search_options = list(pull_metadata = TRUE, - allow_remote = TRUE))) + search_options = options)) expect_length(root$b$index$data()$metadata, 3) expect_equal(nrow(root$b$index$data()$location), 4) # compare with above! diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index d43c16d4..75b169f3 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -38,39 +38,6 @@ test_that("nontrivial location implies allow_remote", { }) -test_that("can convert into search options", { - opts <- orderly_search_options(location = "x", - allow_remote = FALSE, - pull_metadata = FALSE) - expect_equal(as_orderly_search_options(NULL), - orderly_search_options()) - expect_equal(as_orderly_search_options(list(location = "x")), - modifyList(orderly_search_options(), - list(location = "x", allow_remote = TRUE))) - expect_equal(as_orderly_search_options(unclass(opts)), - opts) - expect_equal(as_orderly_search_options(NULL, list(allow_remote = TRUE)), - orderly_search_options(allow_remote = TRUE)) - expect_equal(as_orderly_search_options(list(location = "a"), - list(allow_remote = TRUE)), - orderly_search_options(location = "a", allow_remote = TRUE)) - expect_equal(as_orderly_search_options(list(allow_remote = FALSE, - location = "a"), - list(allow_remote = TRUE)), - orderly_search_options(allow_remote = FALSE, location = "a")) -}) - - -test_that("validate inputs to outpack search options", { - expect_error( - as_orderly_search_options(c(allow_remote = FALSE)), - "Expected 'options' to be an 'orderly_search_options' or a list of options") - expect_error( - as_orderly_search_options(list(allow_remote = FALSE, other = FALSE)), - "Invalid option passed to 'orderly_search_options': 'other'") -}) - - test_that("Can run very basic queries", { root <- create_temporary_root(use_file_store = TRUE) ids <- vcapply(1:3, function(i) create_random_packet(root)) From af4e93cf480c5c6df5d0ebc26e52dcffef9212e8 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 09:53:15 +0100 Subject: [PATCH 11/17] Move old function behind deprecation warning --- R/interactive.R | 6 +++--- R/location.R | 9 +++------ R/metadata.R | 2 +- R/outpack_helpers.R | 11 +++++----- R/outpack_packet.R | 2 +- R/outpack_tools.R | 9 +++------ R/prune.R | 2 +- R/query_explain.R | 16 ++++++--------- R/query_search.R | 25 +++++++++++++++-------- R/run.R | 6 +++--- R/validate.R | 8 +++----- tests/testthat/test-outpack-packet.R | 4 ++-- tests/testthat/test-query-index.R | 10 +++++----- tests/testthat/test-query-search.R | 30 ++++++++++++++-------------- tests/testthat/test-run.R | 2 +- vignettes/collaboration.Rmd | 6 +++--- 16 files changed, 73 insertions(+), 75 deletions(-) diff --git a/R/interactive.R b/R/interactive.R index 2d362a6e..00ba070d 100644 --- a/R/interactive.R +++ b/R/interactive.R @@ -82,9 +82,9 @@ detect_orderly_interactive_path <- function( orderly_interactive_set_search_options <- function(location = NULL, allow_remote = NULL, pull_metadata = FALSE) { - options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) + options <- build_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) .interactive$search_options <- options } diff --git a/R/location.R b/R/location.R index f735d93a..9ec04a69 100644 --- a/R/location.R +++ b/R/location.R @@ -342,18 +342,15 @@ orderly_location_pull_packet <- function(expr, recursive = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - options <- orderly_search_options(location = location, - allow_remote = TRUE, - pull_metadata = pull_metadata) if (expr_is_literal_id(expr, name)) { ids <- expr } else { ids <- orderly_search(expr, name = name, - location = options$location, - allow_remote = options$allow_remote, - pull_metadata = options$pull_metadata, + location = location, + allow_remote = TRUE, + pull_metadata = pull_metadata, root = root) } diff --git a/R/metadata.R b/R/metadata.R index 6c171b25..8b06c757 100644 --- a/R/metadata.R +++ b/R/metadata.R @@ -285,7 +285,7 @@ orderly_dependency <- function(name, query, files) { ctx <- orderly_context(rlang::caller_env()) subquery <- NULL query <- orderly_query(query, name = name, subquery = subquery) - search_options <- ctx$search_options %||% orderly_search_options() + search_options <- ctx$search_options %||% build_search_options() ## TODO: this separation of codepaths here is quite weird. We ## should do the copy here and have the outpack function probably diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 63875794..0efdd68c 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -87,9 +87,11 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, envir = parent.frame(), root = NULL) { root <- root_open(root, require_orderly = FALSE) - options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) + ## Validate options here so we can refer to the computed value of + ## allow_remote later in error messages. + options <- build_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) ## Validate files and dest early; it gives a better error where this ## was not provided with names. @@ -104,7 +106,6 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, arg = expr) } } else { - ## TODO: we may drop options here id <- orderly_search(expr, name = name, parameters = parameters, @@ -152,7 +153,7 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, } else if (!options$allow_remote) { cli::cli_abort( c("Unable to copy files, as they are not available locally", - i = "To fetch from a location, try again with allow_remote = TRUE"), + i = "To fetch from a location, try again with 'allow_remote = TRUE'"), parent = e) } copy_files_from_remote(id, plan$there, plan$here, dest, overwrite, root, diff --git a/R/outpack_packet.R b/R/outpack_packet.R index f378692a..404eb96e 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -162,7 +162,7 @@ outpack_packet_use_dependency <- function(packet, query, files, overwrite = TRUE) { packet <- check_current_packet(packet) query <- as_orderly_query(query, arg = "query") - search_options <- search_options %||% orderly_search_options() + search_options <- search_options %||% build_search_options() assert_is(search_options, "orderly_search_options") if (!query$info$single) { diff --git a/R/outpack_tools.R b/R/outpack_tools.R index 1bfef626..9f1a3634 100644 --- a/R/outpack_tools.R +++ b/R/outpack_tools.R @@ -198,17 +198,14 @@ orderly_metadata_extract <- function(expr = NULL, name = NULL, location = NULL, extract = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) if (expr_is_literal_id(expr, name)) { ids <- expr } else { ids <- orderly_search(expr, name = name, - location = options$location, - allow_remote = options$allow_remote, - pull_metadata = options$pull_metadata, + location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata, root = root) } extract <- parse_extract(extract, environment()) diff --git a/R/prune.R b/R/prune.R index 21536d5d..1ab05b06 100644 --- a/R/prune.R +++ b/R/prune.R @@ -29,7 +29,7 @@ orderly_prune_orphans <- function(root = NULL) { return(invisible(id)) } - idx <- new_query_index(root, orderly_search_options(location = local)) + idx <- new_query_index(root, build_search_options(location = local)) is_used <- lengths(lapply(id, idx$get_packet_uses, Inf)) > 0 if (any(is_used)) { cli::cli_alert_info( diff --git a/R/query_explain.R b/R/query_explain.R index 5b7071b6..8c20d02b 100644 --- a/R/query_explain.R +++ b/R/query_explain.R @@ -19,17 +19,13 @@ orderly_query_explain <- function(expr, name = NULL, scope = NULL, envir = parent.frame(), location = NULL, allow_remote = NULL, - pull_metadata = FALSE, root = NULL) { root <- root_open(root, require_orderly = FALSE) query <- as_orderly_query(expr, name, scope, subquery) - options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) found <- orderly_search(query, parameters = parameters, envir = envir, - location = options$location, - allow_remote = options$allow_remote, - pull_metadata = options$pull_metadata, + location = location, + allow_remote = allow_remote, + pull_metadata = FALSE, root = root) query_simplified <- query_simplify(query) ret <- list(found = found, @@ -42,9 +38,9 @@ orderly_query_explain <- function(expr, name = NULL, scope = NULL, found <- orderly_search(expr, parameters = parameters, envir = envir, - location = options$location, - allow_remote = options$allow_remote, - pull_metadata = FALSE, # not again. + location = location, + allow_remote = allow_remote, + pull_metadata = FALSE, root = root) ret$parts[[name]] <- list( name = name, diff --git a/R/query_search.R b/R/query_search.R index e0e5ea57..7334548b 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -31,9 +31,9 @@ orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL, pull_metadata = FALSE, root = NULL) { root <- root_open(root, require_orderly = FALSE) query <- as_orderly_query(expr, name, scope, subquery) - options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) + options <- build_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) validate_parameters(parameters, environment()) orderly_query_eval(query, parameters, envir, options, root, call = environment()) @@ -76,19 +76,28 @@ orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL, orderly_search_options <- function(location = NULL, allow_remote = NULL, pull_metadata = FALSE) { - ## TODO: Later, we might allow something like "before" here too to - ## control searching against some previous time on a location. + cli::cli_warn( + c("Use of 'orderly_search_options' is deprecated", + i = paste("You should just pass these arguments directly into functions", + "that previously accepted 'options'")), + .frequency = "regularly", + .frequency_id = "orderly_search_options") +} + + +build_search_options <- function(location = NULL, allow_remote = NULL, + pull_metadata = FALSE, call = parent.frame()) { if (!is.null(location)) { - assert_character(location) + assert_character(location, call = call) } has_remote_location <- !is.null(location) && length(setdiff(location, c("local", "orphan")) > 0) - assert_scalar_logical(pull_metadata) + assert_scalar_logical(pull_metadata, call = call) if (is.null(allow_remote)) { allow_remote <- has_remote_location || pull_metadata } else { - assert_scalar_logical(allow_remote) + assert_scalar_logical(allow_remote, call = call) } ret <- list(location = location, allow_remote = allow_remote, diff --git a/R/run.R b/R/run.R index fdd87c86..037462b3 100644 --- a/R/run.R +++ b/R/run.R @@ -173,9 +173,9 @@ orderly_run <- function(name, parameters = NULL, envir = NULL, echo = TRUE, parameters <- check_parameters(parameters, dat$parameters, environment()) orderly_validate(dat, src) - search_options <- orderly_search_options(location = location, - allow_remote = allow_remote, - pull_metadata = pull_metadata) + search_options <- build_search_options(location = location, + allow_remote = allow_remote, + pull_metadata = pull_metadata) id <- outpack_id() path <- file.path(root_src, "draft", name, id) diff --git a/R/validate.R b/R/validate.R index 44dd127b..3f1aa861 100644 --- a/R/validate.R +++ b/R/validate.R @@ -53,13 +53,11 @@ orderly_validate_archive <- function(expr = NULL, name = NULL, if (expr_is_literal_id(expr, name)) { ids <- expr } else { - ## TODO: we may drop options here - options <- orderly_search_options(location = "local", allow_remote = FALSE) ids <- orderly_search(expr, name = name, - location = options$location, - allow_remote = options$allow_remote, - pull_metadata = options$pull_metadata, + location = "local", + allow_remote = FALSE, + pull_metadata = FALSE, root = root) } diff --git a/tests/testthat/test-outpack-packet.R b/tests/testthat/test-outpack-packet.R index 4c5bd228..0da4cc38 100644 --- a/tests/testthat/test-outpack-packet.R +++ b/tests/testthat/test-outpack-packet.R @@ -689,7 +689,7 @@ test_that("can pull in dependency from specific location", { p <- outpack_packet_start_quietly(path_src, "example", root = root$a) query <- quote(latest(name == "data" && parameter:p > 2)) - options <- orderly_search_options(location = c("x", "y"), allow_remote = FALSE) + options <- build_search_options(location = c("x", "y"), allow_remote = FALSE) expect_error( outpack_packet_use_dependency(p, query, c("data.rds" = "data.rds"), search_options = options), @@ -740,7 +740,7 @@ test_that("can pull in dependency when not found, if requested", { expect_equal(nrow(root$a$index$data()$location), 0) expect_equal(length(root$a$index$data()$unpacked), 0) - options <- orderly_search_options(pull_metadata = TRUE, allow_remote = TRUE) + options <- build_search_options(pull_metadata = TRUE, allow_remote = TRUE) suppressMessages( outpack_packet_use_dependency(p_a, query, c("data.rds" = "data.rds"), search_options = options)) diff --git a/tests/testthat/test-query-index.R b/tests/testthat/test-query-index.R index 5a6eb82d..2da2dd41 100644 --- a/tests/testthat/test-query-index.R +++ b/tests/testthat/test-query-index.R @@ -9,8 +9,8 @@ test_that("index can include only unpacked packets", { x2 <- create_random_packet(root$src, "x") orderly_location_pull_metadata(root = root$dst) - opts_all <- orderly_search_options(allow_remote = TRUE) - opts_unpacked <- orderly_search_options(allow_remote = FALSE) + opts_all <- build_search_options(allow_remote = TRUE) + opts_unpacked <- build_search_options(allow_remote = FALSE) index <- new_query_index(root$dst, opts_all) index_unpacked <- new_query_index(root$dst, opts_unpacked) @@ -33,7 +33,7 @@ test_that("index includes depends info", { ids <- create_random_packet_chain(root, 3) ids["d"] <- create_random_dependent_packet(root, "d", ids[c("b", "c")]) - index <- new_query_index(root, orderly_search_options()) + index <- new_query_index(root, build_search_options()) expect_setequal(index$index$id, ids) expect_equal(index$get_packet_depends(ids["a"], 1), character(0)) @@ -55,7 +55,7 @@ test_that("index includes uses info", { ids <- create_random_packet_chain(root, 3) ids["d"] <- create_random_dependent_packet(root, "d", ids[c("b", "c")]) - index <- new_query_index(root, orderly_search_options()) + index <- new_query_index(root, build_search_options()) expect_setequal(index$index$id, ids) expect_setequal(index$get_packet_uses(ids["a"], 1), ids["b"]) @@ -88,7 +88,7 @@ test_that("can apply a location filter to index", { orderly_location_pull_metadata(root = root$a) idx_with_location <- function(location) { - options <- orderly_search_options(location = location, allow_remote = TRUE) + options <- build_search_options(location = location, allow_remote = TRUE) new_query_index(root$a, options) } diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index 75b169f3..f7969476 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -1,5 +1,5 @@ test_that("can construct search options", { - defaults <- orderly_search_options() + defaults <- build_search_options() expect_s3_class(defaults, "orderly_search_options") expect_mapequal( unclass(defaults), @@ -7,8 +7,8 @@ test_that("can construct search options", { allow_remote = FALSE, pull_metadata = FALSE)) - opts <- orderly_search_options(location = c("x", "y"), - pull_metadata = TRUE) + opts <- build_search_options(location = c("x", "y"), + pull_metadata = TRUE) expect_s3_class(opts, "orderly_search_options") expect_mapequal( unclass(opts), @@ -19,22 +19,22 @@ test_that("can construct search options", { test_that("pull_metadata implies allow_remote", { - opts <- orderly_search_options(pull_metadata = TRUE) - expect_equal(opts, orderly_search_options(location = NULL, - allow_remote = TRUE, - pull_metadata = TRUE)) + opts <- build_search_options(pull_metadata = TRUE) + expect_equal(opts, build_search_options(location = NULL, + allow_remote = TRUE, + pull_metadata = TRUE)) }) test_that("nontrivial location implies allow_remote", { - expect_false(orderly_search_options(location = NULL)$allow_remote) - expect_false(orderly_search_options(location = "local")$allow_remote) + expect_false(build_search_options(location = NULL)$allow_remote) + expect_false(build_search_options(location = "local")$allow_remote) expect_false( - orderly_search_options(location = c("local", "orphan"))$allow_remote) + build_search_options(location = c("local", "orphan"))$allow_remote) expect_true( - orderly_search_options(location = "server")$allow_remote) + build_search_options(location = "server")$allow_remote) expect_true( - orderly_search_options(location = c("local", "server"))$allow_remote) + build_search_options(location = c("local", "server"))$allow_remote) }) @@ -73,7 +73,7 @@ test_that("Can run very basic queries", { ids) expect_equal( orderly_search(bquote(latest(id == .(ids[[1]]) || id == .(ids[[2]]))), - root = root), + root = root), ids[[2]]) }) @@ -252,8 +252,8 @@ test_that("scope and allow_local can be used together to filter query", { y2 <- create_random_packet(root$src, "y", list(p = 1)) orderly_location_pull_metadata(root = root$dst) - options_local <- orderly_search_options(allow_remote = FALSE) - options_remote <- orderly_search_options(allow_remote = TRUE) + options_local <- build_search_options(allow_remote = FALSE) + options_remote <- build_search_options(allow_remote = TRUE) expect_equal( orderly_search(quote(latest(parameter:p == 1)), allow_remote = TRUE, diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index fcd3169d..8bd2fc48 100644 --- a/tests/testthat/test-run.R +++ b/tests/testthat/test-run.R @@ -822,7 +822,7 @@ test_that("can select location when querying dependencies interactively", { orderly_interactive_set_search_options(location = "prod") expect_equal(.interactive$search_options, - orderly_search_options(location = "prod")) + build_search_options(location = "prod")) envir2 <- new.env() path_src <- file.path(path[["us"]], "src", "depends") diff --git a/vignettes/collaboration.Rmd b/vignettes/collaboration.Rmd index 6b105481..9d698f70 100644 --- a/vignettes/collaboration.Rmd +++ b/vignettes/collaboration.Rmd @@ -197,10 +197,10 @@ We have seen several broad patterns of distributing packets. This approach works well where a primary goal is confidence that the packets everyone works with as dependencies are created under a "clean" environment with no unexpected global dependencies. With the web version, you can also enforce things like only running off the default branch. Alice and Bob will then end up with a collection of packets in their local archives that are a mix of canonical ones (from the server) and locally-created ones, but the local ones are always a dead end as they never get shared with anyone. As a result, Alice and Bob may delete their archive directories without any great concern. -When running their own packets, to make sure that the packet will run in a similar way to a packet run on the server, Bob may want to ensure that only dependencies that could be found on the server will be considered. To that, he can create search options +When running their own packets, to make sure that the packet will run in a similar way to a packet run on the server, Bob may want to ensure that only dependencies that could be found on the server will be considered. To that, he can pass the `location` argument to `orderly_run()`, which controls how search is performed: -```{r, as="bob"} -search_options <- orderly2::orderly_search_options(location = "server") +```{.r, as="bob"} +orderly_run(..., location = "server") ``` and then pass this in to `orderly2::orderly_run` when running a report. In developing, Bob may find passing this into `orderly2::orderly_interactive_set_search_options()` helpful as that will mean that any calls to `orderly2::orderly_description()` will use only packets from the server. From 064522102435e80bf930caec33c3797c316b8db9 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 10:19:19 +0100 Subject: [PATCH 12/17] Add compatibility layer --- R/location.R | 2 ++ R/outpack_helpers.R | 3 +- R/outpack_tools.R | 4 ++- R/query_search.R | 26 ++++++++++++++++- R/run.R | 43 +++++++++++++++-------------- man/orderly_copy_files.Rd | 5 ++++ man/orderly_location_pull_packet.Rd | 5 ++++ man/orderly_metadata_extract.Rd | 5 ++++ man/orderly_query_explain.Rd | 10 ------- man/orderly_run.Rd | 38 +++++++++++++------------ man/orderly_search.Rd | 5 ++++ 11 files changed, 95 insertions(+), 51 deletions(-) diff --git a/R/location.R b/R/location.R index 9ec04a69..ad1b0bd6 100644 --- a/R/location.R +++ b/R/location.R @@ -340,8 +340,10 @@ orderly_location_pull_packet <- function(expr, location = NULL, pull_metadata = FALSE, recursive = NULL, + options = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) + compatibility_fix_options(options, "orderly_location_pull_packet") if (expr_is_literal_id(expr, name)) { ids <- expr diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 0efdd68c..08133ad5 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -83,10 +83,11 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, name = NULL, location = NULL, allow_remote = NULL, pull_metadata = FALSE, - parameters = NULL, + parameters = NULL, options = NULL, envir = parent.frame(), root = NULL) { root <- root_open(root, require_orderly = FALSE) + compatibility_fix_options(options, "orderly_copy_files") ## Validate options here so we can refer to the computed value of ## allow_remote later in error messages. options <- build_search_options(location = location, diff --git a/R/outpack_tools.R b/R/outpack_tools.R index 9f1a3634..4f56c17d 100644 --- a/R/outpack_tools.R +++ b/R/outpack_tools.R @@ -195,8 +195,10 @@ ##' @export orderly_metadata_extract <- function(expr = NULL, name = NULL, location = NULL, allow_remote = NULL, pull_metadata = FALSE, - extract = NULL, root = NULL) { + extract = NULL, options = NULL, + root = NULL) { root <- root_open(root, require_orderly = FALSE) + compatibility_fix_options(options, "orderly_metadata_extract") if (expr_is_literal_id(expr, name)) { ids <- expr diff --git a/R/query_search.R b/R/query_search.R index 7334548b..b32a7c41 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -19,6 +19,10 @@ ##' @inheritParams orderly_query ##' @inheritParams orderly_search_options ##' +##' @param options **DEPRECATED**. Please don't use this any more, and +##' instead use the arguments `location`, `allow_remote` and +##' `pull_metadata` directly. +##' ##' @return A character vector of matching ids. In the case of no ##' match from a query returning a single value (e.g., `latest(...)` ##' or `single(...)`) this will be a character missing value @@ -28,8 +32,10 @@ orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL, parameters = NULL, envir = parent.frame(), location = NULL, allow_remote = NULL, - pull_metadata = FALSE, root = NULL) { + pull_metadata = FALSE, options = NULL, + root = NULL) { root <- root_open(root, require_orderly = FALSE) + compatibility_fix_options(options, "orderly_search") query <- as_orderly_query(expr, name, scope, subquery) options <- build_search_options(location = location, allow_remote = allow_remote, @@ -85,6 +91,24 @@ orderly_search_options <- function(location = NULL, } +compatibility_fix_options <- function(options, name, + arg = deparse(substitute(options)), + env = parent.frame()) { + if (!is.null(options)) { + cli::cli_warn( + c("Use of '{arg}' in '{name}()' is deprecated and will be removed soon", + i = paste("Please pass the arguments to options ('location',", + "'allow_remote' and 'pull_metadata') directly to '{name}'"), + "!" = paste("If you have {.strong also} passed these options in", + "to your function I am about to silently overwrite them")), + .frequency = "frequently", + .frequency_id = paste0("use_options:", name), + call = env) + list2env(options, env) + } +} + + build_search_options <- function(location = NULL, allow_remote = NULL, pull_metadata = FALSE, call = parent.frame()) { if (!is.null(location)) { diff --git a/R/run.R b/R/run.R index 037462b3..b7af6c81 100644 --- a/R/run.R +++ b/R/run.R @@ -24,12 +24,10 @@ ##' network connection (but *not* pulling in the packets could mean ##' that your packet fails to run). ##' -##' To allow for control over this you can pass in an argument -##' `search_options`, which is a [orderly2::orderly_search_options] -##' object, and allows control over the names of the locations to -##' use, whether metadata should be refreshed before we pull -##' anything and if packets that are not currently downloaded should -##' be considered candidates. +##' To allow for control over this you can pass in an arguments to +##' control the names of the locations to use, whether metadata +##' should be refreshed before we pull anything and if packets that +##' are not currently downloaded should be considered candidates. ##' ##' This has no effect when running interactively, in which case you ##' can specify the search options (root specific) with @@ -37,19 +35,18 @@ ##' ##' @section Which packets might be selected from locations?: ##' -##' The `search_options` argument controls where outpack searches for -##' packets with the given query and if anything might be moved over -##' the network (or from one outpack archive to another). By default -##' everything is resolved locally only; that is we can only depend -##' on packets that are unpacked within our current archive. If you -##' pass a `search_options` argument that contains `allow_remote = -##' TRUE` (see [orderly2::orderly_search_options] then packets -##' that are known anywhere are candidates for using as dependencies -##' and *if needed* we will pull the resolved files from a remote -##' location. Note that even if the packet is not locally present -##' this might not be needed - if you have the same content anywhere -##' else in an unpacked packet we will reuse the same content -##' without re-fetching. +##' The arguments `location`, `allow_remote` and `pull_metadata` +##' control where outpack searches for packets with the given query +##' and if anything might be moved over the network (or from one +##' outpack archive to another). By default everything is resolved +##' locally only; that is we can only depend on packets that are +##' unpacked within our current archive. If you pass `allow_remote +##' = TRUE`, then packets that are known anywhere are candidates for +##' using as dependencies and *if needed* we will pull the resolved +##' files from a remote location. Note that even if the packet is +##' not locally present this might not be needed - if you have the +##' same content anywhere else in an unpacked packet we will reuse +##' the same content without re-fetching. ##' ##' If `pull_metadata = TRUE`, then we will refresh location metadata ##' before pulling, and the `location` argument controls which @@ -128,6 +125,10 @@ ##' ##' @inheritParams orderly_search ##' +##' @param search_options **DEPRECATED**. Please don't use this any +##' more, and instead use the arguments `location`, `allow_remote` +##' and `pull_metadata` directly. +##' ##' @param root The path to the root directory, or `NULL` (the ##' default) to search for one from the current working ##' directory. This function **does** require that the directory is @@ -151,10 +152,12 @@ ##' orderly2::orderly_metadata_extract(name = "data", root = path) orderly_run <- function(name, parameters = NULL, envir = NULL, echo = TRUE, location = NULL, allow_remote = NULL, - pull_metadata = FALSE, root = NULL) { + pull_metadata = FALSE, search_options = NULL, + root = NULL) { env_root_src <- Sys.getenv("ORDERLY_SRC_ROOT", NA_character_) root <- root_open(root, require_orderly = is.na(env_root_src), call = environment()) + compatibility_fix_options(search_options, "orderly_run") if (is.na(env_root_src)) { root_src <- root$path diff --git a/man/orderly_copy_files.Rd b/man/orderly_copy_files.Rd index 6a366f97..bcc8808b 100644 --- a/man/orderly_copy_files.Rd +++ b/man/orderly_copy_files.Rd @@ -14,6 +14,7 @@ orderly_copy_files( allow_remote = NULL, pull_metadata = FALSE, parameters = NULL, + options = NULL, envir = parent.frame(), root = NULL ) @@ -83,6 +84,10 @@ needlessly slow.} \item{parameters}{Optionally, a named list of parameters to substitute into the query (using the \verb{this:} prefix)} +\item{options}{\strong{DEPRECATED}. Please don't use this any more, and +instead use the arguments \code{location}, \code{allow_remote} and +\code{pull_metadata} directly.} + \item{envir}{Optionally, an environment to substitute into the query (using the \verb{environment:} prefix). The default here is to use the calling environment, but you can explicitly pass this in diff --git a/man/orderly_location_pull_packet.Rd b/man/orderly_location_pull_packet.Rd index 2033d17d..cc2a1f18 100644 --- a/man/orderly_location_pull_packet.Rd +++ b/man/orderly_location_pull_packet.Rd @@ -10,6 +10,7 @@ orderly_location_pull_packet( location = NULL, pull_metadata = FALSE, recursive = NULL, + options = NULL, root = NULL ) } @@ -38,6 +39,10 @@ specified in \code{id}. This might copy a lot of data! If \code{NULL}, we default to the value given by the the configuration option \code{require_complete_tree}.} +\item{options}{\strong{DEPRECATED}. Please don't use this any more, and +instead use the arguments \code{location}, \code{allow_remote} and +\code{pull_metadata} directly.} + \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function does not require that the directory is diff --git a/man/orderly_metadata_extract.Rd b/man/orderly_metadata_extract.Rd index 1c807698..421dfa52 100644 --- a/man/orderly_metadata_extract.Rd +++ b/man/orderly_metadata_extract.Rd @@ -11,6 +11,7 @@ orderly_metadata_extract( allow_remote = NULL, pull_metadata = FALSE, extract = NULL, + options = NULL, root = NULL ) } @@ -45,6 +46,10 @@ needlessly slow.} \item{extract}{A character vector of columns to extract, possibly named. See Details for the format.} +\item{options}{\strong{DEPRECATED}. Please don't use this any more, and +instead use the arguments \code{location}, \code{allow_remote} and +\code{pull_metadata} directly.} + \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function does not require that the directory is diff --git a/man/orderly_query_explain.Rd b/man/orderly_query_explain.Rd index f89902cf..83bce048 100644 --- a/man/orderly_query_explain.Rd +++ b/man/orderly_query_explain.Rd @@ -13,7 +13,6 @@ orderly_query_explain( envir = parent.frame(), location = NULL, allow_remote = NULL, - pull_metadata = FALSE, root = NULL ) } @@ -50,15 +49,6 @@ you might pull a large quantity of data. The default \code{NULL} is vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} -\item{pull_metadata}{Logical, indicating if we should pull -metadata immediately before the search. If \code{location} is given, -then we will pass this through to -\link{orderly_location_pull_metadata} to filter locations -to update. If pulling many packets in sequence, you \emph{will} want -to update this option to \code{FALSE} after the first pull, otherwise -it will update the metadata between every packet, which will be -needlessly slow.} - \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function does not require that the directory is diff --git a/man/orderly_run.Rd b/man/orderly_run.Rd index ad381a45..65e7e042 100644 --- a/man/orderly_run.Rd +++ b/man/orderly_run.Rd @@ -12,6 +12,7 @@ orderly_run( location = NULL, allow_remote = NULL, pull_metadata = FALSE, + search_options = NULL, root = NULL ) } @@ -52,6 +53,10 @@ to update this option to \code{FALSE} after the first pull, otherwise it will update the metadata between every packet, which will be needlessly slow.} +\item{search_options}{\strong{DEPRECATED}. Please don't use this any +more, and instead use the arguments \code{location}, \code{allow_remote} +and \code{pull_metadata} directly.} + \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function \strong{does} require that the directory is @@ -89,12 +94,10 @@ seconds to hours depending on their size and the speed of your network connection (but \emph{not} pulling in the packets could mean that your packet fails to run). -To allow for control over this you can pass in an argument -\code{search_options}, which is a \link{orderly_search_options} -object, and allows control over the names of the locations to -use, whether metadata should be refreshed before we pull -anything and if packets that are not currently downloaded should -be considered candidates. +To allow for control over this you can pass in an arguments to +control the names of the locations to use, whether metadata +should be refreshed before we pull anything and if packets that +are not currently downloaded should be considered candidates. This has no effect when running interactively, in which case you can specify the search options (root specific) with @@ -104,18 +107,17 @@ can specify the search options (root specific) with \section{Which packets might be selected from locations?}{ -The \code{search_options} argument controls where outpack searches for -packets with the given query and if anything might be moved over -the network (or from one outpack archive to another). By default -everything is resolved locally only; that is we can only depend -on packets that are unpacked within our current archive. If you -pass a \code{search_options} argument that contains \code{allow_remote = TRUE} (see \link{orderly_search_options} then packets -that are known anywhere are candidates for using as dependencies -and \emph{if needed} we will pull the resolved files from a remote -location. Note that even if the packet is not locally present -this might not be needed - if you have the same content anywhere -else in an unpacked packet we will reuse the same content -without re-fetching. +The arguments \code{location}, \code{allow_remote} and \code{pull_metadata} +control where outpack searches for packets with the given query +and if anything might be moved over the network (or from one +outpack archive to another). By default everything is resolved +locally only; that is we can only depend on packets that are +unpacked within our current archive. If you pass \code{allow_remote = TRUE}, then packets that are known anywhere are candidates for +using as dependencies and \emph{if needed} we will pull the resolved +files from a remote location. Note that even if the packet is +not locally present this might not be needed - if you have the +same content anywhere else in an unpacked packet we will reuse +the same content without re-fetching. If \code{pull_metadata = TRUE}, then we will refresh location metadata before pulling, and the \code{location} argument controls which diff --git a/man/orderly_search.Rd b/man/orderly_search.Rd index 60948ba3..8dc90f24 100644 --- a/man/orderly_search.Rd +++ b/man/orderly_search.Rd @@ -14,6 +14,7 @@ orderly_search( location = NULL, allow_remote = NULL, pull_metadata = FALSE, + options = NULL, root = NULL ) } @@ -59,6 +60,10 @@ to update this option to \code{FALSE} after the first pull, otherwise it will update the metadata between every packet, which will be needlessly slow.} +\item{options}{\strong{DEPRECATED}. Please don't use this any more, and +instead use the arguments \code{location}, \code{allow_remote} and +\code{pull_metadata} directly.} + \item{root}{The path to the root directory, or \code{NULL} (the default) to search for one from the current working directory. This function does not require that the directory is From ec2356bcae7b5f684754da45aea92dfa3118b061 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 10:34:39 +0100 Subject: [PATCH 13/17] Add tests of warnings --- R/query.R | 6 ----- R/query_search.R | 5 ++-- tests/testthat/test-query-search.R | 38 ++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/R/query.R b/R/query.R index 811ab934..7cba8dee 100644 --- a/R/query.R +++ b/R/query.R @@ -49,12 +49,6 @@ orderly_query <- function(expr, name = NULL, scope = NULL, subquery = NULL) { } -dots_is_literal_id <- function(...) { - ...length() == 1 && is.null(...names()) && is.character(..1) && - all(grepl(re_id, ..1)) -} - - expr_is_literal_id <- function(expr, ...) { all(vlapply(list(...), is.null)) && is.character(expr) && diff --git a/R/query_search.R b/R/query_search.R index b32a7c41..3312810b 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -88,6 +88,7 @@ orderly_search_options <- function(location = NULL, "that previously accepted 'options'")), .frequency = "regularly", .frequency_id = "orderly_search_options") + build_search_options(location, allow_remote, pull_metadata) } @@ -101,8 +102,8 @@ compatibility_fix_options <- function(options, name, "'allow_remote' and 'pull_metadata') directly to '{name}'"), "!" = paste("If you have {.strong also} passed these options in", "to your function I am about to silently overwrite them")), - .frequency = "frequently", - .frequency_id = paste0("use_options:", name), + .frequency = "regularly", + .frequency_id = paste0("orderly_use_options:", name), call = env) list2env(options, env) } diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index f7969476..3f20858e 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -1003,3 +1003,41 @@ test_that("&& takes precedence over ||", { root = root), c(x1, y1)) }) + + +test_that("Warn on use of search options", { + rlang::reset_warning_verbosity("orderly_search_options") + expect_warning( + orderly_search_options(), + "Use of 'orderly_search_options' is deprecated") + expect_no_warning( + orderly_search_options()) +}) + + +test_that("Warn, but honour, on use of search options to search", { + rlang::reset_warning_verbosity("orderly_use_options:orderly_search") + root <- list() + root$a <- create_temporary_root(use_file_store = TRUE) + root$b <- create_temporary_root(use_file_store = TRUE) + orderly_location_add_path("b", path = root$b$path, root = root$a) + + ids <- vcapply(1:3, function(i) { + create_random_packet(root$b, "data", list(p = i)) + }) + orderly_location_pull_metadata(root = root$a) + + expect_equal(orderly_search(NULL, root = root$a), + character()) + options <- suppressWarnings(orderly_search_options(location = "b")) + + expect_warning( + res <- orderly_search(NULL, root = root$a, options = options), + "Use of 'options' in 'orderly_search()' is deprecated", + fixed = TRUE) + expect_equal(res, ids) + + expect_no_warning( + res2 <- orderly_search(NULL, root = root$a, options = options)) + expect_equal(res2, res) +}) From 1c9f3f238e6b8e9c885a85a0ae65748a6bc42a5c Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 10:37:55 +0100 Subject: [PATCH 14/17] Tidy up --- R/location.R | 7 +++---- R/util.R | 5 ----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/R/location.R b/R/location.R index ad1b0bd6..c5544fc6 100644 --- a/R/location.R +++ b/R/location.R @@ -357,11 +357,10 @@ orderly_location_pull_packet <- function(expr, } if (length(ids) == 0 || (length(ids) == 1 && is.na(ids))) { - if (pull_metadata) { - pull_arg <- cli_nbsp("pull_metadata = TRUE") + if (!pull_metadata) { hint <- c(i = paste("Did you forget to pull metadata? You can do this", - "by using the argument '{pull_arg}' in the call", - "to 'orderly_location_pull_packet()', or", + "by using the argument {.code pull_metadata = TRUE}", + "in the call to 'orderly_location_pull_packet()', or", "by running 'orderly_location_pull_metadata()'")) } else { hint <- NULL diff --git a/R/util.R b/R/util.R index 486bec8f..85753bdd 100644 --- a/R/util.R +++ b/R/util.R @@ -714,8 +714,3 @@ fill_missing_names <- function(x) { } x } - - -cli_nbsp <- function(x) { - gsub(" ", "\u00a0", x, fixed = TRUE) -} From ef8358110e5a32e9fc40aeafc75f4875e0fe2746 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 21 Oct 2024 11:04:29 +0100 Subject: [PATCH 15/17] Fix lint --- R/outpack_helpers.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 08133ad5..f280fef2 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -154,7 +154,8 @@ orderly_copy_files <- function(expr, files, dest, overwrite = TRUE, } else if (!options$allow_remote) { cli::cli_abort( c("Unable to copy files, as they are not available locally", - i = "To fetch from a location, try again with 'allow_remote = TRUE'"), + i = paste("To fetch from a location, try again with", + "{.code allow_remote = TRUE}")), parent = e) } copy_files_from_remote(id, plan$there, plan$here, dest, overwrite, root, From 6bb231b3dd951506c6a22904bf118456b246015f Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Tue, 22 Oct 2024 09:27:37 +0100 Subject: [PATCH 16/17] Redocument --- man/orderly_copy_files.Rd | 2 +- man/orderly_interactive_set_search_options.Rd | 2 +- man/orderly_metadata_extract.Rd | 2 +- man/orderly_query_explain.Rd | 2 +- man/orderly_run.Rd | 2 +- man/orderly_search.Rd | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/man/orderly_copy_files.Rd b/man/orderly_copy_files.Rd index bcc8808b..bc8e9abe 100644 --- a/man/orderly_copy_files.Rd +++ b/man/orderly_copy_files.Rd @@ -67,7 +67,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} diff --git a/man/orderly_interactive_set_search_options.Rd b/man/orderly_interactive_set_search_options.Rd index 3b9414d0..02290ba6 100644 --- a/man/orderly_interactive_set_search_options.Rd +++ b/man/orderly_interactive_set_search_options.Rd @@ -18,7 +18,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} diff --git a/man/orderly_metadata_extract.Rd b/man/orderly_metadata_extract.Rd index 421dfa52..0d7dbed1 100644 --- a/man/orderly_metadata_extract.Rd +++ b/man/orderly_metadata_extract.Rd @@ -29,7 +29,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} diff --git a/man/orderly_query_explain.Rd b/man/orderly_query_explain.Rd index 83bce048..a1fb9a7b 100644 --- a/man/orderly_query_explain.Rd +++ b/man/orderly_query_explain.Rd @@ -44,7 +44,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} diff --git a/man/orderly_run.Rd b/man/orderly_run.Rd index 65e7e042..5173ee82 100644 --- a/man/orderly_run.Rd +++ b/man/orderly_run.Rd @@ -39,7 +39,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} diff --git a/man/orderly_search.Rd b/man/orderly_search.Rd index 8dc90f24..0413e553 100644 --- a/man/orderly_search.Rd +++ b/man/orderly_search.Rd @@ -46,7 +46,7 @@ might in future expand this to allow wildcards or exceptions.} to be found that are not currently unpacked (i.e., are known only to a location that we have metadata from). If this is \code{TRUE}, then in conjunction with \link{orderly_dependency} -you might pull a large quantity of data. The default \code{NULL} is +you might pull a large quantity of data. The default is \code{NULL}. This is \code{TRUE} if remote locations are listed explicitly as a character vector in the \code{location} argument, or if you have specified \code{pull_metadata = TRUE}, otherwise \code{FALSE}.} From 6712e1b3131868c5cf50ae8c997cd663d6e87001 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Tue, 22 Oct 2024 13:41:24 +0100 Subject: [PATCH 17/17] Bump version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 92a2fa76..44405753 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: orderly2 Title: Orderly Next Generation -Version: 1.99.45 +Version: 1.99.46 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"), email = "rich.fitzjohn@gmail.com"), person("Robert", "Ashton", role = "aut"),