Skip to content

Add exclude_rows argument to g_forest() (Solves #1498). #1503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wwojciech wants to merge 2 commits into from
+91 −8
Closed

Add exclude_rows argument to g_forest() (Solves #1498). #1503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
336c20c
Add exclude_rows arg to g_forest() (#1498).
wwojciech Jul 28, 2026
3bdd1b1
addressing PR review comments #1498.
wwojciech Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# tern 0.9.11.9000

### Enhancements
* 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
when converting character or logical columns. Supported methods: `"sort_auto"` (default,
locale-aware, preserves original behavior), `"sort_radix"` (byte-order / ASCII sort), and
Expand Down
34 changes: 27 additions & 7 deletions R/g_forest.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@
#' is no longer used.
#' @param newpage `r lifecycle::badge("deprecated")` `g_forest` is now generated as a `ggplot` object. This argument
#' is no longer used.
#' @param exclude_rows (`integerish` or `NULL`)\cr vector of positive row
#' indices specifying rows to exclude from the forest plot. Row indices are
#' specified relative to the data frame obtained by applying
#' [rtables::as_result_df()] to `tbl`. No elements of `exclude_rows` may be
#' missing.
#' The specified rows are removed before plotting. This can be used to omit
#' rows that should not be displayed in the forest plot, such as rows
#' containing non-plottable values. Defaults to `NULL`, meaning that all rows
#' are considered for plotting.
#'
#' @return `ggplot` forest plot and table.
#'
Expand Down Expand Up @@ -77,6 +86,7 @@
#' tbl <- basic_table() |>
#' tabulate_rsp_subgroups(df)
#' g_forest(tbl)
#' g_forest(tbl, exclude_rows = 1)
#'
#' # Odds ratio only table.
#'
Expand Down Expand Up @@ -177,7 +187,8 @@ g_forest <- function(tbl,
as_list = FALSE,
gp = lifecycle::deprecated(),
draw = lifecycle::deprecated(),
newpage = lifecycle::deprecated()) {
newpage = lifecycle::deprecated(),
exclude_rows = NULL) {
# Deprecated argument warnings
if (lifecycle::is_present(width_row_names)) {
lifecycle::deprecate_warn(
Expand Down Expand Up @@ -220,6 +231,11 @@ g_forest <- function(tbl,
checkmate::assert_number(font_size, lower = 0)
checkmate::assert_character(col, null.ok = TRUE)
checkmate::assert_true(is.null(col) | length(col) == 1 | length(col) == nrow(tbl))
tbl_df <- as_result_df(tbl)
checkmate::assert_integerish(
exclude_rows,
lower = 1L, upper = nrow(tbl_df), any.missing = FALSE, min.len = 1L, null.ok = TRUE
)

# Extract info from table
mat <- matrix_form(tbl, indent_rownames = TRUE)
Expand All @@ -240,7 +256,9 @@ g_forest <- function(tbl,
arms <- NULL
}

tbl_df <- as_result_df(tbl)
if (!is.null(exclude_rows)) {
tbl_df <- tbl_df[-exclude_rows, ]

@Melkiades Melkiades Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand the need of this parameter inside this function when you can add the filtering before the input i.e. doing tbl_df <- tbl_df[-exclude_rows, ] just before running g_forest()

The rest is fine, it makes the function more stable ^^

@wwojciech wwojciech Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbl_df <- tbl_df[-exclude_rows, ]

The issue is that using g_forest(tbl[-exclude_rows, ]) removes rows from the table itself. The intention is to keep these rows visible in the table while only excluding them from the forest plot.

@Melkiades Melkiades Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense! sorry I had to double check its behavior again ^^ then it is is good to go!

}
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)
Expand All @@ -252,7 +270,9 @@ 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 (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]]) != 2) {
stop("CI column must have two elements (lower and upper limits).")
}

if (!is.null(col_x)) {
x_col <- col_x + 1
Expand All @@ -277,18 +297,18 @@ g_forest <- function(tbl,
if (is.null(x_at)) x_at <- union(xlim, vline)
x_labels <- x_at

# Apply log transformation
if (logx) {
# Apply log transformation.
# When nrow(tbl_df) == 0, x, lwr, and upr are NULL, so log() would fail.
if (logx && nrow(tbl_df) >= 1) {
Comment thread
wwojciech marked this conversation as resolved.
x_t <- log(x)
lwr_t <- log(lwr)
upr_t <- log(upr)
xlim_t <- log(xlim)
} else {
x_t <- x
lwr_t <- lwr
upr_t <- upr
xlim_t <- xlim
}
xlim_t <- if (logx) log(xlim) else xlim
Comment thread
wwojciech marked this conversation as resolved.

# Set up plot area
gg_plt <- ggplot(data = tbl_df) +
Expand Down
14 changes: 13 additions & 1 deletion man/g_forest.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions tests/testthat/test-g_forest.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,55 @@ 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 validates exclude_rows", {
tbl <- basic_table() |>
tabulate_rsp_subgroups(df)

testthat::expect_error(
g_forest(tbl, exclude_rows = 0)
)

testthat::expect_error(
g_forest(tbl, exclude_rows = -1)
)

testthat::expect_error(
g_forest(tbl, exclude_rows = NA_integer_)
)

testthat::expect_error(
g_forest(tbl, exclude_rows = nrow(as_result_df(tbl)) + 1)
)

testthat::expect_error(
g_forest(tbl, exclude_rows = "1")
)
})

testthat::test_that("g_forest exclude_rows works", {
tbl <- basic_table() |>
tabulate_rsp_subgroups(df)

testthat::expect_silent(
p <- g_forest(tbl, exclude_rows = c(2, 4))
)

expect_snapshot_ggplot("g_forest_exclude_rows", p, width = 15, height = 3)
})

testthat::test_that("g_forest works when all rows are excluded", {
tbl <- basic_table() |>
tabulate_rsp_subgroups(df)

exclude_rows <- seq_len(nrow(as_result_df(tbl)))

testthat::expect_silent(
p <- g_forest(tbl, exclude_rows = exclude_rows)
)

expect_snapshot_ggplot("g_forest_exclude_all_rows", p, width = 15, height = 3)
})

testthat::test_that("g_forest argument deprecation warnings work", {
tbl <- basic_table() |>
tabulate_rsp_subgroups(df)
Expand Down
Loading