Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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
28 changes: 23 additions & 5 deletions R/g_forest.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
#' 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 [rtables::as_result_df()] applied to `tbl`. Values
Comment thread
wwojciech marked this conversation as resolved.
Outdated
#' must be between 1 and the number of rows in the result data frame, with no
#' missing values. The specified rows are removed before plotting. This can be
Comment thread
wwojciech marked this conversation as resolved.
Outdated
#' 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 +85,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 +186,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 +230,10 @@ 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))
checkmate::assert_integerish(
exclude_rows,
lower = 1L, upper = nrow(as_result_df(tbl)), any.missing = FALSE, min.len = 1L, null.ok = TRUE
)

# Extract info from table
mat <- matrix_form(tbl, indent_rownames = TRUE)
Expand All @@ -241,6 +255,9 @@ g_forest <- function(tbl,
}

tbl_df <- as_result_df(tbl)
Comment thread
wwojciech marked this conversation as resolved.
Outdated
if (!is.null(exclude_rows)) {
tbl_df <- tbl_df[-exclude_rows, ]

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 ^^

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.

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 +269,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 @@ -278,17 +297,16 @@ g_forest <- function(tbl,
x_labels <- x_at

# Apply log transformation
if (logx) {
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
13 changes: 12 additions & 1 deletion man/g_forest.Rd

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
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