From f38435c32b1158f49e948e34bbe537e12280b33d Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 12:17:57 +0200 Subject: [PATCH 01/10] update g_forest() to support col_x and col_ci in the same column - issue 1499. --- NEWS.md | 2 ++ R/g_forest.R | 51 +++++++++++++++++++++++++--------- man/g_forest.Rd | 24 ++++++++++++---- tests/testthat/test-g_forest.R | 14 ++++++++++ 4 files changed, 72 insertions(+), 19 deletions(-) diff --git a/NEWS.md b/NEWS.md index a9f4c7a6ab..e78982c4da 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,8 @@ # tern 0.9.11.9000 ### Enhancements +* Updated `g_forest()` to support point estimates and confidence intervals + stored in a single column. (#1499) * Added the `exclude_rows` argument to `g_forest()` to allow excluding selected rows from the forest plot before plotting. (#1498) * Added `factor_level_method` argument to `df_explicit_na()` to control factor level ordering diff --git a/R/g_forest.R b/R/g_forest.R index 0a61ccaea3..9fa03da10d 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -10,12 +10,14 @@ #' @inheritParams argument_convention #' @param tbl (`VTableTree`)\cr `rtables` table with at least one column with a single value and one column with 2 #' values. -#' @param col_x (`integer(1)` or `NULL`)\cr column index with estimator. By default tries to get this from -#' `tbl` attribute `col_x`, otherwise needs to be manually specified. If `NULL`, points will be excluded -#' from forest plot. -#' @param col_ci (`integer(1)` or `NULL`)\cr column index with confidence intervals. By default tries to get this from -#' `tbl` attribute `col_ci`, otherwise needs to be manually specified. If `NULL`, lines will be excluded -#' from forest plot. +#' @param col_x (`integer(1)` or `NULL`)\cr column index with estimator. +#' By default tries to get this from `tbl` attribute `col_x`, otherwise needs +#' to be manually specified. If `NULL`, points will be excluded from forest plot. +#' @param col_ci (`integer(1)` or `NULL`)\cr column index with confidence intervals. +#' By default tries to get this from `tbl` attribute `col_ci`, otherwise needs +#' to be manually specified. If `NULL`, lines will be excluded from forest plot. +#' The estimator and confidence interval can be stored in the same column. +#' In this case, `col_x` and `col_ci` must be the same. #' @param vline (`numeric(1)` or `NULL`)\cr x coordinate for vertical line, if `NULL` then the line is omitted. #' @param forest_header (`character(2)`)\cr text displayed to the left and right of `vline`, respectively. #' If `vline = NULL` then `forest_header` is not printed. By default tries to get this from `tbl` attribute @@ -89,6 +91,16 @@ #' g_forest(tbl) #' \donttest{ #' g_forest(tbl, exclude_rows = 1) +#' +#' # Estimates and confidence intervals in the same column. +#' +#' tbl <- rtable( +#' header = rheader(rrow("", "point est (CI)")), +#' rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), +#' rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) +#' ) +#' tbl +#' g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) #' } #' #' # Odds ratio only table. @@ -245,7 +257,7 @@ g_forest <- function(tbl, mat_strings <- formatters::mf_strings(mat) nlines_hdr <- formatters::mf_nlheader(mat) nrows_body <- nrow(mat_strings) - nlines_hdr - tbl_stats <- mat_strings[nlines_hdr, -1] + tbl_stats <- make.unique(mat_strings[nlines_hdr, -1]) # Generate and modify table as ggplot object gg_table <- rtable2gg(tbl, fontsize = font_size, colwidths = width_columns, lbl_col_padding = lbl_col_padding) + @@ -273,8 +285,8 @@ g_forest <- function(tbl, tbl_df[["empty_ci"]] <- rep(list(c(NA_real_, NA_real_)), nrow(tbl_df)) ci_col <- which(names(tbl_df) == "empty_ci") } - if (nrow(tbl_df) >= 1 && length(tbl_df[, ci_col][[1]]) != 2) { - stop("CI column must have two elements (lower and upper limits).") + if (nrow(tbl_df) >= 1 && length(tbl_df[, ci_col][[1]]) <= 1) { + stop("CI column must have at least two elements (lower and upper limits).") } if (!is.null(col_x)) { @@ -289,10 +301,23 @@ g_forest <- function(tbl, sym_size <- rep(1, nrow(tbl_df)) } - tbl_df[, c("ci_lwr", "ci_upr")] <- t(sapply(tbl_df[, ci_col], unlist)) - x <- unlist(tbl_df[, x_col]) - lwr <- unlist(tbl_df[["ci_lwr"]]) - upr <- unlist(tbl_df[["ci_upr"]]) + x_ci <- if (nrow(tbl_df) >= 1) { + x_ci_df <- tbl_df[, unique(c(x_col, ci_col)), drop = FALSE] + x_ci_list <- lapply(x_ci_df, function(col) { + matrix( + unlist(col), + nrow = nrow(tbl_df), + byrow = length(col[[1]]) != 1L + ) + }) + do.call(cbind, x_ci_list) + } else { + NULL + } + + x <- x_ci[, 1] + lwr <- x_ci[, 2] + upr <- x_ci[, 3] row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) if (is.null(col)) col <- "#343cff" diff --git a/man/g_forest.Rd b/man/g_forest.Rd index 08f9325e51..ac56eff648 100644 --- a/man/g_forest.Rd +++ b/man/g_forest.Rd @@ -33,13 +33,15 @@ g_forest( \item{tbl}{(\code{VTableTree})\cr \code{rtables} table with at least one column with a single value and one column with 2 values.} -\item{col_x}{(\code{integer(1)} or \code{NULL})\cr column index with estimator. By default tries to get this from -\code{tbl} attribute \code{col_x}, otherwise needs to be manually specified. If \code{NULL}, points will be excluded -from forest plot.} +\item{col_x}{(\code{integer(1)} or \code{NULL})\cr column index with estimator. +By default tries to get this from \code{tbl} attribute \code{col_x}, otherwise needs +to be manually specified. If \code{NULL}, points will be excluded from forest plot.} -\item{col_ci}{(\code{integer(1)} or \code{NULL})\cr column index with confidence intervals. By default tries to get this from -\code{tbl} attribute \code{col_ci}, otherwise needs to be manually specified. If \code{NULL}, lines will be excluded -from forest plot.} +\item{col_ci}{(\code{integer(1)} or \code{NULL})\cr column index with confidence intervals. +By default tries to get this from \code{tbl} attribute \code{col_ci}, otherwise needs +to be manually specified. If \code{NULL}, lines will be excluded from forest plot. +The estimator and confidence interval can be stored in the same column. +In this case, \code{col_x} and \code{col_ci} must be the same.} \item{vline}{(\code{numeric(1)} or \code{NULL})\cr x coordinate for vertical line, if \code{NULL} then the line is omitted.} @@ -144,6 +146,16 @@ tbl <- basic_table() |> g_forest(tbl) \donttest{ g_forest(tbl, exclude_rows = 1) + +# Estimates and confidence intervals in the same column. + +tbl <- rtable( + header = rheader(rrow("", "point est (CI)")), + rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), + rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) +) +tbl +g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) } # Odds ratio only table. diff --git a/tests/testthat/test-g_forest.R b/tests/testthat/test-g_forest.R index 731f1a32d7..942c46e5b8 100644 --- a/tests/testthat/test-g_forest.R +++ b/tests/testthat/test-g_forest.R @@ -148,6 +148,20 @@ testthat::test_that("g_forest works when all rows are excluded", { expect_snapshot_ggplot("g_forest_exclude_all_rows", p, width = 15, height = 3) }) +testthat::test_that("g_forest works for point estimates and confidence intervals in the same column", { + tbl <- rtable( + header = rheader(rrow("", "point est (CI)")), + rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), + rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) + ) + + testthat::expect_silent( + p <- g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + expect_snapshot_ggplot("g_forest_the_same_x_ci", p, width = 15, height = 3) +}) + testthat::test_that("g_forest argument deprecation warnings work", { tbl <- basic_table() |> tabulate_rsp_subgroups(df) From bda537e44a5be967e6b8449ab69f51ed0f7ed06c Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 13:25:31 +0200 Subject: [PATCH 02/10] g_forest() - cosmetic code update to use x_ci for the log transformation. --- R/g_forest.R | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index 9fa03da10d..792791d4b6 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -304,40 +304,35 @@ g_forest <- function(tbl, x_ci <- if (nrow(tbl_df) >= 1) { x_ci_df <- tbl_df[, unique(c(x_col, ci_col)), drop = FALSE] x_ci_list <- lapply(x_ci_df, function(col) { - matrix( - unlist(col), - nrow = nrow(tbl_df), - byrow = length(col[[1]]) != 1L - ) + byrow <- length(col[[1]]) != 1L + matrix(unlist(col), nrow = nrow(tbl_df), byrow = byrow) }) do.call(cbind, x_ci_list) } else { NULL } - + # `x`, `lwr`, and `upr` are NULL when x_ci = NULL. x <- x_ci[, 1] lwr <- x_ci[, 2] upr <- x_ci[, 3] row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) - if (is.null(col)) col <- "#343cff" - if (length(col) == 1) col <- rep(col, nrow(tbl_df)) - if (is.null(x_at)) x_at <- union(xlim, vline) - x_labels <- x_at - # Apply log transformation. - # When nrow(tbl_df) == 0, x, lwr, and upr are NULL, so log() would fail. - if (logx && nrow(tbl_df) >= 1) { - x_t <- log(x) - lwr_t <- log(lwr) - upr_t <- log(upr) + x_ci_t <- if (logx && !is.null(x_ci)) { + log(x_ci) } else { - x_t <- x - lwr_t <- lwr - upr_t <- upr + x_ci } + x_t <- x_ci_t[, 1] + lwr_t <- x_ci_t[, 2] + upr_t <- x_ci_t[, 3] xlim_t <- if (logx) log(xlim) else xlim + if (is.null(col)) col <- "#343cff" + if (length(col) == 1) col <- rep(col, nrow(tbl_df)) + if (is.null(x_at)) x_at <- union(xlim, vline) + x_labels <- x_at + # Set up plot area gg_plt <- ggplot(data = tbl_df) + theme( From 92ba78354ed53a68ced439abe8b67d5c825f1158 Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 14:21:15 +0200 Subject: [PATCH 03/10] g_forest() cosmetic update (variable rename). --- R/g_forest.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index 792791d4b6..7e80649dce 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -302,8 +302,8 @@ g_forest <- function(tbl, } x_ci <- if (nrow(tbl_df) >= 1) { - x_ci_df <- tbl_df[, unique(c(x_col, ci_col)), drop = FALSE] - x_ci_list <- lapply(x_ci_df, function(col) { + tbl_x_ci <- tbl_df[, unique(c(x_col, ci_col)), drop = FALSE] + x_ci_list <- lapply(tbl_x_ci, function(col) { byrow <- length(col[[1]]) != 1L matrix(unlist(col), nrow = nrow(tbl_df), byrow = byrow) }) From 7be9dded4e351002df45309f975b3eb195d027b0 Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 14:44:06 +0200 Subject: [PATCH 04/10] g_forest() cosmetic update (doc). --- R/g_forest.R | 28 +++++++++++++--------------- man/g_forest.Rd | 26 ++++++++++++-------------- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index 7e80649dce..6184913644 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -83,17 +83,25 @@ #' variables = list(rsp = "rsp", arm = "ARM", subgroups = c("SEX", "STRATA2")), #' data = adrs #' ) -#' # Full commonly used response table. #' +#' # Full commonly used response table. #' tbl <- basic_table() |> #' tabulate_rsp_subgroups(df) -#' +#' tbl #' g_forest(tbl) #' \donttest{ #' g_forest(tbl, exclude_rows = 1) -#' +#' } +#' # Odds ratio only table. +#' tbl_or <- basic_table() |> +#' tabulate_rsp_subgroups(df, vars = c("n_tot", "or", "ci")) +#' tbl_or +#' g_forest( +#' tbl_or, +#' forest_header = c("Comparison\nBetter", "Treatment\nBetter") +#' ) +#' \donttest{ #' # Estimates and confidence intervals in the same column. -#' #' tbl <- rtable( #' header = rheader(rrow("", "point est (CI)")), #' rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), @@ -102,16 +110,6 @@ #' tbl #' g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) #' } -#' -#' # Odds ratio only table. -#' -#' tbl_or <- basic_table() |> -#' tabulate_rsp_subgroups(df, vars = c("n_tot", "or", "ci")) -#' g_forest( -#' tbl_or, -#' forest_header = c("Comparison\nBetter", "Treatment\nBetter") -#' ) -#' #' # Survival forest plot example. #' adtte <- tern_ex_adtte #' # Save variable labels before data processing steps. @@ -277,6 +275,7 @@ g_forest <- function(tbl, dat_cols <- seq(which(names(tbl_df) == "node_class") + 1, ncol(tbl_df)) tbl_df <- tbl_df[, c(which(names(tbl_df) == "row_num"), dat_cols)] names(tbl_df) <- c("row_num", tbl_stats) + row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) # Check table data columns if (!is.null(col_ci)) { @@ -315,7 +314,6 @@ g_forest <- function(tbl, x <- x_ci[, 1] lwr <- x_ci[, 2] upr <- x_ci[, 3] - row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) # Apply log transformation. x_ci_t <- if (logx && !is.null(x_ci)) { diff --git a/man/g_forest.Rd b/man/g_forest.Rd index ac56eff648..5b68027088 100644 --- a/man/g_forest.Rd +++ b/man/g_forest.Rd @@ -138,17 +138,25 @@ df <- extract_rsp_subgroups( variables = list(rsp = "rsp", arm = "ARM", subgroups = c("SEX", "STRATA2")), data = adrs ) -# Full commonly used response table. +# Full commonly used response table. tbl <- basic_table() |> tabulate_rsp_subgroups(df) - +tbl g_forest(tbl) \donttest{ g_forest(tbl, exclude_rows = 1) - +} +# Odds ratio only table. +tbl_or <- basic_table() |> + tabulate_rsp_subgroups(df, vars = c("n_tot", "or", "ci")) +tbl_or +g_forest( + tbl_or, + forest_header = c("Comparison\nBetter", "Treatment\nBetter") +) +\donttest{ # Estimates and confidence intervals in the same column. - tbl <- rtable( header = rheader(rrow("", "point est (CI)")), rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), @@ -157,16 +165,6 @@ tbl <- rtable( tbl g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) } - -# Odds ratio only table. - -tbl_or <- basic_table() |> - tabulate_rsp_subgroups(df, vars = c("n_tot", "or", "ci")) -g_forest( - tbl_or, - forest_header = c("Comparison\nBetter", "Treatment\nBetter") -) - # Survival forest plot example. adtte <- tern_ex_adtte # Save variable labels before data processing steps. From dcd0348892a6f861ac69f97ff315945e99868967 Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 17:56:59 +0200 Subject: [PATCH 05/10] Update g_forest() to allow col_x or col_ci to be NULL, hiding x or ci when the point estimate and CI share a column. --- R/g_forest.R | 66 +++++++++-------- man/g_forest.Rd | 6 +- tests/testthat/test-g_forest.R | 127 ++++++++++++++++++++++++++++++++- 3 files changed, 166 insertions(+), 33 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index 6184913644..d8ab23d310 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -16,8 +16,12 @@ #' @param col_ci (`integer(1)` or `NULL`)\cr column index with confidence intervals. #' By default tries to get this from `tbl` attribute `col_ci`, otherwise needs #' to be manually specified. If `NULL`, lines will be excluded from forest plot. +#' #' The estimator and confidence interval can be stored in the same column. -#' In this case, `col_x` and `col_ci` must be the same. +#' In this case, `col_x` and `col_ci` must be the same, and the values in each +#' row of the column indicated by `col_x`/`col_ci` must be ordered as the +#' point estimate, lower confidence interval bound, and upper confidence +#' interval bound, respectively. #' @param vline (`numeric(1)` or `NULL`)\cr x coordinate for vertical line, if `NULL` then the line is omitted. #' @param forest_header (`character(2)`)\cr text displayed to the left and right of `vline`, respectively. #' If `vline = NULL` then `forest_header` is not printed. By default tries to get this from `tbl` attribute @@ -255,7 +259,6 @@ g_forest <- function(tbl, mat_strings <- formatters::mf_strings(mat) nlines_hdr <- formatters::mf_nlheader(mat) nrows_body <- nrow(mat_strings) - nlines_hdr - tbl_stats <- make.unique(mat_strings[nlines_hdr, -1]) # Generate and modify table as ggplot object gg_table <- rtable2gg(tbl, fontsize = font_size, colwidths = width_columns, lbl_col_padding = lbl_col_padding) + @@ -272,36 +275,26 @@ g_forest <- function(tbl, if (!is.null(exclude_rows)) { tbl_df <- tbl_df[-exclude_rows, ] } - dat_cols <- seq(which(names(tbl_df) == "node_class") + 1, ncol(tbl_df)) - tbl_df <- tbl_df[, c(which(names(tbl_df) == "row_num"), dat_cols)] - names(tbl_df) <- c("row_num", tbl_stats) row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) + dat_cols <- seq(which(names(tbl_df) == "node_class") + 1, ncol(tbl_df)) + tbl_df <- tbl_df[, dat_cols, drop = FALSE] + names(tbl_df) <- make.unique(mat_strings[nlines_hdr, -1]) # Check table data columns if (!is.null(col_ci)) { - ci_col <- col_ci + 1 - } else { - tbl_df[["empty_ci"]] <- rep(list(c(NA_real_, NA_real_)), nrow(tbl_df)) - ci_col <- which(names(tbl_df) == "empty_ci") - } - if (nrow(tbl_df) >= 1 && length(tbl_df[, ci_col][[1]]) <= 1) { - stop("CI column must have at least two elements (lower and upper limits).") - } - - if (!is.null(col_x)) { - x_col <- col_x + 1 - } else { - tbl_df[["empty_x"]] <- NA_real_ - x_col <- which(names(tbl_df) == "empty_x") + if (nrow(tbl_df) >= 1 && length(tbl_df[, col_ci][[1]]) <= 1) { + stop("CI column must have at least two elements (lower and upper limits).") + } } - if (!is.null(col_symbol_size)) { - sym_size <- unlist(tbl_df[, col_symbol_size + 1]) + # col_symbol_size + sym_size <- if (!is.null(col_symbol_size)) { + unlist(tbl_df[, col_symbol_size]) } else { - sym_size <- rep(1, nrow(tbl_df)) + rep(1, nrow(tbl_df)) } x_ci <- if (nrow(tbl_df) >= 1) { - tbl_x_ci <- tbl_df[, unique(c(x_col, ci_col)), drop = FALSE] + tbl_x_ci <- tbl_df[, unique(c(col_x, col_ci)), drop = FALSE] x_ci_list <- lapply(tbl_x_ci, function(col) { byrow <- length(col[[1]]) != 1L matrix(unlist(col), nrow = nrow(tbl_df), byrow = byrow) @@ -310,10 +303,6 @@ g_forest <- function(tbl, } else { NULL } - # `x`, `lwr`, and `upr` are NULL when x_ci = NULL. - x <- x_ci[, 1] - lwr <- x_ci[, 2] - upr <- x_ci[, 3] # Apply log transformation. x_ci_t <- if (logx && !is.null(x_ci)) { @@ -321,9 +310,26 @@ g_forest <- function(tbl, } else { x_ci } - x_t <- x_ci_t[, 1] - lwr_t <- x_ci_t[, 2] - upr_t <- x_ci_t[, 3] + + # Extract vectors: x, lwr, and upr, and their log transformations. + # Note that x, lwr, and upr are NULL when x_ci is NULL. + # x + if (is.null(col_x)) { + x <- x_t <- rep(NA_real_, nrow(tbl_df)) + } else { + x <- x_ci[, 1L] + x_t <- x_ci_t[, 1L] + } + # ci + if (is.null(col_ci)) { + lwr <- upr <- lwr_t <- upr_t <- rep(NA_real_, nrow(tbl_df)) + } else { + lwr <- x_ci[, ncol(x_ci) - 1] + upr <- x_ci[, ncol(x_ci)] + lwr_t <- x_ci_t[, ncol(x_ci_t) - 1] + upr_t <- x_ci_t[, ncol(x_ci_t)] + } + xlim_t <- if (logx) log(xlim) else xlim if (is.null(col)) col <- "#343cff" diff --git a/man/g_forest.Rd b/man/g_forest.Rd index 5b68027088..b7fd20f391 100644 --- a/man/g_forest.Rd +++ b/man/g_forest.Rd @@ -40,8 +40,12 @@ to be manually specified. If \code{NULL}, points will be excluded from forest pl \item{col_ci}{(\code{integer(1)} or \code{NULL})\cr column index with confidence intervals. By default tries to get this from \code{tbl} attribute \code{col_ci}, otherwise needs to be manually specified. If \code{NULL}, lines will be excluded from forest plot. + The estimator and confidence interval can be stored in the same column. -In this case, \code{col_x} and \code{col_ci} must be the same.} +In this case, \code{col_x} and \code{col_ci} must be the same, and the values in each +row of the column indicated by \code{col_x}/\code{col_ci} must be ordered as the +point estimate, lower confidence interval bound, and upper confidence +interval bound, respectively.} \item{vline}{(\code{numeric(1)} or \code{NULL})\cr x coordinate for vertical line, if \code{NULL} then the line is omitted.} diff --git a/tests/testthat/test-g_forest.R b/tests/testthat/test-g_forest.R index 942c46e5b8..98253087a8 100644 --- a/tests/testthat/test-g_forest.R +++ b/tests/testthat/test-g_forest.R @@ -99,6 +99,47 @@ testthat::test_that("g_forest as_list argument works", { expect_snapshot_ggplot("g_forest_plot_only", g_forest_plot_only, width = 2, height = 3) }) +testthat::test_that("g_forest handles NULL col_x/col_ci", { + tbl <- rtable( + header = rheader(rrow("", "est", "CI")), + rrow("row 1", rcell(10), rcell(c(8, 12), format = "(xx, xx)")), + rrow("row 2", rcell(11), rcell(c(7, 13), format = "(xx, xx)")) + ) + + # col_x = NULL + testthat::expect_silent( + pN1 <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + pN1_logF <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + # col_ci = NULL + testthat::expect_silent( + p1N <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + p1N_logF <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + # col_x, col_ci = NULL + testthat::expect_silent( + pNN <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + pNN_logF <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + expect_snapshot_ggplot("g_forest_x_NULL", pN1, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_x_NULL_logF", pN1_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_ci_NULL", p1N, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_ci_NULL_logF", p1N_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_NULL", pNN, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_NULL_logF", pNN_logF, width = 15, height = 3) +}) + testthat::test_that("g_forest validates exclude_rows", { tbl <- basic_table() |> tabulate_rsp_subgroups(df) @@ -148,7 +189,7 @@ testthat::test_that("g_forest works when all rows are excluded", { expect_snapshot_ggplot("g_forest_exclude_all_rows", p, width = 15, height = 3) }) -testthat::test_that("g_forest works for point estimates and confidence intervals in the same column", { +testthat::test_that("g_forest works for point est. and CI in the same column", { tbl <- rtable( header = rheader(rrow("", "point est (CI)")), rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), @@ -159,7 +200,89 @@ testthat::test_that("g_forest works for point estimates and confidence intervals p <- g_forest(tbl, col_x = 1, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) ) - expect_snapshot_ggplot("g_forest_the_same_x_ci", p, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci", p, width = 15, height = 3) +}) + +testthat::test_that("g_forest handles NULL col_x/col_ci in same column", { + tbl <- rtable( + header = rheader(rrow("", "point est (CI)")), + rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), + rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) + ) + + # col_x = NULL + testthat::expect_silent( + pN1 <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + pN1_logF <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + # col_ci = NULL + testthat::expect_silent( + p1N <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + p1N_logF <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + # col_x, col_ci = NULL + testthat::expect_silent( + pNN <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) + ) + testthat::expect_silent( + pNN_logF <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + ) + + expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL", pN1, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL_logF", pN1_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL", p1N, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL_logF", p1N_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_same_x_ci_NULL", pNN, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_NULL_logF", pNN_logF, width = 15, height = 3) +}) + +testthat::test_that("g_forest handles NULL col_x/col_ci in same column (all rows excluded)", { + tbl <- rtable( + header = rheader(rrow("", "point est (CI)")), + rrow("row 1", rcell(c(10, 8, 12), format = "xx. (xx. - xx.)")), + rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) + ) + + # col_x = NULL + testthat::expect_silent( + pN1 <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1) + ) + testthat::expect_silent( + pN1_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1, logx = FALSE) + ) + + # col_ci = NULL + testthat::expect_silent( + p1N <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL) + ) + testthat::expect_silent( + p1N_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL, logx = FALSE) + ) + + # col_x, col_ci = NULL + testthat::expect_silent( + pNN <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL) + ) + testthat::expect_silent( + pNN_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL, logx = FALSE) + ) + + expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL", pN1, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL_logF", pN1_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL", p1N, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL_logF", p1N_logF, width = 15, height = 3) + + expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL", pNN, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL_logF", pNN_logF, width = 15, height = 3) }) testthat::test_that("g_forest argument deprecation warnings work", { From ca0405110d2df8cf2ed00880a3ea6e3526722ef0 Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 18:18:11 +0200 Subject: [PATCH 06/10] g_forest() update - updated CI column check. --- R/g_forest.R | 11 ++-- tests/testthat/test-g_forest.R | 93 ++++++++++++++++++---------------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index d8ab23d310..0fdba11a76 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -280,12 +280,17 @@ g_forest <- function(tbl, tbl_df <- tbl_df[, dat_cols, drop = FALSE] names(tbl_df) <- make.unique(mat_strings[nlines_hdr, -1]) - # Check table data columns - if (!is.null(col_ci)) { - if (nrow(tbl_df) >= 1 && length(tbl_df[, col_ci][[1]]) <= 1) { + # Check CI column. + if (nrow(tbl_df) >= 1 && !is.null(col_ci)) { + ci_len <- length(tbl_df[, col_ci][[1]]) + if (ci_len < 2) { stop("CI column must have at least two elements (lower and upper limits).") } + if (!is.null(col_x) && col_x == col_ci && ci_len != 3) { + stop("x / CI column must have three elements (point estimate, lower and upper limits).") + } } + # col_symbol_size sym_size <- if (!is.null(col_symbol_size)) { unlist(tbl_df[, col_symbol_size]) diff --git a/tests/testthat/test-g_forest.R b/tests/testthat/test-g_forest.R index 98253087a8..d1207dbd1d 100644 --- a/tests/testthat/test-g_forest.R +++ b/tests/testthat/test-g_forest.R @@ -106,38 +106,38 @@ testthat::test_that("g_forest handles NULL col_x/col_ci", { rrow("row 2", rcell(11), rcell(c(7, 13), format = "(xx, xx)")) ) - # col_x = NULL + # nolint: col_x = NULL testthat::expect_silent( - pN1 <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15)) + p_n2 <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - pN1_logF <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15), logx = FALSE) + p_n2_logf <- g_forest(tbl, col_x = NULL, col_ci = 2, vline = 10, xlim = c(5, 15), logx = FALSE) ) - # col_ci = NULL + # nolint: col_ci = NULL testthat::expect_silent( - p1N <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) + p_1n <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - p1N_logF <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + p_1n_logf <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) ) - # col_x, col_ci = NULL + # nolint: col_ci = NULL testthat::expect_silent( - pNN <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) + p_nn <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - pNN_logF <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + p_nn_logf <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) ) - expect_snapshot_ggplot("g_forest_x_NULL", pN1, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_x_NULL_logF", pN1_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_x_NULL", p_n2, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_x_NULL_logf", p_n2_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_ci_NULL", p1N, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_ci_NULL_logF", p1N_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_ci_NULL", p_1n, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_ci_NULL_logf", p_1n_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_NULL", pNN, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_NULL_logF", pNN_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_NULL", p_nn, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_NULL_logf", p_nn_logf, width = 15, height = 3) }) testthat::test_that("g_forest validates exclude_rows", { @@ -210,38 +210,38 @@ testthat::test_that("g_forest handles NULL col_x/col_ci in same column", { rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) ) - # col_x = NULL + # nolint: col_x = NULL testthat::expect_silent( - pN1 <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15)) + p_n1 <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - pN1_logF <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) + p_n1_logf <- g_forest(tbl, col_x = NULL, col_ci = 1, vline = 10, xlim = c(5, 15), logx = FALSE) ) - # col_ci = NULL + # nolint: col_ci = NULL testthat::expect_silent( - p1N <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) + p_1n <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - p1N_logF <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + p_1n_logf <- g_forest(tbl, col_x = 1, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) ) - # col_x, col_ci = NULL + # nolint: col_x, col_ci = NULL testthat::expect_silent( - pNN <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) + p_nn <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15)) ) testthat::expect_silent( - pNN_logF <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) + p_nn_logf <- g_forest(tbl, col_x = NULL, col_ci = NULL, vline = 10, xlim = c(5, 15), logx = FALSE) ) - expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL", pN1, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL_logF", pN1_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL", p_n1, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_x_NULL_logf", p_n1_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL", p1N, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL_logF", p1N_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL", p_1n, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_ci_NULL_logf", p_1n_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_NULL", pNN, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_NULL_logF", pNN_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_NULL", p_nn, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_NULL_logf", p_nn_logf, width = 15, height = 3) }) testthat::test_that("g_forest handles NULL col_x/col_ci in same column (all rows excluded)", { @@ -251,38 +251,41 @@ testthat::test_that("g_forest handles NULL col_x/col_ci in same column (all rows rrow("row 2", rcell(c(11, 7, 13), format = "xx. (xx. - xx.)")) ) - # col_x = NULL + # nolint: col_x = NULL testthat::expect_silent( - pN1 <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1) + p_n1 <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1) ) testthat::expect_silent( - pN1_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1, logx = FALSE) + p_n1_logf <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = 1, logx = FALSE) ) - # col_ci = NULL + # nolint: col_ci = NULL testthat::expect_silent( - p1N <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL) + p_1n <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL) ) testthat::expect_silent( - p1N_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL, logx = FALSE) + p_1n_logf <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = 1, col_ci = NULL, logx = FALSE) ) - # col_x, col_ci = NULL + # nolint: col_x, col_ci = NULL testthat::expect_silent( - pNN <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL) + p_nn <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL) ) testthat::expect_silent( - pNN_logF <- g_forest(tbl, exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL, logx = FALSE) + p_nn_logf <- g_forest( + tbl, + exclude_rows = 1:2, vline = 10, xlim = c(5, 15), col_x = NULL, col_ci = NULL, logx = FALSE + ) ) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL", pN1, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL_logF", pN1_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL", p_n1, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_x_NULL_logf", p_n1_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL", p1N, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL_logF", p1N_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL", p_1n, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_ci_NULL_logf", p_1n_logf, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL", pNN, width = 15, height = 3) - expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL_logF", pNN_logF, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL", p_nn, width = 15, height = 3) + expect_snapshot_ggplot("g_forest_same_x_ci_excl_NULL_logf", p_nn_logf, width = 15, height = 3) }) testthat::test_that("g_forest argument deprecation warnings work", { From d82660c4738b8b44f24f6492b7e9238b46d0f885 Mon Sep 17 00:00:00 2001 From: Wojtek Date: Mon, 10 Aug 2026 18:23:49 +0200 Subject: [PATCH 07/10] g_forest() code comment update. --- R/g_forest.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/g_forest.R b/R/g_forest.R index 0fdba11a76..4e296e9422 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -317,7 +317,7 @@ g_forest <- function(tbl, } # Extract vectors: x, lwr, and upr, and their log transformations. - # Note that x, lwr, and upr are NULL when x_ci is NULL. + # Note that when x_ci is NULL, x_ci[, i] is NULL for any index i. # x if (is.null(col_x)) { x <- x_t <- rep(NA_real_, nrow(tbl_df)) From e0b165c78948f318a3c25f9a9056969fe258db5a Mon Sep 17 00:00:00 2001 From: Wojtek Date: Tue, 11 Aug 2026 12:25:54 +0200 Subject: [PATCH 08/10] g_forest() update some assertions, add comments to the code. --- R/g_forest.R | 72 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/R/g_forest.R b/R/g_forest.R index 4e296e9422..1b8fc12c70 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -242,8 +242,8 @@ g_forest <- function(tbl, } checkmate::assert_class(tbl, "VTableTree") - checkmate::assert_number(col_x, lower = 0, upper = ncol(tbl), null.ok = TRUE) - checkmate::assert_number(col_ci, lower = 0, upper = ncol(tbl), null.ok = TRUE) + checkmate::assert_int(col_x, lower = 1L, upper = ncol(tbl), null.ok = TRUE) + checkmate::assert_int(col_ci, lower = 1L, upper = ncol(tbl), null.ok = TRUE) checkmate::assert_number(col_symbol_size, lower = 0, upper = ncol(tbl), null.ok = TRUE) checkmate::assert_number(font_size, lower = 0) checkmate::assert_character(col, null.ok = TRUE) @@ -261,47 +261,46 @@ g_forest <- function(tbl, nrows_body <- nrow(mat_strings) - nlines_hdr # Generate and modify table as ggplot object - gg_table <- rtable2gg(tbl, fontsize = font_size, colwidths = width_columns, lbl_col_padding = lbl_col_padding) + + gg_table <- rtable2gg( + tbl, + fontsize = font_size, colwidths = width_columns, lbl_col_padding = lbl_col_padding + ) + theme(plot.margin = margin(0, 0, 0, 0.025, "npc")) gg_table$scales$scales[[1]]$expand <- c(0.01, 0.01) gg_table$scales$scales[[2]]$limits[2] <- nrow(mat_strings) + 1 - if (nlines_hdr == 2) { + arms <- if (nlines_hdr == 2) { gg_table$scales$scales[[2]]$expand <- c(0, 0) - arms <- unique(mat_strings[1, ][nzchar(trimws(mat_strings[1, ]))]) + unique(mat_strings[1, ][nzchar(trimws(mat_strings[1, ]))]) } else { - arms <- NULL + NULL } + # Optionally exclude rows and keep only the data columns in `tbl_df`. if (!is.null(exclude_rows)) { tbl_df <- tbl_df[-exclude_rows, ] } row_num <- nrow(mat_strings) - tbl_df[["row_num"]] - as.numeric(nlines_hdr == 2) - dat_cols <- seq(which(names(tbl_df) == "node_class") + 1, ncol(tbl_df)) - tbl_df <- tbl_df[, dat_cols, drop = FALSE] + node_class_idx <- match("node_class", names(tbl_df)) + tbl_df <- tbl_df[, -seq_len(node_class_idx), drop = FALSE] names(tbl_df) <- make.unique(mat_strings[nlines_hdr, -1]) - # Check CI column. - if (nrow(tbl_df) >= 1 && !is.null(col_ci)) { - ci_len <- length(tbl_df[, col_ci][[1]]) - if (ci_len < 2) { - stop("CI column must have at least two elements (lower and upper limits).") + # Validate the number of elements in a CI cell. + if (nrow(tbl_df) >= 1L && !is.null(col_ci)) { + # Since an `rtables` column is homogeneous, use the first row to determine the CI cell length. + ci_len <- length(tbl_df[, col_ci][[1L]]) + if (ci_len < 2L) { + stop("A CI cell must contain at least two elements (lower and upper limits).") } - if (!is.null(col_x) && col_x == col_ci && ci_len != 3) { - stop("x / CI column must have three elements (point estimate, lower and upper limits).") + if (!is.null(col_x) && col_x == col_ci && ci_len != 3L) { + stop("An x / CI cell must contain three elements (point estimate, lower and upper limits).") } } - # col_symbol_size - sym_size <- if (!is.null(col_symbol_size)) { - unlist(tbl_df[, col_symbol_size]) - } else { - rep(1, nrow(tbl_df)) - } - - x_ci <- if (nrow(tbl_df) >= 1) { + x_ci <- if (nrow(tbl_df) >= 1L) { tbl_x_ci <- tbl_df[, unique(c(col_x, col_ci)), drop = FALSE] x_ci_list <- lapply(tbl_x_ci, function(col) { - byrow <- length(col[[1]]) != 1L + # Since an `rtables` column is homogeneous, use the first row to determine the fill direction. + byrow <- length(col[[1L]]) != 1L matrix(unlist(col), nrow = nrow(tbl_df), byrow = byrow) }) do.call(cbind, x_ci_list) @@ -309,7 +308,7 @@ g_forest <- function(tbl, NULL } - # Apply log transformation. + # Optionally apply a log transformation to `x_ci`. x_ci_t <- if (logx && !is.null(x_ci)) { log(x_ci) } else { @@ -317,26 +316,39 @@ g_forest <- function(tbl, } # Extract vectors: x, lwr, and upr, and their log transformations. - # Note that when x_ci is NULL, x_ci[, i] is NULL for any index i. - # x + # + # If `nrow(tbl_df) == 0`, `x_ci` (`x_ci_t`) is `NULL`, so + # `x_ci[, i]` (`x_ci_t[, i]`) is `NULL` for any `i`. + # + # At this point, `x_ci` (`x_ci_t`) has at least one column whenever + # `nrow(tbl_df) >= 1` and `col_x` is not `NULL`. if (is.null(col_x)) { x <- x_t <- rep(NA_real_, nrow(tbl_df)) } else { x <- x_ci[, 1L] x_t <- x_ci_t[, 1L] } - # ci + # At this point, `x_ci` (`x_ci_t`) has at least two columns whenever + # `nrow(tbl_df) >= 1` and `col_ci` is not `NULL` (see the + # "Validate the number of elements in a CI cell" section above). if (is.null(col_ci)) { lwr <- upr <- lwr_t <- upr_t <- rep(NA_real_, nrow(tbl_df)) } else { - lwr <- x_ci[, ncol(x_ci) - 1] + lwr <- x_ci[, ncol(x_ci) - 1L] upr <- x_ci[, ncol(x_ci)] - lwr_t <- x_ci_t[, ncol(x_ci_t) - 1] + lwr_t <- x_ci_t[, ncol(x_ci_t) - 1L] upr_t <- x_ci_t[, ncol(x_ci_t)] } xlim_t <- if (logx) log(xlim) else xlim + # col_symbol_size + sym_size <- if (!is.null(col_symbol_size)) { + unlist(tbl_df[, col_symbol_size]) + } else { + rep(1L, nrow(tbl_df)) + } + if (is.null(col)) col <- "#343cff" if (length(col) == 1) col <- rep(col, nrow(tbl_df)) if (is.null(x_at)) x_at <- union(xlim, vline) From a61b76ffada463f3db1c658cda470b70f66466bc Mon Sep 17 00:00:00 2001 From: Wojtek Date: Wed, 12 Aug 2026 13:08:45 +0200 Subject: [PATCH 09/10] update g_forest(): add assertions for x_ci and x_ci_t. --- R/g_forest.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/g_forest.R b/R/g_forest.R index 1b8fc12c70..63c8879228 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -334,6 +334,8 @@ g_forest <- function(tbl, if (is.null(col_ci)) { lwr <- upr <- lwr_t <- upr_t <- rep(NA_real_, nrow(tbl_df)) } else { + checkmate::assert_matrix(x_ci, min.cols = 2, null.ok = TRUE) + checkmate::assert_matrix(x_ci_t, min.cols = 2, null.ok = TRUE) lwr <- x_ci[, ncol(x_ci) - 1L] upr <- x_ci[, ncol(x_ci)] lwr_t <- x_ci_t[, ncol(x_ci_t) - 1L] From 37c642675f5fecbc812be1e2e3e4c20e0707b71e Mon Sep 17 00:00:00 2001 From: Wojtek Date: Wed, 12 Aug 2026 14:28:18 +0200 Subject: [PATCH 10/10] g_forest() - add assertion for tbl_df column homogenity for col_ci. --- R/g_forest.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/g_forest.R b/R/g_forest.R index 63c8879228..943a8107ed 100644 --- a/R/g_forest.R +++ b/R/g_forest.R @@ -286,8 +286,11 @@ g_forest <- function(tbl, # Validate the number of elements in a CI cell. if (nrow(tbl_df) >= 1L && !is.null(col_ci)) { - # Since an `rtables` column is homogeneous, use the first row to determine the CI cell length. ci_len <- length(tbl_df[, col_ci][[1L]]) + # Assert that `tbl_df[, col_ci]` column is homogeneous. + checkmate::assert_true( + all(sapply(tbl_df[, col_ci], length) == ci_len) + ) if (ci_len < 2L) { stop("A CI cell must contain at least two elements (lower and upper limits).") }