quarto-dev / quarto-cli

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

Table caption showing even when `eval: false` and there is no table #7520

Closed EllaKaye closed 11 months ago

EllaKaye commented 12 months ago

Bug description

Given a code chunk that produces a table and sets a table caption, but with eval: false, the caption for the table still prints in the output, even though there is no table there.

This is very similar to https://github.com/quarto-dev/quarto-cli/issues/3629, but I'm raising it separately, because in that issue, the poster only had the issue in docx, and explicitly did not have it for html (so the issue is labelled docx), whereas I'm getting the same thing in both html and pdf as well.

(I also commented in #3629, and if this is the same issue, please do close this one).

Steps to reproduce

---
title: "Quarto table caption bug"
format: html
---

```{r}
#| label: tbl-bug
#| tbl-cap: "I shouldn't see this caption since `eval: false` is set and no table in printed."
#| eval: false
data.frame(x = 1:3, y = 4:6) |> 
  knitr::kable()
I've got the whole thing at <https://github.com/EllaKaye/quarto-tbl-cap-bug>, with the output at <https://ellakaye.github.io/quarto-tbl-cap-bug/>, generated under v1.4.487.

With v1.3.450 I see "Table 1: ?(caption)" and with v1.4.487 I see the full caption "Table 1: I shouldn’t see this caption since eval: false is set and no table is printed."

### Expected behavior

There shouldn't be a table caption without a table

### Actual behavior

There is a table caption

### Your environment

- macOS 14.1
- RStudio 2023.09.1+494

### Quarto check output

```bash
Quarto 1.4.487
[✓] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.8: OK
      Dart Sass version 1.55.0: OK
      Deno version 1.33.4: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
      Version: 1.4.487
      Path: /Applications/quarto/bin

[✓] Checking tools....................OK
      TinyTeX: (external install)
      Chromium: (not installed)

[✓] Checking LaTeX....................OK
      Using: TinyTex
      Path: /Users/ellakaye/Library/TinyTeX/bin/universal-darwin
      Version: 2022

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
      Version: 3.9.6
      Path: /Library/Developer/CommandLineTools/usr/bin/python3
      Jupyter: (None)

      Jupyter is not available in this Python installation.
      Install with python3 -m pip install jupyter

[✓] Checking R installation...........OK
      Version: 4.3.1
      Path: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources
      LibPaths:
        - /Users/ellakaye/Library/R/arm64/4.3/library
        - /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library
      knitr: 1.42
      rmarkdown: 2.21

[✓] Checking Knitr engine render......OK
mcanouil commented 12 months ago

The issue is that eval: false does not mean there will be nothing, only that code is not evaluated.

I would either use knitr::knit_child() if you have complex conditional, otherwise I would recommend to use conditional content divs surrounding your code. This won't work as the figure/table will still be counted.

EllaKaye commented 12 months ago

Thanks @mcanouil. This came up in my thesis because at some point I had a table, then decided I probably didn't need it but didn't want to delete the code for it in case I changed my mind again, so instead I set eval: false. I realise now what I should have done was include: false instead. But still, I think it's reasonable to assume that if there's no table printed then there shouldn't be a table caption printed either, as is the case when eval: false.

mcanouil commented 12 months ago

Maybe, but it feels a bit weird to make the code cell content cross-referenceable and at the same time asking to not have code cell content. I don't know if there should be a failsafe here or if instead "we" should teach the proper way. Use include: false to not include something (and eval: false to also avoid unnecessary computation).

EllaKaye commented 11 months ago

It's definitely not clear-cut. I wouldn't have seen this issue if I hadn't made a mistake, but it still feels buggy to me to have a caption for a table (or any other cross-referenceable content) that isn't computed, even if it's not technically a bug. I can't think of any reason why anyone would intentionally want to display a caption for content that isn't evaluated/printed, or to reference it in the text. If I'd left a reference in the text to a table that isn't computed, I'd want that to be flagged up in some way. So I can see the value of some kind of failsafe here, whether that be not printing the caption in these cases, or else throwing up a warning or error in the rendering process to alert the user to the likelihood that they've made a mistake.

cderv commented 11 months ago

Personally, I think tbl-cap should indeed not be set if eval: false is set. The chunk is creating the table, and eval: false means to not evaluate the chunk, so no table. We implies to me no caption as it is associated with the tables.

However, the current behavior is for now consistent with Python, as setting eval: false will not compute a table either, but still show the caption.

This is because Quarto does handle the knitr option and this could be by design. I am interested in your opinion @cscheid regarding all this. I know we may have some slightly different view on this sometimes.

Generated Markdown ````markdown ::: {#tbl-bug .cell tbl-cap='I shouldn\'t see this caption since `eval: false` is set and no table in printed.'} ```{.r .cell-code} data.frame(x = 1:3, y = 4:6) |> knitr::kable() ``` ::: ````

More considerations below on current situation


what about fig-cap ?

If we look at fig-cap, if eval: false, it will create no plot, and as caption is handled in a hook for plots, no plot = no caption. It also means no reference possible for now (as the label is not set as an id is not set to reference to).

This is different for python in Quarto. This time if eval: false is set, the caption and the label are set, which make reference possible (for now plot shown). This is coherent with tbl-cap for python

So if we consider the current tbl-cap behavior to be the good one for quarto, then we may have an issue to fix for fig-cap in R document.

About Markdown Syntax

I believe the behavior for tbl-cap option in knitr and jupyter currently make the output of the computation coherent with the raw markdow way

::: {#tbl-bug}

```{r}
#| eval: false
data.frame(x = 1:3, y = 4:6) |> 
  knitr::kable()

Table caption :::



Where when you set `eval: false` it would only not show the caption. 

Not sure I would make the two equivalent. I still think that if you set `eval: false` on a chunk no caption should be shown in the output. 

So by experience, I know I may be too much biased by how R Markown is working. 

Waiting for @cscheid point of view on this now to decide what we should do. 
cscheid commented 11 months ago

I don't think this is a bug in quarto. Quarto doesn't know the context of eval: false vs not to be able to make a decision, and can only rely on the presence of the caption itself.

An argument could be made for knitr (or quarto's knitr engine, I don't know who's truly responsible here) to change the emitted markdown so that *-cap isn't emitted when eval: false, but I think that's wrong. I think that the crossreferenceable element should still be there even if eval: false, so that references in the .qmd don't break. The user will get a placeholder element, and is responsible for making the appropriate changes.

This also enables eval: false executable code cells to behave as listings if lst-label is used.

```{r}
#| lst-label: lst-1
#| lst-cap: A caption for a listing that has invalid code
#| eval: false
if (NULL) 3 else 5 # it's ok, this won't be evaluated

See @lst-1



I'm going to go ahead and close this one.
cderv commented 11 months ago

Thanks for your explanation, clear positioning as always.

I believe this is still an adjustment compared to what knitr can do using its options, and more generally what literate programming can offer, but I understand the position considering Quarto overall for a language agnostic behavior.

The use case is there however. Setting eval: false on chunk conditionally to variable is quite used in R Markdown framework for computational conditional document. When doing that one would not expect any output to be inserted in the document. The caption is set using a cell options for that purpose - being part of the cell execution.

This will make things harder to create some computational conditional document - as eval cell option alone will not be enough - probably requires longer syntax with our conditional content feature for example.

This issue will serve as reference for documentation about this behavior - that is great! Quite an adjustment for previous R Markdown user (only a subset of the user base though).

👍