new unified get_ref_info

[munoztd0]

Pull Request

Fixes #295

Context

junco has at 3 ways to resolve a reference group from ref_path + .spl_context:

1. get_ref_info() exported, returns ref_group + in_ref_col, handles NULL cases. Used by most a_* functions.
2. h_get_trtvar_refpath() exported (should it even be export ?), returns trt_var / ctrl_grp only, not ref_group. Uses tail(...,1) assumption on the split context.
3. Inline in a_summarize_aval_chg_diff_j reimplements both above from scratch, with a different ref_group construction strategy, (direct filter vs .spl_context index).

Fix

Unified all into get_ref_info() and soft deprecated all the others.

Questions

Contributing get_ref_info to tern after we align all ?

Checks

• (Have you updated the NEWS.md ?)
• (Have you added proper tests for new functions/features ?)
• (Have you added new functions to the pkgdown.yml ?)
• (Have you run document() on new functions ?)

[wwojciech]

Hi @munoztd0 - I slightly streamlined the code.

[wwojciech]

why the name trt_var / ctrl_grp ? It does not need to be treatment var or control group, I think.

[gmbecker]

Overall looks good, some small tweaks/additions to the tests/documentation.

Also, a good way to cover something like this is to make a table where the cell values are/represent the info you want to check (in this case what comes out of get_ref_info,

So we could use

introspect_ref_info <- function(df, ref_path, .spl_context) {
refinfo <- get_ref_info(ref_path, .spl_context)
in_rows(trt_var = refinfo$trt_var,
               ctrl_grp = refinfo$ctrl_grp,
               ref_group_dim = dim(refinof$ref_group),
               in_ref_col = refinfo$in_ref_col)

And then compare the table or the cell values to make our assertions. Also useful for documenting what get_ref_info does in various situations in a human readable way (e.g., in a vignette?)

[gmbecker]

technically this is the most recent split's name, which is usually a variable name from the data, but its not guaranteed to be

[gmbecker]

make this tests more specific to the exact length we expect it to be, especially since you are only capturing in risk diff cols here.

[gmbecker]

colvars refers to when split_cols_by_multivar is used, so this description of the test is not accurate, though the test itself seems fine.