diff --git a/DESCRIPTION b/DESCRIPTION index 1cdd8d07..e62ff1d3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: orderly2 Title: Orderly Next Generation -Version: 1.99.43 +Version: 1.99.44 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"), email = "rich.fitzjohn@gmail.com"), person("Robert", "Ashton", role = "aut"), diff --git a/R/outpack_helpers.R b/R/outpack_helpers.R index 99f3cf65..31c14447 100644 --- a/R/outpack_helpers.R +++ b/R/outpack_helpers.R @@ -72,6 +72,8 @@ ##' 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_metadata ##' diff --git a/R/outpack_packet.R b/R/outpack_packet.R index 986644f0..a65ecfee 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -161,7 +161,7 @@ outpack_packet_use_dependency <- function(packet, query, files, search_options = NULL, overwrite = TRUE) { packet <- check_current_packet(packet) - query <- as_orderly_query(query) + query <- as_orderly_query(query, arg = "query") search_options <- as_orderly_search_options(search_options) if (!query$info$single) { diff --git a/R/query.R b/R/query.R index dc214333..1be14753 100644 --- a/R/query.R +++ b/R/query.R @@ -55,17 +55,21 @@ dots_is_literal_id <- function(...) { } -as_orderly_query <- function(expr, ...) { +as_orderly_query <- function(expr, name = NULL, scope = NULL, subquery = NULL, + arg = "expr", call = parent.frame()) { if (missing(expr)) { expr <- NULL } if (inherits(expr, "orderly_query")) { - if (...length() > 0) { - stop("If 'expr' is an 'orderly_query', no additional arguments allowed") + err <- !is.null(name) || !is.null(scope) || !is.null(subquery) + if (err) { + cli::cli_abort( + "If '{arg}' is an 'orderly_query', no additional arguments allowed", + arg = arg, call = call) } expr } else { - orderly_query(expr, ...) + orderly_query(expr, name, scope, subquery) } } diff --git a/R/query_explain.R b/R/query_explain.R index 38bf3967..9dff38ba 100644 --- a/R/query_explain.R +++ b/R/query_explain.R @@ -13,11 +13,12 @@ ##' result. ##' ##' @export -orderly_query_explain <- function(..., parameters = NULL, +orderly_query_explain <- function(expr, name = NULL, scope = NULL, + subquery = NULL, parameters = NULL, envir = parent.frame(), options = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - query <- as_orderly_query(...) + query <- as_orderly_query(expr, name, scope, subquery) options <- as_orderly_search_options(options) found <- orderly_search(query, parameters = parameters, envir = envir, options = options, root = root) diff --git a/R/query_search.R b/R/query_search.R index d9f1d03e..809c5c79 100644 --- a/R/query_search.R +++ b/R/query_search.R @@ -7,9 +7,6 @@ ##' ##' @title Query outpack's database ##' -##' @param ... Arguments passed through to [orderly2::orderly_query], -##' perhaps just a query expression -##' ##' @param parameters Optionally, a named list of parameters to substitute ##' into the query (using the `this:` prefix) ##' @@ -24,6 +21,7 @@ ##' options are used (i.e., `orderly2::orderly_search_options()`) ##' ##' @inheritParams orderly_metadata +##' @inheritParams orderly_query ##' ##' @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 +29,11 @@ ##' (`NA_character_`) ##' ##' @export -orderly_search <- function(..., parameters = NULL, envir = parent.frame(), +orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL, + parameters = NULL, envir = parent.frame(), options = NULL, root = NULL) { root <- root_open(root, require_orderly = FALSE) - query <- as_orderly_query(...) + query <- as_orderly_query(expr, name, scope, subquery) options <- as_orderly_search_options(options) validate_parameters(parameters, environment()) orderly_query_eval(query, parameters, envir, options, root, @@ -53,34 +52,45 @@ orderly_search <- function(..., parameters = NULL, envir = parent.frame(), ##' @param location Optional vector of locations to pull from. We ##' might in future expand this to allow wildcards or exceptions. ##' -##' @param 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 -##' `TRUE`, then in conjunction with -##' [orderly2::orderly_dependency] you might pull a large -##' quantity of data. +##' @param 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 +##' `TRUE`, then in conjunction with [orderly2::orderly_dependency] +##' you might pull a large quantity of data. The default is `NULL`. This is +##' `TRUE` if remote locations are listed explicitly as a character +##' vector in the `location` argument, or if you have specified +##' `pull_metadata = TRUE`, otherwise `FALSE`. ##' ##' @param pull_metadata Logical, indicating if we should pull -##' metadata immediately before the search. If `location` is -##' given, then we will pass this through to -##' [orderly2::orderly_location_pull_metadata] to filter locations to -##' update. If pulling many packets in sequence, you *will* want to -##' update this option to `FALSE` after the first pull. +##' metadata immediately before the search. If `location` is given, +##' then we will pass this through to +##' [orderly2::orderly_location_pull_metadata] to filter locations +##' to update. If pulling many packets in sequence, you *will* want +##' to update this option to `FALSE` after the first pull, otherwise +##' it will update the metadata between every packet, which will be +##' needlessly slow. ##' ##' @return An object of class `orderly_search_options` which should ##' not be modified after creation (but see note about `pull_metadata`) ##' ##' @export orderly_search_options <- function(location = NULL, - allow_remote = FALSE, + 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. if (!is.null(location)) { - assert_character(location, call = environment()) + assert_character(location) + } + has_remote_location <- !is.null(location) && + length(setdiff(location, c("local", "orphan")) > 0) + + assert_scalar_logical(pull_metadata) + if (is.null(allow_remote)) { + allow_remote <- has_remote_location || pull_metadata + } else { + assert_scalar_logical(allow_remote) } - assert_scalar_logical(allow_remote, call = environment()) - assert_scalar_logical(pull_metadata, call = environment()) ret <- list(location = location, allow_remote = allow_remote, pull_metadata = pull_metadata) diff --git a/man/orderly_copy_files.Rd b/man/orderly_copy_files.Rd index 49254b5a..beb7279c 100644 --- a/man/orderly_copy_files.Rd +++ b/man/orderly_copy_files.Rd @@ -15,8 +15,7 @@ orderly_copy_files( ) } \arguments{ -\item{...}{Arguments passed through to \link{orderly_query}, -perhaps just a query expression} +\item{...}{Additional arguments passed through to \link{orderly_search}} \item{files}{Files to copy from the other packet. This can be (1) a character vector, in which case files are copied over without diff --git a/man/orderly_query_explain.Rd b/man/orderly_query_explain.Rd index 20fd0a12..f75161fc 100644 --- a/man/orderly_query_explain.Rd +++ b/man/orderly_query_explain.Rd @@ -5,7 +5,10 @@ \title{Explain a query} \usage{ orderly_query_explain( - ..., + expr, + name = NULL, + scope = NULL, + subquery = NULL, parameters = NULL, envir = parent.frame(), options = NULL, @@ -13,8 +16,17 @@ orderly_query_explain( ) } \arguments{ -\item{...}{Arguments passed through to \link{orderly_query}, -perhaps just a query expression} +\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{scope}{Optionally, a scope query to limit the packets +searched by \code{pars}} + +\item{subquery}{Optionally, named list of subqueries which can be +referenced by name from the \code{expr}.} \item{parameters}{Optionally, a named list of parameters to substitute into the query (using the \verb{this:} prefix)} diff --git a/man/orderly_search.Rd b/man/orderly_search.Rd index c6b99f92..dbb81e74 100644 --- a/man/orderly_search.Rd +++ b/man/orderly_search.Rd @@ -5,7 +5,10 @@ \title{Query outpack's database} \usage{ orderly_search( - ..., + expr, + name = NULL, + scope = NULL, + subquery = NULL, parameters = NULL, envir = parent.frame(), options = NULL, @@ -13,8 +16,17 @@ orderly_search( ) } \arguments{ -\item{...}{Arguments passed through to \link{orderly_query}, -perhaps just a query expression} +\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{scope}{Optionally, a scope query to limit the packets +searched by \code{pars}} + +\item{subquery}{Optionally, named list of subqueries which can be +referenced by name from the \code{expr}.} \item{parameters}{Optionally, a named list of parameters to substitute into the query (using the \verb{this:} prefix)} diff --git a/man/orderly_search_options.Rd b/man/orderly_search_options.Rd index ea84baf5..567d0083 100644 --- a/man/orderly_search_options.Rd +++ b/man/orderly_search_options.Rd @@ -6,7 +6,7 @@ \usage{ orderly_search_options( location = NULL, - allow_remote = FALSE, + allow_remote = NULL, pull_metadata = FALSE ) } @@ -14,19 +14,23 @@ 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.} +\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 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}.} \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.} +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{ An object of class \code{orderly_search_options} which should diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R index 73a70325..bcd2e442 100644 --- a/tests/testthat/test-query-search.R +++ b/tests/testthat/test-query-search.R @@ -8,7 +8,6 @@ test_that("can construct search options", { pull_metadata = FALSE)) opts <- orderly_search_options(location = c("x", "y"), - allow_remote = TRUE, pull_metadata = TRUE) expect_s3_class(opts, "orderly_search_options") expect_mapequal( @@ -19,6 +18,26 @@ 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)) +}) + + +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( + orderly_search_options(location = c("local", "orphan"))$allow_remote) + expect_true( + orderly_search_options(location = "server")$allow_remote) + expect_true( + orderly_search_options(location = c("local", "server"))$allow_remote) +}) + + test_that("can convert into search options", { opts <- orderly_search_options(location = "x", allow_remote = FALSE, @@ -26,7 +45,8 @@ test_that("can convert into search options", { 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"))) + 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)),