Ergonomic improvements for querying

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 = "[email protected]"),
              person("Robert", "Ashton", role = "aut"),

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
 ##'

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) {

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)
   }
 }
 

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)

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,17 +21,19 @@
 ##'   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(...)`
 ##'   or `single(...)`) this will be a character missing value
 ##'   (`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 `NULL` 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)

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,14 +18,35 @@ 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,
                                  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")))
+               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)),