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

Updated mdpi_article #381

Closed
wants to merge 25 commits into from
Closed

Updated mdpi_article #381

wants to merge 25 commits into from

Conversation

mrajeev08
Copy link

@mrajeev08 mrajeev08 commented Apr 6, 2021

Hi there,

Apologies for the previous PR (and the messy commit history), I'm a bit new to this, but here goes. I've just adapted the mdpi_article template for a recent article submission per the newest template (see here), so thought I would submit this PR in case useful. This would close #369.

The biggest changes I encountered are that it's now in a two-column format and they use a custom table environment. I've provided some guidance in the skeleton.Rmd on how to deal with this and some other issues I ran into when adapting my manuscript to this new template (mainly long table doesn't work well with a two-column layout). Not sure if that is the right place for it, but thought it might save others some headaches!

I added in some yaml fields that were missing from the template previously (i.e. dataavailability, informed consent, etc.), and also left a few comments in the template (marked by a comment [MR: from rticles] where I carried over some latex code from the previous rticles template (which I can delete if it all looks good).

Finally, I maintained the directory structure provided by the journal template, i.e. the supporting files (i.e .csl, .bst, images, etc.) in the 'Definitions' folder), as I thought that would be the best bet to make the journal happy:)

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

@CLAassistant
Copy link

CLAassistant commented Apr 6, 2021

CLA assistant check
All committers have signed the CLA.

@cderv cderv self-requested a review April 8, 2021 11:02
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 ! That is really useful to share an update of this articles.

We don't usually update frequently, mainly because we are not aware of updates.

I left a few comments that we could discuss. All the formats needs improvement and it is really great to have users offer some feedback as PR. Thank you very much !

Please update the README also with your PR number and name in the table.


%=================================================================
\documentclass[$journal$,$type$,$status$,moreauthors,pdftex]{mdpi}
\documentclass[$journal$,$type$,$status$,moreauthors,pdftex]{Definitions/mdpi}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Finally, I maintained the directory structure provided by the journal template, i.e. the supporting files (i.e .csl, .bst, images, etc.) in the 'Definitions' folder), as I thought that would be the best bet to make the journal happy:)

Is this a requirement from the journal ? Do they ask for this ?
Just checking as this could break old articles because the template .tex is shared but the resources won't be in the definition format.
If this is a requirement from the journal, we may need to adapt the R function so that it checks for this folder and do something if it does not exist.

We don't want to hardly break old articles without notice at least.

Copy link
Contributor

@mps9506 mps9506 Feb 3, 2023

Choose a reason for hiding this comment

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

Assuming they need the file structure preserved, can we make the document class a yaml variable that runs a quick check to see where the mdpi.cls file is located and returns "mdpi" or "Definitions/mdpi"?

\documentclass[$journal$,$type$,$status$,moreauthors,pdftex]{$cls$}

and in the skeleton:

cls: '`r if(file.exists("mdpi.cls")) "mdpi" else "Definitions/mdpi"`'

I'm thinking that would prevent breaking old articles?


%=================================================================
\documentclass[$journal$,$type$,$status$,moreauthors,pdftex]{mdpi}
\documentclass[$journal$,$type$,$status$,moreauthors,pdftex]{Definitions/mdpi}
Copy link
Collaborator

Choose a reason for hiding this comment

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

From the template file I read

%------------------
% moreauthors
%------------------
% If there is only one author the class option oneauthor should be used. Otherwise use the class option moreauthors.

Should it be made a variable so that it can be changed ?

%---------
% pdftex
%---------
% The option pdftex is for use with pdfLaTeX. If eps figures are used, remove the option pdftex and use LaTeX and dvi2pdf.

Same here ? Does it means pdflatex is required to use this template ?

I am new to this template but usually classoption is used, and one can provide what is needed as a list of option. This can't be done here it seems.

Anyway I wanted to point that out.

Also, maybe the other variable could be documented in skeleton to show what other option can be set (which journals ? What type of article ? Which status ?) I don't think this information is easily accessible for a user currently.

This is often documented into the template.tex but it is not easily accessible for the user. Maybe we should make it more accessible - IDK

Comment on lines +73 to +78
%%%% If original paper need add "Retraction", please release the following command!!%%%%%%
%\retractiondate{Date Month Year} % For example, 13 October 2020
%\retractionnoticeyear{Year}
%\retractionnoticevolume{0}
%\retractionnoticeidnumber{0000}
%\retractionnoticedoi{10.3390/xxx}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it come from the template ?

This is added in the template but won't be actionable by a user.

I wonder what it is here for.


%=================================================================
%% Please use the following mathematics environments: Theorem, Lemma, Corollary, Proposition, Characterization, Property, Problem, Example, ExamplesandDefinitions, Hypothesis, Remark, Definition
%% Please use the following mathematics environments: Theorem, Lemma, Corollary, Proposition, Characterization, Property, Problem, Example, ExamplesandDefinitions, Hypothesis, Remark, Definition, Notation, Assumption
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be updated into the skeleton too ?

% Full title of the paper (Capitalized)
\Title{$title$}

% MDPI internal command: Title for citation in the left column
\TitleCitation{$title$}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this supposed to be the same Title in both ? Would the user need to update it ?

This could be made condition to a new variable if necessary

Comment on lines +211 to +212
%% optional
%\supplementary{The following are available online at \linksupplementary{s1}, Figure S1: title, Table S1: title, Video S1: title.}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be possible to set this from the YAML header ?
Not useful ?

$if(appendix)$
\input{"$appendix$"}
$endif$

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
% Citations and References in Supplementary files are permitted provided that they also appear in the reference list here.
\end{paracol}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this for ? Should it be a \begin{paracol} somewhere ?

Maybe I missed it.

Comment on lines 234 to 236
\begin{Theorem}
Example text of a theorem.
\end{Theorem}
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI interest, with recent rmarkdown, this should now work

::: {.Theorem latex=true}
Example text of a theorem
:::

https://bookdown.org/yihui/rmarkdown-cookbook/custom-blocks.html

Comment on lines 240 to 242
\begin{proof}[Proof of Theorem 1]
Text of the proof. Note that the phrase `of Theorem 1' is optional if it is clear which theorem is being referred to.
\end{proof}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

::: {.proof latex="[Proof of Theorem1]"}
Example text of a theorem
:::


This is an example of an equation:
Note that using default Markdown tables will put these in a normal table environment which may cause issues with numbering. To pass the 'specialtable' environment you can use knitr::kable (and the following arguments get you approximately the same style as the journal's.
Copy link
Collaborator

Choose a reason for hiding this comment

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

That is great !

you can add a link to the doc too maybe ?
https://bookdown.org/yihui/rmarkdown-cookbook/kable.html#customize-latex-tables

@cderv cderv mentioned this pull request Jun 9, 2021
3 tasks
@etiennebr
Copy link

Thanks for the update @mrajeev08! I tested the template (following the call in #325) and it runs fine, at least on one of my mdpi paper.

However, I had some issues with longtable. I think that the new template is narrower than the previous one, doing kabble(..., longtable = FALSE) solved it, but the table is slightly less pleasing. The rest seems fine.

@cderv
Copy link
Collaborator

cderv commented Sep 7, 2021

@mrajeev08 Are still you planning to keep working on this some times?
No hurry. Just trying to see if this is staled or not.

There is a few comments I left that would benefit some feedback.

@etiennebr feel free to jump in if you are a mdpi user.

Thank you.

@JVAQUEROM
Copy link

I wrote #369 some while ago. I would be happy to contribute to this.

@cderv
Copy link
Collaborator

cderv commented Nov 2, 2021

Please @JVAQUEROM , Jump in. Either by helping @mrajeev08 on this PR or starting fresh if this is easier.

@mrajeev08
Copy link
Author

Yes, please do! Apologies for the silence, I was hoping to get to this, but haven't managed. I think they've already updated the template again so may be worth a diff to see if things have changed. Also still seems to be structured with the Definitions folder containing most of the latex files, and from my fuzzy memory I think they did require I do that if I wanted to do edits in the latex version and not the word version as per cderv's q. But may not be worth it if it will mess up other things in rticles. In which case fresh start might be easiest:)

@mrajeev08
Copy link
Author

Thanks for the update @mrajeev08! I tested the template (following the call in #325) and it runs fine, at least on one of my mdpi paper.

However, I had some issues with longtable. I think that the new template is narrower than the previous one, doing kabble(..., longtable = FALSE) solved it, but the table is slightly less pleasing. The rest seems fine.

Also @etiennebr if at all still relevant/useful, here's some info on workarounds for longtable!

@JVAQUEROM
Copy link

JVAQUEROM commented Nov 3, 2021

Hi, I tried @mrajeev08's version of rticles and I get LaTeX compilation error: ! LaTeX Error: File Definitions/mdpi.cls' not found.`

But I see you have talked about this in this issue, so I am not sure if I am getting the latest version. I got it doing:

remotes::install_github("mrajeev08/rticles")

I also suggest (or maybe you already do it?) to keep a version of the original template to be able to track changes more easily.

I have some basic knowledge of git and use it regularly but not in collaboration (i.e. never did a PR, etc.). So I'll try my best but maybe I do something wrong at some point.

EDIT: so I changed in the template de "Definition/mdpi" to "mdpi" and copied the corresponding files in the skeleton folder. However, I get another error

! Undefined control sequence.
l.62 \datereceived
                  {} 

@cderv
Copy link
Collaborator

cderv commented Nov 29, 2021

@JVAQUEROM @mrajeev08 I merged master into this PR so that it can be worked again on a good basis.

@JVAQUEROM regarding how to work on this PR, you can install it using remotes::install_github("rstudio/rticles#381").
Then you can use RStudio IDE menu to create a new R Markdown file based on this template. This should work. The Definition folder thing can be kept that way for now. Other review comment need to be handled.

Regarding git usage, you can find useful resource in https://happygitwithr.com/ and PR helper in usethis https://usethis.r-lib.org/articles/articles/pr-functions.html

Thanks

@cderv
Copy link
Collaborator

cderv commented Apr 17, 2023

I am closing this now. We'll work based on new contributed #515.

Thanks @mrajeev08 for your initial work on this.

@cderv cderv closed this Apr 17, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2023
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.

Template in mdpi_template is outdated
6 participants