-
Notifications
You must be signed in to change notification settings - Fork 213
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
Floating table #1588
Floating table #1588
Conversation
…han longtable Currently I just have a temporary boolean in here, but I need to figure out how to add this as an option to gt somewhere and pull it.
Correction, I did not commit the qmd file because these are apparently ignored by the project. |
With the tabular* environment you need to specify a width for the table. This will default to \linewidth so tables will fill the whole page horizontally by default. However, it will also respect the use of other percentages in the table.width option.
Commit 51da31e add tests on the latex format for the new case. |
I have decided to use the I have also cleaned up all of the issues with I have a qmd file that I have used for testing. It won't upload, so I provide it below.
The file that is produced is attached: |
@AaronGullickson Apologies for not commenting earlier but thank you for the work done here! We have another PR underway that focuses on LaTeX output (#1595). It would be good to have that merged first before reviewing this one. (After that one is merged, the merge conflict here could be also addressed.) |
@rich-iannone Sounds good. One important thing to consider when reviewing is which case (floating table or longtable) should be the default. I set it up here with floating table as the default, because I think that will be a more common case - i.e. if a table is not too long for a page, it should float by default. However, this will be a breaking change for existing documents in the sense that someone with a multipage table will now have it running off the page. I think its worth it for the long term benefit, but wanted to raise the issue here. |
Merge branch 'master' into floating-table # Conflicts: # tests/testthat/test-as_latex.R
Instead of trying to add \begin{table} and \end{table} within various functions, I moved this step to the export.R as_latex function to create a wrapping environment that is either \begingroup-\endgroup for longtables or \begin{table}-\end{table} for floating tables. This will allow floating tables to be able to use fontsize changes and any other future changes that are meant to be added below the enclosing environment.
I made some changes to how the wrapping table environment is injected as described in the comments on commit 0e4503d. Basically, longtable should be surrounded with \begingroup and \endgroup but tabular should be surrounded by \begin{table} and \end{table}. This approach is much saner and makes it easier for both approaches to use things like the font size changes. I have also updated tests and I think its ready to go. Its failing something with word, but I think that is unrelated to anything I am doing. |
I am actually going to make one more change - I want to see if I can give the user the ability to specify the float position with a table option, rather than depend on Quarto. |
Ok, never mind about the floating position. When using captions from a code chunk, pandoc injects caption information into the table immediately after the curly closing bracket of \begin{table} which breaks the pre-existing floating position and you just end up with unintentional text in the table. So its better to specify floating position with the code chunk option I added a news item and all checks are passing. It should be ready for review. |
@AaronGullickson May I ask a question about this PR as an interested user who would love for {gt} to include a float option? I believe that the |
@kbrevoort Thats a good point. I did try to add that as another |
Just wanted to chime in here and offer that the |
Adds a latex.tbl.pos option to tab_options. The value here will be applied to latex output when a floating table is used to specify the floating position. It checks whether we are in a quarto environment to avoid messing up quarto output since floating position should be specified in tbl-pos chunk option in quarto docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! A few comments.
Also, I tested this locally with usethis::pr_fetch(1588)
.
There is something I don't understand.
Given the following code (Rmd):
---
title: "latex-03-pressure"
output:
- pdf_document
---
```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = TRUE)
devtools::load_all(".")
```
Create a display table based on `pressure`.
```{r}
pressure %>% dplyr::bind_rows(pressure) %>% dplyr::bind_rows(pressure) %>%
gt() %>%
fmt_scientific(
columns = pressure,
decimals = 2
) %>%
tab_options(latex.use.longtable = TRUE)
```
I notice that the table headings aren't repeated as the table crosses 2 pages.
![image](https://github.com/rstudio/gt/assets/52606734/7d294501-a5a3-4ce0-8bc5-5b5f9c306396)
I haven't followed the full conversation, so please enlighten me. In the longtable manual, this seems like a supported feature. It could be supported via page.header.use_tbl_headings
@@ -35,7 +35,7 @@ gt( | |||
data = tbl, | |||
groupname_col = "date" | |||
) %>% | |||
tab_header(title = "The Table Title", subtitle = "The subtitle.") %>% | |||
#tab_header(title = "The Table Title", subtitle = "The subtitle.") %>% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you comment that out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into this and if the line is uncommented and tab_options(latex.use.longtable = TRUE)
is removed, we get a LaTeX error when rendering the .Rmd file:
! You can't use `\hrule' here except with leaders.
\caption@hrule ->\hrule
\@height \z@
l.199 }
\\
Error: LaTeX failed to compile latex-13-adding_footnotes.tex. See https://yihui.org/tinytex/r/#debugging for debugging tips. See latex-13-adding_footnotes.log for more info.
Execution halted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AaronGullickson Are you able to fix this? It is important that LaTeX tables don't get compilation issues if they have a header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error is due to the unnumbered caption being placed within the tabular environment when tab_options(latex.use.longtable = FALSE)
. I'm not exactly sure how why the error occurs in the test because this specific problem is solved with the code in lines 727-762 in R/export.R
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I must have commented that out early on and forgot about it. However, when I uncomment the line, the Rmd file renders fine for me with the latest changes from the PR. @rich-iannone Can you please confirm it gives you that error with the latest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AaronGullickson everything is good here now!
tests/testthat/test-as_latex.R
Outdated
groups = 'grp_a', | ||
fmt = ~ fmt_number(., use_seps = TRUE)) %>% | ||
grand_summary_rows(fns =list(list(label = 'grand total', fn = 'sum')), | ||
columns = c('num', 'currency'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem here! You can use styler to restyle the selection in RStudio. https://styler.r-lib.org/
Maybe some \endhead \endfirsthead \endfoot \endlastfoot could be placed strategically From Ch 3 https://mirrors.ibiblio.org/CTAN/macros/latex/required/tools/longtable.pdf I don't know LaTeX that much, and this feels a bit like trial and error to me. |
@olivroy There are a couple of open issues about headers & captions not repeating on tables that span multiple pages (#1630, #1061). The reason I haven't submitted a PR on that myself is my sense is that, while having the column headers repeat on each page would be very easy, repeating the captions (i.e., "Table 1: My Table Title (continued)" on subsequent pages) would be trickier. I'm confident it's doable (and if someone wants to get to it before I do, great!) but it's probably worthy of its own PR. It's also a very different issue than what @AaronGullickson is addressing. This PR provides an alternative environment to longtable with many benefits for users (myself included). But one thing the new floating environment can't do is span multiple pages. It's not my call, but I recommend not holding up this PR because it doesn't fix an unrelated issue. (We can work on #1630 and #1061, and hopefully, the change that @kuriwaki was asking for while people kick the tires of the new environment.) I hope that helps. @AaronGullickson, thanks for implementing a floating environment. I'm looking forward to trying it out once it's merged! |
Co-authored-by: olivroy <52606734+olivroy@users.noreply.github.com>
Co-authored-by: olivroy <52606734+olivroy@users.noreply.github.com>
@kbrevoort thanks for the explanations! @AaronGullickson thanks for sticking with it @rich-iannone I recommend merging this and possibly have longtable be the default at some point. The fact that long tables can now be broken up across pages instead of overflowing is a nice improvement! (EDIT: given the resolution of #1588 (comment) if possible) |
@AaronGullickson if you could make that |
Thanks for all of this helpful feedback. I am away for a few days built will make the corrections first thing Monday. |
tests/testthat/test-tab_footnote.R
Outdated
tab_footnote(footnote = "A footnote.") %>% | ||
tab_options(latex.use.longtable = TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please disregard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the option latex.use.longtable
, could we rename this slightly to latex.use_longtable
? This is just for consistency with the idea that dots separate either locations, contexts, or actions. It seems to me that use_longtable
is a single concrete action here.
Merge commit '294c09c434b492f4346f9339e3632476b052714f' #Conflicts: # NEWS.md # tests/testthat/helper-gt_attr_expectations.R # tests/testthat/test-summary_rows_latex.R
@AaronGullickson when you take this on, please merge my PR first AaronGullickson#4. I took care of resolving some merge conflicts. |
Merge main branch
…into floating-table
I think I have made all the requested changes. I changed the option name from One last issue to consider is the default environment. Currently, this PR uses a floating table environment as the default. I would argue that this is the best approach because tables that are not longer than a page should be floated for the best results. Thus, longtable is only really needed for long tables, as the name implies. This does however change the environment from its previous default (and only environment) which was longtable, which could cause some confusion when users re-render existing documents (for example). |
Thanks again @AaronGullickson for all your time and patience with this. We're finally going to merge this PR; will do so after all CI runs pass! |
Summary
This PR addresses issue #157, specifically the ability to create latex floating environment tables with the use of the
table
+tabular
environments rather than thelongtable
environment. Currently, I believe this is a major limitation ofgt
because tables are often broken up across pages, and most people expect tables to operate in a similar manner to figures.I added an option to
tab_option
calledlatex.use.longtable
which defaults to FALSE. I then use a fewifelse
clauses inutils_render_latex.R
to get the proper output. Specification of the positioning of the float works well from the quarto code chunk optiontbl-pos
.If I run the following code:
Here is the latex output with the option set to FALSE:
Here is the latex output with the option set to TRUE:
I have also supplied an example qmd document in
tests/gt-examples/03-latex/latex-15-floating.qmd
.I am submitting this PR currently as a draft as I have not fully stress tested it nor have I written a proper
testthat
test (not very experienced with those). I am happy to proceed with doing so, if the maintainers think I am on the right path here, but wanted to get feedback before continuing.Related GitHub Issues and PRs
Checklist
testthat
unit tests totests/testthat
for any new functionality.Fixes: #157