-
-
Notifications
You must be signed in to change notification settings - Fork 3
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 functionality to sort & filter ard_stack_hierarchical()
results
#392
base: main
Are you sure you want to change the base?
Conversation
…chical' into 301_sort_filter_ard_stack_hierarchical
Unit Tests Summary 1 files 206 suites 1m 11s ⏱️ Results for commit be0efb0. ♻️ This comment has been updated with latest results. |
Hi @edelarua , is this ready for review? Thanks! |
Hi @ddsjoberg, yes I think this should be ready now! I've also updated the corresponding PR in gtsummary (ddsjoberg/gtsummary#2097) to work with the updated version |
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.
@edelarua this API for the filter function looks amazing!! 🤩
I haven't had a chance to review the sorting function, but have some initial comments on filtering. Thank you for this amazing work!
- [x] Clean up the language/typos -------------------------------------------------------------------------------- Pre-review Checklist (if item does not apply, mark is as complete) - [ ] **All** GitHub Action workflows pass with a ✅ - [ ] PR branch has pulled the most recent updates from master branch: `usethis::pr_merge_main()` - [ ] If a bug was fixed, a unit test was added. - [ ] Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): `devtools::test_coverage()` - [ ] Request a reviewer Reviewer Checklist (if item does not apply, mark is as complete) - [ ] If a bug was fixed, a unit test was added. - [ ] Run `pkgdown::build_site()`. Check the R console for errors, and review the rendered website. - [ ] Code coverage is suitable for any new functions/features: `devtools::test_coverage()` When the branch is ready to be merged: - [ ] Update `NEWS.md` with the changes from this pull request under the heading "`# cards (development version)`". If there is an issue associated with the pull request, reference it in parentheses at the end update (see `NEWS.md` for examples). - [ ] **All** GitHub Action workflows pass with a ✅ - [ ] Approve Pull Request - [ ] Merge the PR. Please use "Squash and merge" or "Rebase and merge". _Optional Reverse Dependency Checks_: Install `checked` with `pak::pak("Genentech/checked")` or `pak::pak("checked")` ```shell # Check dev versions of `cardx`, `gtsummary`, and `tfrmt` which are in the `ddsjoberg` R Universe Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2L, repos = c('https://ddsjoberg.r-universe.dev', 'https://cloud.r-project.org'))" # Check CRAN reverse dependencies but run tests skipped on CRAN Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')" # Check CRAN reverse dependencies in a CRAN-like environment Rscript -e "options(checked.check_envvars = c(NOT_CRAN = FALSE), checked.check_build_args = '--as-cran'); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')" ``` --------- Co-authored-by: Emily de la Rua <emily.de_la_rua@contractors.roche.com>
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 did a brief review, these are nice additions!!
What changes are proposed in this pull request?
sort
andfilter
arguments toard_stack_hierarchical()
to support sorting & filtering of ARD results. (Do we need functions for working with our hierarchical results? #301)Closes #301
Pre-review Checklist (if item does not apply, mark is as complete)
usethis::pr_merge_main()
devtools::test_coverage()
Reviewer Checklist (if item does not apply, mark is as complete)
pkgdown::build_site()
. Check the R console for errors, and review the rendered website.devtools::test_coverage()
When the branch is ready to be merged:
NEWS.md
with the changes from this pull request under the heading "# cards (development version)
". If there is an issue associated with the pull request, reference it in parentheses at the end update (seeNEWS.md
for examples).Optional Reverse Dependency Checks:
Install
checked
withpak::pak("Genentech/checked")
orpak::pak("checked")