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 Wellcome OR latex template #436

Merged
merged 40 commits into from
Dec 10, 2021
Merged

Conversation

arnold-c
Copy link
Contributor

@arnold-c arnold-c commented Sep 3, 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):

  • This project uses a Contributor Licence Agreement (CLA) that you'll be asked to sign when opening a PR. This is required for a significant pull request (it is fine not to sign it if a PR is only intended to fix a few typos). We use a tool called CLA assistant for that.
    You could also, unless you have done it in any other RStudio's projects before, sign the individual or corporate contributor agreement. 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!


Hi,

I'm afraid I'm relatively inexperienced at both package development and LaTeX, so have adapted the template as much as I can, but am currently running into a problem when I try to knit skeleton.Rmd: Error: 'wellcomeor_article' is not an exported object from 'namespace:rticles'. If anyone can point me in the right direction, that'd be much appreciated, and I can see if the template works as expected.

Thanks,

@CLAassistant
Copy link

CLAassistant commented Sep 3, 2021

CLA assistant check
All committers have signed the CLA.

@cderv
Copy link
Collaborator

cderv commented Sep 3, 2021

Thanks for the contribution of this new template.

but am currently running into a problem when I try to knit skeleton.Rmd: Error: 'wellcomeor_article' is not an exported object from 'namespace:rticles'

Did you install the development version of this package (from your branch) ?

Using the Knit button will use the install version. If you are using devtools::load_all() you would need to use command line rmarkdown::render I think

@arnold-c
Copy link
Contributor Author

arnold-c commented Sep 9, 2021

Sorry for the delay in response @cderv, and thanks for reminding me to build and install the development version! I'm now getting this issue

Error compiling template "/Users/user/Library/R/x86_64/4.1/library/rticles/rmarkdown/templates/wellcomeor/resources/template.tex" (line 77, column 1):
unexpected end of input
expecting " ", "\t", "$--", "\n", "\r\n", "\r", "$", "${" or "$$"
Error: pandoc document conversion failed with error 5
Execution halted

Looking in both skeleton.Rmd and template.tex I can't see any code that would be causing these issues, but I'm obviously missing something. I've tried removing the majority of the body text in an attempt to see if some of the LaTeX code was causing the error, but this didn't seem to help.

@cderv
Copy link
Collaborator

cderv commented Sep 10, 2021

This type of error is thrown by Pandoc when something is not correct regarding syntax in the template. There must be a Pandoc template syntax not correct in the template. It could be an $endif$ missing or any other $ missing around variables.

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.

I have left a few comments.

The CI issue is because a $endfor$ is missing I believe

Also you have some file in this PR the concerns the arxiv template, in the arxiv folder. skeleton.pdf and skeleton.tex ? Why are they here ?
I think they should be removed.

I did not do a thorough review yet but can you also fill the template with some missing pieces required by Pandoc ?
At least

% Pandoc syntax highlighting
$if(highlighting-macros)$
$highlighting-macros$
$endif$
% Pandoc citation processing
$if(csl-refs)$
\newlength{\csllabelwidth}
\setlength{\csllabelwidth}{3em}
\newlength{\cslhangindent}
\setlength{\cslhangindent}{1.5em}
% for Pandoc 2.8 to 2.10.1
\newenvironment{cslreferences}%
{$if(csl-hanging-indent)$\setlength{\parindent}{0pt}%
\everypar{\setlength{\hangindent}{\cslhangindent}}\ignorespaces$endif$}%
{\par}
% For Pandoc 2.11+
\newenvironment{CSLReferences}[2] % #1 hanging-ident, #2 entry spacing
{% don't indent paragraphs
\setlength{\parindent}{0pt}
% turn on hanging indent if param 1 is 1
\ifodd #1 \everypar{\setlength{\hangindent}{\cslhangindent}}\ignorespaces\fi
% set entry spacing
\ifnum #2 > 0
\setlength{\parskip}{#2\baselineskip}
\fi
}%
{}
\usepackage{calc} % for calculating minipage widths
\newcommand{\CSLBlock}[1]{#1\hfill\break}
\newcommand{\CSLLeftMargin}[1]{\parbox[t]{\csllabelwidth}{#1}}
\newcommand{\CSLRightInline}[1]{\parbox[t]{\linewidth - \csllabelwidth}{#1}\break}
\newcommand{\CSLIndent}[1]{\hspace{\cslhangindent}#1}
$endif$
$for(header-includes)$
$header-includes$
$endfor$

After theses changes I'll test further the template and check if everything is there.

NEWS.md Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
@arnold-c
Copy link
Contributor Author

Thanks @cderv. I've added the Pandoc requirements you linked, along with your other suggestions, and removed the arXiv files that accidentally were made when I was examining the template.

It is no longer throwing the Pandoc error, but I am now getting the following error

! Undefined control sequence.
<argument> ...sed on Wellcome Open Research \href 
                                                  {https://www.overleaf.com/...
l.62 Latex template}}

l.62 refers to

\emph{Text based on Wellcome Open Research
\href{https://www.overleaf.com/project/6131911644b635ad3aaa4cb2}{Overleaf
Latex template}}

When removing _Text based on Wellcome Open Research [Overleaf Latex template](https://www.overleaf.com/project/6131911644b635ad3aaa4cb2)_ from skeleton.Rmd, I instead get the following error:

! Undefined control sequence.
l.60 \hypertarget
                 {introduction}{% 

Which refers to:

\hypertarget{introduction}{%
\section{Introduction}\label{introduction}}

Thanks

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.

Interesting. I think you get this from again missing part from the template for Pandoc requirement.

First

\IfFileExists{bookmark.sty}{\usepackage{bookmark}}{\usepackage{hyperref}}

This package seems missing and required to use \href command.

then

\providecommand{\tightlist}{%
  \setlength{\itemsep}{0pt}\setlength{\parskip}{0pt}}

required by Pandoc for creating specific list style

Those must be added in the template.

After that, there may be other issue. I get something about font but i'll wait for you to try as I am not sure of my env.

inst/rmarkdown/templates/wellcomeor/template.yaml Outdated Show resolved Hide resolved
@arnold-c
Copy link
Contributor Author

arnold-c commented Sep 21, 2021

I've updated the template and it now produces a pdf. The only remaining issues I couldn't figure out is how to disable link coloring (setting it using hypersetup{} didn't seem to work), and the author affiliation number isn't correct and doesn't allow for multiple authors. I tried to copy across code from the OUP template, which allows for it, but couldn't get it to work I'm afraid. I also get font warning, but it seems like that is also an warning on the Overleaf template, so I don't think it's too much of an issue that should be fixed on our end.

Warning message:
LaTeX Warning: Font shape declaration has incorrect series value `mc'.
               It should not contain an `m'! Please correct it.
               Found on input line 53.

@cderv
Copy link
Collaborator

cderv commented Sep 22, 2021

I couldn't figure out is how to disable link coloring

It is possible this is something produced by Pandoc. Did you try opting out using the variables ?
https://pandoc.org/MANUAL.html#links

Something like

colorlinks: false

in the YAML header ?

the author affiliation number isn't correct and doesn't allow for multiple authors.

This can be tricky and require to modify the template. Several formats have different trick for this kind of thing.
I could try to have a look. That would be helpful if you know what syntax it should look like in the .tex output ?
We could then adapt as template for Pandoc.

Thanks

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.

Hey !

What is the status on this PR ?

I reviewed it as is after merging changes from the main branch to be up to date.

Overall it looks good ! Small questions and feedback before we can merge.

Thanks !

arnold-c and others added 10 commits December 8, 2021 16:31
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Bit of a hack as the affiliation codes have to be listed as in numerical order, and for some reason it doesn't work when `affil` is replaced with `affiliation`
Not ideal to repeat the author names next to the email, but I couldn't think of another way of demonstrating which author corresponded with each email and allow authors to have multiple/share affiliations
And organize closer to other templates
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.

Hi,

thanks a lot for the feedback. We are preparing a releasing so this is good news if we can merge this.

Based on your feedback, and my previous work this past two weeks on rticles I did more comment. Instead of just making them and pushing, I made them as suggestion so that you can validate if that is ok to you.

My aim is for the template to be closest to Pandoc's one so that features work and it is easy to update.

Please, have a look (below and above) and confirm or not my sugestion. Once I'll have your ok, I'll commit and merge everything.

Tests are now passing - some contents where missing.

R/article.R Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/skeleton/skeleton.Rmd Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
inst/rmarkdown/templates/wellcomeor/resources/template.tex Outdated Show resolved Hide resolved
arnold-c and others added 11 commits December 9, 2021 08:27
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
@arnold-c
Copy link
Contributor Author

arnold-c commented Dec 9, 2021

All look good to me so I just went ahead and committed the suggestions. Thanks

@cderv
Copy link
Collaborator

cderv commented Dec 9, 2021

Thanks @arnold-c !

There were also the discussion box above my last comment 😅 sorry for that, Github have separated them from the other. Can you validate those too ?

Especially this one: #436 (comment)

Thanks

arnold-c and others added 3 commits December 9, 2021 16:10
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
@arnold-c
Copy link
Contributor Author

arnold-c commented Dec 9, 2021

Sorry about that. I've just gone through and confirmed it all works as expected after those suggestions (I think I got them all)

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.

Awesome that is great !

We are ready to roll then. Thank you for this PR and you're availability to make the necessary changes!

@cderv
Copy link
Collaborator

cderv commented Dec 9, 2021

We must just redocument following new arguments in function. I'll do that tommorow and merge it.

@cderv cderv changed the title Attempt to update Wellcome OR latex template Add Wellcome OR latex template Dec 10, 2021
@cderv cderv merged commit 4630784 into rstudio:master Dec 10, 2021
cderv added a commit that referenced this pull request Dec 10, 2021
Co-authored-by: Christophe Dervieux <christophe.dervieux@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants