Merge pull request #269 from vimc/vimc-4579

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: orderly
 Title: Lightweight Reproducible Reporting
-Version: 1.2.22
+Version: 1.2.23
 Description: Order, create and store reports from R.  By defining a
     lightweight interface around the inputs and outputs of an
     analysis, a lot of the repetitive work for reproducible research

NEWS.md

@@ -1,3 +1,7 @@
+# orderly 1.2.23
+
+* `orderly_run` with non-boolean `use_draft` and fixed ID dependency works (#259)
+
 # orderly 1.2.20
 
 * `orderly_pull_archive` is now more tolerant of trailing slashes (#260)

R/query.R

@@ -106,7 +106,11 @@ orderly_list_archive <- function(root = NULL, locate = TRUE) {
 ##' @param name Name of the report to find; if \code{NULL} returns the
 ##'   most recent report across all names
 ##'
-##' @param draft Find most recent \emph{draft} report
+##' @param draft Should draft reports be used searched? Valid values
+##'   are logical (\code{TRUE}, \code{FALSE}) or use the string
+##'   \code{newer} to use draft reports where they are newer than
+##'   archive reports. For consistency, \code{always} and \code{never}
+##'   are equivalent to \code{TRUE} and \code{FALSE}, respectively.
 ##'
 ##' @param must_work Throw an error if no report is found.  If FALSE,
 ##'   returns \code{NA_character_}.
@@ -139,22 +143,25 @@ orderly_latest <- function(name = NULL, root = NULL, locate = TRUE,
                            draft = FALSE, must_work = TRUE) {
   config <- orderly_config(root, locate)
 
-  if (is.null(name)) {
-    d <- orderly_list2(draft, config, FALSE)
-    ids <- d$id
-    path <-
-      file.path((if (draft) path_draft else path_archive)(config$root), d$name)
-  } else {
-    path <-
-      file.path((if (draft) path_draft else path_archive)(config$root), name)
-    ids <- orderly_list_dir(path, check_run_rds = draft)
+  draft <- query_check_draft(draft)
+  path_funcs <- switch(draft,
+                       always = list(list_draft),
+                       never = list(list_archive),
+                       newer = list(list_draft, list_archive))
+  what <- switch(draft,
+                 always = "draft",
+                 never = "archive",
+                 newer = "draft or archive")
+
+  ids <- character(0)
+  for (func in path_funcs) {
+    ids <- c(ids, func(name, config))
   }
 
   if (length(ids) == 0L) {
     if (must_work) {
-      type <- if (draft) "draft" else "archive"
       name <- name %||% "any report"
-      stop(sprintf("Did not find any %s reports for %s", type, name))
+      stop(sprintf("Did not find any %s reports for %s", what, name))
     } else {
       return(NA_character_)
     }
@@ -163,6 +170,26 @@ orderly_latest <- function(name = NULL, root = NULL, locate = TRUE,
   latest_id(ids)
 }
 
+list_archive <- function(name, config) {
+  if (is.null(name)) {
+    d <- orderly_list2(FALSE, config, FALSE)
+    d$id
+  } else {
+    path <- file.path(path_archive(config$root), name)
+    orderly_list_dir(path, check_run_rds = FALSE)
+  }
+}
+
+list_draft <- function(name, config) {
+  if (is.null(name)) {
+    d <- orderly_list2(TRUE, config, FALSE)
+    d$id
+  } else {
+    path <- file.path(path_draft(config$root), name)
+    orderly_list_dir(path, check_run_rds = TRUE)
+  }
+}
+
 
 orderly_list2 <- function(draft, root = NULL, locate = TRUE,
                           include_failed = FALSE) {
@@ -207,19 +234,13 @@ orderly_find_report <- function(id, name, config, locate = FALSE,
                                 draft = TRUE, must_work = FALSE) {
   config <- orderly_config(config, locate)
 
-  if (is.character(draft)) {
-    draft <- match_value(draft, c("always", "newer", "never"))
-    search_draft <- draft != "never"
-    search_archive <- draft != "always"
-    what <- switch(draft,
-                   always = "draft",
-                   never = "archive",
-                   newer = "draft or archive")
-  } else {
-    search_draft <- draft
-    search_archive <- !draft
-    what <- if (draft) "draft" else "archive"
-  }
+  draft <- query_check_draft(draft)
+  search_draft <- draft != "never"
+  search_archive <- draft != "always"
+  what <- switch(draft,
+                 always = "draft",
+                 never = "archive",
+                 newer = "draft or archive")
 
   base_archive <- file.path(path_archive(config$root), name)
   base_draft <- file.path(path_draft(config$root), name)

R/query_search.R

@@ -406,7 +406,7 @@ query_check_parameters <- function(parameters) {
 }
 
 
-query_check_draft <- function(draft, as) {
+query_check_draft <- function(draft) {
   if (is.logical(draft)) {
     assert_scalar(draft)
     draft <- if (draft) "always" else "never"

tests/testthat/test-orderly-recipe.R

@@ -282,6 +282,27 @@ test_that("Using draft within a dependency is now a warning", {
 })
 
 
+test_that("VIMC-4579: dependencies draft works with non-boolean use_draft", {
+  path <- prepare_orderly_example("depends", testing = TRUE)
+  id1 <- orderly_run("example", root = path, echo = FALSE)
+
+  filename <- file.path(path, "src", "depend", "orderly.yml")
+  dat <- yaml_read(filename)
+  dat$depends$example$id <- id1
+  dat$depends$example$draft <- NULL
+  yaml_write(dat, filename)
+
+  f <- function(id) {
+    readRDS(path_orderly_run_rds(file.path(path, "draft", "depend", id)))
+  }
+
+  id2 <- orderly_run("depend", root = path, use_draft = "newer", echo = FALSE)
+  id3 <- orderly_run("depend", root = path, use_draft = "always", echo = FALSE)
+  expect_equal(f(id2)$meta$depends$id, id1)
+  expect_equal(f(id3)$meta$depends$id, id1)
+})
+
+
 test_that("data field is optional", {
   path <- prepare_orderly_example("nodata")
   report_path <- file.path(path, "src", "example")

tests/testthat/test-query.R

@@ -145,6 +145,17 @@ test_that("latest", {
   expect_equal(orderly_latest("example", root = path), id2)
 })
 
+test_that("latest works with draft always, never, newer", {
+  skip_on_cran_windows()
+  path <- prepare_orderly_example("minimal")
+  id1 <- orderly_run("example", root = path, echo = FALSE)
+  id2 <- orderly_run("example", root = path, echo = FALSE)
+  orderly_commit(id1, root = path)
+  expect_equal(orderly_latest("example", root = path, draft = "never"), id1)
+  expect_equal(orderly_latest("example", root = path, draft = "always"), id2)
+  expect_equal(orderly_latest("example", root = path, draft = "newer"), id2)
+})
+
 
 test_that("Behaviour with rogue files", {
   testthat::skip_on_cran()