quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.89k stars 320 forks source link

Labeling of gt-tables is broken #5657

Closed snhansen closed 1 year ago

snhansen commented 1 year ago

Bug description

With the newest Quarto version (1.4.84) on Windows, I now get an error when trying to label a table created with gt(). A minimal example:

---
format: html
---

# Title

```{r}
#| echo: false
#| tbl-cap: "Caption."
#| label: tbl-test
library(gt)

gt(mtcars)
```

which results in this error:

Error running filter C:/Users/au234616/AppData/Local/Programs/Quarto/share/filters/main.lua:
...616/AppData/Local/Programs/Quarto/share/filters/main.lua:9301: attempt to call a string value (field 'html_table_caption')
stack traceback:
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:9139: in local 'fn'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:2120: in function <...616/AppData/Local/Programs/Quarto/share/filters/main.lua:2114>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:178: in function 'run_emulated_filter'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:566: in local 'callback'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:577: in upvalue 'run_emulated_filter_chain'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:610: in function <...616/AppData/Local/Programs/Quarto/share/filters/main.lua:607>
stack traceback:
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:178: in function 'run_emulated_filter'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:566: in local 'callback'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:577: in upvalue 'run_emulated_filter_chain'
        ...616/AppData/Local/Programs/Quarto/share/filters/main.lua:610: in function <...616/AppData/Local/Programs/Quarto/share/filters/main.lua:607>

Checklist

mcanouil commented 1 year ago

Thanks for the report!

I can reproduce on the latest development version. The issue is not with the labels, but the caption.

For example, the following works:

---
format: html
---

```{r}
#| label: tbl-cars
library(gt)
gt(mtcars)


I believe it's related to #5551 changes regarding the overhaul "refactoring" of the cross-reference feature (#4944).
mcanouil commented 1 year ago

Side note, should some tests be added for gt? As for knitr-tables-latex.qmd and knitr-tables.qmd

cderv commented 1 year ago

Side note, should some tests be added for gt?

I believe it is tracked by @rich-iannone and he plans to add some (if not already) on gt side to cover a vast variations of tables.

Also we have some basic tests in our smoke-all part (Drawback of the smoke-all folder is that they are not organized by topic. We could regroup. ):

It seems they are passing as we did not detect the issue before 🤔

cderv commented 1 year ago

It seems they are passing as we did not detect the issue before 🤔

This in fact gave us the hint that it is an issue with gt - we are not testing with gt 0.8 and now 0.9 is out.

So issue is with gt 0.9 - @snhansen installing gt 0.8 solve the issue for me, so my guess is that you are using 0.9 too.

Using Quarto 1.3 works with both version.

@rich-iannone FYI as related to gt

snhansen commented 1 year ago

@cderv: Ah, yes. Same behaviour here. Didn't occur to me that it could be related to gt.

cscheid commented 1 year ago

It's still a quarto bug; we shouldn't be crashing at all. Let me investigate.

cscheid commented 1 year ago

The issue is not with gt; it's that our tests were not covering crossref/tables.lua:196. It was a typo.

I've tested a local fix with gt 0.9.0, but I'm not sure how to make our test suite cover multiple R package versions.

mcanouil commented 1 year ago

What changed in gt that lead Quarto to error out here? The only way I know is to use renv profiles or similarly different lockfile (renv or pak) which record different version of the packages.

mcanouil commented 1 year ago

The main "issue" is what packages and how many version to you want to test against? Should Quarto really be compatible with possibly any versions of any packages? Or should any Quarto release worked with the latest release of the packages?

cscheid commented 1 year ago

I found a workaround; I used the 0.9.0 output directly on the qmd input. That triggered the bug independently of the library.

Should Quarto really be compatible with possibly any versions of any packages?

That's not the point here, because this isn't a GT bug. This was literally me using foo() instead of foo on a string value. It only happened that the bug triggered on 0.9.0 because the html formatting changed and it started exercising that specific code path.

cscheid commented 1 year ago

In any case, sorry for the bug! We have a fix and regression test now.