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

automatic DOI links in the reference list #65

Closed
ikashnitsky opened this Issue Jan 9, 2019 · 20 comments

Comments

Projects
None yet
4 participants
@ikashnitsky
Copy link
Contributor

ikashnitsky commented Jan 9, 2019

tl;dr -- maybe default bibliography to APA?

I am specifying a custom .csl using the option (apa.csl is in the parent directory)

# citation style
biblio-style: apa

The resulting reference list looks fine. The only issue -- it omits DOI link at the very end. I think it's useful to have a direct link to the papers from the PDF.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 9, 2019

Yes, that would be nice, and I don't yet know which csl files do this well and which don't. It seems that recent R Journal and/or JSS manage but I haven't looked in detail yet. The jss.bst we ship seems to know about doi per a quick grep.

But I also don't like to add too many extra files as part of the build so hesitant to add a csl. Good enough if we document it?

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 9, 2019

I can (should) also ask Achim about how to enable this for jss.bst. Maybe there is an option...

@statibk

This comment has been minimized.

Copy link

statibk commented Jan 9, 2019

What JSS does is the following:

  • The jss.bst processes doi = {...} fields in the .bib files. This option can be easily enabled in makebst (which is how we produce the jss.bst).
  • Thus, the .bbl produces from such .bib files contains a \newblock with \doi{...}.
  • The \doi{} command that then produces the display and hyperlink is defined in jss.cls as: \newcommand{\doi}[1]{\href{https://doi.org/#1}{\normalfont\texttt{\@doi{#1}}}}.

At some point last year I had a look whether I could quickly produce a jss.csl which would be nice even though we don't have a real need for this in JSS at the moment. But it wasn't straightforward enough for me to accomplish this so I didn't go through with it.

As for APA style: While this is a nice and widely-used style I personally rarely use it, namely because displaying additional information like URLs, access dates, version information, DOIs, etc. is not intended. However, for software-related papers this is needed frequently.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 9, 2019

Thanks, Achim. I guess we need to fiddle more, and probably for now document use of APA as a way out.

I don't feel I want to make APA the default though.

@ikashnitsky

This comment has been minimized.

Copy link
Contributor

ikashnitsky commented Jan 9, 2019

Sorry, it seems that I described the issue vaguely. The resulting APA styled reference list does not have DOI links.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 10, 2019

I had a vague suspicion that PNAS and I might be to blame -- and just "proved it". In short:

  • the PNAS style use a \doi{} command to place a nice footer onto the pages, we carried that over and show how to e.g. put the CRAN package URL there
  • but that means that \doi{} is used / defined ... so when Achim's fine magic produces DOIs for the bibliography these do not see the correct \doi{} and get dropped
  • so by mixing PNAS and jss.bst I created an impasse that was hiding bibliographics dois
  • so as a first pass I did two things:
    • rename the YAML header option to doi_footer
    • rename the command to \doifooter{}
  • now we have DOI in the bibliography -- currently not as a hyperlink.

Achim: Thoughts on whether hacking that into the bst level \doi{} via \href{} may be doable?

In any event the issue of this ticket is now fixed in this new branch so take a look.

Edit: Just made it backwards compatible to doi YAML headers from existing documents should still work.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 10, 2019

Net effect is e.g. on the demo document shipped as the skeleton and its four-item bibliography including one with a DOI:

image

The DOI reference is there (see third reference) but not yet clickable. Might be as simple as 'borrowing' the \doi{} macro from JSS.cls ... To be seen.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 10, 2019

But I just tried copying this from jss.cls and no mas: latex error on redefinition / use different from definition:

%% Borrow \doi{} from jss
\ifx\csname urlstyle\endcsname\relax
  \newcommand\@doi[1]{doi:\discretionary{}{}{}#1}\else
  \newcommand\@doi{doi:\discretionary{}{}{}\begingroup
\urlstyle{tt}\Url}\fi
\newcommand{\doi}[1]{\href{http://dx.doi.org/#1}{\normalfont\texttt{\@doi{#1}}}}
@statibk

This comment has been minimized.

Copy link

statibk commented Jan 11, 2019

To avoid the \@doi stuff, you can also try to do only:

\newcommand{\doi}[1]{\href{http://dx.doi.org/#1}{\normalfont\texttt{doi:#1}}

or

\newcommand{\doi}[1]{\href{http://dx.doi.org/#1}{\normalfont\texttt{doi:\discretionary{}{}{}#1}}

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 11, 2019

@statibk You rock. The second variant, with an added } at the end, yields

image

Which is no nice that we can almost consider making the URL optional. For another time...

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 11, 2019

Thanks again to @ikashnitsky for raising this, and to @statibk for the quick help.

I rolled this up as release 0.0.7 which just got the beloved [CRAN-pretest-publish] treatment and will soon be on a CRAN mirror near you.

@ikashnitsky

This comment has been minimized.

Copy link
Contributor

ikashnitsky commented Jan 11, 2019

I'm having problems with the new doi_footer option

! Undefined control sequence.
l.73 \doifooter
               {\url{https://doi.org/10.1016/j.cities.2016.05.025}} 
@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 11, 2019

Make sure you get a new copy of pinp.cls -- these are usually copied in by rmarkdown::render():

It's in my sources:

edd@rob:~/git/pinp(master)$ ag doifooter
inst/rmarkdown/templates/pdf/resources/template.tex
72:\doifooter{$doi$}
76:\doifooter{$doi_footer$}

inst/rmarkdown/templates/pdf/skeleton/pinp.cls
282:   \fancyfoot[L]{\footerfont\@ifundefined{@doifooter}{}{\@doifooter}}
294:\fancyfoot[LE]{\footerfont\textbf{\thepage}\hspace{7pt}|\hspace{7pt}\@ifundefined{@doifooter}{}{\@doifooter}}
359:\newcommand{\doifooter}[1]{\def\@doifooter{#1}}
edd@rob:~/git/pinp(master)$

and in the installed 0.0.7 package:

edd@rob:~/git/pinp(master)$ ag doifooter /usr/local/lib/R/site-library/pinp/
/usr/local/lib/R/site-library/pinp/rmarkdown/templates/pdf/resources/template.tex
72:\doifooter{$doi$}
76:\doifooter{$doi_footer$}

/usr/local/lib/R/site-library/pinp/rmarkdown/templates/pdf/skeleton/pinp.cls
282:   \fancyfoot[L]{\footerfont\@ifundefined{@doifooter}{}{\@doifooter}}
294:\fancyfoot[LE]{\footerfont\textbf{\thepage}\hspace{7pt}|\hspace{7pt}\@ifundefined{@doifooter}{}{\@doifooter}}
359:\newcommand{\doifooter}[1]{\def\@doifooter{#1}}
edd@rob:~/git/pinp(master)$

That said, it is still entirely possible I messed this up but I ran a few tests over the last few days with/without this set in YAML.

@ikashnitsky

This comment has been minimized.

Copy link
Contributor

ikashnitsky commented Jan 11, 2019

Hmm, this is a funny issue then. The files pinp.cls and jss.bst are copied to the new .Rmd file directory at the moment of its creation. These files are not updated with update of {pinp}.

Also, if I get it right the patch only works for jss.bst. If one uses the YAML option biblio-style: [any style here, e.g. apa] the DOI link are not created.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 11, 2019

These files are not updated with update of pinp

Yup. My "premature optimisation" of not copying existing files. (Which is still the Right Thing (TM) bigger picture I'd argue.)

only works for jss.bst

Yup. See Achim's comments above. You need advanced LaTeX wizard skills for this, which Achim has. This is not a standard feature. His JSS.bst leads to doi entries being extracted into bibitem environments (which made it clear to me that I messed up by hiding \doi{}, now taken care of) but it cannot affect other .bst files and styles. You win some, you loose some.

It is still a big net improvement as only advanced users like you even switch bibliographic styles. And if you do, well then you get to keep the pieces ;-)

@ikashnitsky

This comment has been minimized.

Copy link
Contributor

ikashnitsky commented Jan 12, 2019

Sure, I agree that my concern only touches a minor group of {pinp} users.
Though, I have a feeling that APA increasingly becomes the most accepted citation style. What if I PR a similarly patched apa.bst and it is offered as the second default option?

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 12, 2019

Need to think that through because we then also have to conditionally copy apa.bst in and all that but I guess it could be done.

And from a quick glance APA does not seem all that different. Is is really worth it?

@CosmeSanchezMiguel

This comment has been minimized.

Copy link

CosmeSanchezMiguel commented Jan 14, 2019

I would like to report that the new pinp package version doesn't seem to compile the previous versions of pinp (.rmd). The log file reports an error : ' !undefined control sequence'. \1.50 \doifooter.
I have changed the 'doi' to 'doi_footer' but still does not compile the pdf file.
However, once you copy the code contained in the previous version pinp file (.rmd) into a new pinp file, it compiles the pdf file without any error. Though you have stated that 'the new version supports the old entries for backwards compatibility', could you please check if that is the case? Should I do something extra to be able to use old pinp .rmd files wihtout have to create a new file and paste the code?
Thanks for this package, it is excellent!

@eddelbuettel

This comment has been minimized.

Copy link
Owner

eddelbuettel commented Jan 14, 2019

You need to replace you pinp.cls file in your directory. If one is found no copy is made. This time we need a copy. So just delete the old pinp.cls.

@CosmeSanchezMiguel

This comment has been minimized.

Copy link

CosmeSanchezMiguel commented Jan 14, 2019

It works!! many thanks, Dirk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment