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

setting output_dir in render() will create absolute path for knitr fig.path #2024

Open
cderv opened this issue Jan 29, 2021 · 7 comments
Open
Assignees
Labels
bug an unexpected problem or unintended behavior next to consider for next release theme: paths path related improvment / issue

Comments

@cderv
Copy link
Collaborator

cderv commented Jan 29, 2021

See this reproducible example

xfun::write_utf8(c(
  '```{r plot, fig.cap = "A plot", out.width = "100%"}',
  'plot(mtcars)',
  '```'
), "test.Rmd")

callr::r(function() {  
  res <- rmarkdown::render("test.Rmd", rmarkdown::md_document())
  grep("img", xfun::read_utf8(res), value = TRUE)
})
#> [1] "<img src=\"test_files/figure-markdown_strict/plot-1.png\" alt=\"A plot\" width=\"100%\" />"

callr::r(function() {
  res <- rmarkdown::render("test.Rmd", rmarkdown::md_document(), output_dir = "dummy")
  grep("img", xfun::read_utf8(res), value = TRUE)
})
#> [1] "<img src=\"C:/Users/chris/AppData/Local/Temp/Rtmpy8Seib/reprexe05041605211/dummy/test_files/figure-markdown_strict/plot-1.png\" alt=\"A plot\" width=\"100%\" />"

Only difference between the two calls is setting the output_dir.
I don't think this is expected behavior but this is what happens in render()

  • If output_dir is set, it will be normalized

    rmarkdown/R/render.R

    Lines 336 to 342 in 4b119a9

    # resolve output directory before we change the working directory in
    # preparation for rendering the document
    if (!is.null(output_dir)) {
    if (!dir_exists(output_dir))
    dir.create(output_dir, recursive = TRUE)
    output_dir <- normalize_path(output_dir)
    }

    rmarkdown/R/render.R

    Lines 488 to 492 in 4b119a9

    # if an output_dir was specified then concatenate it with the output file
    if (!is.null(output_dir)) {
    output_file <- file.path(output_dir, basename(output_file))
    }
    output_dir <- dirname(output_file)
  • Then it will be used to determine the path for figures

    rmarkdown/R/render.R

    Lines 500 to 502 in 4b119a9

    # use output filename based files dir
    files_dir_slash <- file.path(output_dir, knitr_files_dir(basename(output_file)))
    files_dir <- pandoc_path_arg(files_dir_slash)
  • And used to set fig.path

    rmarkdown/R/render.R

    Lines 632 to 635 in 4b119a9

    knitr::opts_chunk$set(fig.path = paste0(
    pandoc_path_arg(files_dir_slash, backslash = FALSE),
    "/figure-", base_pandoc_to, "/"
    ))

Maybe in this last step we should explicitly set fig.path to a relative one ? Using xfun::relative_path() ?

The handling of paths in render is always source of unexpected behavior when one the argument is set.

Related in a way to #1508

@cderv cderv added the bug an unexpected problem or unintended behavior label Jan 29, 2021
@yihui
Copy link
Member

yihui commented May 18, 2021

I was aware of this problem long time ago, and tried to make the figure path relative since I didn't like the absolute paths, but I remember the interaction among output_file, output_dir, and intermediate_dir was so complicated that I had to give up. ..

@cderv
Copy link
Collaborator Author

cderv commented May 18, 2021

Maybe we could try for a next version to improve / fix / rethink all the path interaction. I feel this is a larger work as it seems we keep fixing things here and there.

I may try my idea of just setting relative path here for figure in this case, but if we agree on larger work maybe I'll wait for when I have time to tackle all this.

@cderv cderv self-assigned this May 18, 2021
@yihui
Copy link
Member

yihui commented May 18, 2021

I may try my idea of just setting relative path here for figure in this case

Feel free to try. I don't remember what I tried exactly last time or what problem I ran into.

@darckula
Copy link

Hi guys, I just found myself trapped on the same issue. Funny to see that this topic was open "recently".

In my case, I am generating reports with rmarkdown with output format github_document. My directory structure is as follows:

Rstudio project directory:

  • place for markdown documents *.md
  • folder "charts" where I want to locate the graphs generated by markdown when generating the ggplot graphs
  • folder "Rmds" where I locate the rmd source files.

Example:

./charts/
      fig1.png
./Rmds/
      my_file.Rmd
./my_file.md

In order to change the output directory for the md document, I use:
rmarkdown::render("Rmds/my_file.Rmd", output_dir=".")

The problem is that this command changes the fig paths to be absolute (as opposite to relative) to my computer. Reading this thread, I used the chunk option fig.path="../charts" which worked well and put the charts in the intended folder... BUT, the md file generated has also that path for grab the charts, this is, the md document tries to find the graphs under "../charts" and it should be "charts" (not the parent directory but the current).

I cannot find a solution for this... not sure if this example can assist you to figure out the problem. If I can be of any help to test something, I'd be pleased.

thanks!

@cderv
Copy link
Collaborator Author

cderv commented Apr 19, 2023

@yihui as reported in rstudio/rticles#528 this in fact has a big impact on vignettes.

I believe when vignettes are build, output_dir() is set
https://github.com/yihui/knitr/blob/master/R/utils-vignettes.R#L64-L68
This follows a fix you made to prevent a CRAN workaround yihui/knitr@b80ece1

RStudio IDE will also set output_dir in some cases to a temp directory whe clicking on Knit button
https://github.com/rstudio/rstudio/blob/60f5a749a3f469e680655c6c7906610086950604/src/cpp/session/modules/rmarkdown/SessionRMarkdown.cpp#L555-L568
And vignette is one of the case where this happens

I don't know if this is recent or not but definitely happening.

We should definitely try to find a way to improve that for next release.

@yihui
Copy link
Member

yihui commented Apr 19, 2023

It sounds like the problem should affect all PDF vignettes that contain plots? If it is such a serious problem, I wonder why we didn't receive more reports.

Anyway, it's definitely worth fixing, but it may be quite tricky...

@cderv
Copy link
Collaborator Author

cderv commented Apr 19, 2023

It sounds like the problem should affect all PDF vignettes that contain plots? If it is such a serious problem, I wonder why we didn't receive more reports.

Yes I wondered the same. 🤔 I think it is worth looking into to see if we can something scoped to vignette at first maybe. I understand this could be quite tricky.

Maybe this absolute path is not really the issue for vignette (as what happens in rstudio/rticles#528) and the issue in only on Windows environment with absolute path in .tex files and some missing escaping for LaTeX . I'll look into this.

@cderv cderv moved this from To discuss / To plan to Backlog in R Markdown Team Projects May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior next to consider for next release theme: paths path related improvment / issue
Projects
Status: Backlog
Development

No branches or pull requests

3 participants