From 809a33a2dc039df630fd517a020f2374bb3b6ec1 Mon Sep 17 00:00:00 2001
From: wlandau <will.landau@gmail.com>
Date: Sat, 18 Jan 2020 20:12:20 -0500
Subject: [PATCH] Fix #1138

---
 NEWS.md                         |  1 +
 R/cache.R                       |  2 +-
 R/manage_memory.R               |  2 +-
 R/utils.R                       | 12 ++++++++++++
 tests/testthat/test-3-utils.R   |  9 +++++++++
 tests/testthat/test-9-dynamic.R | 17 +++++++++++++++++
 6 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 427cb8f3a..0c87b973f 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -3,6 +3,7 @@
 ## Bug fixes
 
 - Handle unequal list columns in `bind_plans()` (#1136, @jennysjaarda).
+- Handle non-vector sub-targets in dynamic branching (#1138).
 
 ## New features
 
diff --git a/R/cache.R b/R/cache.R
index 695aba2ef..f8948e4c7 100644
--- a/R/cache.R
+++ b/R/cache.R
@@ -324,7 +324,7 @@ get_subtargets.drake_dynamic <- function(hashes, cache, subtargets) {
     hashes <- hashes[subtargets]
   }
   out <- lapply(hashes, cache$get_value, use_cache = FALSE)
-  do.call(vec_c, out)
+  do.call(safe_vec_c, out)
 }
 
 get_subtargets.default <- function(hashes, cache, subtargets) {
diff --git a/R/manage_memory.R b/R/manage_memory.R
index bd4872e0d..1870b39d5 100644
--- a/R/manage_memory.R
+++ b/R/manage_memory.R
@@ -304,7 +304,7 @@ load_dynamic_subdep_impl.group <- function( # nolint
 ) {
   subdeps <- config$cache$get(dep, namespace = "meta")$subtargets[index]
   value <- config$cache$mget(subdeps, use_cache = FALSE)
-  value <- do.call(vec_c, value)
+  value <- do.call(safe_vec_c, value)
   assign(
     x = dep,
     value = value,
diff --git a/R/utils.R b/R/utils.R
index acedbee88..0e46289d9 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -146,6 +146,18 @@ vlapply <- function(X, FUN, ...) {
   vapply(X, FUN, FUN.VALUE = logical(1), ...)
 }
 
+safe_vec_c <- function(...) {
+  tryCatch(
+    vctrs::vec_c(...),
+    vctrs_error_scalar_type = function(e) {
+      list(...)
+    },
+    error = function(e) {
+      stop(e)
+    }
+  )
+}
+
 num_unique <- function(x) {
   length(unique(x))
 }
diff --git a/tests/testthat/test-3-utils.R b/tests/testthat/test-3-utils.R
index 673763e73..f14934fdb 100644
--- a/tests/testthat/test-3-utils.R
+++ b/tests/testthat/test-3-utils.R
@@ -190,3 +190,12 @@ test_with_dir("storage_copy() (#1120)", {
   storage_copy(d1, d2, merge = TRUE, overwrite = TRUE)
   expect_true(file.exists(x))
 })
+
+test_with_dir("safe_vec_c() (#1138)", {
+  expect_equal(safe_vec_c(letters, letters), c(letters, letters))
+  x <- lm(mpg ~ cyl, data = mtcars)
+  y <- lm(mpg ~ wt, data = mtcars)
+  expect_equal(safe_vec_c(x, y), list(x, y))
+  expect_equal(safe_vec_c(x, letters[1]), list(x, letters[1]))
+  expect_error(safe_vec_c(stop("sdklfjiole")), regexp = "sdklfjiole")
+})
diff --git a/tests/testthat/test-9-dynamic.R b/tests/testthat/test-9-dynamic.R
index e9ed6a610..e1558055c 100644
--- a/tests/testthat/test-9-dynamic.R
+++ b/tests/testthat/test-9-dynamic.R
@@ -1667,3 +1667,20 @@ test_with_dir("visualization labels for dynamic targets", {
   label <- x$nodes$label[x$nodes$id == "y"]
   expect_false(grepl("sub-targets", label, fixed = TRUE))
 })
+
+test_with_dir("non-vector sub-targets (#1138)", {
+  f <- function(...) {
+    lm(cyl ~ mpg, data = mtcars)
+  }
+  plan <- drake_plan(
+    index = seq_len(4),
+    model = target(f(index), dynamic = map(index)),
+    result = model
+  )
+  make(plan)
+  models <- readd(result)
+  expect_equal(length(models), 4L)
+  for (model in models) {
+    expect_true(inherits(model, "lm"))
+  }
+})