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

Image sizing inconsistency with knitr github_document's with inline code chunks #310

Open
rundel opened this issue Apr 22, 2021 · 8 comments

Comments

@rundel
Copy link

rundel commented Apr 22, 2021

I'm seeing some fairly bizarre behavior when trying to output magick images in a github_document. Below is a reprex to demonstrate the issue.

---
output: github_document
---

```{r setup, include = FALSE}
library(magick)
```

```{r}
image_read_svg('http://jeroen.github.io/images/tiger.svg', width = 128)
```

Markdown text `r image_read_svg('http://jeroen.github.io/images/tiger.svg', width = 32)` more text.

When knit the Rmd produces a document where the first tiger image is blurred as the base64enc image is only 32x32 and being scaled back up to 128x128.

If the inline code chunk is removed the first image displays properly at 128x128 - more bizarrely if image_read_svg('http://jeroen.github.io/images/tiger.svg', width = 32) is included in the chunk before the 128x128 image then all three images will be correctly sized but this is not the case if this line is included in a different chunk.

Varying the size of this proceeding image also has some strange behavior, making it smaller than 32x32 everything appears to work but the resulting image is actually intrinsically 32x32 but being shrunk to the smaller size. If this image is bigger, then the original issue occurs for this image but now the 2nd 128x128 image is fine.

I believe similar behavior is also happening with html_documents but I haven't explored it as much and it is a bit confounded by the fig.retina option. If the underlying issue actually is with knitr, I'm happy to move this over there.

@rundel
Copy link
Author

rundel commented Apr 22, 2021

After digging a bit deeper this seems like a knitr bug - inline code chunks have the same label as the proceeding chunk so the behavior I am seeing is because inline chunk is overwritting unnamed-chunk-1-1.png.

Maybe this is intended but it seems like plot_counter() is being reset between the two chunk types which is causing this issue.

@cderv
Copy link

cderv commented Apr 22, 2021

I have commented in the other issue but writing here the main why I think this happens.
In the knit_print method in this package internal knitr function are used to create the filename using smae knitr's logic

magick/R/base.R

Lines 117 to 131 in 56f746b

"knit_print.magick-image" <- function(x, ...){
if(!length(x))
return(invisible())
plot_counter <- utils::getFromNamespace('plot_counter', 'knitr')
in_base_dir <- utils::getFromNamespace('in_base_dir', 'knitr')
ext <- ifelse(all(tolower(image_info(x)$format) == "gif"), "gif", "png")
tmp <- knitr::fig_path(ext, number = plot_counter())
# save relative to 'base' directory, see discussion in #110
in_base_dir({
dir.create(dirname(tmp), showWarnings = FALSE, recursive = TRUE)
image_write(x, path = tmp, format = ext)
})
knitr::include_graphics(tmp)
}

If this method is used in inline chunk, the current process will be the same (it could be different by catching inline = TRUE) and internal plot_counter() will be used to create figure name with knitr::fig_path but these are currently supposed to be used with block chunk and not inline one, which have no label - internal knitr::opts_current$get("label") is only for block chunk.

We would need to think about supporting creation of fig.path in inline chunk, or the knit_print method here needs to create the filename another way (possibly only when use in inline chunk).

Nice investigation @rundel ! That helped a lot ! thanks.

@rundel
Copy link
Author

rundel commented Apr 27, 2021

@jeroen Any thoughts?

@jeroen
Copy link
Member

jeroen commented Apr 28, 2021

What could I do about this on my end? It sounds like an issue in knitr?

@rundel
Copy link
Author

rundel commented Apr 28, 2021

See the discussion in yihui/knitr#1988 (comment) - the current assumption (which I had also made) was that inline chunks behave the same way as regular chunks which is not the case.

Longer term I hope this will be changed in knitr but in the shorter term it seems like magick may need to check for the chunk type and use an alternative naming approach for images coming from inline chunks.

@jeroen
Copy link
Member

jeroen commented Apr 28, 2021

OK. Can you suggest a PR for that? I also wonder if it makes much sense to show images in inline chunks in the frist place? Do people really use that?

@rundel
Copy link
Author

rundel commented Apr 28, 2021

I ran across this because I was trying to do exactly this with a package rundel/rsicons which Mine and I were planning to use to embed things like the Git button icons in Rmds for teaching purposes. The original plan was to use <img> tags but that doesn't generalize as well (e.g. to pdf). I think there are similar use cases around things like font awesome.

I'm happy to take a stab at a PR and let things progress from there.

@cderv
Copy link

cderv commented Apr 28, 2021

I think it is not as usual to show image in inline code and that is why knitr mechanism for auto naming file from figures created by R does not support it currently. This is not a small change in knitr and we'll need to think about it.

Also, for now packages that needs to this (mainly icon packages it seems) does not rely on internal knitr naming mechanism like magick does. fontawesome package is an example and for HTML it uses inline svg, for other format it will save an image but name the file itself. So the knit_print method there take that into account https://github.com/rstudio/fontawesome/blob/b6a916199ffe34aac2507d770327467d525dc6cf/R/knit_print.R#L44

rsicons package could also do like fontawesome and have an internal knit_print methods for its object to handle inline case correctly.

On magick side, knit_print.magick-image could make sure to work only for block chunk with the current mechanism and works or not for inline chunk. knit_print generic will pass options and inline to methods - inline will be TRUE when this is used inline. See knitr vignette for example: https://cran.r-project.org/web/packages/knitr/vignettes/knit_print.html#customize-printing

Hope the above can help to solve this with the current state of knitr

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Oct 26, 2024
(w3m -insecure -dump -T text https://github.com/yihui/knitr/releases)
knitr 1.48
BUG FIXES

  * Fix regression from 1.46 with collapse = TRUE option not correctly
    collapsing source code and output into one when code chunk returns multiple
    outputs (thanks, @jennybc, @florisvdh, tidyverse/reprex#463).

  * hook_purl() should not write the path of the R script to the output
    document (thanks, @fenguoerbian, #2348).

knitr 1.47
NEW FEATURES

  * For kable(), you can set the global option knitr.kable.max_rows to limit
    the number of rows to show in the table, e.g., options(knitr.kable.max_rows
    = 30). This is a way to prevent kable() from generating a huge table from a
    large data object by accident.

  * write_bib() now escapes all non-escaped "&" in the bibliography by default.
    Previously, it only escaped the title field of the package citation. You
    can disable the escape with the argument tweak = FALSE (thanks, @HedvigS #
    2335, @atusy #2342).

BUG FIXES

  * Fixed a bug that write_bib() fails to use the first URL of a package when
    multiple URLs are provided in DESCRIPTION and separated by \n (thanks,
    @bastistician, #2343).

MINOR CHANGES

  * The syntax highlighting LaTeX commands for Rnw documents, \hlstr and \
    hlstd, were renamed to \hlsng and \hldef, respectively, to maintain
    consistency with Andrew Simon's highlight package (thanks, @dcser123, #2341
    ).

knitr 1.46
NEW FEATURES

  * Added a new chunk option tab.cap to specify the table caption for kable()
    (thanks, @ulyngs, #1679). Previously, the caption could only be specified
    via the caption argument of kable(). Now you can set it in the chunk header
    if you want. Please note that this chunk option only works with a single
    kable() in each code chunk, and its value must be of length 1.

  * spin() now recognizes # %% as a valid code chunk delimiter (thanks,
    @kylebutts, #2307).

  * spin() also recognizes #| comments as code chunks now (thanks, @kylebutts,
    #2320).

  * Chunk hooks can have the ... argument now. Previously, only arguments
    before, options, envir, and name are accepted. If a chunk hook function has
    the ... argument, all the aforementioned four arguments are passed to the
    hook. This means the hook function no longer has to have the four arguments
    explicitly in its signature, e.g., function(before, ...) is also valid if
    only the before argument is used inside the hook. See https://yihui.org/
    knitr/hooks/#chunk-hooks for more information.

  * For package vignettes, PNG plots will be optimized by optipng and pngquant
    if they are installed and can be found from the system PATH variable. This
    can help reduce the package size if vignettes contain PNG plots (thanks,
    @nanxstats, https://nanx.me/blog/post/rpkgs-pngquant-ragg/).

BUG FIXES

  * spin() stopped working with input that cannot be parsed as R code due to #
    1605. Now it works again (thanks, @Hemken, #1773).

  * write_bib() generated empty entries for packages without URLs (thanks,
    @bastistician, #2304).

  * The family argument was not passed to the pdf device (thanks, @sebkopf,
    rstudio/rmarkdown#2526).

  * Trailing spaces escaped by \ should not be trimmed in kable() (thanks,
    @mjsmith037, #2308).

  * kable() fails when the value of the caption argument is of length > 1
    (thanks, @LeeMendelowitz, #2312).

  * include_graphics() may provide an incorrect plot width to LaTeX when the
    locale setting for LC_NUMERIC is not C because the decimal separator may
    not be a dot (thanks, @tdhock, rstudio/rmarkdown#2525).

  * When TinyTeX and the LaTeX package pdfcrop are installed, knitr::pdf_crop()
    is unable to find pdfcrop (thanks, @dmkaplan2000, rstudio/tinytex#435).

MAJOR CHANGES

  * Unbalanced chunk delimiters (fences) in R Markdown documents are no longer
    allowed, as announced two years ago at https://yihui.org/en/2021/10/
    unbalanced-delimiters/ (#2306). This means the opening delimiter must
    strictly match the closing delimiter, e.g., if a code chunk starts with
    four backticks, it must also end with four; or if a chunk header is
    indented by two spaces, the closing fence must be indented by exactly two
    spaces. For authors who cannot update their R Markdown documents for any
    reason at the moment, setting options(knitr.unbalanced.chunk = TRUE) (e.g.,
    in .Rprofile) can temporarily prevent knitr from throwing an error, but it
    is strongly recommended that you fix the problems as soon as possible,
    because this workaround will be removed in future.

  * Package vignettes are tangled by default during R CMD check, per request
    from CRAN maintainers (d0d1b47). The consequence is that R CMD check will
    check R scripts tangled from vignettes by default, unless you set the
    environment variable _R_CHECK_VIGNETTES_SKIP_RUN_MAYBE_=true. Previously,
    knitr would skip tangling vignettes during R CMD check, because R scripts
    tangled from vignettes are not guaranteed to valid. With the skip undone, R
    CMD check may fail in places other than CRAN (because CRAN has set the
    environment variable).

MINOR CHANGES

  * Fixed broken vignettes, improved CSS for HTML vignettes, and reduced the
    file sizes.

  * SQL code chunks that run ALTER statements are only executed and not tried
    to fecth a result (thanks, @maxschmi, #2330).

  * The function imgur_upload() has been moved to (and enhanced in) the xfun
    package as xfun::upload_imgur() so it is no longer tied to knitr and can be
    reused by other pakages. Now knitr::imgur_upload() is only a wrapper
    function of xfun::upload_imgur(). You are recommended to use the latter (#
    2325).

  * spin() dropped support for #- as the chunk delimiter token. Please use #+
    or # %% or #| instead.

  * Faster processing of cache dependencies in dep_auto() (thanks, @knokknok, #
    2318).

  * Removed some S3 methods that are used internally and changed them to normal
    functions: print.block -> print_block, print.inline -> print_inline,
    process_group.block/process_group.inline -> process_group, and
    process_tangle.block/process_tangle.inline -> process_tangle.

knitr 1.45
NEW FEATURES

  * Improved the error message to contain more specific information when YAML
    chunk options could not be parsed (thanks, @pedropark99, #2294).

BUG FIXES

  * Special characters in the chunk option fig.alt are properly escaped now
    (thanks, @jay-sf, #2290).

  * Negative numbers returned from inline R expressions lost their minus signs
    when formatted in the scientific notation (thanks, @fkohrt, #2288).

  * convert_chunk_header(type = 'yaml') will now use dash option name for known
    knitr options, and numeric option are kept with same significant digits,
    e.g fig.width = 10 is converted to fig-width: 10.

  * Add the necessary \newline to the last subfigure (thanks, @slrellison,
    rstudio/rmarkdown#2518).

  * Percent signs (%) in LaTeX figure captions and short captions are properly
    escaped now (thanks, @s-u, #2302).

MAJOR CHANGES

  * The object opts_current will be restored after each code chunk has finished
    executing. Previously, it would not be restored, which means even for
    inline R expressions, opts_current$get() will inherit chunk options from a
    previous code chunk (thanks, @rundel, #1988). Besides, opts_current$get
    ('label') will return a unique label for inline expressions. One possible
    application is to construct unique figure paths via fig_path() (e.g.,
    ropensci/magick#310).

  * opts_current$set() without opts_current$lock(FALSE) will trigger a warning
    instead of an error for now and it will become an error in future (#2296).

knitr 1.44
NEW FEATURES

  * kable() can generate Emacs org-mode tables now via kable(..., format =
    'org') (thanks, @xvrdm #1372, @maxecharel #2258).

  * Added support for the qmd (Quarto) output format to spin(), e.g., spin
    ('script.R', format = 'qmd') (thanks, @cderv, #2284).

  * write_bib() has a new argument packageURL to control whether to use a URL
    from the DESCRIPTION file or the one generated by utils::citation()
    (thanks, @dmurdoch, #2264).

  * Updated the package vignette vignette('knit_print', 'knitr') to mention
    that package authors no longer have to make knitr a hard dependency if they
    want to define S3 methods for knitr::knit_print with R >= 3.6.0 (thanks,
    @cderv, #1929).

  * Added a new function download_image() to download an image from a URL and
    include it via include_graphics(). This is mainly for including online
    images when the output format is not HTML (e.g., LaTeX), because the URL
    will not work as the image path, and it has to be downloaded beforehand
    (thanks, @bayeslearner, #2274).

BUG FIXES

  * Make the internal function add_html_caption() work with Quarto <= v1.3.353
    (thanks, @giabaio, #2261).

  * Fixed a bug in spin(format = 'Rnw') reported by @Tarious14 at yihui/
    yihui.org#769 (reply in thread)

  * When the chunk option dev = 'svglite', the svglite device should be used to
    record plots (thanks, @Darxor, #2272).

  * Figure captions are no longer escaped for reStructuredText output, and the
    alt text can also be specified via the fig.alt chunk option now (thanks,
    @trevorld, #2023).

  * Use the correct type of progress bar when rendering Quarto documents in
    RStudio, which takes place in RStudio's background jobs pane or build pane
    (thanks, @hadley, #2271).

  * The opts_current object can no longer be modified within code chunks via
    its $set() method (thanks, @AshesITR, #1798).

  * The argument col.names of kable() can be used to specify the column name of
    row names now, e.g., kable(head(mtcars), col.names = c("car", names
    (mtcars))) ("car" will be the column name of row names in the first column)
    (thanks, @iago-pssjd, #1933).

MAJOR CHANGES

  * Dashes (-) in the names of all chunk options are normalized to dots (.)
    now, e.g., fig-height will be converted to fig.height. This is to make
    knitr more compatible with Quarto since Quarto always use dashes in chunk
    option names (#2282).

MINOR CHANGES

  * In-body chunk options (#|) are now preserved when extracting code from a
    document via purl() (thanks, @LuisLauM, #2268).

  * A warning message will be issued when taking PDF screenshots for HTML
    widgets with the webshot2 package, because webshot2 doesn't use the correct
    figure size at the moment (thanks, @icejean, #2276).

  * If the title argument of knit2wp() is omitted and a title field is
    specified in the YAML metadata of the input document, the YAML title will
    be used (thanks, @arencambre, #1924).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants