-
Notifications
You must be signed in to change notification settings - Fork 524
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
Mdpi update #515
Mdpi update #515
Conversation
There is a MDPI special issue deadline next Thursday. It would be awesome if this PR could be merged before then :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR. Sorry for the delay. I am going back on rticles now.
I have a few comments and questions.
If you don't have time anymore on this, I can understand. I'll do the change myself if so. Please do tell me.
Thank you
% pandoc conditionals added to preserve backwards compatibility with previous versions of rticles | ||
$if(classoption)$ | ||
\documentclass[$for(classoption)$$classoption$$sep$,$endfor$]{$cls$} | ||
$else$ | ||
\documentclass[$journal$,$type$,$status$,moreauthor,pdftex]{$cls$} | ||
$endif$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the change, though I wonder if we shouldn't mix both.
It seemed useful to not rely of the order of classoption
but use named field in YAML for the expected option by mdpi.cls
We can do both either from root yaml or ven something more targeted to format function
output:
mdpi_article:
journal: ...
Just curious if you think that letting classoption
be free is really best for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the open classoption felt more consistent with what I saw in other templates so I adopted it. I think I agree that using named fields makes sense here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do both
- Named field to make it easier for authors, and insure correct order
classoption
for further customization
We could do
journal: sports
type: article
status: submit
classoption:
- moreauthors # use when multiple author
- pdftex # use when using latex_engine pdflatex
output: mdpi_article
Or set everything under mdpi_article
directly, not sure which is better.
We could probably also handle moreauthors
and pdftex
insertion from R format function, by (1) checking if multiple authors in yaml header and (2) checking latex_engine
value
Does it sounds helpful ?
I am also fine with leaving classoption
free. Easier to maintain, but more responsability for authors who need to know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moreauthors
and pdftex
are now set by the format function so are not needed in the yaml header. journal
, type
, and status
are named fields. classoption
is no longer needed in this case.
@cderv I'll work on the changes. |
Awesome. Do not hesitate to tell me if you need me to help or if I should do it instead. |
Thanks for the work @mps9506 ! I'll review it now. As stated in the PR template, can you be sure to sign the CLA document and send it to the email ? Thank you
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - this is really good PR !
I'll do a few tweaks myself, and then merge once you have confirmed me you sent the CLA document.
Thank you again !
Otherwise, a user would not be able to modify it and there would be an error if passed in `...`
@mps9506 do you have aslo these warnings locally ?
Is this by default with their template ? |
@cderv Those warnings are issues in mdpi cls or template file. I get them when compiling the template directly in TexLive. I'll send over the CLA shortly. |
ok good to know. Hopefully they'll fix them |
Useful to help document possible values from official LaTeX templates
All good now. Just waiting for confirmation of the CLA and then I'll merge |
@cderv I sent over the CLA. Just a note that the repository template file for contributors has the [email protected] address while the actual CLA form indicates to send it to [email protected]. I copied both since I was unsure. |
Oh thanks. I'll check that. Merging now then ! Thanks a lot for the work on this ! |
Co-authored-by: Michael Schramm <[email protected]> Co-authored-by: Christophe Dervieux <[email protected]>
This is a fresh pull request with the newest version of the mdpi template circa 2022-12 (PR #381 seemed to stall out and was already outdated). There are numerous updates to the yaml header to make better use of the mdpi.cls and template file.
Also fixes #502
How to contribute a new output format ?
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). Unless you have done it in any other RStudio / Posit projects before, sign the individual or corporate contributor agreement as appropriate. You can send the signed copy to [email protected].
Add the
journalname_article()
function toR/article.R
if the output format is simple enough, otherwise create a separateR/journalname_article.R
.Document your function using roxygen2. Markdown syntax is supported. Refer to https://roxygen2.r-lib.org/articles/rd-formatting.html for formatting references.
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 viatinytex::parse_packages(files = "FILENAME"")
(e.g., whenFILENAME
isplain.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 linetest_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!