GeoBosh / Rdpack

R package Rdpack provides functions and macros facilitating writing and management of R documentation.
https://geobosh.github.io/Rdpack/
28 stars 6 forks source link

Using `\doi` in place of `\url` #24

Closed ms609 closed 4 months ago

ms609 commented 2 years ago

I'm rendering documentation in a CRAN package, using the \insertAllCited{} to generate a bibliography.

When running devtools::check_win_devel(), CRAN's pre-submission checks return a note:

Found the following URLs which should use \doi (with the DOI name only): File 'SomeFile.Rd': https://doi.org/10.2307/some-doi

It looks like Rdpack is generating a \url field where a \doi would do. Is this something that can be modified in the package?

GeoBosh commented 2 years ago

Thanks for the report. Could you give the bibentry that causes this? I get this message sometimes but it invariably turns out that I have put the URL in the bib. Rdpack doesn't expand \doi.

Another possibility is that \doi has changed in recent R-devel to expand at install time (previously at build time). I noticed this in a discussion today on R-devel. In that case, if the package is not build with R-devel, the doi would be in the partially built database in the tarball. If that is the case, building the package with R-devel should help. I don't believe that this is the cause though.

GeoBosh commented 2 years ago

I updated my R-devel and run the checks on some of my packages. if they are not build with R-devel I see complaints like the above about doi's. This is eliminated when I build the packages with R-devel.

That said, doi's in a couple of bib entries still cause trouble:

 https://doi.org/10.1016/S0304-4149(96)00104-4
 https://doi.org/10.1016/S0024-3795(01)00475-X

These entries from the bib file in package pcts differ from the rest in that they contain parentheses and end in dash something' but I don't see immediately how this causes different behaviour from the rest. But I did some time ago include also url for doi's so there maybe I haven't removed that entirely.

ms609 commented 2 years ago

Thanks for looking at this. I don't see the messages locally (on R4.1.2, Win 10), but CRAN reports them with R-devel. (Possibly CRAN use a stricter environment variable to activate the check?)

I don't think that URL entries in the Bibfile are the issue; nor are brackets to blame.

CRAN notes (Flavor: r-devel-windows-x86_64; r-devel-linux-x86_64-debian-gcc):

    File 'ArtificialExtinction.Rd':
      https://doi.org/10.1093/sysbio/syab072
    File 'CharacterInformation.Rd':
      https://doi.org/10.1080/10635150118627
    File 'CladisticInfo.Rd':
      https://doi.org/10.1111/j.1096-0031.1992.tb00054.x
      https://doi.org/10.1016/0025-5564(82)90112-2
      https://doi.org/10.1007/978-3-642-72253-0
    File 'Consensus.Rd':
      https://doi.org/10.1007/BF01908061
    File 'Lobo.data.Rd':
      https://doi.org/10.1098/rsbl.2016.0492
    File 'MSTEdges.Rd':
      https://doi.org/10.2307/2346439
    File 'N1Spr.Rd':
      https://doi.org/10.1007/s00026-001-8006-8
    File 'NRooted.Rd':
      https://doi.org/10.1137/0403005
      https://doi.org/10.1111/j.1558-5646.1967.tb03411.x
    File 'ReadCharacters.Rd':
      https://doi.org/10.1093/sysbio/46.4.590
    File 'RoguePlot.Rd':
      https://doi.org/10.1371/journal.pone.0212942
      https://doi.org/10.1093/sysbio/syab099
    File 'Stemwardness.Rd':
      https://doi.org/10.1093/sysbio/syab072
    File 'TotalCopheneticIndex.Rd':
      https://doi.org/10.1016/j.mbs.2012.10.005
    File 'TreeNumber.Rd':
      https://doi.org/10.1007/3-540-61332-3_168
    File 'UnrootedTreesMatchingSplit.Rd':
      https://doi.org/10.1137/0403005
GeoBosh commented 2 years ago

From the link I see that the above is from package TreeTools.

Running R-devel checks on the tarball of TreeTools on CRAN and on a local build of the current github version with a released version of R results in a lot of complaints about doi's. However, building with R-devel eliminates all but that for the following doi:

https://doi.org/10.1016/0025-5564(82)90112-2

This confirms the observation from my previous comment regarding the change in R-devel of when the \doi macro is expanded.

The doi that remains problematic has the same format as the ones in my comment above. I need to investigate this.

For packages already on CRAN this check seems to not have been activated (I looked up the CRAN results for several packages there). When you said that you got the above from CRAN was it from a submission of a new version?

ms609 commented 2 years ago

Yes, sorry not to have been clear - this is for the submission of the 1.7.0 version (containing the linked bibfile). It remains to be seen whether the CRAN team see the warnings as the basis for rejecting the package, but based on your detective work with R-devel it sounds like this might just be a transitory issue with R devel.

GeoBosh commented 2 years ago

No problem, I guessed. Thanks a lot for reporting this and let me know if the package gets rejected because of this issue.

ms609 commented 2 years ago

Alas, the package has indeed been rejected – I explained the situation but received the message:

I guess Rdpack provides DOI markup that you can use? If not, do not use Rdpack?

GeoBosh commented 2 years ago

Thanks for letting me know. So, I need to fix this quickly.

Rdpack uses the \doi from R, so "there is markup for \doi", the most compatible one possible. Frankly, the response from the CRAN is misleading, especially given that it relates to a feature that is very fresh and not necessarily ironed out. I will check again but these warnings seem to be issued for \doi's whenever a package is not compiled with he current version of R-devel.

ms609 commented 2 years ago

What I find strange is that I've never witnessed these warnings before, despite using Rdpack for a number of years – unless the change could be related to my replacing \insertRef{} with \insertCite{} & \insertAllCited{}?

GeoBosh commented 2 years ago

You probably missed my comment about recent changes in R-devel. :) This change is new in R-devel. They even have not been activated it for the checks on packages already on CRAN since otherwise all most of them would have warnings from R-devel

If you build your package with R-devel (I did it) then only one doi remains problematic (that one may be because of Rdpack).

GeoBosh commented 2 years ago

Rdpack 2.2 (now on CRAN) fixes this issue. It shouldn't matter which version of R is used to build a package, as long as the developer builds their package with Rdpack 2.2 installed. It seems prudent to require Rdpack >= 2.2 in DESCRIPTION.

I will leave the issue open to make it somewhat easier to find and for potential problems that the fix may cause.

ms609 commented 2 years ago

Confirmed - CRAN checks now pass when Rdpack >= 2.2 is required. Thanks for the quick fix!

GeoBosh commented 2 years ago

It seems that I was too quick to publish the fix. At the same time I reported a bug(let) in R-devel and they fixed it (https://stat.ethz.ch/pipermail/r-devel/2022-March/081570.html) but now if the manual is not build with R-devel, the doi's are not expanded.

GeoBosh commented 2 years ago

I reverted the change in Rdpack v2.2, since after the above mentioned fix in R-devel it is not necessary and may not render the doi's. It looks fine but I see other irregularities and will not hurry to update on CRAN in case R-devel makes further changes.

GeoBosh commented 2 years ago

Rdpack v2.3 is on CRAN and reverts the changes in v2.2. As long as a package is not built with v2.2 it should be fine on CRAN but it is advisable to require Rdpack >= 2.3 to eliminate the possibility that a user may install your package on a system with Rdpack 2.2 (in which case the doi's would remain not rendered in the documentation), admittedly a remote possibility since Rdpack v2.2 was on CRAN for several days only.

MLopez-Ibanez commented 4 months ago

You can close an issue but still pin it. This way it will be visible but not count as open. See https://github.com/MLopez-Ibanez/irace/issues