Create formula interface for RMST test

[jdblischak]

Closes #189

xref: #188 (comment)

I added a simple formula interface, rmst.formula, and converted rmst() to an S3 generic function. I say simple because it only extracts the variable names from the formula, and then uses these to call rmst.default(). Converting to the S3 generic required changing the first argument name to something that could stand for a data frame or a formula. I chose x since this is a common S3 pattern (eg see ?aggregate). The main downside I see of making rmst() an S3 generic is that its first argument is no longer data like all the other test functions like wlr() and maxcombo(). UPDATE: After feedback, I instead added an additional argument formula = NULL to the existing rmst() function

Please give it a try and let me know how it can be improved.

[LittleBeannie]

Hello John, I am thrilled to see the remarkable progress that has been achieved! I only have one minor comment to add. This topic would make an excellent presentation for Friday's meeting, where we can gather input and suggestions regarding the formula choice between month ~ event + trt or Surv(month, event, trt).

Thank you for your diligent efforts in moving this forward. It's truly outstanding work. I am fine with merging this pull request first, considering the numerous changes we already have in place.

[LittleBeannie]

I have some concerns regarding whether this formula makes statistical sense. Typically, the ~ symbol is used when fitting a linear model, as in y ~ x1 + x2. However, in the context of rmst(), where the object is a survival object, I am wondering if something like a Surv(month, event, trt) object would be more appropriate and meaningful.

Reference: https://www.rdocumentation.org/packages/survival/versions/2.11-4/topics/Surv

[jdblischak]

That example is to demonstrate that only the order matters. month ~ evntd + trt is equivalent to Surv(month, event, trt). I'm open to updating the documentation in order to make this more clear to end users. I did my best to explain in the Details section that the formula syntax chosen is purely to indicate intent.

[jdblischak]

I attempted to address your feedback in 3ccb493

[LittleBeannie]

Thanks, John!

[nanxstats]

This is great start. Just a few design and style comments that can be easily addressed once the decision is made:

1. I think data should be the first argument, Hadley also has a similar opinion.
2. Would it make sense to name the formula argument as formula instead of x, to be obvious, and to be consistent with other model fitting packages?
3. Make the argument only accept formula, instead of creating a dependency between arguments.
4. If we enforce a single, strict formula syntax that is consistent with the conventions, I think it would encourage people to write correct code. For example, Surv(time, event) ~ treatment.
   Although we can still interpret it not literally, by only using the variable names, like your current implementation.
   tidymodels/censored also has thoughts on related issues.

[jdblischak]

1. I think data should be the first argument, Hadley also has a similar opinion.
2. Would it make sense to name the formula argument as formula instead of x, to be obvious, and to be consistent with other model fitting packages?
3. Make the argument only accept formula, instead of creating a dependency between arguments.

These are all objections to making rmst() an S3 generic. The alternative would be to have two separate functions. Your bullet points describe essentially what I had in the first commit of this PR, 0bbdaa4, prior to converting to an S3 generic

4. If we enforce a single, strict formula syntax that is consistent with the conventions, I think it would encourage people to write correct code. For example, Surv(time, event) ~ treatment.
   Although we can still interpret it not literally, by only using the variable names, like your current implementation.
   tidymodels/censored also has thoughts on related issues.

I worry about the implementation. Dealing with formulas/language objects is difficult to do robustly. I think the end users need to know that Surv() is not executed. For example, what if a user used the named arguments and put them out of order, eg Surv(event = x, time2 = y, time = z). This would be expected to work, but would fail in our setup.

[LittleBeannie]

Looks great to me, thanks!

[nanxstats]

Looking great! Thanks for the update that simplifies things 💯