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

Labeling of gt-tables is broken #5657

Closed
3 of 5 tasks
snhansen opened this issue May 24, 2023 · 11 comments
Closed
3 of 5 tasks

Labeling of gt-tables is broken #5657

snhansen opened this issue May 24, 2023 · 11 comments
Assignees
Labels
bug Something isn't working tables Issues with Tables including the gt integration
Milestone

Comments

@snhansen
Copy link

snhansen commented May 24, 2023

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

  • Please include a minimal, fully reproducible example in a single .qmd file? Please provide the whole file rather than the snippet you believe is causing the issue.
  • Please format your issue so it is easier for us to read the bug report.
  • Please document the RStudio IDE version you're running (if applicable), by providing the value displayed in the "About RStudio" main menu dialog?
  • Please document the operating system you're running. If on Linux, please provide the specific distribution.
  • Please provide the output of quarto check so we know which version of quarto and its dependencies you're running.
@snhansen snhansen added the bug Something isn't working label May 24, 2023
@mcanouil
Copy link
Collaborator

mcanouil commented May 24, 2023

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 mcanouil added crossref triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. labels May 24, 2023
@mcanouil
Copy link
Collaborator

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

@cderv
Copy link
Collaborator

cderv commented May 24, 2023

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
Copy link
Collaborator

cderv commented May 24, 2023

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
Copy link
Author

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

@cderv cderv added the tables Issues with Tables including the gt integration label May 24, 2023
@cscheid
Copy link
Collaborator

cscheid commented May 24, 2023

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

@cscheid
Copy link
Collaborator

cscheid commented May 24, 2023

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
Copy link
Collaborator

mcanouil commented May 24, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

cscheid commented May 24, 2023

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
Copy link
Collaborator

cscheid commented May 24, 2023

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

@cderv cderv removed triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. crossref labels May 25, 2023
@mcanouil mcanouil added this to the v1.4 milestone Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tables Issues with Tables including the gt integration
Projects
None yet
Development

No branches or pull requests

5 participants