Skip to content
This repository was archived by the owner on Feb 13, 2020. It is now read-only.

Link to Vignettes #66

Closed
dchiu911 opened this issue Nov 18, 2014 · 9 comments
Closed

Link to Vignettes #66

dchiu911 opened this issue Nov 18, 2014 · 9 comments

Comments

@dchiu911
Copy link
Member

Since the URL changes token every time we visit the vignette, when we are asked to "link to the vignette" is it sufficient to tell the user to call browseVignettes("gameday") from the R console in order to view it?

@jennybc
Copy link
Member

jennybc commented Nov 19, 2014

@BernhardKonrad Here are your HW submission instructions re: the vignette:

Upload your package to a public repository on GitHub. Add a minimal README.md that shows how to install your package, a minimal example how it works, and links to the vignette.

What did you have in mind for "links to the vignette"? The devtools workflow will put inst/doc into .gitignore, so the rendered vignette won't show up in a student's gameday repository. Only the RMarkdown source will be sitting there in vignettes/.

It's possible we should omit the instruction to link to the vignette and just direct the peer reviewer to read the vignette via browseVignettes() once they've installed the peer's package?

However, I would really like a Markdown version to simply exist somewhere logical because when we blast through all the repos for marking, we may not necessarily install every last one. Or we might want to refer back to a vignette of a previous student.

@dchiu911
Copy link
Member Author

Yeah I am not sure how to edit the YAML of the overview.Rmd to generate a markdown file. Otherwise I would just annotate a link pointing the user there. For now, I have only directed the user to using the R Console method.

@jennybc
Copy link
Member

jennybc commented Nov 19, 2014

General answer about YAML for getting Markdown. Two alternatives:

  • make HTML the target output type and add keep_md: true
  • make Markdown the target output type via output: md_document

But, yeah, @dchiu911 good question: how will this interact with the vignette-building machinery? Does it break it? For that matter, where would the Markdown file go?

@dchiu911
Copy link
Member Author

So the default is output: rmarkdown::html_vignette and using the first option I use:
output: rmarkdown::html_vignette:
keep_md: true

and I get this error message:

Error: processing vignette 'overview.Rmd' failed with diagnostics:
Scanner error: mapping values are not allowed in this context at line 4, column 33.

The second option is:
output: md_document

and I get this error message:

Error: processing vignette 'overview.Rmd' failed with diagnostics:
Failed to locate the ‘weave’ output file (by engine ‘knitr::rmarkdown’) for vignette with name ‘overview’. The following files exist in directory ‘.’: ‘overview.md’, ‘overview.R’, ‘overview.Rmd’.

BUT! in ~/gameday/vignettes/ there now exist a overview.md file, but it is not moved to inst/doc. Hence I am still not sure how to get it to build properly while trying to retain the Markdown.

@daattali
Copy link
Member

@jennybc I changed my YAML from having

output: rmarkdown::html_vignette

to

output:
  md_document:
    toc: true
  rmarkdown::html_vignette:
    toc: true

Is that what you meant?

Running build_vignettes() ignores the markdown output and nothing changed, but you can run it manually using rmarkdown::render("overview.Rmd", "all"). Then the HTML and MD files get created, but they are inside the "vignettes" directory...

Related question: is it bad practice to remove the inst/doc from .gitignore? I wanted to have the built vignette available in my package so I removed that line.

@jennybc
Copy link
Member

jennybc commented Nov 19, 2014

I think @daattali is onto something with his YAML.

However, given that the output: md_document gets ignored by build_vignettes(), there is perhaps no advantage to including md_document in the YAML. If you're going to issue a rmarkdown::render("overview.Rmd", …) command "manually" anyway, you might as well just request Markdown there, i.e. rmarkdown::render("overview.Rmd", "md_document"). I think it's OK that they are created within vignettes/.

I was also wondering about inst/doc in .gitignore. I went so far as to track down why it's there, which is because devtools adds that line when we start to use vignettes:

https://github.com/hadley/devtools/blob/a53e0109d8f79b1470efc6cbea3caed42f739b3e/R/infrastructure.R#L82

I don't see any reason why you should necessarily keep the rendered vignettes out of the repo. Have you suffered any negative consequences downstream, i.e. when building and installing? Or with R CMD check? If we can convince ourselves it's OK, manually creating a Markdown version of the vignette and removing this line from .gitignore might be a solution.

@daattali
Copy link
Member

My package includes inst/doc and suffers no problems:
I was able to install the package on a different machine and use it
perfectly.
I was also able to clone the repository on a different machine and run
"document()", "build_vignettes()", RStudio build button, and "check()"
without problem.

On Tue, Nov 18, 2014 at 4:46 PM, Jennifer (Jenny) Bryan <
notifications@github.com> wrote:

I think @daattali https://github.com/daattali is onto something with
his YAML.

However, given that the output: md_document gets ignored by
build_vignettes(), there is perhaps no advantage to including md_document
in the YAML. If you're going to issue a rmarkdown::render("overview.Rmd",
…) command "manually" anyway, you might as well just request Markdown
there, i.e. rmarkdown::render("overview.Rmd", "md_document"). I think
it's OK that they are created within vignettes/.

I was also wondering about inst/doc in .gitignore. I went so far as to
track down why it's there, which is because devtools adds that line when
we start to use vignettes:

https://github.com/hadley/devtools/blob/a53e0109d8f79b1470efc6cbea3caed42f739b3e/R/infrastructure.R#L82

I don't see any reason why you should necessarily keep the rendered
vignettes out of the repo. Have you suffered any negative consequences
downstream, i.e. when building and installing? Or with R CMD check? If we
can convince ourselves it's OK, manually creating a Markdown version of the
vignette and removing this line from .gitignore might be a solution.


Reply to this email directly or view it on GitHub
#66 (comment)
.

@BernhardKonrad
Copy link
Member

Thank you for bringing this to our attention. I didn't mean the linking to be a hurdle.
Having said that, I agree that it would be nice to have the md version of the vignette show up on GitHub. It sounds like the easiest thing is to run rmarkdown::render("vignettes/overview.Rmd", "md_document") and add the created md file (now in the vignettes directory) to the repository. What do you think, @jennybc? It would be good to announce a solution in class tomorrow. Worst case I'm fine with not linking to the vignette and relying on installing and browseVignettes()
Unfortunately, I can't test my suggestion here because I get a frustrating error (which seems to be unrelated to the problem here, but something I need to fix about my markdown in general)

pandoc: Could not find data file templates/default.markdown
 Error: pandoc document conversion failed with error 97 

@jennybc
Copy link
Member

jennybc commented Nov 22, 2014

We will certainly accept a solution where the vignette is built and available after (and only after) package installation.

It is a nice bonus if you make the vignette available, even just in Markdown form, somewhere -- presumably in the source package repo. Why is this nice? Because it becomes accessible under a much wider set of circumstances that way. The steps @daattali describes above, about removing inst/doc from the .gitignore file and adding to the YAML (or rendering to Markdown manually)will do this for you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants