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

Caption placed incorrect when using multiple include_graphics() #1771

Closed
hadley opened this issue Nov 14, 2019 · 6 comments · Fixed by #1776
Closed

Caption placed incorrect when using multiple include_graphics() #1771

hadley opened this issue Nov 14, 2019 · 6 comments · Fixed by #1776
Labels
bug Bugs

Comments

@hadley
Copy link
Contributor

hadley commented Nov 14, 2019

If I write

```{r fig1, fig.cap = "This is a caption", out.width = "33%", fig.show = "hold", echo = FALSE}
knitr::include_graphics("test-1.png")
knitr::include_graphics("test-2.png")
knitr::include_graphics("test-3.png")
```

The figure caption appears after test-1.png, instead of after test-3.png:

Screenshot 2019-11-14 at 1 43 07 PM

Full reprex here: fig-cap.zip

@hadley
Copy link
Contributor Author

hadley commented Nov 19, 2019

This is making "Mastering Shiny" look a little shoddy: https://mastering-shiny.org/action-dynamic.html#updating-inputs

@cderv
Copy link
Collaborator

cderv commented Nov 21, 2019

Currently with knitr::include_graphics, you can pass a vector of paths like this

knitr::include_graphics(c("test-1.png", "test-2.png", "test-3.png"))

It will render like this
image

Internally, it is expected that the caption is placed after the last figure. a counter options fig.num try to calculate the number of figure in a chunk. Currently, if provided, this chunk options is replaced internally by the number to figure in knitr::include_graphics. That is why if you provide all three pngs in the same vector, the caption is correctly put under.

Did you already try this ? Do you call include_graphics 3 times for a reason here ?

If so, I think there is indeed improvement to make on how fig.num is computed and when.

Hope it helps.

@hadley
Copy link
Contributor Author

hadley commented Nov 21, 2019

I'm generating the screenshots with code, so I can work around the problem (now that I know how, thanks!), but it seems odd that I have to do something different with include_graphics() than I would with (e.g.) plot().

@cderv
Copy link
Collaborator

cderv commented Nov 21, 2019

Yes I agree that knitr should be able to guess by itself. I already identified where to put some small changes

knitr/R/block.R

Lines 241 to 243 in c25df11

# number of plots in this chunk
if (is.null(options$fig.num))
options$fig.num = if (length(res)) sum(sapply(res, evaluate::is.recordedplot)) else 0L

and
options$fig.num = length(x)

It works already with your reprex but it needs more tests before a PR to see if it breaks anything and it it handles all cases.

working in branch fix-1771
Comparaison master...cderv:fix-1771

Just sharing the lead I follow...

yihui pushed a commit that referenced this issue Jan 14, 2020
…is used (#1776)

For each include_graphics(x), the number of plots is length(x).
@yihui
Copy link
Owner

yihui commented Jan 14, 2020

Should be fixed now. Thanks for the report, and thank @cderv for the PR!

@yihui yihui added the bug Bugs label Jan 14, 2020
@github-actions
Copy link

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants