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

Vignettes use absolute paths #553

Closed
zkamvar opened this issue Mar 16, 2018 · 15 comments
Closed

Vignettes use absolute paths #553

zkamvar opened this issue Mar 16, 2018 · 15 comments
Labels
articles ✍️ bug

Comments

@zkamvar
Copy link
Contributor

@zkamvar zkamvar commented Mar 16, 2018

After updating, I notice that the vignettes/articles are using absolute paths to images instead of relative paths.

case in point: grunwaldlab/poppr@5697ce1#diff-f627caa64acaddf31682a506ff09110dR353

@batpigandme
Copy link
Member

@batpigandme batpigandme commented Mar 16, 2018

Hmm, there's something inconsistent happening, it seems, because the logo in your article below is definitely relative.
https://grunwaldlab.github.io/poppr/articles/mlg.html

@zkamvar
Copy link
Contributor Author

@zkamvar zkamvar commented Mar 16, 2018

Ignore the one hosted at grunwaldlab.github.io. That was built with the previous iteration of pkgdown.

This is the one you want:

https://htmlpreview.github.io/?https://github.com/grunwaldlab/poppr/blob/5697ce13af6a0931c82cbc739e4d8c6135fd1844/docs/articles/mlg.html

@batpigandme
Copy link
Member

@batpigandme batpigandme commented Mar 16, 2018

Does the view appear the same for you locally (i.e. without htmlpreview)?

@zkamvar
Copy link
Contributor Author

@zkamvar zkamvar commented Mar 17, 2018

Locally, I will get the correct images showing up, but I suspect if I change computers, this will not be true.

@hadley
Copy link
Member

@hadley hadley commented Mar 17, 2018

I changed how vignettes are built and didn’t have an example of embedding a plot. Oops!

I’ll reconsider my approach again. Might be able to fix by setting working directory instead of using output dir, or could try building in vignette dir with temp name and then copying to articles.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Setting the working directory doesn't seem to help. Now trying the copying approach.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Actually, I can't reproduce the problem. I added a new test vignette containing a figure, but I always get a relative path. How are you building the site?

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

And can you please provide a pointer to the source of the vignette?

@jayhesselberth
Copy link
Collaborator

@jayhesselberth jayhesselberth commented Mar 18, 2018

I still see abs paths in README and vignettes, see #554

I get the same result calling pkgdown::build_site() or using RStudio menu item.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Oh the problem is images, not figures.

@hadley hadley added bug articles ✍️ labels Mar 18, 2018
@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Hmmm setting output_dir to anything (even .) appears to trigger absolute urls in the output.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

And we can't set output_file because that affects the path used for R figures.

I think that implies that we have to build in the directory in which the article lives. We can't even copy that directory because we also have to copy all children, and in the case of README.Rmd that's the whole package.

To figure out which files we need to copy, maybe we can dir_ls() before and after, and then move all new files?

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Hmmm, that strategy will fail if you're previewing locally.

Another idea: copy .Rmd to output directory, build in that directory, then delete the .Rmd. Would also need to copy files found by rmarkdown::find_external_resources(), and document that if a neede file doesn't get imported, you'll need to add it to resource_files in the yaml header.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Alternatively, since the problem is only images, maybe it's easier to simply add another layer of html tweaking?

hadley added a commit that referenced this issue Mar 18, 2018
Setting output_path, automatically causes embedded images (but not figures) to get absolute paths. I considered a number of other solutions in #553, and this seems like the tradeoff.
@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2018

Ok, I've resolved the absolute path problem but it reveals a new problem - images in the vignettes directory are not currently copied to the documentation website. Should they be? R does not (unless you list in .install_extras). But this behaviour was probably designed in the era of pdf vignettes where there was no need to copy inputs. It seems like it would be more convenient if pkgdown copied these files; but precisely what should it copy?

Maybe a reasonable heuristic is to copy any "image" files found by rmarkdown::find_external_resources()? That should get the files you need, without copying over
potentially large files that are needed to generate the article, but not to view it.

...

Oh better, rmarkdown::find_external_resources include a field "web" indicating if the file is needed for rendering on the web.

@hadley hadley closed this as completed in 9fcc3ac Mar 18, 2018
statnmap added a commit to statnmap/SDMSelect that referenced this issue Jun 25, 2019
When images are internal using `knitr::include_graphics`, they are not
copied with pkgdown site. Moreover, copy is not in the same place for
pkgdown and classical vignette to show images.
Here is the workaround:
```{r}
file <- file.path(tmpdir, "Covariate_correlation_crop.png")
silent <- file.copy(file, "Covariate_correlation_crop.png")
if (dir.exists(here::here("docs/articles"))) { # for pkgdown only
  silent <- file.copy(file,
    here::here("docs/articles/Covariate_correlation_crop.png"))
}
knitr::include_graphics("Covariate_correlation_crop.png")
```

Changes due to modifications in {pkgdown}:
r-lib/pkgdown#553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
articles ✍️ bug
Projects
None yet
Development

No branches or pull requests

4 participants