Clean up summary functions
#461
Conversation
…irst and second elements in x$design_par; also use gsub(fixed = TRUE) to avoid escaping parentheses as \(\)
an alternative way is a loop:
for (i in c("design", "n", "event", "time", "bound", "power")) {
# special case: Event -> Events
names(ans)[names(ans) == i] <- if (i == "event") "Events" else {
# capitalize the first letter
sub("^(.)", "\\U\\1", i, perl = TRUE)
}
}
the pattern `x[match(y, names(x))]` could win an unmatched award... it should have been simply `x[y]`
…`filter()` can be simply indexing by name from `col_decimals`; then all `if` statments can be merged into a `for` loop
…ed to access the col_vars
…ame columns in `analyses`
…so been updated, so only need to add the `Null hypothesis` column
…tails`; just use `analyses` and `xy`
yihui
left a comment
yihui
left a comment
There was a problem hiding this comment.
I had to take several breaks while working on this PR, since it almost burned my brain several times. Sorry I can't make it bite-sized this time. Good luck to reviewers :)
| } | ||
|
|
||
| if ("event" %in% names(ans)) { | ||
| ans <- ans %>% dplyr::rename(Events = event) |
There was a problem hiding this comment.
Is there a reason for renaming event to Events (plural) for fixed_design but Event (singular) for gs_design?
There was a problem hiding this comment.
Please use "Events" for both fixed design and group sequential designs.
| # If the method is WLR, change HR to wHR | ||
| if (method == "wlr") xy <- replace_names(xy, c("~hr at bound" = "~whr at bound")) | ||
|
|
||
| # If the method is COMBO, remove the column of "~HR at bound", and remove AHR from header |
There was a problem hiding this comment.
This comment is inconsistent with the code below---the code doesn't remove ~hr at bound or ahr. What should we do?
There was a problem hiding this comment.
The comment is correct, when it is maxcombo test, the "~HR at bound" and "AHR" do not exist.
| \multicolumn{6}{l}{Analysis: 1 Time: 36 N: 471.1 Event: 289 AHR: 0.68 Information fraction: 1\textsuperscript{\textit{3}}} \\[2.5pt] | ||
| \midrule\addlinespace[2.5pt] | ||
| Efficacy & 1.96 & 0.025 & 0.7940584 & 0.9 & 0.025 \\ | ||
| Efficacy & 1.96 & 0.025 & 0.7941 & 0.9 & 0.025 \\ |
There was a problem hiding this comment.
I believe the changes in tests revealed a bug in the original code. That is, wHR at bound should have been rounded to 4 decimal places by default but wasn't. My code fixed the bug, therefore I updated the test results here.
jdblischak
left a comment
jdblischak
left a comment
There was a problem hiding this comment.
No CI jobs are displayed for the PR because your most recent commit contained [ci skip]. The most recent run was this one triggered by the previous push. I ran R CMD check locally with the latest version, and I can confirm there is still only the single NOTE about setNames.
jdblischak
left a comment
jdblischak
left a comment
There was a problem hiding this comment.
Amazing work. Thanks @yihui!
LittleBeannie
left a comment
LittleBeannie
left a comment
There was a problem hiding this comment.
There are lots of work, thank you, @yihui !
…ith fixed design
…ith fixed design
3 participants
Closes #282.