names labels #246
names labels #246
Conversation
R/construct_design.R
Outdated
| # auto_steps <- sapply(lhs, function(x) { lbl <- attr(x, "label"); !is.null(lbl) && grepl("Autogenerated by ", lbl) }) | ||
| auto_steps <- sapply(lhs, function(x) !is_null(attr(x, "auto-generated"))) | ||
|
|
||
| auto_steps <- sapply(lhs, function(x) !is_null(attr(x, "auto-generated"))) |
There was a problem hiding this comment.
pull out predicate to helper file, rewrite as Filter(Negate(is_autogenerated), lhs)
There was a problem hiding this comment.
thx. couldn't get this to work. tried:
is_autogenerated <- function(steps) {
sapply(steps, attr, "auto-generated")
}
lhs <- Filter(Negate(is_autogenerated), lhs)
am I missing something?
R/construct_design.R
Outdated
| @@ -128,31 +119,19 @@ | |||
| construct_design <- function(...) { | |||
|
|
|||
| qs <- quos(...) | |||
| qs <- maybe_add_names_qs(qs) | |||
There was a problem hiding this comment.
note that you don't really use qnames now, can probably purge this and set names(ret) directly.
There was a problem hiding this comment.
we do that a bit lower, but need here to capture names like pop when sent to pop + declare_sampling(n = 50). the name pop is lost if you don't use maybe_add_names on the quosures.
There was a problem hiding this comment.
There's only one place that calls in to conduct_design, should not need any NSE within it - pass the steps directly. See #249
| # it will fail to evaluate - we can catch that and try to curry it | ||
|
|
||
| #' @importFrom rlang quos lang_fn lang_modify eval_tidy | ||
| callquos_to_step <- function(step_call, label="") { |
| if (is.null(step_meta <- attr(step_j, "step_meta"))) next; | ||
| if (attr(step_j, "step_type") == "assignment") { | ||
| if (any(step_meta$assignment_variables %in% attr(step, "step_meta")$assignment_variables)) { | ||
| # warning("Attempting to inject a `declare_reveal(", this_step_meta$outcome_variables, ", ", |
There was a problem hiding this comment.
The warnings should probably come back at some point.
There was a problem hiding this comment.
thx, yes, will remember to come back to this
| ". You can name steps within declare design, e.g. declare_design(my_pop = pop_step)."), call. = FALSE) | ||
| if (names(quotations)[i] == ""){ | ||
| names(quotations)[i] <- namer(quotations[[i]]) | ||
| } |
There was a problem hiding this comment.
whole loop can be vectorized now.
|
See #249 |
refactoring nse in conduct_design -> plus
tests/testthat/test-fanout.R
Outdated
| @@ -84,6 +84,7 @@ test_that("sims expansion is correct",{ | |||
|
|
|||
|
|
|||
| test_that("fan_out ids are correct",{ | |||
| sleep_large <- resample_data(sleep, N = 200) | |||
There was a problem hiding this comment.
that test fails intermittently, was trying to see if upping sample size would help
There was a problem hiding this comment.
oic I didn't actually implement. commit to follow
R/construct_design.R
Outdated
| stop("The right hand side does not appear to be a DeclareDesign object or a function", call. = FALSE) | ||
| } | ||
| while(new_nm %in% union(names(lhs), names(rhs))) | ||
| new_nm <- paste0(new_nm, "._") |
There was a problem hiding this comment.
Needs to add numbers like data.frame does.
There was a problem hiding this comment.
great, just made commits along these lines
|
think this is good to go, subject to @nfultz review |
closes #220
closes #227
closes #246