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

Count figures properly in a chunk when include_graphics is used #1776

merged 8 commits into from Jan 14, 2020


Copy link

cderv commented Nov 21, 2019

This will closes #1771

Captions are placed at the bottom because several include graphics in a chunk are not counted like plots.

This PR aims at allowing several plots and/or included img paths to be supported in the same chunk


This comment has been minimized.

Copy link
Collaborator Author

cderv commented Nov 23, 2019

I tested this based on the reprex from #1771 and also a new document I made and put in a gist.
Here is the code to execute in knitr development project folder

# load dev version of knitr
# get the reprex
zip_reprex <- ""
pins::pin(zip_reprex, name = "fig_cap")
# render the example in a clean dir
dir.create(tmp_dir <- tempfile())
  tmp_dir, {
    file.copy(pins::pin_get("fig_cap"), ".")
    # render reprex 
    output1 <- rmarkdown::render("fig-cap.Rmd")
    # get the new example using the same pngs
    con <- url("")
    content <- xfun::read_utf8(con)
    xfun::write_utf8(content, "manual-test.Rmd")
    output2 <- rmarkdown::render("manual-test.Rmd")

@yihui I also try to make a new knitr-example document to test, but I am really not sure I am able to create a new document.
I added a modified version of to test for caption. However, several issues :

  • figures are handle using a base.url from your website and not locally. I am not sure how to create the new md document that goes with it.
  • Document are rendered only in .md and the issue with fig.cap and fig.num is mainly for html output. Caption are not really put in md output the same way.

Not sure how to create an automated test, and check also that everything is fine anywhere after this two little changes. ☺️ knitr is at the base of everything so feels dangerous...

yihui added 2 commits Jan 14, 2020
…e_graphics(), in which case we need to count length(x) instead of treating one include_graphics() call as one plot
…o do it here
@yihui yihui marked this pull request as ready for review Jan 14, 2020
yihui approved these changes Jan 14, 2020
Copy link

yihui left a comment

Do my changes look good to you?

I'll take care of the tests in 099-include-graphics.Rmd. Thank you very much!


This comment has been minimized.

Copy link
Collaborator Author

cderv commented Jan 14, 2020

Yes, I find it clearer. And reprex is working, so all good ! thanks.

yihui added a commit to yihui/knitr-examples that referenced this pull request Jan 14, 2020

This comment has been minimized.

Copy link

yihui commented Jan 14, 2020

yihui added 2 commits Jan 14, 2020
@yihui yihui merged commit 49cb3a9 into yihui:master Jan 14, 2020
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
clahub All contributors have signed the Contributor License Agreement.
@cderv cderv deleted the cderv:fix-1771 branch Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.