Closes #2142 Supersede get_summary_records() and enhance derive_summary_records()
#2158
Conversation
zdz2101
changed the title
get_summary_records() and enhance derive_summary_records()
|
…https://github.com/pharmaverse/admiral into 2142_derive_summary_records_get_summary_records_mods
DESCRIPTION
Outdated
| @@ -38,6 +38,7 @@ LazyData: true | |||
| Roxygen: list(markdown = TRUE) | |||
| RoxygenNote: 7.2.3 | |||
| Depends: R (>= 4.0) | |||
| Remotes: pharmaverse/admiraldev | |||
There was a problem hiding this comment.
@ddsjoberg is this all I would have to do? or does I need to remove the imports line too?
There was a problem hiding this comment.
You'll want to make one small change to include the min version number required.
Remotes: pharmaverse/admiraldev (>= 0.5.0.9000)
Then if anyone does any dev work, like redocumenting, and they don't have this version installed, they'll get a note that they need to install the updated version.
NEWS.md
Outdated
| @@ -12,6 +12,8 @@ | |||
| were enhanced such that more than one summary variable can be derived, e.g., | |||
| `AVAL` as the sum and `ADT` as the maximum of the contributing records. (#1792) | |||
|
|
|||
| - `derive_summary_records()` was enhanced with the following optional arguments: `dataset_add`, `dataset_ref`, `missing_values`. These arguments respectively, generate summary variables from additional datasets, retain/add specific records that and impute user-defined missing values. (#2142) | |||
There was a problem hiding this comment.
is the that supposed to have something after it?
R/get_summary_records.R
Outdated
| #' switching to using the `dataset_add` argument in `derive_summary_records()`, | ||
| #' which is easier to use, has more features, and still under active development. |
There was a problem hiding this comment.
I don't think we should say easier to use and under active development.
I think this is fine:
#' Development on get_summary_records() is complete, and for new code we recommend
#' switching to using the dataset_add argument in derive_summary_records().
Co-authored-by: Ben Straub <[email protected]>
…https://github.com/pharmaverse/admiral into 2142_derive_summary_records_get_summary_records_mods
|
@bundfussr all feedback should be addressed now, please take a look specifically at test 6 for |
zdz2101
deleted the
2142_derive_summary_records_get_summary_records_mods
branch
Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.
Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
develbranch until you have checked off each task.styler::style_file()to style R and Rmd filesdevtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelyNEWS.mdunder the header# admiral (development version)if the changes pertain to a user-facing function (i.e. it has an@exporttag) or documentation aimed at users (rather than developers)pkgdown::build_site()and check that all affected examples are displayed correctly and that all new functions occur on the "Reference" page.lintr::lint_package()R CMD checklocally and address all errors and warnings -devtools::check()