Add by_level argument to sort_ard_hierarchical() (#548)

[ddsjoberg]

What changes are proposed in this pull request?

• Added the by_level argument to sort_ard_hierarchical(), allowing "descending" sorting to rank variable groups by the counts observed within specific by variable levels (e.g. by_level = list(TRTA = "Placebo")) rather than the sums across all by variable levels. The argument accepts a named list, so any combination of by variables may be used. (Feature Request: sort_ard_hierarchical #548, @rikoprogrammer)

Provide more detail here as needed.

by_level accepts a named list where names are by variables and values are a single level of that variable. When sorting "descending", variable groups are ranked by count sums computed only from rows matching the specified by level(s):

# sort by the counts observed in the "Placebo" arm only
sort_ard_hierarchical(ard, by_level = list(TRTA = "Placebo"))

# any combination of by variables is supported
sort_ard_hierarchical(ard, by_level = list(TRTA = "Placebo", SEX = "F"))

This generalizes the single-by-variable sort_level approach explored in #550 to work with any number of by variables, which was that PR's core limitation.

Implementation notes for reviewers

Since #550 was opened, the sort engine was rewritten for performance (#176), so .append_hierarchy_sums() no longer exists. The feature is integrated into the current vctrs-based engine:

• The original by (as supplied to ard_stack_hierarchical()) is captured before the "highest-severity" extraction that moves the innermost by variable into variables, so by_level is validated and applied against the by variables as the user knows them.
• .hierarchy_by_level_mask() builds a per-row logical mask from the pre-reformat by group columns (which are not reordered by the sort), keeping it positionally aligned with the reformatted frame. It is AND-ed into the keep filter of .hierarchy_sort_group_desc(), so only rows at the requested level(s) contribute to the count sums. Variable groups with no contributing rows sort last.
• .check_by_level() validates the input: must be a fully-named list, names must be by variables, a single level per entry, the level must exist, and at least one by variable must be present.

Correctness was verified with an oracle: sorting with by_level = list(TRTA = "Placebo") on the full ARD produces the same variable-group ordering as the default sort on an ARD built only from Placebo subjects — confirmed for both single- and two-by-variable cases. New unit tests cover the functional behavior, multiple by variables, by_level = NULL equivalence to the default, and all validation error paths (snapshots added).

closes #548

---

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() (branched from current main)
• If a bug was fixed, a unit test was added. (new feature; unit tests 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".

[github-actions]

Unit Tests Summary

  1 files  276 suites   19s ⏱️
276 tests 182 ✅  94 💤 0 ❌
627 runs  500 ✅ 127 💤 0 ❌

Results for commit 0ed7ca5.

♻️ This comment has been updated with latest results.

[github-actions]

Code Coverage Summary

Filename                       Stmts    Miss  Cover    Missing
---------------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------------------------------------------------
R/add_calculated_row.R            53       6  88.68%   51-56
R/apply_fmt_fun.R                116       0  100.00%
R/ard_attributes.R                46       1  97.83%   57
R/ard_formals.R                   13       0  100.00%
R/ard_hierarchical.R             104      12  88.46%   88-93, 187-192
R/ard_identity.R                  13       0  100.00%
R/ard_missing.R                   70       7  90.00%   45-50, 61
R/ard_mvsummary.R                 54       7  87.04%   76-81, 100
R/ard_pairwise.R                  50       0  100.00%
R/ard_stack_hierarchical.R       272       0  100.00%
R/ard_stack.R                     98       0  100.00%
R/ard_strata.R                    38       0  100.00%
R/ard_summary.R                  200       8  96.00%   87-92, 226-227
R/ard_tabulate_rows.R             12       0  100.00%
R/ard_tabulate_value.R            85       8  90.59%   51-56, 73, 97
R/ard_tabulate.R                 629     103  83.62%   108-113, 194, 271, 310, 400, 412-415, 547, 607-612, 643, 819-823, 829-842, 873, 880-885, 889-908, 944-954, 958-977, 981-985, 1015, 1048
R/ard_total_n.R                   19       0  100.00%
R/as_card_fn.R                     8       0  100.00%
R/as_card.R                       12       0  100.00%
R/as_nested_list.R                41       0  100.00%
R/bind_ard.R                      49      11  77.55%   74-85
R/cards-package.R                  1       1  0.00%    14
R/check_ard_structure.R           55       5  90.91%   35-39
R/compare_ard_helpers.R          127      32  74.80%   44-50, 66-69, 113, 148, 153, 158-184, 227, 230
R/compare_ard.R                   26       0  100.00%
R/default_stat_labels.R           20       0  100.00%
R/deprecated.R                    34      30  11.76%   40-41, 56-73, 88-134
R/eval_capture_conditions.R       30       0  100.00%
R/filter_ard_hierarchical.R      209       0  100.00%
R/get_ard_statistics.R            20       0  100.00%
R/maximum_variable_value.R        13       0  100.00%
R/mock.R                         137       2  98.54%   116, 245
R/nest_for_ard.R                  80       1  98.75%   64
R/print_ard_conditions.R          83       0  100.00%
R/print.R                        101       2  98.02%   164-165
R/process_selectors.R            126       1  99.21%   337
R/rename_ard_columns.R            84       0  100.00%
R/rename_ard_groups.R             60       0  100.00%
R/replace_null_statistic.R        11       0  100.00%
R/round5.R                         1       0  100.00%
R/selectors.R                     23       0  100.00%
R/shuffle_ard.R                  167       0  100.00%
R/sort_ard_hierarchical.R        195       0  100.00%
R/summary_functions.R             25       1  96.00%   59
R/tidy_ard_order.R                42       0  100.00%
R/tidy_as_ard.R                   40       0  100.00%
R/unlist_ard_columns.R            28       0  100.00%
R/update_ard.R                    60       6  90.00%   54-59
R/utils.R                         32       1  96.88%   37
TOTAL                           3812     245  93.57%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  --------
R/sort_ard_hierarchical.R      +47       0  +100.00%
TOTAL                          +47       0  +0.08%

Results for commit: 0ed7ca5

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Test Performance Difference

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
ard_stack_hierarchical	💚	$3.37$	$-1.30$	ard_stack_hierarchical_variables_

Results for commit 45db895

♻️ This comment has been updated with latest results.

[ddsjoberg]

Hi @rikoprogrammer, your branch had a merge conflict, and rather than trying to resolve it, I just made a new branch. Can you take a look here? Did I generalize your code correctly?

[rikoprogrammer]

Hi @rikoprogrammer, your branch had a merge conflict, and rather than trying to resolve it, I just made a new branch. Can you take a look here? Did I generalize your code correctly?

Hi @ddsjoberg, yes, the code is well generalized. Thank you