-
Notifications
You must be signed in to change notification settings - Fork 8
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
Standardize the output format of the test functions #222
Comments
First of all: very nice! |
I think the .04 is the z-score, not the p-value
I think it should be possible. I'm not a big fan of list-columns in general (since they make things more complicated), but it would be a way to store this heterogenous data.
Exactly. We can use this Issue to discuss the various descriptions/parameters/statistcs/etc that we need returned from each test function and decide on a uniform output format. |
100% agree with the OOP/S3 method comment - that would provide the interface for the returned data (implementation), as described in the principle code against interface, not implementation. |
After discussion, we are planning to try returning a named vector with the following values. If this becomes too unmanageable, then we can consider trying more sophisticated options like S3 or list-columns.
@LittleBeannie please feel free to adjust these names as you see fit. They are just my suggestions |
Hi John, looks great to me, thanks for the nice summary! Can I raise some minor comments to the names?
|
Those names are all fine by me. Like I said, mine were only suggestions |
Good discussions, how about this - no abbreviation at all to make them easy to search:
This also aligns with the general rule of thumb to prefer long, descriptive names suggested by R4DS. nphsim consistency is a good topic, although I don't think it's that important because of its experimental nature... and we should take its code style with a grain of salt. |
Here are the outputs of the testing functions now that #227 was merged (75d7b67). I am going to work on extending remotes::install_github("Merck/simtrial@2c015f6", upgrade = FALSE)
library("simtrial")
packageVersion("simtrial")
## [1] ‘0.3.2.14’
trial_data <- sim_pw_surv()
trial_data_cut <- cut_data_by_event(trial_data, 150)
wlr(trial_data_cut, weight = fh(rho = 0, gamma = 0))
## $method
## [1] "WLR"
##
## $parameter
## [1] "FH(rho=0, gamma=0)"
##
## $estimation
## [1] -12.3417
##
## $se
## [1] 4.486425
##
## $z
## [1] -2.750898
##
wlr(trial_data_cut, weight = fh(rho = 0, gamma = 0.5))
## $method
## [1] "WLR"
##
## $parameter
## [1] "FH(rho=0, gamma=0.5)"
##
## $estimation
## [1] -10.15942
##
## $se
## [1] 2.848798
##
## $z
## [1] -3.566213
##
wlr(trial_data_cut, weight = mb(delay = 3))
## $method
## [1] "WLR"
##
## $parameter
## [1] "MB(delay = 3, max_weight = Inf)"
##
## $estimate
## [1] -14.61053
##
## $se
## [1] 5.199295
##
## $z
## [1] -2.810097
##
rmst(trial_data_cut, tau = 20)
## $method
## [1] "RMST"
##
## $parameter
## [1] 20
##
## $estimation
## [1] 1.867087
##
## $se
## [1] 1.470548
##
## $z
## [1] 1.269654
##
milestone(trial_data_cut, ms_time = 10)
## $method
## [1] "milestone"
##
## $parameter
## [1] 10
##
## $estimation
## [1] 0.1
##
## $se
## [1] 0.07019972 0.07048404
##
## $z
## [1] 0.9964078
##
maxcombo(trial_data_cut, rho = c(0, 0), gamma = c(0, 0.5))
## $method
## [1] "Maxcombo"
##
## $parameter
## [1] "FH(0, 0) + FH(0, 0.5)"
##
## $z
## [1] -2.750898 -3.566213
##
## $p_value
## [1] 0.0002723921
## |
In #229, I updated all the test functions to return |
Hi @jdblischak, is it okay if we close this issue as complete? |
Yes. The main remaining piece is multiple tests per cut for |
We have ambitious goals to make experiments with different test functions extremely flexible. For example:
sim_gs_n()
to apply a different test at each cut, and even potentially multiple tests per cutmaxcombo()
to accept an arbitrary number of test functions and return the best resultmultitest()
to run multiple tests on a given data set and return the results (currently works by returning a list, but a data frame would be easier for downstream analysis)Currently this flexibility isn't possible. While the test functions all accept a data frame of trial data as their first positional argument, the outputs are not uniform. This makes it impossible to combine the results into a data frame to facilitate downstream analysis. I also don't understand how the
maxcombo()
function can be extended to accept any number of arbitrary tests without a common test statistic to compare them (though I admit I may just not fully understand the future plans formaxcombo()
).The example below demonstrates the diversity of the outputs:
I propose that each test function returns a named vector with 3 elements:
description
: brief description of the test performedtest statistic
: whatever makes sense to return from the testp-value
: to facilitate comparing across tests with different test statisticsIf this is too simplistic, we could include more information (eg the input parameters) by either:
parameters
, which could itself be a named vector. Then we could convert these to a data frame using a list column forparameters
as.data.frame()
would extract a minimal set to combine the results of many tests into a single data frameattributes
of the named vector. But this isn't really friendly for downstream analysis. It would also likely be lost in functions likesim_gs_n()
that return a data framexref: #215
The text was updated successfully, but these errors were encountered: