Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add estimate_KM.formula() method #379

Closed
ddsjoberg opened this issue May 10, 2022 · 6 comments · Fixed by #390
Closed

Add estimate_KM.formula() method #379

ddsjoberg opened this issue May 10, 2022 · 6 comments · Fixed by #390
Assignees
Labels
issue Used for project filtering
Milestone

Comments

@ddsjoberg
Copy link
Collaborator

The visualizations for time to event data in visR are amazing and will be used by many people, including those whose data does not follow CDISC conventions. We can do a couple of things to improve their experience, while not taking away from the experience of those working with CDISC data.

One HUGE difference between most statistical coding and CDISC is the CNSR column: it's the opposite of what many will expect. TO aid these users, adding a formula method to estimate_KM() will allow non-CDISC users to use familiar formula and data syntax, while the CDISC users can continue to utilize the helpful CDISC defaults.

The estimate_KM.formula() method will simply parse the formula, and pass the elements to estamate_KM(data, AVAL, CNSR, strata); a simple function adding great usability to many more users.

@ddsjoberg ddsjoberg added the issue Used for project filtering label May 10, 2022
@ddsjoberg ddsjoberg added this to the v0.3.0 milestone May 11, 2022
@ddsjoberg ddsjoberg self-assigned this May 11, 2022
@SHAESEN2
Copy link
Collaborator

Our main focus is pharma and working with analysis data standards so I would prefer to keep the behavior as is. I don't see how the addition of a new method would improve the user experience. Perhaps you can make a small example?

1 similar comment
@SHAESEN2
Copy link
Collaborator

Our main focus is pharma and working with analysis data standards so I would prefer to keep the behavior as is. I don't see how the addition of a new method would improve the user experience. Perhaps you can make a small example?

@ddsjoberg
Copy link
Collaborator Author

I am suggesting to keep ALL behavior as is, i.e. still focusing on pharma standards. Adding a formula method only improves usability for non-CDISC data.

I don't see how the addition of a new method would improve the user experience.

The formula method is already familiar to every R user who does survival analyses, and the users won't need to learn new syntax. The CDISC format codes the outcome in the opposite way as every other survival analysis method in R. This is dangerous when users are not using CDSIC data!

We would keep the original function un-altered as a data frame method.

estimate_KM.data.frame <- function(data = NULL, strata = NULL, CNSR = "CNSR", AVAL = "AVAL", ...){}

And add a formula method, so users have the option to also use the familiar formula syntax

estimate_KM.formula <- function(formula=, data = NULL, ...){}

I have a function draft of the update in the formula_methods branch

@SHAESEN2
Copy link
Collaborator

I see. Perhaps for a next CRAN release then? I would prefer to first to get the other issues resolved.

@ddsjoberg
Copy link
Collaborator Author

Let's discuss next week. I agree with you about resolving the other issues first. The code to support formula notation is simple and easily integrated. Definitely something we can finish before the CRAN release (which doesn't have a date!).

We could implement with a formula method (as described above). Or we could simple add a formula argument to the existing function. A screenshot of the help files illustrates the proposed changes well.

  1. Add a formula method:
    image

  2. Add a formula argument. The argument defaults to NULL. If users specify it, then the formula is used, rather constructed from the AVAL, CNSR, and strata arguments.
    image

Or something like this
image

@ddsjoberg
Copy link
Collaborator Author

  • implement with formula method @ddsjoberg
  • also look at vignette converting data to CDISC format. Update that code to use formula method. @bailliem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue Used for project filtering
Projects
None yet
2 participants