pharmaverse / ggsurvfit

http://www.danieldsjoberg.com/ggsurvfit/
Other
67 stars 19 forks source link

Plot arithmetic for ggsurvfit? #96

Open ddsjoberg opened 1 year ago

ddsjoberg commented 1 year ago

Is it feasible to export methods for +.ggsurvfit, -.ggsurvfit, /.ggsurvfit, etc. They would work just like the patchwork versions, but would call ggsurvfit_build() to first construct the risktable, then pass the objects on to the ggplot2 methods.

https://patchwork.data-imaginist.com/reference/plot_arithmetic.html

I did some preliminary testing with a built ggsurvfit figure, and it did not seem to be as straight-forward as I was hoping.

This help file on generics for operators is helpful. The class of both sides of the operator are considered before it's dispatched to a method. https://stat.ethz.ch/R-manual/R-devel/library/base/html/groupGeneric.html

ddsjoberg commented 1 year ago

One thing that may make this tricky is that patchwork implements methods for ggplot objects, e.g. \.ggplot. But when we do ggsurvfit_build(combine_plots = FALSE) each of the plots is a gtable instead of ggplot.

ddsjoberg commented 1 year ago

I think the key here will be to use patchwork::wrap_elements() functions around the built ggsurvift figures. Minimally, we'd need to to the following:

  1. Add a "ggsurvfit_build" class to built ggsurvfit plots.
  2. We'd need to export methods for ggsurvift and ggsurvfit_build (need to check on the class of ggcuminc()).
  3. I think all the operators will work, EXCEPT I am unsure what will happen with +.gg or +.ggplot. If I create a +.ggsurvfit method, how will it know the difference between ggsurvfit() + a_new_layer() vs ggsurvfit_plot + ggsurvfit_plot. Will need to investigate the consequences....
library(patchwork)
library(ggsurvfit)
#> Loading required package: ggplot2

ggsurvfit <-
  (survfit2(Surv(time, status) ~ surg, data = df_colon) %>%
  ggsurvfit(size = 1, type = "risk") +
  add_risktable()) %>%
  ggsurvfit_build()

class(ggsurvfit)
#> [1] "patchwork"     "wrapped_patch" "patch"         "gg"           
#> [5] "ggplot"

wrap_elements(ggsurvfit) + wrap_elements(ggsurvfit)

Created on 2022-10-09 with reprex v2.0.2

ddsjoberg commented 1 year ago

My current hypothesis is that operator S3 methods are different from typical S3 methods. I am wondering if there is an initial check that the method being implemented is identical on both sides of the operator. In the example below, the LHS would be dispatched via |.ggplot but the RHS would be dispatched via |.ggsurvfit (even though the |.ggplot method can handle the ggsurvfit class object). Before any code is executed or dispatched via |.ggplot we get the error "Error in tt | p1: operations are possible only for numeric, logical or complex types".

library(ggsurvfit)
#> Loading required package: ggplot2

packageVersion("ggsurvfit")
#> [1] '0.2.0.9999'
packageVersion("patchwork")
#> [1] '1.1.2.9999'

p1 <- 
  survfit2(Surv(time, status) ~ sex, data = df_lung) %>%
  ggsurvfit() +
  add_risktable()

tt <- p1 | p1
#> [1] "We're in the '|.ggsurvfit' method!"
#> [1] "We're in the '|.ggplot' method!"

# print the variosu classes of interest
class(tt)
#> [1] "patchwork"   "plot_filler" "gg"          "ggplot"
class(p1)
#> [1] "ggsurvfit" "gg"        "ggplot"
class(ggsurvfit_build(p1))
#> [1] "patchwork"     "wrapped_patch" "patch"         "gg"           
#> [5] "ggplot"
class(ggsurvfit_build(p1))
#> [1] "patchwork"     "wrapped_patch" "patch"         "gg"           
#> [5] "ggplot"

# these methods work and are dispatched via '|.ggplot'
rr <- tt | patchwork::wrap_elements(ggsurvfit_build(p1))  
#> [1] "We're in the '|.ggplot' method!"
rr <- tt | ggsurvfit_build(p1)  
#> [1] "We're in the '|.ggplot' method!"

# but because this includes class 'ggsurvfit', we get an error
tt | p1  
#> Warning: Incompatible methods ("|.ggplot", "|.ggsurvfit") for "|"
#> Error in tt | p1: operations are possible only for numeric, logical or complex types

# now remove the ggsurvfit class
p2 <- p1
class(p2) <- class(p2) %>% setdiff("ggsurvfit")
class(p2)
#> [1] "gg"     "ggplot"

# and this works...why!?!?!
rr <- tt | p2
#> [1] "We're in the '|.ggplot' method!"

Created on 2022-10-14 with reprex v2.0.2

ddsjoberg commented 1 year ago

OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK, some progress.

All of this works, BUT it does depend on an update to patchwork that would make their methods extendable. I'll revisit this after the survfit v0.2.0 release.

library(ggsurvfit)
#> Loading required package: ggplot2

packageVersion("ggsurvfit")
#> [1] '0.2.0.9999'
packageVersion("patchwork")
#> [1] '1.1.2.9999'

p1 <- 
  survfit2(Surv(time, status) ~ sex, data = df_lung) %>%
  ggsurvfit() +
  add_risktable()

p1 | p1 | p1

Created on 2022-10-14 with reprex v2.0.2

ddsjoberg commented 1 year ago

I wonder if this post can help... https://stackoverflow.com/questions/74072102/force-execution-of-a-specified-s3-method/75830547#75830547

ddsjoberg commented 9 months ago
# FROM HADLEY WICKHAM USING S7 PACKAGE
method(`|`, list(new_S3_class("ggsurvfit"), new_S3_class("ggsurvfit"))) <- function(e1, e2) { 

}
method(`|`, list(new_S3_class("ggsurvfit"), new_S3_class("ggplot"))) <- function(e1, e2) { 

}
method(`|`, list(new_S3_class("ggplot"), new_S3_class("ggsurvfit"))) <- function(e1, e2) { 

}
ddsjoberg commented 8 months ago

This works! Next steps:

library(S7)
library(ggsurvfit)
#> Loading required package: ggplot2

ggsurvfit_S7 <- 
  new_class("ggsurvfit_S7", properties = list(obj = new_S3_class("ggsurvfit"))
)

method(`|`, list(ggsurvfit_S7, new_S3_class("ggplot"))) <- function(e1, e2) { 
  "This is ggsurvfit|ggplot"
}

method(`|`, list(new_S3_class("ggplot"), ggsurvfit_S7)) <- function(e1, e2) { 
  "This is ggplot|ggsurvfit"
}

method(`|`, list(ggsurvfit_S7, ggsurvfit_S7)) <- function(e1, e2) { 
  "This is ggsurvfit|ggsurvfit"
}

plot1 <- ggsurvfit_S7(
  obj = survfit2(Surv(time, status) ~ sex, data = df_lung) |> 
    ggsurvfit() +
    add_risktable()
)

plot2 <- ggplot(mtcars, aes(mpg, cyl)) + geom_point()

plot1 | plot2
#> [1] "This is ggsurvfit|ggplot"

plot2 | plot1
#> [1] "This is ggplot|ggsurvfit"

plot1 | plot1
#> [1] "This is ggsurvfit|ggsurvfit"

method(`|`, list(ggsurvfit_S7, ggsurvfit_S7)) <- function(e1, e2) { 
  patchwork::wrap_elements(ggsurvfit_build(e1@obj)) | patchwork::wrap_elements(ggsurvfit_build(e2@obj))
}
#> Overwriting method |(<ggsurvfit_S7>, <ggsurvfit_S7>)

plot1 | plot1

Created on 2023-10-10 with reprex v2.0.2

ddsjoberg commented 8 months ago

UPDATE FOR THE FUTURE DANIEL TO HOPEFULLY NOT FORGET!

  1. Implementing |.ggsurvfit methods does not work because it will only work correctly for 2 plots. A third plot would then be class ggplot | ggsurvfit and will fail

    `|.ggsurvfit` <- function(e1, e2) {
    patchwork::wrap_elements(ggsurvfit_build(e1)) |
    patchwork::wrap_elements(ggsurvfit_build(e2))
    }
  2. I have decided not to use S7, because I would just like the ggsurvfit objects to seamlessly use any S3 method defined for ggplots. If I were to make ggsurvift objects S7, I would need to re-implement all S3 methods meant for ggplot obejcts. While this is feasible for the methods exports by ggplot (notably, +.gg), any extension package that uses S3 methods would no longer work for ggsurvfit objects.

I am hopeful that patchwork will green-light the new generic function suggested in https://github.com/thomasp85/patchwork/issues/310 . Thomas added the feature tag a couple of months ago, hopefully signaling he's open to the suggestion 🤞🏼

teunbrand commented 5 months ago

Hi Daniel,

I just watched your presentation on ggsurfvit and it was very well presented! If this risk table is giving you grief with patchwork, I have an alternative idea that you might entertain for a bit. The ggplot2 guide update is upon us, so when it hits the fan, why not implement the risk table as an axis guide? I'm totally unqualified to opine on the risk table itself and what the specs are, but custom axes should just gently weave themselves into patchwork.

ddsjoberg commented 5 months ago

Ahhh, very interesting! I need to read up on the new features for the guides! Maybe we can set up a time to chat about the details soon?

I couple of potential issues come to mind:

  1. The numbers in the risk table must align with tick marks. Will it be tricky to to make that happen?
  2. We need to risktable to to remain below the figure. {patchwork} does an awesome job of gathering the guides for the treatment groups. Will it also try to gather up the risktables?

Anyway, would love to chat!

teunbrand commented 5 months ago

The numbers in the risk table must align with tick marks. Will it be tricky to to make that happen?

Nah, you'd be surprised to learn that axis infrastructure is sorta geared towards aligning things at the tick marks 😅

We need to risktable to to remain below the figure. {patchwork} does an awesome job of gathering the guides for the treatment groups. Will it also try to gather up the risktables?

No, patchwork only mops up the non-position guides. Axes should remain in place.

Chat sounds great, just drop me an email if you want to propose dates/times!