236 milestone take the un squared version of the current z statistics #237
Conversation
nanxstats
deleted the
236-milestone-take-the-un-squared-version-of-the-current-z-statistics
branch
I don't understand why this test was skipped. It doesn't run the updated |
🤔 That would be a good question for @LittleBeannie |
There was a problem hiding this comment.
Overall this PR had a much bigger impact than I had anticipated. I assumed it was only changing the Z statistics in z. However, it also changed se to only return a single value instead of two. And the default estimate is also changed by the new test_type of "log-log". I was able to fix estimate for my sim_gs_n() tests by adding test_type = "naive".
We should add some developer tests for milestone() so that it is more transparent how a PR is effecting the output.
| #' | ||
| #' @param test_type Method to build the test statistics. | ||
| #' There are 2 options: | ||
| #' - `"native"`: a native approach by dividing the KM survival difference by its standard derivations, |
There was a problem hiding this comment.
Is the approach "native" or "naive"? The string passed to test_type below is "naive"
There was a problem hiding this comment.
Probably a typo - should be "naive" cc @LittleBeannie
| #' - `method` - The method, always `"milestone"`. | ||
| #' - `parameter` - Milestone time point. | ||
| #' - `estimation` - Survival difference between the experimental and control arm. | ||
| #' - `se` - Standard error of the control and experimental arm. |
There was a problem hiding this comment.
The se result was changed by this PR, but the documentation did not change. Previously it returned two standard error values (from fit_res$std.err), but now it only returns a single standard error. Is it the standard error of the control arm, experimental arm, or some combination? We should update the documentation for this return value
There was a problem hiding this comment.
Agreed, this should be a quick fix for @LittleBeannie
milestone() was recently overhauled to 1. Use the log-log method to calculate the `estimate`. Setting `test_type = "naive"` restored the previous value 1. Return a single value for `se` instead of two 1. Return the unsquared Z statistic for `z` Merck#237
milestone() was recently overhauled to 1. Use the log-log method to calculate the `estimate`. Setting `test_type = "naive"` restored the previous value 1. Return a single value for `se` instead of two 1. Return the unsquared Z statistic for `z` Merck#237
Closes #236