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.

Sign up for GitHub

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

Warning when factorial arguments conflict #251

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Warning when factorial arguments conflict #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4b2de41
Warning when factorial arguments conflict
clarabicalho Aug 2, 2019
068635f
Update check
clarabicalho Aug 2, 2019
3649287
Same warnings for two_by_two
clarabicalho Aug 2, 2019
1794709
Update warning two_by_two
clarabicalho Aug 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions R/factorial_designer.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ factorial_designer <- function(
if(k < 2 || !is_integerish(k)) stop("`k' should be a positive integer > 1.")
if(any(outcome_sds<0)) stop("`outcome_sds' should be nonnegative.")
if(any(assignment_probs <= 0)) stop("`assignment_probs' should have positive values only.")
if(!is.null(sd) && !is.null(outcome_sds) && !identical(outcome_sds, rep(sd, 2^k)))
warning("Both `sd` and `outcome_sd` are specified and have different values. Taking value of `outcome_sds`.")

# pre-objects -------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions R/two_by_two_designer.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ two_by_two_designer <- function(N = 100,
if(max(c(sd_i, outcome_sds) < 0) ) stop("sd_i and outcome_sds must be nonnegative")
if(max(c(prob_A, prob_B) < 0)) stop("prob_ arguments must be nonnegative")
if(max(c(prob_A, prob_B) > 1)) stop("prob_ arguments must not exceed 1")
if(!is.null(outcome_means) && all(!is.null(c(mean_A0B0, mean_A0B1, mean_A1B0, mean_A1B1))) &&
Copy link
Contributor

@nfultz nfultz Aug 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this check will work as intended, it is not user friendly to require people to set outcome_means=NULL when they want to provide all 4 means.

Also is.null is not vectorized, would double check the second half as well.

> is.null(c(2, NULL))
[1] FALSE

Adding some tests that set the mean_AB params should help shake out these issues.

!identical(outcome_means, c(mean_A0B0, mean_A0B1, mean_A1B0, mean_A1B1))) stop("Both `outcome_means` and `mean_*` parameters are specified and have different values. Taking value of `mean_*` arguments.")
{{{

# M: Model
Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/test_designers.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ test_that(desc = "two_arm_covariate_designer errors when it should",
expect_error(two_arm_covariate_designer(rho_WZ = 10))
})

test_that(desc = "factorial_designer errors when it should",
test_that(desc = "factorial_designer errors or warns when it should",
code = {
expect_error(factorial_designer(outcome_name = c("Y ")))
expect_error(factorial_designer(outcome_means = 1, k = 2))
Expand All @@ -260,6 +260,7 @@ test_that(desc = "factorial_designer errors when it should",
expect_error(factorial_designer(k = .5))
expect_error(factorial_designer(outcome_sds = c(-1,-1,-1,-1), k = 2))
expect_error(factorial_designer(assignment_probs = c(-.5,.5), k = 2))
expect_warning(factorial_designer(k = 2, sd = 1, outcome_sds = rep(.5, 4)))
})

test_that(desc = "process_tracing_designer errors when it should",
Expand Down