insightsengineering / rtables

Reporting tables with R
https://insightsengineering.github.io/rtables/
Other
225 stars 48 forks source link

Can vertical and horizontal pagination be mixed? #458

Closed Melkiades closed 1 year ago

Melkiades commented 1 year ago

While testing export_as_pdf and wrapping, I tried to run the following (data set can be loaded from tests in #457):

export_as_pdf(tt_for_wrap, 
              file = "tmptxtf.pdf", 
              paginate = TRUE, 
              lpp = 18,  
              tf_wrap = TRUE, 
              max_width = 20, 
              cpp = 50)

and I get the following:

Error in find_pag(pagdf, start, guess, rlpp = adjrlpp, min_siblings = min_siblings, :
Unable to find any valid pagination between 1 and 3

I do not understand the error and my usual way to go is to change input variables to have them good enough for a nice page split. The problem here is that the error is not really informative to me. If cpp is too low (which it is) or if we do not support horizontal pagination these should be specific error messages imo.

On the other hand, I tried using widths and higher cpp on export_as_txt and it creates correctly horizontal and vertical pages (it seems to me):

clw <- c(5, 7, 6, 6) + 12
export_as_txt(tt_for_wrap, 
              file = tmptxtf, 
              paginate = TRUE, 
              lpp = 18, 
              colwidths = clw,
              tf_wrap = TRUE, 
              max_width = 20, 
              cpp = 75)

Still, when I try the same with export_as_pdf, I get an error (also without colwidths).

export_as_pdf(tt_for_wrap, 
              file = "tmptxtf.pdf", 
              paginate = TRUE, 
              lpp = 18,
              tf_wrap = TRUE, 
              max_width = 20, 
              cpp = 75)

Error is Error in .local(x, ...) : length(widths) == ncol(mat$strings) is not TRUE

It is possible here that the mixed pagination machinery is not there yet for pdf support. As I am unsure of that I label this as question ;)

gmbecker commented 1 year ago

@Melkiades whenever pagination doesn't work for you, the first step is to run it again with verbose = TRUE and see what it thinks the issue is.

The error there indicates that there are no valid paginations under the criteria you specified (lpp, cpp, and min_siblings) for that table. This can happen for some tables and some page dimensions, because our pagination is not strict truncation but instead involves context repeating, cant happen on context rows or within a multi-line row, etc.

gmbecker commented 1 year ago

Right, so This is what we get:

export_as_pdf(tt_to_test_wrapping(), 
+               file = "tmptxtf.pdf", 
+               paginate = TRUE, 
+               lpp = 18,  
+               tf_wrap = TRUE, 
+               max_width = 20, 
+               cpp = 50, verbose = TRUE)
Checking pagination after row 6
    ....................... FAIL: last row is a label or content row
Checking pagination after row 5
    ....................... OK
Checking pagination after row 10
    ....................... OK
Checking pagination after column 3
    ....................... FAIL: columns take up too much space (147 characters)
Checking pagination after column 2
    ....................... FAIL: columns take up too much space (98 characters)
Checking pagination after column 1
    ....................... FAIL: columns take up too much space (49 characters)
Error in find_pag(pagdf, start, guess, rlpp = adjrlpp, min_siblings = min_siblings,  : 
  Unable to find any valid pagination between 1 and 3

Note you're not specifying column widths here, so the defualts are used which are:

> propose_column_widths(tt_to_wrap)
[1] 25 46 46 46

Note the first 'column' is the row labels, which is width 25. This will be repeated on every horizontally paginated page. That means that the minimum total width for a page is 25 + 46 + <col_gap> which is 73, but you told it to paginate to a cpp of 50, which means that not a single non-row-label column fits on the first page, which is (correctly) an error.

The Error in .local(x, ...) : length(widths) == ncol(mat$strings) is not TRUE is a bug that I will be merging in the fix for momentarily

Melkiades commented 1 year ago

Note the first 'column' is the row labels, which is width 25. This will be repeated on every horizontally paginated page. That means that the minimum total width for a page is 25 + 46 + <col_gap> which is 73, but you told it to paginate to a cpp of 50, which means that not a single non-row-label column fits on the first page, which is (correctly) an error.

I see! I did not notice there was a verbose option. I think it is very clear now how it works, thank you!