From f027af31eff054fa57a21b48fbc26e1f28604ed0 Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Thu, 4 Apr 2024 12:24:46 -0700 Subject: [PATCH 1/5] updates --- DESCRIPTION | 2 +- NEWS.md | 4 + R/tidy_survfit.R | 12 +- .../_snaps/add_risktable/sf-negative-time.svg | 351 ++++++++++++ .../add_risktable/sf-sf-start-time.new.svg | 515 ++++++++++++++++++ .../_snaps/add_risktable/sf-sf-start-time.svg | 357 ++++++++++++ tests/testthat/_snaps/tidy_survfit.md | 8 + tests/testthat/test-add_risktable.R | 22 + tests/testthat/test-tidy_survfit.R | 15 +- 9 files changed, 1280 insertions(+), 6 deletions(-) create mode 100644 tests/testthat/_snaps/add_risktable/sf-negative-time.svg create mode 100644 tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg create mode 100644 tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg create mode 100644 tests/testthat/_snaps/tidy_survfit.md diff --git a/DESCRIPTION b/DESCRIPTION index 1cb7570..b4fa794 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: ggsurvfit Title: Flexible Time-to-Event Figures -Version: 1.0.0.9000 +Version: 1.0.0.9001 Authors@R: c( person("Daniel D.", "Sjoberg", , "danield.sjoberg@gmail.com", role = c("aut", "cre", "cph"), comment = c(ORCID = "0000-0003-0862-2018")), diff --git a/NEWS.md b/NEWS.md index d154107..92a0c83 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,10 @@ * Updated legend position syntax to account for changes in ggplot v3.5.0. +* The `tidy_survfit()` (and subsequently `ggsurvfit()`) now honor the `survfit(start.time)` if specified. (#192) + +* We now allow for negative follow-up times in `tidy_survfit()` (and subsequently `ggsurvfit()`). When negative follow-up times are present users should specify `survfit(start.time)` and we print a note to this effect when not set. (#192) + # ggsurvfit 1.0.0 * By default, a model plot created with `ggsurvfit()` or `ggcuminc()` uses the color aesthetic to plot curves by the stratifying variable(s), and further, `ggcuminc()` uses the linetype aesthetic for plots that contain multiple outcomes (i.e. competing events). We now introduce the global option `"ggsurvfit.switch-color-linetype"` to switch these defaults, giving users more flexibility over the output figures. Furthermore, when the `linetype_aes=` argument is called in a situation when it does not apply, it will be silently ignored (previously, an error message _may_ have been thrown). (#166) diff --git a/R/tidy_survfit.R b/R/tidy_survfit.R index 0cf0f97..2cbd6e0 100644 --- a/R/tidy_survfit.R +++ b/R/tidy_survfit.R @@ -39,13 +39,17 @@ tidy_survfit <- function(x, } if (inherits(x, "survfitms")) type <- "cuminc" else if (is.character(type)) type <- match.arg(type) - if (!is.null(times) && any(times < 0)) { - cli_abort("The {.var times} cannot be negative.") - } + # create base tidy tibble ---------------------------------------------------- + if (is.null(x$start.time) && min(x$time) < 0) { + cli::cli_inform(c( + "!" ="Setting start time to {.val {min(x$time)}}.", + "i" = "Specify {.code ggsurvfit::survfit2(start.time)} to override this default." + )) + } df_tidy <- - survival::survfit0(x, start.time = 0) %>% + survival::survfit0(x) %>% broom::tidy() # if a competing risks model, filter on the outcome of interest diff --git a/tests/testthat/_snaps/add_risktable/sf-negative-time.svg b/tests/testthat/_snaps/add_risktable/sf-negative-time.svg new file mode 100644 index 0000000..b679b73 --- /dev/null +++ b/tests/testthat/_snaps/add_risktable/sf-negative-time.svg @@ -0,0 +1,351 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +0.25 +0.50 +0.75 +1.00 + + + + + + + + + + + + + + + + +-10 +0 +10 +20 + + + + + + + + +Time + + + + +Survival Probability + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +sf-negative_time + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +228 +0 +87 +103 +24 +148 +3 +165 + + + + + + + + + + + + + + + + + + + + + + + + +Events +At Risk + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg b/tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg new file mode 100644 index 0000000..5276210 --- /dev/null +++ b/tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg @@ -0,0 +1,515 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +0.25 +0.50 +0.75 +1.00 + + + + + + + + + + + + + + + + + +10 +15 +20 +25 +30 + + + + + + + + +Time + + + + +Survival Probability + + + + + + + + + + + + + + + + + + + + + +sex=Male +sex=Female + + + + + + + + + + + + + + + + +sf-sf_start_time + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +44 +0 +24 +17 +13 +27 +7 +33 +2 +36 + + + + + + + + + + + + + + + + + + + + + + + + +Events +At Risk + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +sex=Male + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +43 +0 +22 +14 +11 +18 +3 +25 +1 +26 + + + + + + + + + + + + + + + + + + + + + + + + +Events +At Risk + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +sex=Female + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg b/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg new file mode 100644 index 0000000..4663089 --- /dev/null +++ b/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg @@ -0,0 +1,357 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +0.25 +0.50 +0.75 +1.00 + + + + + + + + + + + + + + + + + +10 +15 +20 +25 +30 + + + + + + + + +Time + + + + +Survival Probability + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +sf-sf_start_time + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +87 +0 +46 +31 +24 +45 +10 +58 +3 +62 + + + + + + + + + + + + + + + + + + + + + + + + +Events +At Risk + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testthat/_snaps/tidy_survfit.md b/tests/testthat/_snaps/tidy_survfit.md new file mode 100644 index 0000000..4baf173 --- /dev/null +++ b/tests/testthat/_snaps/tidy_survfit.md @@ -0,0 +1,8 @@ +# tidy_survfit() messaging + + Code + invisible(tidy_survfit(survfit(Surv(time - 500, status) ~ 1, df_lung))) + Message + ! Setting start time to -499.835728952772. + i Specify `ggsurvfit::survfit2(start.time)` to override this default. + diff --git a/tests/testthat/test-add_risktable.R b/tests/testthat/test-add_risktable.R index 15e5ba2..1f70f92 100644 --- a/tests/testthat/test-add_risktable.R +++ b/tests/testthat/test-add_risktable.R @@ -298,3 +298,25 @@ test_that("add_risktable() works with Cox models", { ) }) +test_that("add_risktable() works with ggsurvfit() `start.time` and negative times", { + expect_error( + sf_negative_time <- + survfit(Surv(time - 10, status) ~ 1, df_lung, start.time = -10) |> + ggsurvfit() + + add_risktable(), + NA + ) + + expect_error( + sf_start_time <- + survfit(Surv(time, status) ~ sex, df_lung, start.time = 10) |> + ggsurvfit() + + add_risktable(), + NA + ) + + skip_on_ci() + vdiffr::expect_doppelganger("sf-negative_time", sf_negative_time) + vdiffr::expect_doppelganger("sf-sf_start_time", sf_start_time) +}) + diff --git a/tests/testthat/test-tidy_survfit.R b/tests/testthat/test-tidy_survfit.R index 3bef440..af07e37 100644 --- a/tests/testthat/test-tidy_survfit.R +++ b/tests/testthat/test-tidy_survfit.R @@ -207,4 +207,17 @@ test_that("tidy_survfit() works with multi-state models", { ) }) - +test_that("tidy_survfit() messaging", { + # expect a message about the start time when there are negative times and no start.time + expect_snapshot( + survfit(Surv(time - 500, status) ~ 1, df_lung) |> + tidy_survfit() |> + invisible() + ) + # no longer see message when start.time specified + expect_invisible( + survfit(Surv(time - 500, status) ~ 1, df_lung, start.time = -500) |> + tidy_survfit() |> + invisible() + ) +}) From 9c06e4417b7678dc46c294f6b75244edd3cd0179 Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Thu, 4 Apr 2024 12:51:37 -0700 Subject: [PATCH 2/5] Update test-tidy_survfit.R --- tests/testthat/test-tidy_survfit.R | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat/test-tidy_survfit.R b/tests/testthat/test-tidy_survfit.R index af07e37..5a9d4af 100644 --- a/tests/testthat/test-tidy_survfit.R +++ b/tests/testthat/test-tidy_survfit.R @@ -84,7 +84,6 @@ test_that("tidy_survfit() works with survfit2()", { test_that("tidy_survfit() throws appropriate errors", { expect_error(tidy_survfit(mtcars)) - expect_error(tidy_survfit(sf1, times = -5:5)) expect_error(tidy_survfit(sf1, type = "not_a_type")) }) From 1063bb6e8f7886cceb854cffeb1850054fa41d95 Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Thu, 4 Apr 2024 14:49:54 -0700 Subject: [PATCH 3/5] test updates --- .../_snaps/add_risktable/sf-sf-start-time.svg | 357 ------------------ ...f-start-time.new.svg => sf-start-time.svg} | 2 +- tests/testthat/test-add_risktable.R | 2 +- 3 files changed, 2 insertions(+), 359 deletions(-) delete mode 100644 tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg rename tests/testthat/_snaps/add_risktable/{sf-sf-start-time.new.svg => sf-start-time.svg} (99%) diff --git a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg b/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg deleted file mode 100644 index 4663089..0000000 --- a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.svg +++ /dev/null @@ -1,357 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -0.25 -0.50 -0.75 -1.00 - - - - - - - - - - - - - - - - - -10 -15 -20 -25 -30 - - - - - - - - -Time - - - - -Survival Probability - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -sf-sf_start_time - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -87 -0 -46 -31 -24 -45 -10 -58 -3 -62 - - - - - - - - - - - - - - - - - - - - - - - - -Events -At Risk - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg b/tests/testthat/_snaps/add_risktable/sf-start-time.svg similarity index 99% rename from tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg rename to tests/testthat/_snaps/add_risktable/sf-start-time.svg index 5276210..9168eb6 100644 --- a/tests/testthat/_snaps/add_risktable/sf-sf-start-time.new.svg +++ b/tests/testthat/_snaps/add_risktable/sf-start-time.svg @@ -202,7 +202,7 @@ -sf-sf_start_time +sf-start_time diff --git a/tests/testthat/test-add_risktable.R b/tests/testthat/test-add_risktable.R index 1f70f92..8eaf9db 100644 --- a/tests/testthat/test-add_risktable.R +++ b/tests/testthat/test-add_risktable.R @@ -317,6 +317,6 @@ test_that("add_risktable() works with ggsurvfit() `start.time` and negative time skip_on_ci() vdiffr::expect_doppelganger("sf-negative_time", sf_negative_time) - vdiffr::expect_doppelganger("sf-sf_start_time", sf_start_time) + vdiffr::expect_doppelganger("sf-start_time", sf_start_time) }) From 02fb6f5707e17005cca5b31925b3642510f013a9 Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Thu, 4 Apr 2024 15:04:55 -0700 Subject: [PATCH 4/5] Update test-add_risktable.R --- tests/testthat/test-add_risktable.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-add_risktable.R b/tests/testthat/test-add_risktable.R index 8eaf9db..149a74a 100644 --- a/tests/testthat/test-add_risktable.R +++ b/tests/testthat/test-add_risktable.R @@ -301,7 +301,7 @@ test_that("add_risktable() works with Cox models", { test_that("add_risktable() works with ggsurvfit() `start.time` and negative times", { expect_error( sf_negative_time <- - survfit(Surv(time - 10, status) ~ 1, df_lung, start.time = -10) |> + survfit(Surv(time - 10, status) ~ 1, df_lung, start.time = -10) %>% ggsurvfit() + add_risktable(), NA @@ -309,7 +309,7 @@ test_that("add_risktable() works with ggsurvfit() `start.time` and negative time expect_error( sf_start_time <- - survfit(Surv(time, status) ~ sex, df_lung, start.time = 10) |> + survfit(Surv(time, status) ~ sex, df_lung, start.time = 10) %>% ggsurvfit() + add_risktable(), NA From 87fd3be2b651ab60c41dbdd48a524a0aa085b81e Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Thu, 4 Apr 2024 15:13:15 -0700 Subject: [PATCH 5/5] removing base R pipe --- tests/testthat/_snaps/tidy_survfit.md | 2 +- tests/testthat/test-tidy_survfit.R | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/testthat/_snaps/tidy_survfit.md b/tests/testthat/_snaps/tidy_survfit.md index 4baf173..50c4970 100644 --- a/tests/testthat/_snaps/tidy_survfit.md +++ b/tests/testthat/_snaps/tidy_survfit.md @@ -1,7 +1,7 @@ # tidy_survfit() messaging Code - invisible(tidy_survfit(survfit(Surv(time - 500, status) ~ 1, df_lung))) + survfit(Surv(time - 500, status) ~ 1, df_lung) %>% tidy_survfit() %>% invisible() Message ! Setting start time to -499.835728952772. i Specify `ggsurvfit::survfit2(start.time)` to override this default. diff --git a/tests/testthat/test-tidy_survfit.R b/tests/testthat/test-tidy_survfit.R index 5a9d4af..b46e804 100644 --- a/tests/testthat/test-tidy_survfit.R +++ b/tests/testthat/test-tidy_survfit.R @@ -209,14 +209,14 @@ test_that("tidy_survfit() works with multi-state models", { test_that("tidy_survfit() messaging", { # expect a message about the start time when there are negative times and no start.time expect_snapshot( - survfit(Surv(time - 500, status) ~ 1, df_lung) |> - tidy_survfit() |> + survfit(Surv(time - 500, status) ~ 1, df_lung) %>% + tidy_survfit() %>% invisible() ) # no longer see message when start.time specified expect_invisible( - survfit(Surv(time - 500, status) ~ 1, df_lung, start.time = -500) |> - tidy_survfit() |> + survfit(Surv(time - 500, status) ~ 1, df_lung, start.time = -500) %>% + tidy_survfit() %>% invisible() ) })