-
Notifications
You must be signed in to change notification settings - Fork 8
Big refactor to simplify S3 classes #584
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -78,60 +78,22 @@ as_gt <- function(x, ...) { | |||||||
| #' ) |> | ||||||||
| #' summary() |> | ||||||||
| #' as_gt() | ||||||||
| as_gt.fixed_design <- function(x, title = NULL, footnote = NULL, ...) { | ||||||||
| method <- fd_method(x) | ||||||||
| ans <- gt::gt(x) |> | ||||||||
| gt::tab_header(title = title %||% fd_title(method)) | ||||||||
| if (!isFALSE(footnote)) ans <- ans |> | ||||||||
| gt::tab_footnote( | ||||||||
| footnote = footnote %||% fd_footnote(x, method), | ||||||||
| locations = gt::cells_title(group = "title") | ||||||||
| ) | ||||||||
| return(ans) | ||||||||
| } | ||||||||
|
|
||||||||
| get_method <- function(x, methods) intersect(methods, class(x))[1] | ||||||||
| as_gt.fixed_design_summary <- function(x, title = NULL, footnote = NULL, ...) { | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it's worth exposing the true defaults here, i.e.,
Suggested change
Two benefits are: 1) it'd be more transparent; 2) it'd make it possible to actually use I don't have strong opinions on this, so if you prefer the current way, that will be fine to me.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like your proposal, but I don't want to introduce any API changes in this already large PR. Thus for now I am following the documented interface: Line 150 in 936ec20
But given that you were the one that introduced the workaround
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I'll follow up.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided not to change this after looking at |
||||||||
| if (is.null(title)) title <- attr(x, "title") | ||||||||
| if (is.null(footnote)) footnote <- attr(x, "footnote") | ||||||||
|
|
||||||||
| # get the fixed design method | ||||||||
| fd_method <- function(x) { | ||||||||
| get_method(x, c("ahr", "fh", "mb", "lf", "rd", "maxcombo", "milestone", "rmst")) | ||||||||
| } | ||||||||
| ans <- gt::gt(x) |> | ||||||||
| gt::tab_header(title = title) | ||||||||
|
|
||||||||
| # get the default title | ||||||||
| fd_title <- function(method) { | ||||||||
| sprintf("Fixed Design %s Method", switch( | ||||||||
| method, | ||||||||
| ahr = "under AHR", fh = "under Fleming-Harrington", mb = "under Magirr-Burman", | ||||||||
| lf = "under Lachin and Foulkes", maxcombo = "under MaxCombo", | ||||||||
| milestone = "under Milestone", rmst = "under Restricted Mean Survival Time", | ||||||||
| rd = "of Risk Difference under Farrington-Manning" | ||||||||
| )) | ||||||||
| } | ||||||||
| if (!isFALSE(footnote)) { | ||||||||
| ans <- ans |> | ||||||||
| gt::tab_footnote( | ||||||||
| footnote = footnote, | ||||||||
| locations = gt::cells_title(group = "title") | ||||||||
| ) | ||||||||
| } | ||||||||
|
|
||||||||
| # get the default footnote | ||||||||
| fd_footnote <- function(x, method) { | ||||||||
| switch( | ||||||||
| method, | ||||||||
| ahr = "Power computed with average hazard ratio method.", | ||||||||
| fh = paste( | ||||||||
| "Power for Fleming-Harrington test", substring(x$Design, 19), | ||||||||
| "using method of Yung and Liu." | ||||||||
| ), | ||||||||
| lf = paste( | ||||||||
| "Power using Lachin and Foulkes method applied using expected", | ||||||||
| "average hazard ratio (AHR) at time of planned analysis." | ||||||||
| ), | ||||||||
| rd = paste( | ||||||||
| "Risk difference power without continuity correction using method of", | ||||||||
| "Farrington and Manning." | ||||||||
| ), | ||||||||
| maxcombo = paste0( | ||||||||
| "Power for MaxCombo test with Fleming-Harrington tests ", | ||||||||
| substring(x$Design, 9), "." | ||||||||
| ), | ||||||||
| # for mb, milestone, and rmst | ||||||||
| paste("Power for", x$Design, "computed with method of Yung and Liu.") | ||||||||
| ) | ||||||||
| return(ans) | ||||||||
| } | ||||||||
|
|
||||||||
| #' @rdname as_gt | ||||||||
|
|
@@ -243,7 +205,7 @@ fd_footnote <- function(x, method) { | |||||||
| #' summary() |> | ||||||||
| #' as_gt(display_columns = c("Analysis", "Bound", "Nominal p", "Z", "Probability")) | ||||||||
| #' } | ||||||||
| as_gt.gs_design <- function( | ||||||||
| as_gt.gs_design_summary <- function( | ||||||||
| x, | ||||||||
| title = NULL, | ||||||||
| subtitle = NULL, | ||||||||
|
|
@@ -355,7 +317,7 @@ gsd_footnote <- function(method, columns) { | |||||||
| # footnote for non-binding designs | ||||||||
| gsd_footnote_nb <- function(x, x_alpha) { | ||||||||
| full_alpha <- attr(x, "full_alpha") | ||||||||
| if (!inherits(x, "non_binding") || x_alpha >= full_alpha) return() | ||||||||
| if (attr(x, "binding") || x_alpha >= full_alpha) return() | ||||||||
| a1 <- format(x_alpha, scientific = FALSE) | ||||||||
| a2 <- format(full_alpha, scientific = FALSE) | ||||||||
| a3 <- format(full_alpha - x_alpha, scientific = FALSE) | ||||||||
|
|
@@ -387,7 +349,7 @@ gsd_parts <- function( | |||||||
| x, title, subtitle, spannersub, footnote, bound, columns, inf_bound, | ||||||||
| transform = identity | ||||||||
| ) { | ||||||||
| method <- intersect(c("ahr", "wlr", "combo", "rd"), class(x))[1] | ||||||||
| method <- attr(x, "design") | ||||||||
jdblischak marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| if (!inf_bound) x <- filter(x, !is.infinite(Z)) | ||||||||
| # `x` needs a custom transformation in as_rtf() | ||||||||
| x2 <- transform(x) | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we could disable the footnote via
footnote = FALSE. Now we wouldn't if thisifstatement is removed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Fixed by c8609a1