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

Replace group with tags #688

Merged
merged 7 commits into from
Jan 21, 2019
Merged

Replace group with tags #688

merged 7 commits into from
Jan 21, 2019

Conversation

wlandau
Copy link
Member

@wlandau wlandau commented Jan 21, 2019

Summary

Before, we had

drake_plan(
  x = target(
    command,
    transform = map(y = c(1, 2)),
    group = my_group
  )
)

The whole separate "group" slot was confusing. It conflicted with the term "grouping variable", which describes y above, and it's a dangling slot of target().

This PR cleans up and completes the idea. Now, we have tags, which are grouping variables added post-hoc.

  • In-tags: columns with the target names we start with.
  • Out-tags: columns with the new target names that the transformation generates.
library(drake)
drake_plan(
  x = target(
    command,
    transform = map(y = c(1, 2), .tag_in = from, .tag_out = c(to, out))
  ),
  trace = TRUE
)
#> # A tibble: 2 x 7
#>   target command y     x     from  to    out  
#>   <chr>  <chr>   <chr> <chr> <chr> <chr> <chr>
#> 1 x_1    command 1     x_1   x     x_1   x_1  
#> 2 x_2    command 2     x_2   x     x_2   x_2

Created on 2019-01-20 by the reprex package (v0.2.1)

Above,

  • x and y are implicit out-tags.
  • from is an explicit in-tag.
  • to and out are explicit out-tags.

The whole concept is intuitive and symmetric. Users can chain tags together to create much more complicated dependency graphs from individual calls to drake_plan().

Related

#233

Checklist

  • I have read drake's code of conduct, and I agree to follow its rules.
  • I have listed any substantial changes in the development news.
  • I have added testthat unit tests to tests/testthat to confirm that any new features or functionality work correctly.
  • I have tested this pull request locally with devtools::check()
  • This pull request is ready for review.
  • I think this pull request is ready to merge.

@codecov-io
Copy link

codecov-io commented Jan 21, 2019

Codecov Report

Merging #688 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #688   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          72     72           
  Lines        5715   5723    +8     
=====================================
+ Hits         5715   5723    +8
Impacted Files Coverage Δ
R/api-plan.R 100% <ø> (ø) ⬆️
R/api-dsl.R 100% <100%> (ø) ⬆️
R/utils-utils.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a92d095...5b7ccbc. Read the comment docs.

@wlandau wlandau merged commit f527cc7 into master Jan 21, 2019
@wlandau wlandau deleted the tags branch January 21, 2019 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants