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 new article template for Bioinformatics #297

Merged
merged 15 commits into from
Aug 5, 2020
Merged

Add new article template for Bioinformatics #297

merged 15 commits into from
Aug 5, 2020

Conversation

ShixiangWang
Copy link
Contributor

@ShixiangWang ShixiangWang commented Jul 4, 2020

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


I add this new template based on #83, so two authors are added to DESCRIPTION. Please correct my modification if there are something wrong.

This PR can close #83.

Best,
Shixiang

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 didn't check if there are other LaTeX files that are unnecessary (see the PR checklist), but at least plain.bst is from a standard LaTeX package, hence shouldn't be included in the PR. I'm not sure where natbib.bst comes from since it is not from a standard LaTeX package (is it provided by the journal?). The eps figure is relatively large in file size. I wonder if it is possible to use pdf (which will usually be much smaller). I'm asking because we should keep the package size as small as possible, otherwise R CMD check will generate a NOTE in the log.

Thank you!

DESCRIPTION Outdated Show resolved Hide resolved
@ShixiangWang
Copy link
Contributor Author

Thanks for your comments

@ShixiangWang
Copy link
Contributor Author

@yihui please see the updates, the files are provided by the journal at https://academic.oup.com/bioinformatics/pages/submission_online. I try to do my best to remove useless files. You can check them.

@ShixiangWang
Copy link
Contributor Author

Please also remember to close #83 if this PR is accepted :).

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.

Okay. Now I only have two questions:

  1. The file OUP_First_SBk_Bot_8401-eps-converted-to.pdf doesn't seem to be used anywhere. I wonder it can actually be removed. You can try it and see if the test fails. If it still succeeds, I'd just remove this file.

  2. Does the journal have a guideline on whether to use csl or bst for the bibliography style? Both are provided in this PR. If either is okay, I'd prefer using csl. Then delete natbib.bst and the default argument citation_package = 'natbib' in the function bioinformatics_article().

Thanks!

@ShixiangWang
Copy link
Contributor Author

@yihui According to the description from https://academic.oup.com/bioinformatics/pages/submission_online, bst file is necessary, so I delete csl file.

image

And the pdf is also necessary, in my test that removing the pdf, an error occurred.

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.

Great! You are all set now. The only thing we need from you is the contributor agreement, i.e. the first bullet in the checklist above. Have you signed that and emailed to JJ?

After we receive the agreement, we'll make a few very minor revisions to this PR and merge it. Thanks again for your contribution!


bibliography: bibliography.bib
# biblio-style: natbib
# csl: bioinformatics.csl
Copy link
Member

Choose a reason for hiding this comment

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

Given that this file has been removed, perhaps this line could also be removed?

citation_package: natbib

bibliography: bibliography.bib
# biblio-style: natbib
Copy link
Member

Choose a reason for hiding this comment

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

On line #65 in template.tex, the bibliography style has already been hard-coded, so I don't think this biblio-style field is necessary here. It can confuse users because it may give them a wrong impression that the biblio-style is customizable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yihui Thanks, I agree with you. Could you directly change it in this PR web editor?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I will.

@ShixiangWang
Copy link
Contributor Author

ShixiangWang commented Aug 4, 2020

@yihui I have done the contributor agreement. :)

@yihui
Copy link
Member

yihui commented Aug 4, 2020

Okay. I'll let @cderv tweak and merge the PR. Thanks again!

@cderv
Copy link
Collaborator

cderv commented Aug 5, 2020

I can confirm that OUP_First_SBk_Bot_8401-eps-converted-to.pdf is necessary and in used in bioinfo.cls. In fact it is looking for OUP_First_SBk_Bot_8401.eps and does not found it. When converting to pdf, the pdf version is found, so everything goes well after (except a warning because the eps file is not found) - seems ok as is.

@cderv cderv merged commit 86e3892 into rstudio:master Aug 5, 2020
@cderv
Copy link
Collaborator

cderv commented Aug 5, 2020

Thanks @ShixiangWang !

For next time, can you try to create a branch in your repo before sending a PR ?
It is good practice and easier to have PR sent from feature branch. Thank you !

@ShixiangWang
Copy link
Contributor Author

@cderv Thank you. Learned from it. :)

@yihui
Copy link
Member

yihui commented Aug 18, 2020

I just saw a NOTE in R CMD check: https://travis-ci.org/github/rstudio/rticles/builds/715211132#L877-L884

* checking for portable file names ... NOTE
Found the following non-portable file path:
  rticles/inst/rmarkdown/templates/bioinformatics_article/skeleton/OUP_First_SBk_Bot_8401-eps-converted-to.pdf
Tarballs are only required to store paths of up to 100 bytes and cannot
store those of more than 256 bytes, with restrictions including to 100
bytes for the final component.
See section ‘Package structure’ in the ‘Writing R Extensions’ manual.

We'll have to either rename bioinformatics_article or OUP_First_SBk_Bot_8401-eps-converted-to.pdf. We need to cut at least 8 bytes off from the path:

> nchar('rticles/inst/rmarkdown/templates/bioinformatics_article/skeleton/OUP_First_SBk_Bot_8401-eps-converted-to.pdf')
[1] 108

@cderv
Copy link
Collaborator

cderv commented Aug 18, 2020

The pdf file is the one provided with the template. I don't know if it is important to keep the name or not.
We could also provide the eps file instead OUP_First_SBk_Bot_8401.eps. It is 398 kb instead of 10kb in size. This eps file be converted at rendering in the pdf file OUP_First_SBk_Bot_8401-eps-converted-to.pdf locally in the user folder. (tested on my computer)
Used here

\hfill\includegraphics{OUP_First_SBk_Bot_8401.eps}}

If we do that we can reduce by more than 8 characters.

Otherwise, we rename the file but we would need to modify the bioinfo.cls I think or we rename the function to bioinf_article 😉

@yihui
Copy link
Member

yihui commented Aug 18, 2020

Perhaps we just get rid of the _article suffix in the folder names (which is exactly 8 characters):

$ ls inst/rmarkdown/templates/
acm_article             bioinformatics_article  jss_article             rsos_article
acs_article             biometrics_article      mdpi_article            rss_article
aea_article             copernicus_article      mnras_article           sage_article
agu_article             ctex                    oup_article             sim_article
amq_article             elsevier_article        peerj_article           springer_article
ams_article             frontiers_article       plos_article            tf_article
arxiv_article           ieee_article            pnas_article
asa_article             joss_article            rjournal_article

The suffix seems to be redundant anyway.

@cderv
Copy link
Collaborator

cderv commented Aug 18, 2020

Yes that is another way. We can do that. This folder name is only internal the _article does not help particularly.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 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.

3 participants