From fea3bcff4ba5500875a7b49c102a9c6856c4b2c4 Mon Sep 17 00:00:00 2001 From: Davide Garolini Date: Tue, 12 Mar 2024 09:33:49 +0100 Subject: [PATCH 1/2] Fix `section_div` in case of multiple var analyzed (`AnalyzeMultiVars`) (#836) * Fixing section div * [skip style] [skip vbump] Restyle files * other test * [skip style] [skip vbump] Restyle files * empty --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- NEWS.md | 1 + R/00tabletrees.R | 3 ++- R/tt_dotabulation.R | 2 ++ R/utils.R | 2 +- tests/testthat/setup-fakedata.R | 7 +++++++ tests/testthat/test-accessors.R | 6 ------ tests/testthat/test-exporters.R | 2 +- tests/testthat/test-printing.R | 28 ++++++++++++++++++++++++++++ 8 files changed, 42 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 941820ea3..75553504a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,7 @@ * Fixed issue with `rtables_root` not being removed when using `as_result_df`. * Fixed edge case bug in `as_result_df` where rows of the table have only `"root"` as path index. * Fixed `sort_at_path` pathing to ignore leading `"root"` element (regardless of actual root element name) to match current `tt_at_path` behavior. + * Fixed `section_div` for analysis of multiple variables (`AnalyzeMultiVars`). ## rtables 0.6.6 ### New Features diff --git a/R/00tabletrees.R b/R/00tabletrees.R index 18419f95c..40e0ef901 100644 --- a/R/00tabletrees.R +++ b/R/00tabletrees.R @@ -814,6 +814,7 @@ AnalyzeMultiVars <- function(var, cformat <- .repoutlst(cformat, nv) ## split_format = .repoutlst(split_format, nv) inclNAs <- .repoutlst(inclNAs, nv) + section_div_if_multivar <- if (length(var) > 1) NA_character_ else section_div pld <- mapply(AnalyzeVarSplit, var = var, split_name = child_names, @@ -830,7 +831,7 @@ AnalyzeMultiVars <- function(var, label_pos = show_kidlabs, split_format = split_format, split_na_str = split_na_str, - section_div = section_div + section_div = section_div_if_multivar ), ## rvis), SIMPLIFY = FALSE ) diff --git a/R/tt_dotabulation.R b/R/tt_dotabulation.R index 06d4f9de7..cdafe5b7d 100644 --- a/R/tt_dotabulation.R +++ b/R/tt_dotabulation.R @@ -692,6 +692,8 @@ setMethod( ... )) + kids <- .set_kids_section_div(kids, spl_section_div(spl), "VTableTree") + ## XXX this seems like it should be identical not !identical ## TODO FIXME if (!identical(make_lrow, FALSE) && !have_controws && length(kids) == 1) { diff --git a/R/utils.R b/R/utils.R index 5f347ef99..0b93413a2 100644 --- a/R/utils.R +++ b/R/utils.R @@ -108,7 +108,7 @@ paste_vec <- function(vec) { # Utility for checking if a package is installed check_required_packages <- function(pkgs) { for (pkgi in pkgs) { - if (!requireNamespace(pkgi)) { + if (!requireNamespace(pkgi, quietly = TRUE)) { stop( "This function requires the ", pkgi, " package. ", "Please install it if you wish to use it" diff --git a/tests/testthat/setup-fakedata.R b/tests/testthat/setup-fakedata.R index c39215679..f22111193 100644 --- a/tests/testthat/setup-fakedata.R +++ b/tests/testthat/setup-fakedata.R @@ -223,3 +223,10 @@ tt_for_nl <- tt_to_test_newline_chars() nchar(str) - nchar(gsub(chr, "", str, fixed = TRUE)) } } + +# Utility function for section_div tests +check_pattern <- function(element, letter, len) { + # Regular expression to match exactly len of the same letter + regex <- paste0(rep(letter, len), collapse = "") + return(grepl(regex, element, fixed = TRUE)) +} diff --git a/tests/testthat/test-accessors.R b/tests/testthat/test-accessors.R index 479cf1907..1512d86b6 100644 --- a/tests/testthat/test-accessors.R +++ b/tests/testthat/test-accessors.R @@ -225,12 +225,6 @@ test_that("header sep setting works", { }) # section_div tests ------------------------------------------------------------ -check_pattern <- function(element, letter, len) { - # Regular expression to match exactly len of the same letter - regex <- paste0(rep(letter, len), collapse = "") - return(grepl(regex, element, fixed = TRUE)) -} - test_structure_with_a_getter <- function(tbl, getter, val_per_lev) { # Main table obj expect_identical(tbl %>% getter(), val_per_lev$global) diff --git a/tests/testthat/test-exporters.R b/tests/testthat/test-exporters.R index dd5f5a93f..ec10f6793 100644 --- a/tests/testthat/test-exporters.R +++ b/tests/testthat/test-exporters.R @@ -390,7 +390,7 @@ test_that("Can create flextable object that works with different styles", { # internal package check not_a_pkg <- "bwrereloakdosirabttjtaeerr" - suppressMessages(expect_error(check_required_packages(c("flextable", not_a_pkg)), not_a_pkg)) + expect_error(check_required_packages(c("flextable", not_a_pkg)), not_a_pkg) }) test_that("export_as_doc works thanks to tt_to_flextable", { diff --git a/tests/testthat/test-printing.R b/tests/testthat/test-printing.R index 2488c43c5..15de7d526 100644 --- a/tests/testthat/test-printing.R +++ b/tests/testthat/test-printing.R @@ -397,6 +397,34 @@ test_that("section_div works throughout", { expect_identical(length(mylns), 31L) ## sect div not printed for last one }) +test_that("section_div works when analyzing multiple variables", { + # Regression test for #835 + lyt <- basic_table() %>% + split_rows_by("Species", section_div = "|") %>% + analyze(c("Petal.Width", "Petal.Length"), + afun = function(x) list("m" = mean(x), "sd" = sd(x)), section_div = "-" + ) + + tbl <- build_table(lyt, iris) + out <- strsplit(toString(tbl), "\n")[[1]] + + expect_true(check_pattern(out[11], "|", length(out[1]))) + expect_true(check_pattern(out[16], "-", length(out[1]))) + + # One-var still works + lyt <- basic_table() %>% + split_rows_by("Species", section_div = "|") %>% + analyze("Petal.Width", + afun = function(x) list("m" = mean(x), "sd" = sd(x)), section_div = "-" + ) + + tbl <- build_table(lyt, iris) + out <- strsplit(toString(tbl), "\n")[[1]] + + expect_true(check_pattern(out[7], "|", length(out[1]))) + expect_true(check_pattern(out[10], "-", length(out[1]))) +}) + test_that("Inset works for table, ref_footnotes, and main footer", { general_inset <- 3 From 3cd599f63b8f8d43d258aa9773ef2cd841fd0aa0 Mon Sep 17 00:00:00 2001 From: Pawel Rucki <12943682+pawelru@users.noreply.github.com> Date: Fri, 15 Mar 2024 15:46:41 +0100 Subject: [PATCH 2/2] fix verdepcheck (#841) * reintroduce rmarkdown; vbump formatters; fix typo in gh ref * test Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> * Update .github/workflows/scheduled.yaml Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> * mv image file used in vignette * rm test Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> --------- Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> --- DESCRIPTION | 7 ++- inst/WORDLIST | 59 +++++++++--------- .../images}/rtables-basics.png | Bin vignettes/introduction.Rmd | 2 +- 4 files changed, 35 insertions(+), 33 deletions(-) rename {man/figures => vignettes/images}/rtables-basics.png (100%) diff --git a/DESCRIPTION b/DESCRIPTION index 4f4151007..7dc49c283 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -29,7 +29,7 @@ URL: https://github.com/insightsengineering/rtables, https://insightsengineering.github.io/rtables/ BugReports: https://github.com/insightsengineering/rtables/issues Depends: - formatters (>= 0.5.5), + formatters (>= 0.5.5.9005), magrittr (>= 1.5), methods, R (>= 2.10) @@ -46,6 +46,7 @@ Suggests: knitr (>= 1.42), officer (>= 0.5.0), r2rtf (>= 0.3.2), + rmarkdown (>= 2.19), survival (>= 3.3-1), testthat (>= 3.0.4), tibble (>= 3.2.1), @@ -56,9 +57,9 @@ VignetteBuilder: knitr Config/Needs/verdepcheck: insightsengineering/formatters, tidyverse/magrittr, mllg/checkmate, rstudio/htmltools, - gogolewski/stringi, tidymodels/broom, cran/car, tidyverse/dplyr, + gagolews/stringi, tidymodels/broom, cran/car, tidyverse/dplyr, davidgohel/flextable, yihui/knitr, davidgohel/officer, Merck/r2rtf, - r-lib/testthat, tidyverse/tibble, tidyverse/tidyr, r-lib/withr, + rstudio/rmarkdown, r-lib/testthat, tidyverse/tibble, tidyverse/tidyr, r-lib/withr, r-lib/xml2 Encoding: UTF-8 Language: en-US diff --git a/inst/WORDLIST b/inst/WORDLIST index b64fd5939..ece47f93b 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -1,37 +1,11 @@ +amongst Arg -CRAN's Carreras +charset Cheatsheet Chohan -FFFL -Godwin -Heng -Kelkhoff -Layouting -Lewandowski -Maximo -Modelling -NSE -Paszty -Pharma -Phuse -Postprocessing -Pre -Qi -RStudio -Resync -STUDYID -Saibah -Stoilova -Subtable -Subtables -TableTree -Tadeusz -Unstratified -Yung -amongst -charset combinatorial +CRAN's customizations decrementing dimensioned @@ -39,44 +13,71 @@ dplyr emph facetted facetting +FFFL formatter getter getters +Godwin +Heng ing initializer integerish iteratively +Kelkhoff labelled +Layouting layouting +Lewandowski mandatorily +Maximo +Modelling multivariable +NSE orderable +Paszty pathing +Pharma +Phuse postfix +Postprocessing postprocessing +Pre pre priori programmatically +Qi quartiles reindexed repo repped responder +Resync reusability roadmap +RStudio +rtables +Saibah sortable spl +Stoilova +STUDYID +Subtable subtable subtable's +Subtables subtables summarization tableone +TableTree +Tadeusz todo unaggregated unicode univariable unnested unpruned +Unstratified unstratified useR xtable +Yung diff --git a/man/figures/rtables-basics.png b/vignettes/images/rtables-basics.png similarity index 100% rename from man/figures/rtables-basics.png rename to vignettes/images/rtables-basics.png diff --git a/vignettes/introduction.Rmd b/vignettes/introduction.Rmd index d5d11712b..681ce562d 100644 --- a/vignettes/introduction.Rmd +++ b/vignettes/introduction.Rmd @@ -51,7 +51,7 @@ table object, a formatted table can be printed in ASCII format, or exported to a variety of other formats (`.txt`, `.pdf`, `.docx`, etc.). ```{r echo=FALSE, fig.align='center'} -knitr::include_graphics("../man/figures/rtables-basics.png") +knitr::include_graphics("./images/rtables-basics.png") ``` ## Data