Skip to content

Commit

Permalink
Fix #1138
Browse files Browse the repository at this point in the history
  • Loading branch information
wlandau-lilly committed Jan 19, 2020
1 parent 581d882 commit 809a33a
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 2 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion R/cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion R/manage_memory.R
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-3-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
17 changes: 17 additions & 0 deletions tests/testthat/test-9-dynamic.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
})

0 comments on commit 809a33a

Please sign in to comment.