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

Support pagination of lists of rtables/rlistings objects #213

Merged
merged 35 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
149a47b
First draft
edelarua Oct 13, 2023
c2f271a
Simplify condition, use page_break
edelarua Oct 13, 2023
26d56ea
Update NEWS
edelarua Oct 13, 2023
36c7466
Correct default rep_cols for list of listings
edelarua Oct 14, 2023
0e04f6f
Merge branch 'main' into 212_page_by_listings@main
edelarua Mar 5, 2024
b14fdd7
Fix lint
edelarua Mar 5, 2024
fa7b8bf
Update processing of lists
edelarua Mar 6, 2024
42674ff
Simplify
edelarua Mar 6, 2024
f42f3f5
Clean up code
edelarua Mar 6, 2024
944a0db
Update NEWS
edelarua Mar 7, 2024
7e28129
rework
Melkiades Mar 18, 2024
359b6b5
styling
Melkiades Mar 18, 2024
1cead74
styling
Melkiades Mar 18, 2024
5b22f43
does it work with pdfs?
Melkiades Mar 18, 2024
3c09e79
[skip roxygen] [skip vbump] Roxygen Man Pages Auto Update
dependabot-preview[bot] Mar 18, 2024
29accaf
fixes and tests
Melkiades Mar 22, 2024
f7163a5
styler
Melkiades Mar 22, 2024
5b5da85
Merge branch '212_page_by_listings@main' of github.com:insightsengine…
Melkiades Mar 22, 2024
8fdd70c
[skip roxygen] [skip vbump] Roxygen Man Pages Auto Update
dependabot-preview[bot] Mar 22, 2024
c217ce2
Merge branch 'main' into 212_page_by_listings@main
Melkiades Mar 22, 2024
b6b2a1f
lintr fix
Melkiades Mar 22, 2024
d8925e1
Apply suggestions from code review
Melkiades Mar 25, 2024
04f92ee
still to fix ncols
Melkiades Mar 25, 2024
c906e18
solving all issues but ncols
Melkiades Mar 26, 2024
7a20619
styling
Melkiades Mar 26, 2024
cc713d3
Fixes of tests
Melkiades Mar 27, 2024
50b3064
it should work
Melkiades Mar 27, 2024
57ed1c0
fix
Melkiades Mar 27, 2024
29daaff
fix
Melkiades Mar 27, 2024
6015748
adding tests and checks for num_rep_cols
Melkiades Apr 2, 2024
f89e089
fix
Melkiades Apr 2, 2024
8e6c9d9
[skip style] [skip vbump] Restyle files
github-actions[bot] Apr 2, 2024
20dfe40
fixes for no breaking changes
Melkiades Apr 2, 2024
3a86591
further fixes
Melkiades Apr 2, 2024
25396b0
Merge branch '212_page_by_listings@main' of github.com:insightsengine…
Melkiades Apr 2, 2024
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 @@ -10,6 +10,7 @@
* Fixed mismatch between pagination and exports regarding the value assigned to parameter `max_width`. Introduced general handler `.handle_max_width` for pagination, exports, and `toString`.
* Fixed bug in `format_value` causing a warning for vectors containing both NA and non-NA values.
* Fixed issue with `var_label` assignment that needed to be of non-named strings.
* Updated `export_as_txt` to allow lists of tables/listings as input. This enables listing pagination with pages by parameter.

## formatters 0.5.5
* Applied `styler` and resolved package lint. Changed default indentation from 4 spaces to 2.
Expand Down
33 changes: 28 additions & 5 deletions R/matrix_form.R
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ MatrixPrintForm <- function(strings = NULL,
main_title = "",
subtitles = character(),
page_titles = character(),
listing_keycols = NULL,
main_footer = "",
prov_footer = character(),
header_section_div = NA_character_,
Expand Down Expand Up @@ -315,6 +316,7 @@ MatrixPrintForm <- function(strings = NULL,
header_section_div = header_section_div,
horizontal_sep = horizontal_sep,
col_gap = col_gap,
listing_keycols = listing_keycols,
table_inset = as.integer(table_inset),
has_topleft = has_topleft,
indent_size = indent_size,
Expand Down Expand Up @@ -915,8 +917,12 @@ basic_matrix_form <- function(df, parent_path = "root", ignore_rownames = FALSE,
has_topleft = FALSE,
nlines_header = 1,
nrow_header = 1,
has_rowlabs = TRUE
has_rowlabs = isFALSE(ignore_rownames)
)

# Check for ncols
stopifnot(mf_has_rlabels(ret) == isFALSE(ignore_rownames))

ret <- mform_build_refdf(ret)

if (add_decoration) {
Expand All @@ -936,7 +942,7 @@ basic_matrix_form <- function(df, parent_path = "root", ignore_rownames = FALSE,
#'
#' @param keycols character. Vector of `df` column names that are printed first and
#' repeated values are assigned to `""`. This format is characteristic of a listing matrix form.
#' When `NULL`, no key columns are used. Defaults to `c("vs", "gear")` for `mtcars` default dataset.
#' When `NULL`, no key columns are used.
#' @return A valid `MatrixPrintForm` object representing `df` as a listing,
#' ready for ASCII rendering.
#'
Expand All @@ -946,7 +952,7 @@ basic_matrix_form <- function(df, parent_path = "root", ignore_rownames = FALSE,
#'
#' @export
basic_listing_mf <- function(df,
keycols = c("vs", "gear"),
keycols = names(df)[1], # c("vs", "gear")
ignore_rownames = FALSE,
add_decoration = TRUE) {
checkmate::assert_data_frame(df)
Expand All @@ -958,6 +964,9 @@ basic_listing_mf <- function(df,
add_decoration = add_decoration
)

# keycols addition to MatrixPrintForm (should happen in the constructor)
dfmf$listing_keycols <- keycols

# Modifications needed for making it a listings
mf_strings(dfmf)[1, ] <- colnames(mf_strings(dfmf)) # set colnames

Expand Down Expand Up @@ -994,7 +1003,8 @@ basic_listing_mf <- function(df,

dfmf$aligns[seq(2, nrow(dfmf$aligns)), ] <- "center" # the default for listings

dfmf$formats[] <- 1 # the default for listings is numeric??
# the default for listings is a 1 double??
dfmf$formats <- matrix(1, nrow = nrow(dfmf$formats), ncol = ncol(dfmf$formats))

# row info
ri <- dfmf$row_info
Expand All @@ -1003,11 +1013,23 @@ basic_listing_mf <- function(df,
ri$path <- as.list(NA_character_) # same format of listings
ri$node_class <- "listing_df"
# l_ri$pos_in_siblings # why is it like this in rlistings?? also n_siblings
class(ri$path) <- "AsIs" # Artifact from I()
dfmf$row_info <- ri

# colwidths need to be sorted too!!
dfmf$col_widths <- dfmf$col_widths[colnames(mf_strings(dfmf))]

# Adjustment for presence of rownames column
# if (isFALSE(ignore_rownames)) {
# attr(dfmf, "ncols") <- attr(dfmf, "ncols") + 1
# }
Melkiades marked this conversation as resolved.
Show resolved Hide resolved

if (!add_decoration) {
# This is probably a forced behavior in the original matrix_form in rlistings
main_title(dfmf) <- character()
main_footer(dfmf) <- character()
}

dfmf
}

Expand Down Expand Up @@ -1047,7 +1069,7 @@ reconstruct_basic_fnote_list <- function(mf) {
tmp_strmat <- mf_strings(mf)[i_mat, j_mat, drop = FALSE]

# Only for listings
if (nrow(tmp_strmat) > 0 && .is_listing(mf)) { # safe check for empty listings
if (nrow(tmp_strmat) > 0 && .is_listing_mf(mf)) { # safe check for empty listings

# Fix for missing labels in key columns (only for rlistings)
empty_keycols <- !nzchar(tmp_strmat[-seq_len(nlh), keycols, drop = FALSE][1, ])
Expand Down Expand Up @@ -1089,6 +1111,7 @@ reconstruct_basic_fnote_list <- function(mf) {
} else {
newspans <- mf_spans(mf)[i_mat, j_mat, drop = FALSE]
}

mf_spans(mf) <- newspans
mf_formats(mf) <- mf_formats(mf)[i_mat, j_mat, drop = FALSE]

Expand Down
65 changes: 45 additions & 20 deletions R/mpf_exporters.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,17 @@
colwidths = NULL,
min_siblings = 2,
nosplitin = character(),
rep_cols = num_rep_cols(x),
rep_cols = NULL,
verbose = FALSE,
page_break = "\\s\\n",
page_num = default_page_number()) {
# Processing lists of tables or listings
if (.is_list_of_tables_or_listings(x)) {
if (isFALSE(paginate)) {
warning("paginate is FALSE, but x is a list of tables or listings, so paginate will automatically be updated to TRUE")

Check warning on line 68 in R/mpf_exporters.R

View workflow job for this annotation

GitHub Actions / SuperLinter 🦸‍♀️ / Lint R code 🧶

file=R/mpf_exporters.R,line=68,col=121,[line_length_linter] Lines should not be more than 120 characters. This line is 124 characters.
}
paginate <- TRUE
}

if (paginate) {
pages <- paginate_to_mpfs(
Expand Down Expand Up @@ -101,7 +108,7 @@
)
}

## we dont' set widths here because we already but that info on mpf
## we don't set widths here because we already put that info in mpf
## so its on each of the pages.
strings <- vapply(
pages, toString, "",
Expand All @@ -118,7 +125,22 @@
}
}

.is_list_of_tables_or_listings <- function(a_list) {
all_matrix_forms <- FALSE
obj_are_tables_or_listings <- FALSE

if (is(a_list[[1]], "MatrixPrintForm")) {
all_matrix_forms <- all(sapply(a_list, is, class2 = "MatrixPrintForm"))
} else {
obj_are_tables_or_listings <- all(
sapply(a_list, function(list_i) {
is(list_i, "listing_df") || is(list_i, "VTableTree")
})
)
}

is(a_list, "list") && (obj_are_tables_or_listings || all_matrix_forms)
}

## ## TODO this needs to be in terms of a MPF, so ncol(tt) needs to change

Expand Down Expand Up @@ -416,7 +438,7 @@

export_as_rtf <- function(x,
file = NULL,
colwidths = propose_column_widths(matrix_form(x, TRUE)),
colwidths = NULL,
page_type = "letter",
pg_width = page_dim(page_type)[if (landscape) 2 else 1],
pg_height = page_dim(page_type)[if (landscape) 1 else 2],
Expand All @@ -425,27 +447,26 @@
font_size = 8,
font_family = "Courier",
...) {
# Processing lists of tables or listings
if (.is_list_of_tables_or_listings(x)) {
if (isFALSE(paginate)) {
warning("paginate is FALSE, but x is a list of tables or listings, so paginate will automatically be updated to TRUE")

Check warning on line 453 in R/mpf_exporters.R

View workflow job for this annotation

GitHub Actions / SuperLinter 🦸‍♀️ / Lint R code 🧶

file=R/mpf_exporters.R,line=453,col=121,[line_length_linter] Lines should not be more than 120 characters. This line is 124 characters.
}
paginate <- TRUE
}

if (!requireNamespace("r2rtf")) {
stop("RTF export requires the r2rtf package, please install it.")
}
if (is.null(names(margins))) {
names(margins) <- marg_order
}

fullmf <- matrix_form(x, indent_rownames = TRUE)
req_ncols <- ncol(fullmf) + as.numeric(mf_has_rlabels(fullmf))
if (!is.null(colwidths) && length(colwidths) != req_ncols) {
stop(
"non-null colwidths argument must have length ncol(x) (+ 1 if row labels are present) [",
req_ncols, "], got length ", length(colwidths)
)
}
Melkiades marked this conversation as resolved.
Show resolved Hide resolved

true_width <- pg_width - sum(margins[c("left", "right")])
true_height <- pg_height - sum(margins[c("top", "bottom")])

mpfs <- paginate_to_mpfs(
fullmf,
x,
Melkiades marked this conversation as resolved.
Show resolved Hide resolved
font_family = font_family, font_size = font_size,
pg_width = true_width,
pg_height = true_height,
Expand Down Expand Up @@ -543,16 +564,20 @@
cpp = NULL,
hsep = "-",
indent_size = 2,
rep_cols = num_rep_cols(x),
tf_wrap = TRUE,
max_width = NULL,
colwidths = propose_column_widths(x)) {
Melkiades marked this conversation as resolved.
Show resolved Hide resolved
colwidths = NULL) {
stopifnot(tools::file_ext(file) != ".pdf")
if (!is.null(colwidths) && length(colwidths) != ncol(x) + 1) {
stop(
"non-null colwidths argument must have length ncol(x) + 1 [",
ncol(x) + 1, "], got length ", length(colwidths)
)

# Processing lists of tables or listings
if (.is_list_of_tables_or_listings(x)) {
if (isFALSE(paginate)) {
warning("paginate is FALSE, but x is a list of tables or listings, so paginate will automatically be updated to TRUE")

Check warning on line 576 in R/mpf_exporters.R

View workflow job for this annotation

GitHub Actions / SuperLinter 🦸‍♀️ / Lint R code 🧶

file=R/mpf_exporters.R,line=576,col=121,[line_length_linter] Lines should not be more than 120 characters. This line is 124 characters.
}
paginate <- TRUE
}

gp_plot <- grid::gpar(fontsize = font_size, fontfamily = font_family)

if (!is.null(height)) {
Expand Down Expand Up @@ -604,7 +629,7 @@
max_width = max_width,
indent_size = indent_size,
verbose = FALSE,
rep_cols = num_rep_cols(x),
rep_cols = rep_cols,
page_num = page_num
)
} else {
Expand Down
Loading
Loading