Add a type = "cloglog" for log minus log survival plots
#195
Conversation
|
Thank you for the PR!! I will likely to get to this next week. |
|
it looks like there are some test failures. can you take a look at the details? |
|
FYI, I have just heard from the maintainer of the survival package that they are planning a release that has breaking changes in ggsurvfit. This means that I will prepping a new release soon. If you want to address the failures in the checks in the next week, we may be able to add this to the next release. Let me know if you have questions. Thanks, Daniel |
|
Hi Daniel, I'm a physician, and my coding/GitHub/package development skills are pretty basic. Despite my best efforts, I am struggling with the remaining test failures. I have managed to address some, but others are beyond my ability to resolve efficiently. I'm reaching out to see if someone within the community could assist in refining these fixes. I believe the updates would be beneficial, especially for users conducting medical research. Any support or direction on how to move forward with these specific issues would be greatly appreciated. Thank you again for your patience and for considering these modifications. |
R/ggsurvfit.R
Outdated
| @@ -73,13 +73,17 @@ ggsurvfit <- function(x, type = "survival", | |||
| # prep data to be passed to ggplot() ----------------------------------------- | |||
| df <- | |||
| tidy_survfit(x = x, type = type) %>% | |||
| dplyr::filter(type != "cloglog" | .data$time > 0) %>% | |||
There was a problem hiding this comment.
@avishaitsur can you explain what the purpose of this addition is? Thank you!
There was a problem hiding this comment.
This is the line that is causing the GitHub Actions Checks to fail, and I don't understand why/if it's needed.
There was a problem hiding this comment.
The transformation in log minus log survival is cloglog = function(y) log(-log(y)).
In time 0, by definition, the survival y is 1, and therefore, the result is undefined (log of 1 is 0, and log of -0 is undefined).
For this reason, the transformation and the visual do not apply to time 0.
There was a problem hiding this comment.
I wonder the best way to handle this as survfit() allows negative times. 🤔
There was a problem hiding this comment.
The transformation and the visual are implemented in the survival and survminer packages. Maybe we could explore their solutions?
There was a problem hiding this comment.
worst case, we get a NaN or Inf value and it won't appear on the plot.
I am going to go ahead and merge this now (assuming the current tests complete without error). I am also going to make a release
|
FYI I removed the call to |
What changes are proposed in this pull request?
Add a
type = "cloglog"for log minus log survival plots totidysurvfit()andggsurvfit(). These plots are widely used to assess the assumption of proportional hazards before running a Cox regression model and are implemented in thesurvivalandsurvminerpackages.If there is an GitHub issue associated with this pull request, please provide link.
closes #194
Reviewer Checklist (if item does not apply, mark as complete)
renv::install()_pkgdown.ymlpkgdown::build_site(). Check the R console for errors, and review the rendered website.withr::with_envvar(list(CI = TRUE), code = devtools::test_coverage()). Begin in a fresh R session without any packages loaded.usethis::use_spell_check()runs with no spelling errors in documentationWhen the branch is ready to be merged into master:
NEWS.mdwith the changes from this pull request under the heading "# ggsurvfit (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.mdfor examples).usethis::use_version(which = "dev")usethis::use_spell_check()again