Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loadd(deps = TRUE) should work even if the target is missing. #830

Closed
3 tasks done
wlandau opened this issue Apr 9, 2019 · 3 comments
Closed
3 tasks done

loadd(deps = TRUE) should work even if the target is missing. #830

wlandau opened this issue Apr 9, 2019 · 3 comments
Assignees

Comments

@wlandau
Copy link
Member

wlandau commented Apr 9, 2019

Prework

@wlandau wlandau self-assigned this Apr 9, 2019
@wlandau
Copy link
Member Author

wlandau commented Apr 10, 2019

This needs to work for debugging. Reprex:

library(drake)
plan <- drake_plan(a = "x", b = a + 1)
make(plan)
#> target a
#> target b
#> fail b
#> Error: Target `b` failed. Call `diagnose(b)` for details. Error message:
#>   non-numeric argument to binary operator
config <- drake_config(plan)
loadd(b, config = config, deps = TRUE)
#> Error in loadd(b, config = config, deps = TRUE): object 'b' not found

Created on 2019-04-09 by the reprex package (v0.2.1)

@wlandau
Copy link
Member Author

wlandau commented Apr 10, 2019

Important note: we will need to disable tidyselect functionality when deps is TRUE. For example, loadd(starts_with("model_"), config = config, deps = TRUE)
will not work. For the selection mechanism to work, the model_* targets to need to already be in the cache, which is not always the case when you are debugging your projects. To help drake understand what you mean, you will need to name the targets explicitly when deps is TRUE, e.g. loadd(model_A, model_B, config = config, deps = TRUE).

Also, @kendonB, I am still thinking about #823. I do agree that supplying config is an inconvenience, but none of the alternatives I can think of would scale well. We would at least need to store config$layout$EVERY_TARGET$deps_build$memory or config$graph, which could eat up a lot of space.

@wlandau
Copy link
Member Author

wlandau commented Apr 10, 2019

Fixed in a83e7bb. Also added the above note in the loadd() help file under deps, plus an informative console message when the deps and tidyselect arguments are both TRUE.

@wlandau wlandau reopened this Apr 10, 2019
@wlandau wlandau closed this as completed Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant