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 ims_article() #372

Merged
merged 12 commits into from
Feb 23, 2021
Merged

add ims_article() #372

merged 12 commits into from
Feb 23, 2021

Conversation

auzaheta
Copy link
Contributor

@auzaheta auzaheta commented Feb 4, 2021

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.

  • 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/resources/template.tex.

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

  • Add a description of the template inst/rmarkdown/templates/journalname/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. If you are using TinyTeX or TeX Live, you can verify if a package is available on CTAN via tinytex::parse_packages(files = "FILENAME"") (e.g., when FILENAME is plain.bst, it should return "bibtex", which means this file is from a standard CTAN package). Please keep the number of new files absolutely minimal (e.g., do not include PDF output files), 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 too many 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. Please add your Github username and the full name of the journal (follow other examples in the list).

  • Add a test to tests/testit/test-formats.R by adding a line test_format("journalname"). We try to keep them in alphabetical order.

  • 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!

@CLAassistant
Copy link

CLAassistant commented Feb 4, 2021

CLA assistant check
All committers have signed the CLA.

@auzaheta auzaheta marked this pull request as ready for review February 8, 2021 19:11
@cderv cderv self-requested a review February 9, 2021 12:42
@cderv cderv linked an issue Feb 9, 2021 that may be closed by this pull request
Copy link
Collaborator

@cderv cderv left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution ! This is a really great first contribution to rticles !
Thanks a lot for having following closely the guide! This is working well and there are very few questions on my side.

First of all, I understand that the article is based on the template of AOAS journal. Do you think or know if this is generic to every journals they have ? I see several: https://www.e-publications.org/ims/support/ims-instructions.html and did not compare the templatex .tex

Does changing only the journal: aoas field in the YAML will work for any journal they have ?

I ask because usually the function is journalname_article(). So it seems it should be called aosas_article instead as this is the name of the journal. But if what I said before, maybe we could make an exception.

If not we could keep one function, but may need to modify a bit. Like providing an argument to the function ims_article(journal = "aoas) that would switch between the correct template for IMS and using the share component. If there is no shared component, then maybe it is one function in rticles by journal.

For the rest of the comments, see below. There is a few things that I think needs to change (depending on your feedback). For the rest this is mainly for discussion before merging.

For example ,I made some suggestion to include a bit less LaTeX syntax and use more markdown in the skeleton. Pandoc and rmarkdown allow to write some markdown syntax to generate the expected latex. What do you think ?

inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/LICENSE Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
auzaheta and others added 3 commits February 10, 2021 12:04
…estions to skeleton.rmd

Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Merge remote-tracking branch 'upstream/master'

# Conflicts:
#	NEWS.md
@auzaheta
Copy link
Contributor Author

auzaheta commented Feb 11, 2021

Thanks a lot for the suggestions to improve the template and the detailed review.

The template is generic for all the journals they have; being precise, all templates use the same style file. I checked the five journals directly related to IMS Journals and publications, and I added a comment in the skeleton indicating the acronym of the journals and additional variable that should be used for some of them. I didn't find a way of adding an example of this variable's type of content that allows an easy way of uncommenting it.

In summary, it works for these five journals. All of them have the same tex template. I tried to emphasize the annals of applied statistics in the news and the readme because I think it is the one most of us could use.

This template could be used for other journals beyond these five (also IMS). It would require developing the function with the argument with an indication of the journal and create independent skeletons (as you suggested). However, this is beyond what I know how to do and what I will need soon. Happy to learn how to do it in any case.

I tried to follow your recommendation to have more Rmarkdown syntax in the skeleton.

@auzaheta auzaheta requested a review from cderv February 11, 2021 14:54
Copy link
Collaborator

@cderv cderv left a comment

Choose a reason for hiding this comment

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

In summary, it works for these five journals. All of them have the same tex template. I tried to emphasize the annals of applied statistics in the news and the readme because I think it is the one most of us could use.

That is really great news ! We can keep ims_article() then and have the journal variable for switching between journal. Maybe we should add somewhere that it should work for the five journals and list them ? maybe add the link of https://www.e-publications.org/ims/support/ims-instructions.html ?
It could be in the function help page.

I didn't find a way of adding an example of this variable's type of content that allows an easy way of uncommenting it.

See below

I tried to follow your recommendation to have more Rmarkdown syntax in the skeleton.

Awesome! Thank you.

This is very cool. Thanks for this contribution!

Only last thing is indeed the rmarkdown 2.7 dependency. I let you see the commend below and let you time to react before merging.

inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/ims/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
auzaheta and others added 3 commits February 15, 2021 11:39
…leton.Rmd

Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
- Homogenize YAML variable names to lowercase
- Use current custom blocks syntax
- Define a `journal` parameter to activate a part of the template accordingly
- Extend documentation with journal acronym
R/article.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@cderv cderv left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@cderv cderv changed the title add ims_article(), solves #234 add ims_article() Feb 23, 2021
@cderv cderv merged commit 14cf24d into rstudio:master Feb 23, 2021
stefanocoretta pushed a commit to stefanocoretta/rticles that referenced this pull request Apr 3, 2021
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2021
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.

Add IMS templates
3 participants