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

Remove warnings for manually created enrollment and failure rates #324

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

jdblischak
Copy link
Collaborator

Closes #316

I refactored the define_*_rate() and check_*_rate() functions so that the end user will no longer receive a warning message if they self-constructed enroll_rate and fail_rate. However, if they provide an invalid input, the error message recommends the appropriate define_*_rate() function

library("gsDesign2")

enroll_rate <- define_enroll_rate(
  duration = c(2, 2, 10),
  rate = c(3, 6, 9)
)

# new behavior: the column stratum was added by define_enroll_rate()
enroll_rate
## # A tibble: 3 × 3
## stratum duration  rate
## <chr>      <dbl> <dbl>
##   1 All            2     3
## 2 All            2     6
## 3 All           10     9

class(enroll_rate)
## [1] "enroll_rate" "tbl_df"      "tbl"         "data.frame" 
gs_design_ahr(enroll_rate = enroll_rate)

# remove the class
class(enroll_rate) <- class(enroll_rate)[-1]
class(enroll_rate)
## [1] "tbl_df"     "tbl"        "data.frame"

# new behavior: no warning for missing class
gs_design_ahr(enroll_rate = enroll_rate)

# New behavior: Recommend define_enroll_rate() in error message when invalid
# input is detected
enroll_rate_invalid <- enroll_rate[, -3]
gs_design_ahr(enroll_rate = enroll_rate_invalid)
## Error in check_enroll_rate(enroll_rate) : 
##   The variable `rate` can't be NULL. Try the helper function define_enroll_rate() to prepare valid input.

@LittleBeannie
Copy link
Collaborator

Hello @jdblischak , thank you for making the changes! I observed that the commit is forked in your personal GitHub repository. Could you please guide me on how to test the jdblischak:remove-input-class-warning branch? Should I clone it from your repository?

@jdblischak
Copy link
Collaborator Author

Two options for testing locally:

  1. Use {remotes} in the R console
remotes::install_github("jdblischak/gsDesign2@remove-input-class-warning")
  1. Use Git in the terminal
git remote add john https://github.com/jdblischak/gsDesign2.git
git fetch john
git checkout remove-input-class-warning
# Build and test the package in R

# When you are done, return to main branch
git checkout main

Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

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

Now with this patch it's much clearer. Moving the validation logic to the validator check_*() and call the validator in the constructor define_*() makes this much better aligned with the patterns described in the S3 chapter of Advanced R.

@nanxstats nanxstats merged commit 0caf7a4 into Merck:main Jan 25, 2024
8 checks passed
@jdblischak jdblischak deleted the remove-input-class-warning branch January 25, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve input constructor implementations
3 participants