search

chjackson / flexsurv

The flexsurv R package for flexible parametric survival and multi-state modelling
http://chjackson.github.io/flexsurv/
55 stars 27 forks source link

Update to "tidy" format for predictions #155

Closed hfrick closed 1 year ago

hfrick commented 1 year ago

In parsnip and censored, we now refer to the time at which to calculate predicted survival probability (or hazard) as "evaluation time". This is reflected in a change in column name of the output of predict(): the column .time is now called .eval_time.

We've been using your predict() methods basically out of the box because @mattwarkentin made the output fit with tidymodels principals like a glove! Would you consider updating the column name?

R CMD check fails for me locally with an error in creating a vignette but the tests pass fine.

chjackson commented 1 year ago

OK - I've applied this, under the assumption that the idea of this function is to make flexsurv compatible with tidymodels, hence the community of people who use this function are willing to keep up with changes in tidymodels!

hfrick commented 1 year ago

Thank you! That's really helpful. Happy to provide PRs for this sort of "maintenance".

chjackson commented 6 months ago

Hi @hfrick and @mattwarkentin. I'd like to update flexsurv on CRAN, but it's currently failing the reverse dependencies check for censored, which I think is assuming that this column in the predict output is still called .time.

What would you suggest? I don't want to interfere with the release schedule for tidymodels, but it's been over a year since the last flexsurv CRAN release and there's been lots of changes.

Do you think it'd be reasonable to provide both .eval.time and .time in the output of predict.flexsurvreg, document that the use of .time is deprecated, then withdraw it after the censored package is next updated?

hfrick commented 5 months ago

@chjackson I've prepared a PR (linked above) to adapt censored to dev flexsurv. censored was renaming the column from .time to .eval_time without checking if it existed first. So now it does check for the flexsurv version number before doing that.

I can send that in as a patch release now or you send flexsurv first and I'll send censored in right after. Do you have a preference?