1089 always avail alt df

NEWS.md

@@ -5,6 +5,7 @@
  * Exported previously internal `make_subset_expr` for use when constructing custom splitting behavior
  * Exported previously internal `splv_extra` and `splv_extra<-` accessors for getting and setting child-specific extra arguments on `SplitValue` objects.
  * Exported previously internal `value_expr` accessor for retrieving the subsetting expression from a `SplitValue` or `ValueWrapper` object.
+ * The `.alt_df*` family of afun arguments now receive subsets of `df` when `alt_counts_df` is not specified in the `build_table` call; previously resulted in an error.
  * Added accessor methods for `RowsVerticalSection objects`: `row_cells`, `obj_format`, `obj_format<-`, `obj_na_str`, `obj_na_str<-`, `cell_values`
  * Added `c` method for directly combining `RowsVerticalSection` objects
  * Added vignette: Guided Tour (Advanced) @gmbecker

R/colby_constructors.R

@@ -1044,7 +1044,11 @@ NULL
 #'   \item{.all_col_exprs}{List of expressions. Each of them represents a different column splitting.}
 #'   \item{.all_col_counts}{Vector of integers. Each of them represents the global count for each column. It differs
 #'     if `alt_counts_df` is used (see [build_table()]).}
-#' }
+#'
+#' For the `.alt_df*` family of parameters, these will be passed data
+#' subsets based on `df` if no `alt_counts_df` is specified in the
+#' `build_table` call. In `rtables` versions `<= 0.6.13`, this instead
+#' resulted in an error.  }
 #'
 #' @note If any of these formals is specified incorrectly or not present in the tabulation machinery, it will be
 #'   treated as if missing. For example, `.ref_group` will be missing if no baseline is previously defined during

R/tt_dotabulation.R

@@ -22,7 +22,13 @@ match_extra_args <- function(f,
       .N_row = .N_row,
       .df_row = .df_row,
       .all_col_exprs = .all_col_exprs,
-      .all_col_counts = .all_col_counts
+      .all_col_counts = .all_col_counts,
+      ## always available as of fix for https://github.com/insightsengineering/rtables/issues/1089
+      ## still will only be passed to afun if it's asked for by the formals, same as
+      ## .N_col, etc
+      .alt_df = .alt_df,
+      .alt_df_row = .alt_df_row,
+      .alt_df_full = .alt_df_full
     ),
     extras
   )
@@ -35,16 +41,6 @@ match_extra_args <- function(f,
   if (!is.null(.ref_group)) {
     possargs <- c(possargs, list(.ref_group = .ref_group))
   }
-  if (!is.null(.alt_df_row)) {
-    possargs <- c(possargs, list(.alt_df_row = .alt_df_row))
-  }
-  if (!is.null(.alt_df)) {
-    possargs <- c(possargs, list(.alt_df = .alt_df))
-  }
-
-  if (!is.null(.alt_df_full)) {
-    possargs <- c(possargs, list(.alt_df_full = .alt_df_full))
-  }
 
   if (!is.null(.ref_full)) {
     possargs <- c(possargs, list(.ref_full = .ref_full))
@@ -119,6 +115,9 @@ gen_onerv <- function(csub, col, count, cextr, cpath,
     dat <- dat[!is.na(dat[[col]]), , drop = FALSE]
   }
 
+  ## firstarg will be df or x (col vec), dat will always be the df
+  firstarg <- dat
+
   fullrefcoldat <- cextr$.ref_full
   if (!is.null(fullrefcoldat)) {
     cextr$.ref_full <- NULL
@@ -133,11 +132,19 @@ gen_onerv <- function(csub, col, count, cextr, cpath,
   ## behavior for x/df and ref-data (full and group)
   ## match
   if (!is.null(col) && !takesdf) {
-    dat <- dat[[col]]
+    firstarg <- firstarg[[col]]
     fullrefcoldat <- fullrefcoldat[[col]]
     baselinedf <- baselinedf[[col]]
   }
-  args <- list(dat)
+  args <- list(firstarg)
+
+  ## replace alt_df (potential) args with their df versions if alt_counts_df not set
+  ## in build_table call
+  if (is.null(alt_df_full)) {
+    alt_df_full <- if (NROW(spl_context) > 0) spl_context$full_parent_df[[1]] else dfpart
+    alt_dfpart <- dfpart
+    alt_dfpart_fil <- dat
+  }
 
   names(all_col_counts) <- names(all_col_exprs)
 
@@ -1405,14 +1412,6 @@ build_table <- function(lyt, df,
   lyt <- set_def_child_ord(lyt, df)
   lyt <- fix_analyze_vis(lyt)
   df <- fix_split_vars(lyt, df, char_ok = is.null(col_counts))
-  alt_params <- check_afun_cfun_params(lyt, c(".alt_df", ".alt_df_row"))
-  if (any(alt_params) && is.null(alt_counts_df)) {
-    stop(
-      "Layout contains afun/cfun functions that have optional parameters ",
-      ".alt_df and/or .alt_df_row, but no alt_counts_df was provided in ",
-      "build_table()."
-    )
-  }
 
   rtpos <- TreePos()
   cinfo <- create_colinfo(lyt, df, rtpos,

tests/testthat/test-tab_afun_cfun.R

@@ -166,9 +166,6 @@ test_that(".spl_context and afun extra parameters contain information about comb
 
   # NB: If you add keep_levels = c("all_X") to add_combo_levels the other
   #     column expressions are missing -> Expected!
-  expect_error(lyt |> build_table(DM),
-    regexp = "Layout contains afun\\/cfun functions that have optional*"
-  )
 
   tbl <- lyt |> build_table(DM, alt_counts_df = ex_adsl)
 
@@ -352,10 +349,6 @@ test_that(".alt_df_row appears in cfun but not in afun.", {
     split_rows_by("ARMCD") |>
     analyze("BMRKR1", afun = afun_tmp)
 
-  expect_error(
-    lyt |> build_table(ex_adsl),
-    "Layout contains afun/cfun functions that have optional*"
-  )
   expect_error(
     lyt |> build_table(ex_adsl, alt_counts_df = DM),
     "alt_counts_df appears incompatible with column-split*"
@@ -384,3 +377,31 @@ test_that("full alt_counts_df is accessible from afun/cfun via .alt_df_full", {
   cvals <- unlist(cell_values(tbl))
   expect_true(all(cvals == "ok"))
 })
+
+test_that(".alt_df* argument behavior is correct when alt_counts_df is not set", {
+  check_alt_dfs <- function(df, .df_row, .alt_df_row, .alt_df, .alt_df_full) {
+    expect_identical(df, .alt_df)
+    expect_identical(.df_row, .alt_df_row)
+    expect_false(is.null(.alt_df_full))
+    TRUE
+  }
+
+  afun <- function(df, .df_row, .alt_df_row, .alt_df, .alt_df_full) {
+    res <- check_alt_dfs(df, .df_row, .alt_df_row, .alt_df, .alt_df_full)
+    in_rows("afun result" = "OK")
+  }
+  cfun <- function(df, labelstr, .df_row, .alt_df_row, .alt_df, .alt_df_full) {
+    res <- check_alt_dfs(df, .df_row, .alt_df_row, .alt_df, .alt_df_full)
+    in_rows("cfun result" = "OK", .formats = list("cfun result" = "xx"))
+  }
+
+  first_2_levs <- function(vec) levels(vec)[1:2]
+
+  lyt <- basic_table() |>
+    split_cols_by("ARM", split_fun = keep_split_levels(first_2_levs(ex_adsl$ARM))) |>
+    split_rows_by("STRATA1", split_fun = keep_split_levels(first_2_levs(ex_adsl$STRATA1))) |>
+    summarize_row_groups("STRATA1", cfun = cfun) |>
+    analyze("AGE", afun = afun)
+
+  expect_no_error(build_table(lyt, ex_adsl))
+})