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

get rid of setwd() usage #33

Closed
AlexAxthelm opened this issue Oct 12, 2023 · 3 comments · Fixed by #50
Closed

get rid of setwd() usage #33

AlexAxthelm opened this issue Oct 12, 2023 · 3 comments · Fixed by #50
Assignees

Comments

@AlexAxthelm
Copy link
Collaborator

At
Line 561 in 54b3ff3
and later...
Line 591 in 54b3ff3

My memory of the need for this was that getting bookdown/knitr to run from a directory but in a different directory was problematic. I'm hoping maybe things have improved since then (?) and we can find a way to avoid doing this, because it is error prone and problematic for its own reasons.

Along with this, it probably would be good to not run directly in the template directory, but rather copy the template directory to a temp directory, because bookdown/knitr creates files in the template directory (which may not be possible if, for instance, we were to mount the template directories into the docker image into a read-only directory), and if anything goes wrong during the knitting it leaves behind some of these files in the template directory (ostensibly, to facilitate figuring out what went wrong), which is also undesirable in our use case.

For further context... I believe we were always targeting the index.Rmd file, originally with the full path to the index.Rmd, then by setting the directory to where it is and targeting just "index.Rmd". {bookdown} v0.22 brought problems because it changed how it processed the first input argument. We adapted to that here https://github.com/RMI-PACTA/create_interactive_report/pull/445. I think what {bookdown} v0.22 enabled/should have prompted us to do is to simply specify the directory and let it find the index.Rmd inside it, which is probably more in line with what we were going for in the first place.

Migration of https://github.com/RMI-PACTA/pacta.interactive.report/issues/198

@AlexAxthelm
Copy link
Collaborator Author

@cjyetman I've had some success passing normalizePath()'d paths to the output_dir, and intermediates_dir arguments of bookdown::render(), but the slight trick there is that normalizePath() fails if the directory doesn't exist first.

@cjyetman
Copy link
Member

dir.create(warn = FALSE) first to make sure it's there?

@AlexAxthelm AlexAxthelm self-assigned this Nov 7, 2023
@AlexAxthelm
Copy link
Collaborator Author

bumping this issue, since I ran into it again while writing #46

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

Successfully merging a pull request may close this issue.

2 participants