Add add_hierarchical_zero_rows() for unobserved hierarchical levels - #609
Add add_hierarchical_zero_rows() for unobserved hierarchical levels#609Melkiades wants to merge 5 commits into
Conversation
Stacked hierarchical ARDs include only observed levels because the tabulation routes through the strata (observed-only) branch, so predefined categories such as SMQ/CQ baskets, SOCs, and preferred terms disappear instead of showing a zero count. add_hierarchical_zero_rows() appends zero-count rows for unobserved levels using a mapping of the expected level universe. A single mapping argument (named list or data.frame) covers both an unobserved top-level category and an unobserved child of an observed parent, and preserves the by structure and denominators so percentages remain correct.
| #' @param variables ([`tidy-select`][dplyr::dplyr_tidy_select])\cr | ||
| #' the hierarchical variables used to create `x`, in the same order. The first | ||
| #' variable is the top level; the second, when present, is the nested child. | ||
| #' @param mapping (named `list` or `data.frame`)\cr |
There was a problem hiding this comment.
I was thinking a bit more about this. Would the named list require positional matching to the variables added in ard_hierarcihcal(variables, include)? If so, that is probably not the best, and providing a data frame is more robust.
There was a problem hiding this comment.
Do we have a way to handle a situation like:
ard <- ard_heirarchical(variables = c(AESOC, AEPT), include = AEPT)
add_zero_rows(ard, ....) # how do we know to only add the AEPT zeros and not the AESOC?There was a problem hiding this comment.
Both of these were from before I reworked the API, and I think the new version resolves them.
It's now driven by variables instead of the mapping, you name which levels or level to complete, and it reads the expected levels straight from the stored factor levels(). So your include = AEPT case works directly:
ard <- ard_stack_hierarchical(adae, variables = c(AESOC, AEPT), include = AEPT, ...)
# completes only PT — AESOC is left untouched
add_hierarchical_unobserved_levels(ard, variables = AEPT) # TODO: find a good name ahahNothing is positional: variables is tidy-selected by name against the ARD, and the (now optional) mapping list is keyed by parent-level value, not variable order. mapping is only needed for the one thing factor levels can't express: children under a parent that itself never occurs.
So for the common cases there's no data-frame requirement. factor levels cover them. If needed you can add a data.frame too. Do you still want the data.frame form as the primary documented input, or is factor-levels-first fine with you?
There was a problem hiding this comment.
I like more the mapping list as it looks intuitive to me ^^
What do you mean by percentages stay correct? |
Read the expected level universe from the factor levels the ARD already carries, so top-level and nested completion need only `variables`. `mapping` becomes optional and additive, used only for children of an unobserved parent or a bespoke universe that factor levels cannot express.
Rename the function (and its file/tests/docs) to add_hierarchical_unobserved_levels(), matching the intent of adding unobserved factor levels rather than raw zero-rows. Drop the user-facing `statistic` argument; the count-style stats are zeroed internally while denominators carry over, so callers no longer manage that detail. Rewrite the documentation to lead with the single-variable case and cross-link gtsummary::tbl_hierarchical() so users can discover it.
A never-observed level has no one at risk, so its proportion is 0 / 0 -- undefined, not zero. Set only the counts (n, n_cum) to zero and leave p/p_cum as NaN, letting the display layer recode them rather than asserting zero in the ARD.
I was mapping it to 0 which is wrong. now for an added unobserved level the proportion is 0 / 0, which is undefined ,so I leave p/p_cum as NaN rather than asserting 0 in the ARD. The "show as 0" is a display choice and gets recoded downstream, so the ARD stays correct!! |
| @@ -1,5 +1,7 @@ | |||
| # cards 0.9.0.9000 | |||
|
|
|||
| * Added `add_hierarchical_unobserved_levels()` to add zero-count rows for unobserved levels (top-level categories and nested children) to a stacked hierarchical ARD. Expected levels are read from the variables' factor levels. (#602) | |||
There was a problem hiding this comment.
Do you want to give yourself credit for this change??
| #' set.seed(1) | ||
| #' adae <- data.frame( | ||
| #' USUBJID = sprintf("S%03d", 1:20), | ||
| #' AESOC = factor(sample(c("Cardiac", "GI"), 20, TRUE), levels = c("Cardiac", "GI", "Vascular")), |
There was a problem hiding this comment.
I don't like the auto-reading of the factor levels, because it feels very unnatural to assign SOC and PT as factors (and all nesting variables like this). This also gives differential behaviour for the first variable (e.g. SOC) compared to the subsequent variables (e.g. PT). I would prefer to see consistent for all variables.
| #' ard |> | ||
| #' add_hierarchical_unobserved_levels( | ||
| #' variables = c(AESOC, AEDECOD), | ||
| #' mapping = list(Vascular = c("PT1", "PT2")) |
There was a problem hiding this comment.
This is the positional matching I am not a fan of. Vascular is matched to AESOC just by it's position, and hte same for PT1 and PT2.
It's clearer input to have just a data frame as the input with column names that match the variables/include from the primary function call.
| #' variables = c(AESOC, AEDECOD), | ||
| #' mapping = list(Vascular = c("PT1", "PT2")) | ||
| #' ) | ||
| NULL |
There was a problem hiding this comment.
Do we need a statistic argument that lists the stats to add? The default could be to just match what is seen passed ARD (so it works well for rates and counts), or is that what it does now and that is good enough and no need to add the argument.
2 participants
Adds
add_hierarchical_zero_rows(), an ARD-level helper that appends zero-count rows for unobserved hierarchical levels using amappingof the expected level universe.A single
mappingargument (named list or two-column data.frame) covers:Preserves the
bystructure and carries denominators over so percentages stay correct (p = 0, no NaN).closes #602