-
Notifications
You must be signed in to change notification settings - Fork 31
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
Adding agument estimate_KM(formula=)
#390
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great but needs more work for UAT. Also the documentation needs to highlight that our main focus is data standards. We tolerate non-standard formats through the formula argument, not the other way around :)
#' | ||
#' @description This function is a wrapper around \code{survival::survfit.formula} to perform a Kaplan-Meier analysis, assuming right-censored data. | ||
#' The function expects that the data has been filtered on the parameter (PARAM/PARAMCD) of interest. All NA values in the CNSR, AVAL and strata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be kept. You can generalize it if you prefer, but it is important information for the programmer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this text to a section in the help file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll recheck later this week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One proposal is to mention CDISC compliant data model and explicitly ADTTE and how the function leverages it in the decription. For example, we could add the following qualifer:
The function is designed to leverage the \link[CDISC ADaM ADTTE data model](https://www.cdisc.org/standards/foundational/adam/adam-basic-data-structure-bds-time-event-tte-analyses-v1-0) and more explicity the conventions and controlled vocabulary of the data model. Functionality is also supplied to handle non ADTTE compiant data sets.
@@ -17,15 +14,19 @@ | |||
#' | |||
#' @seealso \code{\link[survival]{survfit.formula} \link[survival]{survfitCI}} | |||
#' | |||
#' @param data The name of the dataset for Time-to-Event analysis based on the Analysis Data Model (ADaM) principles. The dataset is expected to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be kept. Our main focus still is data standards. Non-standard data is the exception that we are allowing for, not what we design for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CDISC is mentioned 1 million times in the documentation: there is no missing is. This argument now applies to both non-CDISC data and CDISC, and there is no value added adding details about CDISC here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is added value for statistical programmers. They like things explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 5 references to CDISC data in this single help file. The line directly below is "default values follow the naming conventions in CDISC." This is quite explicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above - I think a statement at the start then is enough throughout the rest of the help file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An update to the vignette - one that is not about CDISC - should be sufficient IMO.
} | ||
} | ||
else if (!inherits(formula, "formula")) { | ||
stop("Argument `formula=` must be class 'formula'.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you include a requirement test for this: 1. That we allow for the argument. That the argument is tested, the argument cancels out aval and cnsr, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll recheck later this week.
tests/testthat/test-estimate_KM.R
Outdated
@@ -43,6 +43,9 @@ | |||
#' T7.2 The function prefixes the function call with survival | |||
#' T8. Piped datasets still return accurate results | |||
#' T8.1 Piped datasets still return accurate results | |||
#' T9. The user can specify formula argument | |||
#' T9.1 The formula method returns the same results and the data method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rephrase as we dont work with methods anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll recheck later this week.
tests/testthat/test-estimate_KM.R
Outdated
@@ -43,6 +43,9 @@ | |||
#' T7.2 The function prefixes the function call with survival | |||
#' T8. Piped datasets still return accurate results | |||
#' T8.1 Piped datasets still return accurate results | |||
#' T9. The user can specify formula argument | |||
#' T9.1 The formula method returns the same results and the data method. | |||
#' T9.2 The formula method triggers error messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? what does this mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll recheck later this week.
@@ -43,6 +43,9 @@ | |||
#' T7.2 The function prefixes the function call with survival | |||
#' T8. Piped datasets still return accurate results | |||
#' T8.1 Piped datasets still return accurate results | |||
#' T9. The user can specify formula argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw more things you implemented as safety nets and requirements eg when the string is empty you put the formula to NULL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You will have to review above testing to be more specific eg we say "The function relies on the presence of two numeric variables, specified through
AVAL
andCNSR
, to be present indata
" but this is now only true when the formula is NULL. - how are we testing the presence of the actual variables inside the formula argument? Are the NAs also removed from those variables, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eg when the string is empty you put the formula to NULL.
i am not sure what you're speaking to, this is not done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will have to review above testing to be more specific eg we say "The function relies on the presence of two numeric variables, specified through
AVAL
andCNSR
, to be present indata
" but this is now only true when the formula is NULL.
formula is marked experimental...no need to update these in case implementation is changed
how are we testing the presence of the actual variables inside the formula argument? Are the NAs also removed from those variables, ...
test added to ensure error. NAs are removed.
All comments have been addressed.
The documentation already heavily highlights CDSIC format, and the formula argument is not merely "tolerated": it will in fact be the method the vast majority of the users use to interact with this function. |
@bailliem do you want to provide any further review before the PR is merged? |
#' | ||
#' @description This function is a wrapper around \code{survival::survfit.formula} to perform a Kaplan-Meier analysis, assuming right-censored data. | ||
#' The function expects that the data has been filtered on the parameter (PARAM/PARAMCD) of interest. All NA values in the CNSR, AVAL and strata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One proposal is to mention CDISC compliant data model and explicitly ADTTE and how the function leverages it in the decription. For example, we could add the following qualifer:
The function is designed to leverage the \link[CDISC ADaM ADTTE data model](https://www.cdisc.org/standards/foundational/adam/adam-basic-data-structure-bds-time-event-tte-analyses-v1-0) and more explicity the conventions and controlled vocabulary of the data model. Functionality is also supplied to handle non ADTTE compiant data sets.
@@ -17,15 +14,19 @@ | |||
#' | |||
#' @seealso \code{\link[survival]{survfit.formula} \link[survival]{survfitCI}} | |||
#' | |||
#' @param data The name of the dataset for Time-to-Event analysis based on the Analysis Data Model (ADaM) principles. The dataset is expected to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above - I think a statement at the start then is enough throughout the rest of the help file.
@@ -17,15 +14,19 @@ | |||
#' | |||
#' @seealso \code{\link[survival]{survfit.formula} \link[survival]{survfitCI}} | |||
#' | |||
#' @param data The name of the dataset for Time-to-Event analysis based on the Analysis Data Model (ADaM) principles. The dataset is expected to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An update to the vignette - one that is not about CDISC - should be sufficient IMO.
Lets discuss today. I proposed one clarification at the start of the description to make it clear that an aim is to leverage a standard data model. But we also accomodation other data. Given the conventions in the survival package, I think this functionality adds value to the package. Maybe we can provide a clearer vignette that goes through this. I think this will be helpful also when we are working with ADAE data and want to do a TTE or competing risks analysis and ADTTE is not set up for it. |
* Wrote and executed watchdog * Add 'thevalidatoR' gh-action to generate a report on the release see https://github.com/marketplace/actions/r-package-validation-report * fixed formatting * Prototype * reformatting * major cleanup * added more sanity checks * Implemented compatibility for y-axis transformations and wrote some tests * More tests and now robust to fun=log * now robust against all function input? * Changed behaviour so that test aso works on CRAN and not only local * Added tests for incorrect arguments * Minor formatting changes * Dummy for the auto-deploy to shinyapps * Update deploy_to_shinyapps.yml * Update DESCRIPTION * Implemented Stevens suggestions * Removed parcats from imports, moved DT and gt to suggests, updated mans * align_plots: make more user friendly * add_risktabel: adjust ylabel according to increase in 4th column in grob of graph * Updated testing * - split visr testing file for each method - added more tests for visr survfit - updated visR to align with expectations - default method should not be a test. It should provide a default way of plotting objects * Camelcase to snakecase; minor formatting; initial vdiffr * forgot to save vdiffr changes * fixed bug since bools get translated to 0/1 which gets forwarded to base::plot and doesn't fail * Fixed formatting for CRAN compatibility * Fixed bug in testing * in progress * Update estimate_CUMINC.R * working on merge conflict * Wrote and executed watchdog * fixed formatting * reformatting * major cleanup * Implemented Stevens suggestions * working on merge conflict * Delete deploy_to_shinyapps.yml * package check update * in progress * Update check-standard.yaml * updates * Update check-standard.yaml * Update estimate_CUMINC.R * updates * Update DESCRIPTION * Update DESCRIPTION * Update DESCRIPTION * updates after review * in progress * updates * doc updates * first draft complete * init * Update test-estimate_CUMINC.R * misc updates * doc updates * Delete .Rprofile * Update DESCRIPTION * fix, now working on tests * Converting output to data.frame now * Extended watchdog to append END OF CODE section at the end of the file * addition of END OF CODE sections * Changed commit-date to ISO format (YYYY-MM-DDTHH:MM:SS) * updates * Updated testing * updates after review * updates * Update DESCRIPTION * rename * re-document * Update test-estimate_CUMINC.R * minor edit on test * Removed glue dependency * Re-build README.Rmd * Updated test 6.4 * Updated last-update-when-header * Attempt to fix riskmetric score in readme * Update README.Rmd * Attempt to fix riskmetric score in Readme * Update README.Rmd * Update README.Rmd * Removed unused snaps * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Update README.Rmd * Update README.Rmd * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Update README.Rmd * Update README.Rmd * Update test-coverage.yaml * Update rebuild-readme.yaml * Update README.Rmd * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Update rebuild-readme.yaml * Re-build README.Rmd * Update rebuild-readme.yaml * Update README.Rmd * Update test-coverage.yaml * Fixed small error in numbering of tests * Adding cumulative events and censored obs to survfit `*_risktable()` fns * Updated to align legend labels with strata specified * Re-build README.Rmd * remove tidyselect dependency * removing magrittr dep * don't reexport %||% * Regenerated man file for estimate_cuminc after rename * Update get_risktable.R * renamed cumulative.* to cum.* * Updated testing for PR310 * Included survfit object to tidyme output as attribute Included broom colname nomenclature in tidyme output * Re-build README.Rmd * update naming in visR as well * fixed bug * Updated documentation and added CRAN check \value tag exceptions to watchdog * Fixed typo in test, now package check is back at 0/0/0 * Fixed threeway test * Made new test less ambigous; minor formatting * Readress issue * adressed comments * Re-build README.Rmd * risktable updates * Re-build README.Rmd * rlang prefix updates * Re-build README.Rmd * adding images of rendered tableone() tables in help file * Re-build README.Rmd * Update README.Rmd * Re-build README.Rmd * Specified riskmetric version to avoid breaking 0.1.2 * Reverted useless change * Re-build README.Rmd * Integrate learnR vignette and shinyapps.io autodeploy (#339) * Update DESCRIPTION * Created deploy_to_shinyapps.yml * Changed branch to be deployed to develop * Added gt to suggests * Re-build README.Rmd * Added test for external data used in vignettes * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> * Added link target of shinyapps autodeploy * Adding R CMD Checks for older versions of R (#324) * workflow updates * Re-build README.Rmd * plot() fix * Fix broken code and updated vignettes * Update test-watchdog_testfiles.R * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: Tim Treis <tim.treis@outlook.de> * Build `README.Rmd` on main branch instead of all PRs (#342) * build on main instead of all PRs * Update rebuild-readme.yaml Co-authored-by: Tim Treis <tim.treis@outlook.de> * Changed R version to be shinyapps.io compatible * Reverted "build README on main" because bot can't push to write-protected branch * Updates to website (#345) * first draft of updated website * Update _pkgdown.yml * Updated NEWS with changelog (#349) * Updated NEWS with changelog Quick fixes for readme to remove contributor stats and cover stats (can be accessed through the badge) * fixed typo * fixed typo * Re-build README.Rmd * update ReadMe and trigger readme flow from inside pushed branch * Re-build README.Rmd * Implemented review and adjusted readme.RMD * Re-build README.Rmd * fixed readme * fixed readme * dunno * Re-build README.Rmd * Updated contributors * Re-build README.Rmd * Adressed review comments - Badges side-by-side - Updated NEWS Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: GitHub Actions <actions@github.com> * Create contributing (#326) * Create contributing * Re-build README.Rmd * Updated contributing * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Mark Baillie <bailliem@gmail.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * Updated the strata levels and label in `visr()` (#348) * updating default strata labels/headers * label updates * Re-build README.Rmd * Update DESCRIPTION * Update add_CNSR.R * Re-build README.Rmd * updates after review * doc update * Update test-estimate_KM.R * Re-build README.Rmd * Update estimate_cuminc.R * updates after review * doc updates * Re-build README.Rmd * review updates * more review updates * estimate_KM Updated tests to be more clear Updated comments to make more clear Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * NEWS entry update (#351) * updated news and increment version number * Update NEWS.md * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> * Create PR template (#330) * Create PULL_ * Re-build README.Rmd * Update PR template * Updated based on review * Re-build README.Rmd * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: Tim Treis <tim.treis@outlook.de> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Adding vignette example for estimate_cuminc() (#355) * adding vignette example for estimate_cuminc() * Re-build README.Rmd * Updated vignettes * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * Added update_documentation to quickly update everything and generate … (#356) * Added update_documentation to quickly update everything and generate documentation/visR.pdf Updated S3 method roxygen * Removed update_documentation implemented review comments * Increment version number to 0.2.0.9001 Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Updates to `class()` calls (#359) * class updates * Update test-watchdog_testfiles.R * Re-build README.Rmd * Further updates to class vs inherits * Delete visR.pdf Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * Review of Joanna testing (#361) * Review of Joanna testing estimate_cuminc -------------- - Initialize data argument - focus on arguments that are required for ADaM eg confint can be defined through ... Tidycmprisk should deal with confint variations - add more informative messages - Do we want to add an overall strata? We can put Overall instead of 1 inside the formula and then add a column strat99= Overall - %||% is not defined. When strata is NULL then the formula becomes ...... ~ NULL in the final object without any error => used ifelse statement testing ------- - we need to focus on what the wrapper is doing, not on what the tidycmprisk function -- test arguments -- test excecuting surrounding the main function -- do not test how the main function works - downstream processing of this object needs to be tested in the respective test files, not in the estimate function - we expect tidycmprisk to do the heavy lifting and testing on the final object. We only verify wether the wrapper does not change the object. If it do * Re-build README.Rmd * review updates * Update _pkgdown.yml * adding `visR::` prefix in examples * review updates from call * Re-build README.Rmd * update for conf.int * Re-build README.Rmd * redocument Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Test/add annotation cuminc updates (#364) * update T1 to include cuminc test * update T2 to include cuminc tests * update T3 to include cuminc * update T5 to include cuminc * update T6 * Fix T3.2 extracted_lbl * spell check updates (#372) * spell check updates * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> * Update README.Rmd by tableone (#369) * Update README.Rmd by tableone * Re-build README.Rmd * installing and rendering readme * Re-build README.Rmd * updates * Update README.md * Update NEWS.md * Increment version number to 0.2.0.9002 * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Added table.height to add_risktable (#353) * Added table.height to add_risktable * Re-build README.Rmd * unit tests were added * Complete tests + rename var + doc * Added small fixes * Delete test-estimate_CUMINC.R * fix doc reference * Re-build README.Rmd * Update rowgutter default value Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Update readme tableone display (#376) * update readme tableone display * Re-build README.Rmd * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> * Add na.rm option to remove the strata variable in rows (#368) * Add na.rm option to remove the strata variable in rows * update unit tests * Re-build README.Rmd * remove na.rm and applied strata filteration * Re-build README.Rmd * Update test-get_tableone.R * Update WORDLIST * increment version number * Re-build README.Rmd Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> Co-authored-by: GitHub Actions <actions@github.com> * Removing warning in `visr()` when PARAMCD not present (#380) * Update visr.R * Update test-visr_survfit.R * Re-build README.Rmd * documentation update * doc updates * increment version number * Re-build README.Rmd Co-authored-by: GitHub Actions <actions@github.com> * Replaced `survfit` call with quo (#366) * replace survfit call with quo * Re-build README.Rmd * Update test-get_pvalue.R * Re-build README.Rmd * Update get_COX_HR.R * Re-build README.Rmd * Update test-watchdog_testfiles.R * Re-build README.Rmd * small adjustments to improve readability * Re-build README.Rmd * updates from review * Re-build README.Rmd * Re-build README.Rmd * doc update * Re-build README.Rmd * Re-build README.Rmd * Added prefix for testthat. Updated documentation for estimate_KM. Removed warning param/paramcd. * Re-build README.Rmd * review updates * Re-build README.Rmd * increment version number Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * Adding unit tests with cumulative inc. (#387) * adding unit tests * Update get_risktable.Rd * visr survfit check (#389) * WIP: start documenting all functions in render.R * WIP: add roygen skeleton * WIP: remove export for all summarize functions * WIP: remove export for align_plots function * WIP: remove export from get_tableone function * WIP: remove export from get_summary function * WIP: remove export from lhs/pipe function * WIP: remove export for get_risktable and quantilte funcs * Remove lifecycle badge and text. Move remaining badges under installation. * Re-build README.Rmd * add lifecycle to description and experimental badge to render function * document royxgen headings for all functions in render.R. compile documentation. * export what is needed * export what is needed * Re-build README.Rmd * move badge location to under first section - * Re-build README.Rmd * adding lifecycle to wordlist * bump package increment * export * get_summary * align_plots * package bump * tidy up methods of the same s3 class and remove duplication in docs * Re-build README.Rmd * roll back some of the export deletions. trigging test failures and also required for vignettes, examples. * get_* * utils pipe * utilts table * Re-build README.Rmd * WIP: connect the s3 methods for summarize through document. * Tidy up tags * clean up typo and build docs * bump package add experimental tag to get_tableone * Adding agument `estimate_KM(formula=)` (#390) * adding formula argument * adding lifecycle badges * doc update * Re-build README.Rmd * Update test-estimate_KM.R * Re-build README.Rmd * cleaning up before requesting review * review comment updates * typo * Update estimate_KM.R * doc update * Re-build README.Rmd * updates * increment version number Co-authored-by: GitHub Actions <actions@github.com> * Updated to GH Actions V2 * Re-build README.Rmd * Add instruction to check if the readme has been rendered manually. (#404) * Add instruction to check if the readme has been rendered manually. * build readme * Updates from review estimate_KM (#405) * Updated estimate KM testing Updated estimate KM documentation to ensure to user knows that we allow non-CDISC. This was not clear. * Ran `usethis::use_tidy_description()` to put DESCRIPTION entries in standard order * docs * re-adding the error message that was removed. * docs Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com> * Documentation updates (#409) * doc updates * doc updates * Adding `Surv_CNSR()` function (#397) * Create Surv_CDISC.R * updates * doc update * Re-build README.Rmd * Update Surv_CDISC.R * Re-build README.Rmd * Update Surv_CDISC.R * Re-build README.Rmd * doc updates * Re-build README.Rmd * Update NAMESPACE * merge conflict fix * Re-build README.Rmd * updates * Re-build README.Rmd * Update test-Surv_CDISC.R * Re-build README.Rmd * readme * doc updates from review * doc updates * Update test-Surv_CDISC.R * Update testing: - consistent with CDISC philosophy - check the actual requirement - remove req T2.5 as this is not a requirement of our function, but something tested in the backend of survival * removed requirement 2.3 in TOC * change function name * Update testing Included CNSR info in estimate_KM to ensure user will restrict to 0/1 * re-documenting Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Mark Baillie <bailliem@gmail.com> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> * Update test-Surv_CNSR.R (#410) * fix issues associated with this function * issue fix * Add @nord to internal functions. (#412) Add questioning to functions under development / review * Added test for case when not all colours are defined. * Tiny bugfix, two more tests * Update testing of theming * Updated code/testing for coverage * Include legend title testing * Knit readme Move lifecycle badges to under description Add questioning badge to tabelone * add update to news,md * styler + spell check (#414) * Update pull_request_template.md Co-authored-by: Tim Treis <tim.treis@outlook.de> Co-authored-by: James Black <james@epijim.uk> Co-authored-by: shaesen2 <haesendonckx.steven@gmail.com> Co-authored-by: Tim Treis <tim.treis@stud.uni-heidelberg.de> Co-authored-by: shaesen2 <shaesen2@its.jnj.com> Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Steven Haesendonckx <47894155+SHAESEN2@users.noreply.github.com> Co-authored-by: Mark Baillie <bailliem@gmail.com> Co-authored-by: Joana M. Barros <94060211+joanacmbarros@users.noreply.github.com> Co-authored-by: ardeeshany <ardeeshany@gmail.com>
What changes are proposed in this pull request?
estimate_KM()
. When the formula argument is used, the AVAL, CNSR, and strata args are not used to construct the formula. (Addestimate_KM.formula()
method #379)Did you include unit tests for the proposed change/bug fix (https://testthat.r-lib.org/)?
Yes
If there is an GitHub issue associated with this pull request, please provide link.
closes #379
Checklist for PR reviewer
_pkgdown.yml
pkgdown::build_site()
. Check the R console for errors, and review the rendered website.withr::with_envvar(new = c("NOT_CRAN" = "true"), covr::report())
. Before you run, begin a fresh R session without any packages loaded.usethis::use_spell_check()
runs with no spelling errors in documentationNEWS.md
been updated with the changes from this pull request under the heading indicating the latest version. If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.md
for examples).usethis::use_version(which = "dev")