Allow orderly_shared_resource to be used with unnamed arguments.

The previous behaviour was inconsistent with orderly_resource and often
redundant, as users commonly want to copy the resource using the same
name as the original file.

It is now possible to use `orderly_shared_resource("foo.txt")`, in which
case the source and destination name are assumed to be the same.

The old syntax is still available in case the user wants to change the
name while copying, and it is even possible to mix-and-match, renaming
some files but not others.

R/metadata.R

@@ -325,9 +325,9 @@ static_orderly_dependency <- function(args) {
 ##'
 ##' @title Copy shared resources into a packet directory
 ##'
-##' @param ... Named arguments corresponding to shared resources to
-##'   copy. The name will be the destination filename, while the value
-##'   is the filename within the shared resource directory.
+##' @param ... The shared resources to copy. If arguments are named, the name
+##'   will be the destination file while the value is the filename within the
+##'   shared resource directory.
 ##'
 ##' @return Invisibly, a data.frame with columns `here` (the fileames
 ##'   as as copied into the running packet) and `there` (the filenames
@@ -351,21 +351,23 @@ orderly_shared_resource <- function(...) {
   invisible(files)
 }
 
-
 validate_shared_resource <- function(args, call) {
   if (length(args) == 0) {
     cli::cli_abort("'orderly_shared_resource' requires at least one argument",
                    call = call)
   }
-  assert_named(args, unique = TRUE, call = environment())
+
   is_invalid <- !vlapply(args, function(x) is.character(x) && length(x) == 1)
   if (any(is_invalid)) {
     cli::cli_abort(
-      sprintf(
-        "Invalid shared resource %s: entries must be strings",
-        paste(squote(names(args)[is_invalid]), collapse = ", ")),
+      "Arguments to 'orderly_shared_resource' must be strings",
       call = call)
   }
+
+  args <- fill_missing_names(args)
+  assert_unique_names(args, call = call,
+                      name = "Arguments to 'orderly_shared_resource'")
+
   list_to_character(args)
 }
 
@@ -405,7 +407,8 @@ copy_shared_resource <- function(path_root, path_dest, config, files, call) {
 
 
 static_orderly_shared_resource <- function(args) {
-  unlist(lapply(args, static_character_vector, TRUE), FALSE, TRUE)
+  list_to_character(
+    fill_missing_names(lapply(args, static_character_vector, TRUE)))
 }
 
 

R/util.R

@@ -644,3 +644,16 @@ is_testing <- function() {
   # https://github.com/r-lib/testthat/blob/fe50a22/R/test-env.R#L20
   identical(Sys.getenv("TESTTHAT"), "true")
 }
+
+#' Given a character vector or list, missing names are filled using the value.
+fill_missing_names <- function(x) {
+  if (!is.null(x)) {
+    if (is.null(names(x))) {
+      names(x) <- list_to_character(x)
+    } else {
+      missing <- is.na(names(x)) | names(x) == ""
+      names(x)[missing] <- x[missing]
+    }
+  }
+  x
+}

R/util_assert.R

@@ -44,18 +44,25 @@ assert_simple_scalar_atomic <- function(x, name = deparse(substitute(x)),
   invisible(x)
 }
 
+assert_unique_names <- function(x, name = deparse(substitute(x)),
+                                arg = name, call = NULL) {
+  if (any(duplicated(names(x)))) {
+      dups <- unique(names(x)[duplicated(names(x))])
+      cli::cli_abort(
+        c("'{name}' must have unique names",
+          i = "Found {length(dups)} duplicate{?s}: {collapseq(dups)}"),
+        call = call, arg = arg)
+  }
+}
+
 assert_named <- function(x, unique = FALSE, name = deparse(substitute(x)),
                          arg = name, call = NULL) {
   ## TODO: we get bad quotes here from static_orderly_parameters
   if (is.null(names(x))) {
     cli::cli_abort("'{name}' must be named", call = call, arg = arg)
   }
-  if (unique && any(duplicated(names(x)))) {
-    dups <- unique(names(x)[duplicated(names(x))])
-    cli::cli_abort(
-      c("'{name}' must have unique names",
-        i = "Found {length(dups)} duplicate{?s}: {collapseq(dups)}"),
-      call = call, arg = arg)
+  if (unique) {
+    assert_unique_names(x, name = name, arg = arg, call = call)
   }
 }
 

tests/testthat/examples/shared-shorthand/shared-shorthand.R

@@ -0,0 +1,7 @@
+orderly2::orderly_shared_resource("data.csv")
+orderly2::orderly_artefact("A graph of things", "mygraph.png")
+
+data <- read.csv("data.csv", stringsAsFactors = FALSE)
+png("mygraph.png")
+plot(data)
+dev.off()

tests/testthat/helper-orderly.R

@@ -36,9 +36,9 @@ copy_examples <- function(examples, path_src) {
   }
 
   fs::dir_create(path_src)
-  if (any(c("shared", "shared-dir") %in% examples)) {
+  if (any(c("shared", "shared-shorthand", "shared-dir") %in% examples)) {
     fs::dir_create(file.path(path_src, "shared"))
-    if ("shared" %in% examples) {
+    if (any(c("shared", "shared-shorthand") %in% examples)) {
       fs::file_copy(test_path("examples/explicit/data.csv"),
                     file.path(path_src, "shared"))
     }

tests/testthat/test-cleanup.R

@@ -153,6 +153,29 @@ test_that("can clean up shared resources", {
   expect_equal(status$delete, "shared_data.csv")
 })
 
+test_that("can clean up shared resources with shorthand syntax", {
+  path <- test_prepare_orderly_example("shared-shorthand")
+  path_src <- file.path(path, "src", "shared-shorthand")
+  file.create(file.path(path_src, "data.csv"))
+  status <- orderly_cleanup_status("shared-shorthand", root = path)
+
+  files <- c("data.csv", "shared-shorthand.R")
+  expect_setequal(rownames(status$role), files)
+  expect_equal(
+    status$role,
+    cbind(orderly = set_names(c(FALSE, TRUE), files),
+          resource = FALSE,
+          shared_resource = c(TRUE, FALSE),
+          dependency = FALSE,
+          artefact = FALSE))
+  expect_equal(
+    status$status,
+    cbind(source = set_names(c(FALSE, TRUE), files),
+          derived = c(TRUE, FALSE),
+          ignored = NA))
+  expect_equal(status$delete, "data.csv")
+})
+
 
 test_that("can clean up dependencies", {
   path <- test_prepare_orderly_example(c("data", "depends"))

tests/testthat/test-run.R

@@ -261,19 +261,64 @@ test_that("can run manually with shared resources", {
 })
 
 
+test_that("can run with shared resources using shorthand arguments", {
+  path <- test_prepare_orderly_example("shared-shorthand")
+  envir <- new.env()
+  id <- orderly_run_quietly("shared-shorthand", root = path, envir = envir)
+  expect_setequal(
+    dir(file.path(path, "archive", "shared-shorthand", id)),
+    c("data.csv", "mygraph.png", "shared-shorthand.R"))
+  meta <- orderly_metadata(id, root = path)
+  expect_equal(nrow(meta$custom$orderly$shared), 1)
+  expect_equal(meta$custom$orderly$shared,
+               data_frame(here = "data.csv", there = "data.csv"))
+  expect_equal(
+    meta$custom$orderly$role,
+    data_frame(path = c("shared-shorthand.R", "data.csv"),
+               role = c("orderly", "shared")))
+})
+
+
 test_that("can validate shared resource arguments", {
   expect_error(
     validate_shared_resource(list(), NULL),
     "'orderly_shared_resource' requires at least one argument")
+
   expect_error(
     validate_shared_resource(list(input = c("a", "b")), NULL),
-    "Invalid shared resource 'input': entries must be strings")
+    "Arguments to 'orderly_shared_resource' must be strings")
   expect_error(
     validate_shared_resource(list(a = 1, b = TRUE, c = "str"), NULL),
-    "Invalid shared resource 'a', 'b': entries must be strings")
+    "Arguments to 'orderly_shared_resource' must be strings")
+  expect_error(
+    validate_shared_resource(list(1, TRUE, "str"), NULL),
+    "Arguments to 'orderly_shared_resource' must be strings")
+  expect_error(
+    validate_shared_resource(list(a = 1, TRUE, "str"), NULL),
+    "Arguments to 'orderly_shared_resource' must be strings")
+
+  expect_error(
+    validate_shared_resource(list(a = "A", a = "B"), NULL),
+    "'Arguments to 'orderly_shared_resource'' must have unique names")
+  expect_error(
+    validate_shared_resource(list("a", "a"), NULL),
+    "'Arguments to 'orderly_shared_resource'' must have unique names")
+  expect_error(
+    validate_shared_resource(list("a", a = "B"), NULL),
+    "'Arguments to 'orderly_shared_resource'' must have unique names")
+
   expect_equal(
     validate_shared_resource(list(a = "A", b = "B"), NULL),
     c(a = "A", b = "B"))
+  expect_equal(
+    validate_shared_resource(list(a = "A", b = "A"), NULL),
+    c(a = "A", b = "A"))
+  expect_equal(
+    validate_shared_resource(list("a", "b"), NULL),
+    c(a = "a", b = "b"))
+  expect_equal(
+    validate_shared_resource(list("a", b = "B"), NULL),
+    c(a = "a", b = "B"))
 })
 
 
@@ -1243,6 +1288,31 @@ test_that("can read about assigned shared resources", {
   expect_equal(res$shared_resource, c(shared_data = "data"))
 })
 
+test_that("can read about shared resources with shorthand syntax", {
+  path <- test_prepare_orderly_example("shared-shorthand")
+  path_src <- file.path(path, "src", "shared-shorthand")
+  code <- readLines(file.path(path_src, "shared-shorthand.R"))
+  code <- sub("orderly2::orderly_shared_resource",
+              "r <- orderly2::orderly_shared_resource",
+              code)
+  code <- c(code, 'saveRDS(r, "resources.rds")')
+  writeLines(code, file.path(path_src, "shared-shorthand.R"))
+
+  id <- orderly_run_quietly("shared-shorthand", root = path, envir = new.env())
+  r <- readRDS(file.path(path, "archive", "shared-shorthand",
+                         id, "resources.rds"))
+  expect_equal(r, data_frame(here = "data.csv", there = "data.csv"))
+
+  res <- withr::with_dir(
+    path_src,
+    withVisible(orderly_shared_resource("data.csv")))
+  expect_equal(res$visible, FALSE)
+  expect_equal(res$value, r)
+
+  res <- orderly_read(path_src)
+  expect_equal(res$shared_resource, c(data.csv = "data.csv"))
+})
+
 
 test_that("can read about dependencies", {
   path <- test_prepare_orderly_example(c("data", "depends"))

tests/testthat/test-util-assert.R

@@ -26,6 +26,14 @@ test_that("assert_simple_scalar_atomic", {
   expect_error(assert_simple_scalar_atomic(list(1)), "must be atomic")
 })
 
+test_that("assert_unique_names", {
+  expect_error(assert_unique_names(setNames(1:3, c("a", "a", "c"))),
+               "must have unique names")
+  expect_error(assert_unique_names(setNames(1:3, c("a", "c", "a"))),
+               "must have unique names")
+  expect_silent(assert_unique_names(setNames(1:3, c("a", "b", "c"))))
+})
+
 
 test_that("assert_named", {
   expect_error(assert_named(1), "must be named")

tests/testthat/test-util.R

@@ -334,3 +334,18 @@ test_that("read_string strips newlines", {
   result <- expect_silent(read_string(path))
   expect_equal(result, "12345678")
 })
+
+test_that("fill_missing_names works", {
+  expect_equal(fill_missing_names(NULL), NULL)
+
+  expect_equal(fill_missing_names(c("a", "b")), c(a = "a", b = "b"))
+  expect_equal(fill_missing_names(c("a", "a")), c(a = "a", a = "a"))
+  expect_equal(fill_missing_names(c(x = "a", "a")), c(x = "a", a = "a"))
+
+  expect_equal(fill_missing_names(list("a", "a")),
+               list(a = "a", a = "a"))
+  expect_equal(fill_missing_names(list(x = "a", "a")),
+               list(x = "a", a = "a"))
+  expect_equal(fill_missing_names(list(x = "a", y = "a")),
+               list(x = "a", y = "a"))
+})

vignettes/introduction.Rmd

@@ -311,7 +311,7 @@ We can then write an orderly report `use_shared` that uses this shared file, wit
 ```{r, echo = FALSE, results = "asis"}
 fs::dir_create(file.path(path, "src", "use_shared"))
 writeLines(c(
-  'orderly2::orderly_shared_resource(data.csv = "data.csv")',
+  'orderly2::orderly_shared_resource("data.csv")',
   'orderly2::orderly_artefact("analysis", "analysis.png")',
   "",
   'd <- read.csv("data.csv")',