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

Josis #382

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Josis #382

wants to merge 18 commits into from

Conversation

Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Apr 9, 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 [email protected].

  • 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 Apr 9, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ cderv
❌ Robinlovelace
You have signed the CLA already but the status is still pending? Let us recheck it.

@Robinlovelace
Copy link
Author

First pass, let's see what happens! Any feedback v. welcome.

@Robinlovelace
Copy link
Author

Heads-up @cderv I think this is ready to be reviewed/merged as appropriate. The template generates nice outputs as can be seen here in this in-progress paper I'm working on: https://github.com/zonebuilders/zonebuilder/releases/download/0.0.1/zonebuilder-paper.pdf

@Robinlovelace Robinlovelace mentioned this pull request Apr 17, 2021
2 tasks
@cderv cderv linked an issue Apr 21, 2021 that may be closed by this pull request
2 tasks
@cderv cderv self-requested a review April 21, 2021 16:31
@cderv
Copy link
Collaborator

cderv commented Apr 21, 2021

Thanks I'll add it in the review todo 😅
I should go back to rticles next week I believe.

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 your contribution.

See my comments below. There are a few things that I believe are not working as expected.

And other comments are about improvement that could be made if you want, like t Markdown syntax for example that you could use more in the skeleton.

Comment on lines +14 to +38
%%% JOSIS checks in typesetting
%%% * All titles and sections lower case *EXCEPT short title [ ]
%%% * Remove author postal addresses, only have geographic places and institutions [ ]
%%% * Consistent use of Section, Figure, Table (capitalized and in full) [ ]
%%% * 10 keywords (and all lower case) [ ]
%%% * Remove all avoidable footnotes [ ]
%%% * Use double quotation marks (``'' not "" or `') [ ]
%%% * Punctuation inside quotations [ ]
%%% * E.g. and i.e. followed by comma [ ]
%%% * cf. followed by tilde [ ]
%%% * Itemize and enumerate correctly punctuated [e.g., "1. x, 2. y, and 3. x." ]
%%% * And/or lists using American English punctuation (e.g., "x, y, and z") [ ]
%%% * Bibliography (e.g., en-dashes for number ranges, consistent "Proc.~" for Proceedings of..., etc.) []
%%% * Acknowledgment style use section* [ ]
%%% * et al. no italics, but with dot [ ]
%%% * All captions end with full stop [ ]
%%% * Table captions under, not over table [ ]
%%% * Adjust urls with burlalt [ ]
%%% * Check correct use of hyphens, emdashes, endashes [ ]
%%% * Perform spell check [ ]

%%% JOSIS checks directly before publication
%%% Check DOI, page numbers on article and web site. [ ]
%%% Update web site with final title, abstract, keywords. [ ]
%%% Build with distiller for DOI links. [ ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we need to add this checklist (or adapt it) somewhere in the documentation accessible to author from R ?

Like in the R help section for the format or in the skeleton ?

Comment on lines +50 to +53
% Suggested packages for algorithm formatting
\usepackage{algorithm}
%\usepackage{algorithmic}
\usepackage{algpseudocode}
Copy link
Collaborator

Choose a reason for hiding this comment

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

As these are suggested, should it be make optional through a YAML option to use one or all of these ?

Using Pandoc variable can help with that as in other template.

For example, a R user won't be able to uncomment %\usepackage{algorithmic} for its Rmd file.

Or maybe should all be activated by default as it does not harm really ?

$endif$
\providecommand{\tightlist}{%
\setlength{\itemsep}{0pt}\setlength{\parskip}{0pt}}
\usepackage{longtable,booktabs,array}
Copy link
Collaborator

Choose a reason for hiding this comment

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

booktabs is already loaded before.
https://github.com/rstudio/rticles/pull/382/files#diff-7242a8d3731225c175e35b36ec39868cee3b909d2a0c3234030cd9939d8382c8R45
I don't know if this is an issue to load it twice. Probably not

Comment on lines +112 to +114
%\renewcommand{\UrlLeft}{http:\sslash}
%\DeclareUrlCommand\myurl{\def\UrlLeft{}\def\UrlRight{}%
%\urlstyle{tt}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, does this needs to be activated somehow ?

If so, then a variables should be added so that it could be included in the output.

Comment on lines 9 to 12
author1: Nameof Author1
affil1: Department, Institution, Country
author2: Nameof Author2
affil2: Department, Institution, Country
Copy link
Collaborator

Choose a reason for hiding this comment

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

For author template this is usually below a single authors field which is usual for a Rmd.

See examples in other templates:

author:
- name: Alice Anonymous
firstname: Alice
surname: Anonymous
email: [email protected]
affiliation: Some Institute of Technology
- name: Bob Security
firstname: Bob
surname: Security
email: [email protected]
affiliation: Another University

if this could work here too, it would be better. This requires change in the pandoc template with a for loop I guess. I know some templates use this like you did but it would also not be extensible to more then two.

In the future we plan to standardize this more: #324

Comment on lines +98 to +111
\begin{table}
\centering
\begin{tabular}{lrr}
\hline
Text column & Numerical column 1 & Numerical column 2\\
\hline
First row& 10 & 0.003\\
Second row& 52& 10.037\\
Third row& 729 & 150.315\\
...& ...& ...\\
\hline
\end{tabular}
\caption{Example table with preferred line rules and alignment.}\label{tab:1}
\end{table}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You show table above with knitr : is this interested to keep ?

other search engines and publisher pages (e.g., Scopus, SpringerLink).


\section{About JOSIS}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would be header 1 in Markdown as the other above, wouldn't it ?


JOSIS encourages submissions from topics including, but not limited to spatial and spatiotemporal information systems; computational geometry, geocomputation, spatial algorithms; geovisualization, cartography, and geographical user interfaces; computing with spatiotemporal information under uncertainty; spatial cognition and qualitative spatial reasoning; spatial data models and structures; conceptual models of space and geoontology; distributed and parallel spatial computing, web-based GIS, and interoperability; context- and location-aware computing; and applications to GIS, spatial databases, location-based services, geosensor networks, and geosensor web. The journal publishes full-length original research articles, as well as survey-style review papers. In addition, the journal publishes shorter articles in three sections: reports from community activities, letters to the editors, and book reviews.

\section*{Acknowledgments}
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI in Pandoc's markdown this would be

# Acknowledgement {-}

or

# Acknowledgement {.unnumbered}

Comment on lines +168 to +171
$if(bibliography)$
\bibliographystyle{josisacm}
\bibliography{$bibliography$}
$endif$
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this syntax requires to use natbib as citation processing engine.
Currently, your format sets "default" meaning use Pandoc processing and biliography seems to be added twice.
See
image

See this is because one of the reference is done using Pandoc's syntax (https://pandoc.org/MANUAL.html#citations) and the others are made using \cite I believe so Pandoc
does not process them.

So maybe you need to default to "natbib" for this format.

Usually with Pandoc a CSL is used by I don't see one for ACM Josis

#' \samp{https://www.overleaf.com/latex/templates/journal-of-spatial-information-science-template/bmdhbgxnhtqx}.
#' @export
#' @rdname article
josis_article <- function(..., keep_tex = TRUE, citation_package = 'default') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See commend about bibliography in template: I believe it should be natbib here not default

@Robinlovelace
Copy link
Author

Thanks for all the feedback @cderv this is great and highly educational. Will look to update the PR and respond to your comments in due course.

Comment on lines +76 to +97
$if(csl-refs)$
\newlength{\cslhangindent}
\setlength{\cslhangindent}{1.5em}
\newlength{\csllabelwidth}
\setlength{\csllabelwidth}{3em}
\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}
\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$
Copy link
Collaborator

Choose a reason for hiding this comment

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

I forgot to mention that this needs to be extended. I believe CI error are because of this See for example

$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$

The part for ealier Pandoc version needs to be added

Suggested change
$if(csl-refs)$
\newlength{\cslhangindent}
\setlength{\cslhangindent}{1.5em}
\newlength{\csllabelwidth}
\setlength{\csllabelwidth}{3em}
\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}
\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$
$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$

@cderv
Copy link
Collaborator

cderv commented Sep 7, 2021

Hi @Robinlovelace,

Friendly ping as I am going through PR for next version.

Is this something you are still planning to spend a bit of time off ?

No hurry. I just want to check the status of this PR. Thanks !

@Robinlovelace
Copy link
Author

Sorry for slow reply. I still plan to do this but may be ~1 month before I get the dedicated time needed to move this forward. Looking forward to that time as it will be a learning experience!

@cderv
Copy link
Collaborator

cderv commented Nov 26, 2021

Hi @Robinlovelace ,

Just to inform you that I took the time to merge master in this PR and fix the conflicts so that you can start on a better basis when you'll find time to continue the work.

Cheers,

@nuest
Copy link
Contributor

nuest commented Oct 13, 2022

Hi @Robinlovelace ! Just came across this while looking for another PR. Big JOSIS fan, so if you're still not seeing a window to take another look, I'll find time before the end of the year.

@Robinlovelace
Copy link
Author

Many thanks @nuest let's do this. The PR is almost there but year, needs a bit of TLC. FYI the editors are aware and in support of this effort.

@cderv
Copy link
Collaborator

cderv commented Apr 19, 2023

@nuest @Robinlovelace just a friendly ping to see if this is still a project. If you need my help to make JOSIS format happens, happy to schedule some time on this.

@cderv cderv added the WIP label Apr 19, 2023
@Robinlovelace
Copy link
Author

May need some help. Thanks for patience @cderv, will be worth it! JOSIS is a great open access journal and we're almost there. I will try to get some of the editorial team involved who my have comments also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

josis template
4 participants