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

Add JOSS and JOSE format #229

Merged
merged 8 commits into from Sep 13, 2019
Merged

Add JOSS and JOSE format #229

merged 8 commits into from Sep 13, 2019

Conversation

noamross
Copy link
Contributor

@noamross noamross commented Jun 2, 2019

While JOSS and JOSE take markdown submissions, this seemed the best place to put the pipeline so that R authors could locally preview the rendered PDF.

@karthik @arfon @eveskew


To contribute a new article template to this package, please make sure you have done the following things (note that journalname_article below is only an example name):

  • Unless you have done it in any other RStudio's projects before, please sign the individual or corporate contributor agreement for a significant pull request (it is fine not to sign it if a PR is only intended to fix a few typos). You can send the signed copy to jj@rstudio.com.

Previously signed

  • Add the journalname_article() function to R/article.R if the output format is simple enough, otherwise create a separate R/journalname_article.R.

  • Add the Pandoc LaTeX template inst/rmarkdown/templates/journalname_article/resources/template.tex.

  • Add a skeleton article inst/rmarkdown/templates/journalname_article/skeleton/skeleton.Rmd.

  • Add a description of the template inst/rmarkdown/templates/journalname_article/template.yaml.

  • Please include the document class file (*.cls) if needed, but please do not include standard LaTeX packages (*.sty) that can be downloaded from CTAN. Please keep the number of new files absolutely minimal, and also make examples minimal (e.g., if you need a .bib example, try to only leave one or two bibliography entries in it, and don't include one hundred items in it without using all of them).

  • Update Rd and namespace (could be done by devtools::document()).

  • Update NEWS.

  • Update README with a link to the newly supported journal.

  • Add a test to tests/testit/test-formats.R.

  • Add your name to the list of authors Authors@R in DESCRIPTION. You don't need to bump the package version in DESCRIPTION.

Lastly, please try your best to do only one thing per pull request (e.g., if you want to add two output formats, do them in two separate pull requests), and refrain from making cosmetic changes in the code base: https://yihui.name/en/2018/02/bite-sized-pull-requests/

Thank you!

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, have you run optipng on PNG images?

Thank you!

R/joss_article.R Outdated
"Journal of Open Source Software",
"Journal of Open Source Education")

template_variables <- c(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about setting these variables in the YAML frontmatter of the Rmd document? That way, you won't have to worry about escaping certain special characters in the values of these variables (although I guess it would be unlikely that these values could contain characters like double quotes). It will also make the R function much simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid needing these variables in the YAML so that a standard paper.md would compile without setting them, but now I realized that the document will compile without most of them. The logo and journal name still make sense to define here, but I can drop the rest.

R/joss_article.R Outdated
base$knitr$opts_chunk$fig.width <- 6
base$knitr$opts_chunk$fig.height <- 4.15
base$keep_md <- keep_md
base$clean_supporting <- !keep_md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All above options can be passed to pdf_document_format() so you don't need to hack the base object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, except keep_md, which is not supported in pdf_document() (rstudio/rmarkdown#1001)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added support for keep_md in rmarkdown, so you can get rid of this hack, too.

R/joss_article.R Outdated

pandoc_args <- c(
template_variables,
"--csl", find_resource("joss_article", "apa.csl")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope the csl option could be set in the YAML metadata of the Rmd document, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid doing this because I wanted the format to compile correctly on a standard paper.md without modifying the YAML.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other templates in this package include csl under the skeleton/ directory (instead of resources/) so the csl file will be copied over when creating a new article using rticles::draft(), e.g. https://github.com/rstudio/rticles/tree/master/inst/rmarkdown/templates/acm_article/skeleton Then in Rmd: https://github.com/rstudio/rticles/blob/master/inst/rmarkdown/templates/acm_article/skeleton/skeleton.Rmd#L15

I'm aware of the potential disadvantages of this approach (e.g. the csl file may change in the upstream), but I'd like this template to be consistent with other templates. Similarly, I hope logo_path, journal_name, and graphics variables could all be set in the YAML frontmatter of skeleton.Rmd (the png logos need to be moved to skeleton/ accordingly).

@noamross
Copy link
Contributor Author

noamross commented Jun 3, 2019

OK, optimized the images, and simplified the function now that I realize that the article will compile without all those variable set. I have left the csl coded into the function. I think that the most common workflow will be for authors with already-created paper.md files to want to compile them to check references, so want to keep the need to change the YAML minimal.

@noamross
Copy link
Contributor Author

Just checking in. Is this version mergable?

@yihui
Copy link
Member

yihui commented Jun 17, 2019

Sorry, I haven't had a chance to look at the revision yet.

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a good chance that this format can be simplified and moved to https://github.com/rstudio/rticles/blob/master/R/article.R. Thanks!

R/joss_article.R Outdated
pandoc_args = pandoc_args,
citation_package = "none",
fig_width = 6, fig_height = 4.15,
dev = "png"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you hard-coded these options, are users not supposed to customize fig_width, fig_height, or dev?

BTW, it seems you forgot to pass the ... argument here.

R/joss_article.R Outdated

pandoc_args <- c(
template_variables,
"--csl", find_resource("joss_article", "apa.csl")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other templates in this package include csl under the skeleton/ directory (instead of resources/) so the csl file will be copied over when creating a new article using rticles::draft(), e.g. https://github.com/rstudio/rticles/tree/master/inst/rmarkdown/templates/acm_article/skeleton Then in Rmd: https://github.com/rstudio/rticles/blob/master/inst/rmarkdown/templates/acm_article/skeleton/skeleton.Rmd#L15

I'm aware of the potential disadvantages of this approach (e.g. the csl file may change in the upstream), but I'd like this template to be consistent with other templates. Similarly, I hope logo_path, journal_name, and graphics variables could all be set in the YAML frontmatter of skeleton.Rmd (the png logos need to be moved to skeleton/ accordingly).

R/joss_article.R Outdated
base$knitr$opts_chunk$fig.width <- 6
base$knitr$opts_chunk$fig.height <- 4.15
base$keep_md <- keep_md
base$clean_supporting <- !keep_md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added support for keep_md in rmarkdown, so you can get rid of this hack, too.

R/joss_article.R Outdated
paste(metadata$authors[[1]]$name, "et. al."),
metadata$authors[[1]]$name
)
return(c("-V", paste0("citation_author=", citation_author)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I wish this variable could be set in YAML. I'm definitely being selfish here: I want the R source code of this package to be as simple as possible, and I guess the price for users to pay is not that high (typing out a name).

@noamross
Copy link
Contributor Author

It's not the upstream that I'm concerned with here as the downstream. With JOSS, the .md file will be submitted, not the LaTeX or PDF, and then the file goes through their automated build system. So I'm concerned that user settings in the YAML will clash with that. Perhaps @arfon can comment here - if users set additional fields such as csl: in their YAML, will that break the JOSS/JOSE build in any way?

@karthik
Copy link

karthik commented Jun 26, 2019

Arfon will have to weigh in but yes setting csl in the yaml will not be used in the automated build system. It's hard coded here

@yihui
Copy link
Member

yihui commented Jun 28, 2019

Since csl is hard-coded in the build system, user's setting in YAML will be ignored. See https://pandoc.org/MANUAL.html#citation-rendering:

Set the csl field in the document’s metadata to FILE, overriding any value set in the metadata.

@arfon
Copy link
Contributor

arfon commented Jul 3, 2019

Arfon will have to weigh in but yes setting csl in the yaml will not be used in the automated build system. It's hard coded here

Yep, we don't support custom CSL definitions for JOSS papers.

@yihui yihui force-pushed the master branch 3 times, most recently from 033a19f to d98786f Compare August 20, 2019 20:57
@noamross
Copy link
Contributor Author

noamross commented Sep 3, 2019

I've moved csl and other variables to the YAML rather than the function. I have kept the logic in the function to switch between JOSE and JOSS articles. This is slightly more complex than the simplest article functions but it's DRYer than having two formats for JOSS and JOSE.

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@yihui yihui merged commit 4f2ded0 into rstudio:master Sep 13, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants