yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.39k stars 878 forks source link

opts_chunk$get("output.dir") contains a mangled file path? #1471

Open mtmorgan opened 6 years ago

mtmorgan commented 6 years ago

This repo https://github.com/mtmorgan/outputdir has an Rnw vignette. The vignette has a main document and a child document. The code chunk to include the child document includes fig.path that references opts_chunk$get("output.dir"). When built and run in a directory that contains a '.', e.g.,

~/tmp/R-3.4.2/check$ R CMD build outputdir && R CMD check outputdir_1.10.0.tar.gz 

the check, specifically the step * checking re-building of vignette outputs ... OK creates a new directory hierarchy where the '.' in R-3.4.2 is changed to '_', R-3_4_2:

$ tree ~/tmp/R-3_4_2/
/home/mtmorgan/tmp/R-3_4_2/
└── check
    └── outputdir_Rcheck
        └── vign_test
            └── outputdir
                └── vignettes
                    ├── Xunnamed-chunk-1-1.pdf
                    └── Xunnamed-chunk-1-1.png

5 directories, 2 files

which is very undesirable. I can't tell whether this is an unfortunate name collision, bad form in the original package, or a bug.

yihui commented 6 years ago

I'm not sure, either, but this is an undocumented option, and I don't recommend that you use it: https://yihui.name/knitr/options/

mtmorgan commented 6 years ago

We'll stop using it, but it seems like its a bug in knitr anyway...

yihui commented 6 years ago

Okay, I'll investigate it when I have time. Thanks for the report!

cderv commented 3 years ago

Here is how to reproduce and why we get this behavior

dir.create(tmp_dir <- tempfile(pattern = "R-3.2.3"))
tmp_dir
#> [1] "C:\\Users\\chris\\AppData\\Local\\Temp\\Rtmp8AKB5r\\R-3.2.382703ad777be"
owd <- setwd(tmp_dir)
# let's reproduce
gert::git_clone("https://github.com/mtmorgan/outputdir")
xfun::Rcmd(c("build", "outputdir"))
xfun::Rcmd(c("check", "outputdir_1.10.0.tar.gz"))
# the folder with _ is here 
fs::dir_tree("..", recurse = FALSE)
#> ..
#> +-- file82702e9a265a
#> +-- file827077bf6bcb.dll
#> +-- R-3.2.382703ad777be
#> \-- R-3_2_382703ad777be
folder <- dir("..", "R-3_2_3")
fs::dir_tree(file.path("..", folder), recurse = TRUE)
#> ../R-3_2_382703ad777be
#> \-- outputdir_Rcheck
#>     \-- vign_test
#>         \-- outputdir
#>             \-- vignettes
#>                 +-- Xunnamed-chunk-1-1.pdf
#>                 \-- Xunnamed-chunk-1-1.png

unlink(file.path("..", folder), recursive = T)

# it comes indeed from knitr, you'll get a warning if you knit the file
knitr::knit("outputdir/vignettes/main.Rnw")
#> processing file: outputdir/vignettes/main.Rnw
#> output file: main.tex
#> [1] "main.tex"

#> Warning message:
#>In (if (out_format(c("latex", "sweave", "listings"))) sanitize_fn else paste0)(path,  :
#>  dots in figure paths replaced with _ ("C:/Users/chris/AppData/Local/Temp/RtmpofFca4/R-3_2_37f3059ef625f/Xunnamed-chunk-1")

# specifically this function is called by fig.path to 
# explicitly replace dots by _
knitr:::sanitize_fn("R-3.2.2")
#> Warning in knitr:::sanitize_fn("R-3.2.2"): dots in figure paths replaced with _
#> ("R-3_2_2")
#> [1] "R-3_2_2"

setwd(owd)
unlink(tmp_dir)

As shown above this is from internal sanitize_fn() used in fig.path() when LaTeX is used. It explicitely replace . with _ except ../ and ./ in file path for figures.

The side effect is what you observe.

@yihui investigation done. It seems by design. Should we keep this open as a bug or close as won't fix ?

yihui commented 3 years ago

@cderv Thanks for the investigation! So the problem is that output.dir is an absolute path that contains ., and the periods were sanitized by knitr due to LaTeX issues. Normally I don't recommend using absolute paths. It seems that @mtmorgan wanted the child Rnw document to generate plots to a specific directory. I don't know what to do here. I can provide an option to disable knitr:::sanitize_fn(), if the periods in paths are no longer problems in LaTeX (this was problematic at least around 2013 and I'm not sure if LaTeX is better at dealing with paths containing . now).

cderv commented 3 years ago

I don't think we have something to do here. opts_knit$get("output.dir") is absolute path and it is not to be used in fig.path that could be relative.

This does not seems to be a very common issue, but it is good to have it documented.

I don't really know about the dot issue in LaTeX.