Skip to content
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.

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

Fix formatted values different from cell values #988

Merged
merged 7 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

### Bug Fixes
* Fixed issue with `split_cols_by_multivar()` when having more than one value. Now `as_result_df(make_ard = TRUE)` adds a predefined split name for each of the `multivar` splits.
* Fixed bug happening when format functions were changing the number of printed values. Now `as_result_df(make_ard = TRUE)` uses the cell values for `stat_strings` for these exceptions.

## rtables 0.6.11

Expand Down
57 changes: 51 additions & 6 deletions R/tt_as_df.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#' @param add_tbl_name_split (`flag`)\cr when `TRUE` and when the table has more than one
#' `analyze(table_names = "<diff_names>")`, the table names will be present as a group split named
#' `"<analysis_spl_tbl_name>"`.
#' @param verbose (`flag`)\cr when `TRUE`, the function will print additional information for
#' `data_format != "full_precision"`.
#' @param ... additional arguments passed to spec-specific result data frame function (`spec`).
#'
#' @return
Expand All @@ -46,6 +48,7 @@ as_result_df <- function(tt, spec = NULL,
keep_label_rows = FALSE,
add_tbl_name_split = FALSE,
simplify = FALSE,
verbose = FALSE,
...) {
data_format <- data_format[[1]]
checkmate::assert_class(tt, "VTableTree")
Expand All @@ -56,6 +59,7 @@ as_result_df <- function(tt, spec = NULL,
checkmate::assert_flag(keep_label_rows)
checkmate::assert_flag(simplify)
checkmate::assert_flag(add_tbl_name_split)
checkmate::assert_flag(verbose)

if (nrow(tt) == 0) {
return(sanitize_table_struct(tt))
Expand All @@ -70,7 +74,7 @@ as_result_df <- function(tt, spec = NULL,
if (is.null(spec)) {
# raw values
rawvals <- cell_values(tt)
cellvals <- .make_df_from_raw_data(rawvals, nr = nrow(tt), nc = ncol(tt))
cellvals_init <- .make_df_from_raw_data(rawvals, nr = nrow(tt), nc = ncol(tt))

if (data_format %in% c("strings", "numeric")) {
# we keep previous calculations to check the format of the data
Expand All @@ -80,15 +84,17 @@ as_result_df <- function(tt, spec = NULL,
mf_result_numeric <- .make_numeric_char_mf(mf_result_chars)
mf_result_chars <- as.data.frame(mf_result_chars)
mf_result_numeric <- as.data.frame(mf_result_numeric)
if (!setequal(dim(mf_result_numeric), dim(cellvals)) || !setequal(dim(mf_result_chars), dim(cellvals))) {
cond1 <- !setequal(dim(mf_result_chars), dim(cellvals_init))
cond2 <- !setequal(dim(mf_result_numeric), dim(cellvals_init))
if (cond1 || cond2) {
stop(
"The extracted numeric data.frame does not have the same dimension of the",
" cell values extracted with cell_values(). This is a bug. Please report it."
) # nocov
}

colnames(mf_result_chars) <- colnames(cellvals)
colnames(mf_result_numeric) <- colnames(cellvals)
colnames(mf_result_chars) <- colnames(cellvals_init)
colnames(mf_result_numeric) <- colnames(cellvals_init)
if (data_format == "strings") {
cellvals <- mf_result_chars
if (isTRUE(make_ard)) {
Expand All @@ -101,6 +107,41 @@ as_result_df <- function(tt, spec = NULL,
cellvals <- mf_result_numeric
}
}

diff_in_cellvals <- length(unlist(cellvals_init)) - length(unlist(cellvals))
if (make_ard && abs(diff_in_cellvals) > 0) {
warning_msg <- paste0(
"Found ", abs(diff_in_cellvals), " cell values that differ from ",
"printed values. This is possibly related to conditional formatting. "
)

# number of values difference mask between cellvals and cellvals_init (TRUE if different)
dmc <- lengths(unlist(cellvals, recursive = FALSE)) != lengths(unlist(cellvals_init, recursive = FALSE))
dmc <- matrix(dmc, nrow = nrow(cellvals), ncol = ncol(cellvals))

# Mainly used for debugging
selected_rows_to_print <- mf_strings(mf_tt)[-seq_len(mf_nlheader(mf_tt)), , drop = FALSE]
selected_rows_to_print <- cbind(
which(apply(dmc, 1, any, simplify = TRUE)),
as.data.frame(selected_rows_to_print[apply(dmc, 1, any), , drop = FALSE])
)
colnames(selected_rows_to_print) <- c("row_number", "row_name", colnames(cellvals_init))
warning_msg <- if (verbose) {
paste0(
warning_msg,
"\n",
"The following row names were modified: ",
paste(selected_rows_to_print$row_name, sep = ", ", collapse = ", "),
"\n"
)
} else {
paste0(warning_msg, "To see the affected row names use `verbose = TRUE`.")
}
warning(warning_msg)
cellvals[dmc] <- cellvals_init[dmc]
}
} else {
cellvals <- cellvals_init
}

rdf <- make_row_df(tt)
Expand All @@ -115,7 +156,11 @@ as_result_df <- function(tt, spec = NULL,
# Correcting maxlen for even number of paths (only multianalysis diff table names)
maxlen <- max(lengths(df$path))
if (maxlen %% 2 != 0) {
maxlen <- maxlen + 1
maxlen <- if (add_tbl_name_split) {
maxlen + 1
} else {
maxlen - 1
}
}

# Loop for metadata (path and details from make_row_df)
Expand Down Expand Up @@ -299,7 +344,7 @@ as_result_df <- function(tt, spec = NULL,
if (!"already_done" %in% names(list(...))) {
stat_string_ret <- as_result_df(
tt = tt, spec = spec, data_format = "numeric",
make_ard = TRUE, already_done = TRUE, ...
make_ard = TRUE, already_done = TRUE, verbose = verbose, ...
)
ret_w_cols <- cbind(ret_w_cols, "stat_string" = stat_string_ret$stat)
}
Expand Down
4 changes: 4 additions & 0 deletions man/data.frame_export.Rd

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

26 changes: 25 additions & 1 deletion tests/testthat/test-result_data_frame.R
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ test_that("Result Data Frame generation works v0", {
analyze(c("AGE", "SEX"))

tbl4 <- build_table(lyt4, DM)
result_df4 <- as_result_df(tbl4)
result_df4 <- as_result_df(tbl4, add_tbl_name_split = TRUE)

expect_identical(
names(result_df4),
Expand Down Expand Up @@ -566,3 +566,27 @@ test_that("make_ard works with split_cols_by_multivar", {
expect_silent(out <- as_result_df(tbl, make_ard = TRUE))
expect_true(all(out$group3 == "multivar_split1"))
})
test_that("make_ard works when printed format differs from cell values", {
mean_sd_custom <- function(x, ...) {
rcell(c(1, 2),
label = "Mean (SD)", format = function(xf, ...) {
return(as.character(xf[1]))
}
)
}

test_out <- basic_table() %>%
split_rows_by("ARM") %>%
split_cols_by("ARM") %>%
analyze(vars = "AGE", afun = mean_sd_custom) %>%
build_table(DM)

expect_warning(
out <- as_result_df(test_out, make_ard = TRUE, verbose = TRUE),
"Found 9 cell"
)
expect_equal(
out$stat,
out$stat_string
)
})
Loading