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

Convert wlr() to S3 generic to accept tte_data or counting_process #276

Merged
merged 4 commits into from
Aug 28, 2024

Conversation

jdblischak
Copy link
Collaborator

Closes #275

Added the class "tte_data" to the output of cut_data_by_event() and cut_data_by_date(), then converted wlr() to an S3 generic. The "counting_process" class was already added to the output of counting_process() in #267

@jdblischak jdblischak self-assigned this Aug 20, 2024
R/wlr.R Show resolved Hide resolved
@jdblischak
Copy link
Collaborator Author

I rebased onto #273, which has broken my strategy for converting wlr() to an S3 generic. The goal was to pre-compute counting_process() prior to running multiple iterations of wlr(), but now the pre-counting-process data is required to compute the new ratio

simtrial/R/wlr.R

Lines 89 to 95 in 2ed558b

x <- data |> counting_process(arm = "experimental")
# calculate the sample size and randomization ratio
n <- nrow(data)
ratio <- sum(data$treatment == "experimental") / sum(data$treatment == "control")
q_e <- ratio / (1 + ratio)
q_c <- 1 - q_e

@LittleBeannie
Copy link
Collaborator

Got you! I guess one possible way is to get ratio as an attribute of data.

ratio <- sum(data$treatment == "experimental") / sum(data$treatment == "control")

@jdblischak
Copy link
Collaborator Author

I guess one possible way is to get ratio as an attribute of data.

Ok, I proceeded with that strategy. Though instead of passing the ratio, I passed the number of experimental and control treatments.

R/counting_process.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@LittleBeannie LittleBeannie left a comment

Choose a reason for hiding this comment

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

Thank you, @jdblischak! This PR has been merged.

@LittleBeannie LittleBeannie merged commit 5fe51b9 into Merck:main Aug 28, 2024
7 checks passed
@jdblischak jdblischak deleted the wlr-s3 branch August 30, 2024 18:00
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.

Make wlr enable a S3 method for both tte_data object and counting_process object
2 participants