[REVIEW]: RcppCNPy: Read-Write Support for NumPy Files in R #55

Closed
whedon opened this Issue Aug 28, 2016 · 74 comments

Projects

None yet

5 participants

@whedon
Collaborator
whedon commented Aug 28, 2016 edited

Submitting author: @eddelbuettel (Dirk Eddelbuettel)
Repository: https://github.com/eddelbuettel/rcppcnpy
Version: 0.2.5
Editor: @cMadan
Reviewer: @mdnunez
Archive: 10.5281/zenodo.155066

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/6afd88394721d1f421f5a023467a8b86"><img src="http://joss.theoj.org/papers/6afd88394721d1f421f5a023467a8b86/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/6afd88394721d1f421f5a023467a8b86/status.svg)](http://joss.theoj.org/papers/6afd88394721d1f421f5a023467a8b86)

Reviewer questions

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Version: Does the release version given match the GitHub release (0.2.5)?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: Have any performance claims of the software been confirmed?

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

Paper PDF: 10.21105.joss.00055.pdf

  • Authors: Does the paper.md file include a list of authors with their affiliations?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?
@whedon whedon added the review label Aug 28, 2016
@arfon
Collaborator
arfon commented Aug 28, 2016

/ cc @openjournals/joss-reviewers - would anyone be willing to review this submission?

If you would like to review this submission then please comment on this thread so that others know you're doing a review (so as not to duplicate effort). Something as simple as :hand: I am reviewing this will suffice.

Reviewer instructions

  • Please work through the checklist at the start of this issue.
  • If you need any further guidance/clarification take a look at the reviewer guidelines here http://joss.theoj.org/about#reviewer_guidelines
  • Please make a publication recommendation at the end of your review

Any questions, please ask for help by commenting on this issue! πŸš€

@arfon arfon changed the title from Submission: RcppCNPy: Read-Write Support for NumPy Files in R to [REVIEW]: RcppCNPy: Read-Write Support for NumPy Files in R Sep 20, 2016
@arfon
Collaborator
arfon commented Sep 20, 2016

@whedon commands

@whedon
Collaborator
whedon commented Sep 20, 2016

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the reviewer of this submission
@whedon assign @username as reviewer

# List the GitHub usernames of the JOSS editors
@whedon list editors

# List of JOSS reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Open the review issue
@whedon start review

🚧 Important 🚧

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

@arfon
Collaborator
arfon commented Sep 20, 2016

@whedon list editors

@whedon
Collaborator
whedon commented Sep 20, 2016

Current JOSS editors:

@acabunoc
@arfon
@cMadan
@danielskatz
@jakevdp
@karthik
@katyhuff
@kyleniemeyer
@labarba
@mgymrek
@pjotrp
@tracykteal
@arfon
Collaborator
arfon commented Sep 20, 2016

@whedon assign @cMadan editor

@arfon
Collaborator
arfon commented Sep 20, 2016

@whedon assign @cMadan as editor

@cMadan cMadan was assigned by whedon Sep 20, 2016
@whedon
Collaborator
whedon commented Sep 20, 2016

OK, the editor is @cMadan

@mdnunez
Collaborator
mdnunez commented Sep 20, 2016

βœ‹ I am reviewing this

@cMadan
Member
cMadan commented Sep 20, 2016

@whedon assign @mdnunez as reviewer

@cMadan cMadan assigned cMadan and unassigned cMadan Sep 20, 2016
@cMadan
Member
cMadan commented Sep 20, 2016

@mdnunez I tried to assign you as a reviewer so you can modify the checklist, but that didn't work--will sort this out, but feel free to try the package

@arfon do reviewers need to be part of the joss-reviewers list to be assign-able?

@mdnunez
Collaborator
mdnunez commented Sep 20, 2016

Okay. Let me know. I'm not yet on a joss-reviewer list.

@arfon
Collaborator
arfon commented Sep 20, 2016

@whedon assign @mdnunez as reviewer

@whedon
Collaborator
whedon commented Sep 20, 2016

OK, the reviewer is @mdnunez

@arfon
Collaborator
arfon commented Sep 20, 2016

@cMadan - apologies. There was a small bug with who could assign reviewers. I've fixed this now and @mdnunez you should be set up with collaborator access on this repo so you can update the checklist (you don't need to be in the @openjournals/joss-reviewers) team any longer.

@cMadan
Member
cMadan commented Sep 20, 2016

@arfon - thanks for sorting that out!

@mdnunez - take a look at the reviewer guidelines here (http://joss.theoj.org/about). Let me know if you have any questions!

@mdnunez
Collaborator
mdnunez commented Sep 20, 2016

I will have some comments. Should I leave them here or make a rcppcnpy repo issue?

@arfon
Collaborator
arfon commented Sep 20, 2016

I will have some comments. Should I leave them here or make a rcppcnpy repo issue?

@mdnunez if they are related to the review then please leave comments here for @eddelbuettel to respond to.

If they are issues with things such as installation troubles (i.e. things that would be of interest to any user of the package) then please open issues in the https://github.com/eddelbuettel/rcppcnpy repository.

@eddelbuettel

Hi @mdnunez -- thanks for volunteering to review! I am subscribed here, so I would see them either way.

From what I gather from the other joss-reviews I have seen here, feedback in this thread seems common. But as @arfon said, any "structural" issues in the package can go to the repo as well.

In case it helps, the package also has binaries for Windows and OS X via its CRAN page and the normal CRAN installs.

Looking forward to your comments.

@mdnunez
Collaborator
mdnunez commented Sep 20, 2016

Review of RcppCNPy: Read-Write Support for NumPy Files in R

Reviewer questions

Conflict of interest

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

  • Repository: Is the source code for this software available at the repository url?

Yes, it is also available from CRAN with install.packages('rcppcnpy')

  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?

Yes, except it is GNU 2, not GNU 3 or later. Also, should GNU statement be at the top of each script?

  • Version: Does the release version given match the GitHub release (0.2.5)?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?

README does not list installation instructions, easy fix
library(rcppcnpy) did not work for me
It seems library('RcppCNPy') command is case sensitive.

  • Functionality: Have the functional claims of the software been confirmed?
  • Performance: Have any performance claims of the software been confirmed?

N/A? I did not see any performance claims in README, I found performance claims in https://cran.r-project.org/web/packages/RcppCNPy/vignettes/RcppCNPy-intro.pdf

Documentation

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

Target audience could be better stated in README and vignette

  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

I'm not sure if the authors would also like to include the list of Python dependencies since Python example code is given in the README

  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?

Functionality is better stated in vignette. README does not contain exhaustive list of functionality

  • Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?

Tests were performed, but I did not find a .md or README if that is required by this journal

  • Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Community guidelines for contributions could be better reported

Software paper

Paper PDF: 10.21105.joss.00055.pdf

  • Authors: Does the paper.md file include a list of authors with their affiliations?

Author names present in paper.md but did not compile to pdf

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

References do not have a DOI in pdf

Recommendations to editor

While the software works well and quickly, some additional documentation is required by JOSS for publication.

Recommendations to author

This reviewer would like to see reading and writing capability of .npz in the future. npyLoad of integer arrays produces incorrect values when not given the flag "integer." Multidimensional arrays should also be easily loadable. I was impressed that arrays of sizes (24,) , (1,24), and (24,1) all loaded correctly in R with the existing functionality.

Recommendations to JOSS

Reviewers should be left with the choice to be anonymous

@eddelbuettel

If the above was accessible in markdown for me, it would make my reply a little easier.

For the time being I am copying-and-pasting-and-reformatting which feel very 1990s to me...

@mdnunez
Collaborator
mdnunez commented Sep 21, 2016

review.txt

@eddelbuettel I've attached the markdown script

@eddelbuettel
eddelbuettel commented Sep 21, 2016 edited

@mdnunez Very kind, thank you -- I had already committed a cobbled-together copy (NB: URL may disappear once I merge that branch). Will probably update to yours.

@eddelbuettel
eddelbuettel commented Sep 22, 2016 edited

Reply to Review Comments on RcppCNPy: Read-Write Support for NumPy Files in R

Thank you for your very detailed review and comments.

The package and paper have been updated updated to reflect these suggestions, and the
additional documentation is very beneficial.

This reply contains two major section. The first is 'open issues' and addresess items
which were brought up as required improvements or changes. The second is 'comments' and
either responds to some minor items, or provides other clarification.

Open Issues

Functionality: Installation

R packages need to be loaded, so library("RcppCNPy") (which is case-sensitive) is indeed
required.

We do have a section in the README.md showing this as floating point (shown below) and
also integer:

R> library(RcppCNPy)
R> fmat <- npyLoad("fmat.npy")

The package contains a demo file (which can be executed via demo("timings", package="RcppCNPy")
showing this, and the vignette also shows it.

To be even more explicit we added this also to the 'Examples' sections of the help pages.

That provides four different instances showing how to load the package and perform a few
first steps which should be sufficient for users of the package.

Documentation: Automated Tests

The package already contains tests which are executed each time R CMD check runs. The
standard R mechanism of using files in the top-level directory tests/ is used.

There are two levels of tests.

Files ending in .Rout.save provide (persistent) reference output which is compared
character-by-character to freshly created output during a test. Any differences are
reported (but are not fatal).

Additionally, the file tests/loadFiles.R contains five stopifnot() tests which would
abort unless the tested condition (of comparing read content to expected content) are met
exactly. That provides stringent unit testing.

It is unclear whether an additional markdown or README file is required as the package
conforms to standard R and CRAN practice.

Documentation: Community Guidelines

The DESCRIPTION file lists the standard GitHub facilities:

BugReports: https://github.com/eddelbuettel/rcppcnpy/issues

By being on GitHub and listing the repository, the standard and well-known participation
venues offered by GitHub are available.

Following this suggestion, we made this more explicit by adding a short section to the
README.md as well.

Software Paper: References

We added the DOIs (as supplied by the publisher) for one of the two references. The
second reference does not yet have a DOI.

We also added the DOI for this submission as a 'badge' to the main README.md.

Comments

General Checks: Repository

The installation command for R is case-sensitive so the most recent released version can
be installed via `install.packages("RcppCNPy").

Should this be desired, R also allows installation directly from the GitHub repository by
supplying eddelbuettel/rcppcnpy as the (unique) 'author/repo' token. But as RcppCNPy is
a CRAN package so we prefer the more common first approach using install.packages().

General Check: License

Line 17 of DESCRIPTION clearly states License: GPL (>= 2). This exact form is described in the corresponding R manuals, and e.g. expands on the CRAN webpage for the package
to 'GPL-2 | GPL-3'. Both licenses are OSI approved, and used by R itself in the same form.

The GNU statement was already present at src/cnpyMod.cpp, the main C++ intgegration
script. Files src/cnpy.cpp and src/cnpy.h carry both a statement for the MIT license
of the underlying cnpy library by Rogers, and the a short sentence stating GPL for the
additional integration provided by this package.

The remaining source file R/cnpy.R is a one-liner so that adding a copyright header
seemed excessive.

Functionality: Performance

In general, code from a vignette can always be extracted via Stangle(). Here, we chose
to keep the result fixed and static to not endure the (still short) simulation on each
latex-compilation of the vignette. However, the package contains a demo detailing this
which can be seen via demo("timings", package="RcppCNPy"). It contains the same
benchmark. This permits each user to validate performance locally.

Documentation: Statement of Need

The package provides an input/ouput facility so that R users can read and/or write files
in the NumPy format. As such, it the target audience is not restricted to any class or
set of users, members of discipline, or methodological school.

The key focus is stated is stated in the package description, in the first full sentence
in the README.md, and in the first line of the abstract of the vignette.

Documentation: Installation Instructions

As an R package, there should be rather a limited need for the Python specifics
particularly as every NumPy version appears to provide the identical output. As such, we
have not seen any need to provide more detail.

Documentation: Functionality Documentation

As an R package, primary documentation is provided via the help pages, and then the vignette. Both detail how to use the package.

Recommendation To The Author

Adding .npz files has long been noted as possible extensions, this is also explicitly
stated in the vignette. Development of the package was needs-driven: we needed to process
certain files, and the npz was not a format we used. Pull-requests by users with the
need to process such files are therefore strongly encouraged.

The need to explicitly request integer format is a limitation of the format and the
underlying library. There appears to be no meta-data signalling the content.

Recommendation To JOSS

I second this recommendation.

(NB: This has also been committed along with the changes mentioned above; currently in a branch 'feature/joss')

@mdnunez
Collaborator
mdnunez commented Sep 22, 2016

It is in my view that @eddelbuettel has fulfilled all the requirements for publication in JOSS.

I feel the author has fulfilled the Automated Test requirement due to matching CRAN standards. However I await guidance from @cMadan to check this box as I'm not sure if JOSS requires written text markdown or README in addition to the included test script tests/loadFiles.R

I also noticed there is a minor mistake / redundancy in the example Python code in README.md:

>>> im = np.arange(3,4)
>>> im = np.arange(12).reshape(3,4)
@eddelbuettel

NB: there are actually two test scripts: tests/saveAndLoad.R, and tests/loadFiles.R.

And I removed the redundant Python line np.arrange(3.4); that was a copy-and-paste left over from the package webpage where I removed it too.

@cMadan cMadan referenced this issue in openjournals/joss Sep 22, 2016
Closed

Anonymous reviewers #174

@eddelbuettel
eddelbuettel commented Sep 22, 2016 edited

Concerning @mdnunez concern about

if JOSS requires written text markdown or README in addition to the included test script

I went back to the Author guidelines which are clearer:

Tests

Authors are strongly encouraged to include an automated test suite covering the core functionality of > their software.

Good: An automated test suite hooked up to an external service such as Travis-CI or similar
OK: Documented manual steps that can be followed to check the expected functionality of the software (e.g. a sample input file to assert behaviour)
Bad (not acceptable): No way for you the reviewer to check whether the software works

which we fully support in the package:

  • unit tests are automatically executed when the standard R CMD check .... command is running,
  • we have Travis CI connected (and all results are available) to run checks on each commit or pull request.

I hope this provides additional clarification. Thanks again for the prompt and detailed review.

@mdnunez
Collaborator
mdnunez commented Sep 22, 2016

@eddelbuettel gave convincing enough arguments to check the Automated Test box. (Btw: Are the Travis CI results linked to from the repository?)

No problem! This was overall a very pleasant review process.

@cMadan
Member
cMadan commented Sep 22, 2016

Thank you for the quick and thorough review, @mdnunez!

I have a few additional comments/suggestions before acceptance.

(1) paper.md didn't pull the author information properly (hopefully @arfon can look into that), however, the package vignette on CRAN (https://cran.r-project.org/web/packages/RcppCNPy/vignettes/RcppCNPy-intro.pdf) includes a second author, also listed in the README.md, that isn't included in the JOSS submission. They should probably also be included in paper.md.

(2) Since the package vignette in CRAN includes more information, it would be good to also reference/point-to that as well (in paper.md).

(3) README.md should have a small section for the installation instructions. I think it should include both the install.packages("RcppCNPy"), as well as install.packages("eddelbuettel/rcppcnpy"), should the CRAN package be a slightly older version than that found on GitHub.

(4) README.md should also include a sentence pointing to the tests directory. A short description of each test (in README.md or in a separate file) would also be beneficial.

(5) It would be good to cite NumPy in the paper, see https://www.scipy.org/citing.html.

@eddelbuettel
eddelbuettel commented Sep 22, 2016 edited

Briefly

(1) I will add Wush as he is a contributor to the package.
(2) Yes, that occurred to me too, and I saw other JOSS submission do it. Will do.
(3) I will add the usual two-liner about install.packages(). The delta is temporary and are the changes I made in the 24 hours in response to this very issue ticket / review, and are in a branch. (I consider CRAN to be the primary and always push releases. GitHub is my development repository. This policy has been consistent and transparent for ten+ years across two dozen packages. I do not plan to change that.)
(4) No. It is a CRAN package, and R users understand how to tickle tests. The README in the repository is not a replacement for the R manuals.
(5) I looked for such citation information, but found it wanting. If anything, the cnpy repository should be cited (and I will add this). RcppCNPy does not contain NumPy code or functionality so this is a bit of a stretch. But I will add it.

@cMadan
Member
cMadan commented Sep 22, 2016

(3) I still think it would be good to add install.packages("RcppCNPy") to the README.md, to be overtly clear how the package can be installed from CRAN. This also may be useful in preventing confusion related to the case-sensitivity (also since the GitHub repo is named in all lowercase).

(5) Despite the package not using any NumPy code, it is built as a means to read-write from the NumPy data format--that's why I think citing NumPy makes sense. I hope the rationale is more clear now.

@eddelbuettel
eddelbuettel commented Sep 22, 2016 edited

(3) Yes I agree (and I edited the above commented twice). I checked, and this package does not have the short block most my other packages have -- ie preference for install.packages(). I will add that.

(5) Yes. No point to debate this to death. Taken further, we could also cite R and Python. But NumPy is appropriate, and cnpy even more so. Any tips on citing 'unpublished' github repos?

@eddelbuettel

@arfon: Regarding paper.md and paper.pdf, I raise a white flag. I must be using the wrong pandoc invocation. I even compared carefully against the files in two already-published JOSS pieces.

I have used

$ pandoc paper.md --output paper.pdf --filter pandoc-citeproc

without additional bells and whistles. Do I need some? Pandoc is 1.16.0.2 on Ubuntu 16.04.

@cMadan
Member
cMadan commented Sep 22, 2016

For citing unpublished GitHub repos, take a look at this:
http://academia.stackexchange.com/a/14015

As for compiling paper.md, normally @arfon does this (not the author). After you commit these last changes, I'll be happy to accept your submission for publication in JOSS.

@eddelbuettel

Yes, agreed, @Misc or @Manual are good BibTeX types for that. I just updated the .bib and .pdf once more. I also added a reference for the actual package RcppCNPy.

More to come, will report back once done.

@cMadan
Member
cMadan commented Sep 23, 2016

Sounds good!

Just since I took a look, typo "DRead" in the bib. For NumPy, it would be better to cite the suggested 2011 paper (https://www.scipy.org/citing.html), rather than the SciPy website.

@eddelbuettel

Typo fixed. NumPy reference fixed; thanks for catching that. It's been a long day...

@cMadan
Member
cMadan commented Sep 23, 2016

No worries! Let me know when I should check everything over.

@eddelbuettel

I just expanded README.md as discussed, and with that should have addressed the points you made yesterday. All changes are still in the features/joss branch which I intend to merge once we have finalized the review.

@cMadan
Member
cMadan commented Sep 23, 2016

Perfect, I am happy to accept the submission in it's current form.

@arfon - for archiving the code, does CRAN count as a suitable repository or should it also be in zenodo/figshare ?

@arfon
Collaborator
arfon commented Sep 23, 2016

@arfon - for archiving the code, does CRAN count as a suitable repository or should it also be in zenodo/figshare ?

No, it also needs to be in Zenodo or figshare.

@eddelbuettel

I have resisted Zenodo so far because the 'one id per release' seems "wrong" -- for some active packages of mine (such as Rcpp or RcppArmadillo) we have six or more releases a year, every year.

Do you want me to set the Zenodo id up for release 0.2.5 that the submission was based on?

Also, I now have the orcid id of my coauthor so there will be one more commit.

@eddelbuettel

Now done in this commit.

@cMadan
Member
cMadan commented Sep 24, 2016

Thanks for adding the orcid!

I would say the Zenodo archive doi should correspond to the 'current' version. I.e., version 0.2.5, or newer if you are incrementing the version number based on the changes since the initial submission.

@eddelbuettel
eddelbuettel commented Sep 24, 2016 edited

Well I was planning to finalize this submission, review and publication (submitted as 0.2.5). I would then update the package, include the paper and metadata etc pp and call that 0.2.6.

@eddelbuettel

I created a Zenodo doi: DOI

@arfon
Collaborator
arfon commented Sep 24, 2016

@eddelbuettel thanks. Are you planning on making a final Zenodo release with all of the changes (such as including the ORCID of your coauthor) or does DOI already include these changes?

@eddelbuettel
eddelbuettel commented Sep 24, 2016 edited

DOI archives the current CRAN release 0.2.5 on which the submission was based. Wush is a co-author in the vignette included in 0.2.5 as well as in the package itself (see the CRAN page) which was used to back the submission.

I plan to release 0.2.6 once we are done here with all the changes in paper/ and other files which accumulated in the review privess, and could provide another Zenodo DOI for that.

@arfon
Collaborator
arfon commented Sep 25, 2016

I plan to release 0.2.6 once we are done here with all the changes in paper/ and other files which accumulated in the review privess, and could provide another Zenodo DOI for that.

@eddelbuettel - based on @cMadan's comment I think we're good to accept in the current state. If you want to make the 0.2.6 release and the corresponding Zenodo archive I can move forward with accepting the submission.

@arfon arfon closed this Sep 25, 2016
@arfon arfon reopened this Sep 25, 2016
@arfon arfon added the accepted label Sep 25, 2016
@cMadan
Member
cMadan commented Sep 25, 2016

Right, I do think 0.2.6 should be the accepted version, since that's the version that includes the changes made as part of the review.

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

Sounds good, and as I recall that is also how JSS does it. So what if any updated files (or information) do I need from you to finalize 0.2.6?

In other words: I am planning on rolling out 0.2.6 once we are done here. So we appear to be in a deadlock: you are waiting for me, I am waiting for you. All changes I made are in the feature/joss branch.

@arfon
Collaborator
arfon commented Sep 25, 2016

In other words: I am planning on rolling out 0.2.6 once we are done here. So we appear to be in a deadlock: you are waiting for me, I am waiting for you. All changes I made are in the feature/joss branch.

I believe you can move forward and make the 0.2.6 release now based on the feature/joss branch.

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

No -- I cannot reference a JOSS publication. As I understand I am awaiting a joss status badge change and doi from you.

@arfon
Collaborator
arfon commented Sep 25, 2016

No -- I cannot reference a JOSS publication.

Sorry, where do you need to create this reference?

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

It would be part of the post-publication release and tarball, including inter alia a file inst/CITATION providing data for R's citation() function. I need a full complete reference from you to include it.

The whole thing is weird. JSS does the same dance: submission of a version $i$, then revivions until eventual acceptance and somewhat joint publication of a new version $i+1$ and JSS publication.

I can control the CRAN side. I cannot control your end.

@arfon
Collaborator
arfon commented Sep 25, 2016

Ah OK. Well, the DOI for the paper will be http://dx.doi.org/10.21105/joss.00055 (although I've not created the DOI yet). Can you just include that?

@eddelbuettel

Yes I could. Are you saying that you want me to blink first so that you can finalize, ie you need 0.26 present?

(Also see my last round of edits to prev comment)

@arfon
Collaborator
arfon commented Sep 25, 2016

Yes I could. Are you saying that you want me to blink first so that you can finalize, ie you need 0.26 present?

πŸ˜€ that would be great. The thing I really need is the DOI pointing to the 0.2.6 release.

The whole thing is weird

I agree this is a tad strange and we're still working through some of these issues in our editorial process. Thanks for your patience :-)

@eddelbuettel

Let me give it a look over in the morning.

@arfon
Collaborator
arfon commented Sep 25, 2016

Perfect. Thanks.

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

Here is one thing I certainly need from you: volume and number, as well as a general look-over the citation info. The file inst/CITATION currently has

citHeader("To cite RcppCNPy in publications use:")

citEntry(entry = "Article",
  title        = "{RcppCNPy}: Read-Write Support for NumPy Files in R",
  author       = personList(as.person("Dirk Eddelbuettel"), as.person("Wush Wu")),
  journal      = "Journal of Open Source Software",
  publisher    = "The Open Journal",
  year         = "2016",
  volume       = "1",
  number       = "55"
  month        = "September",
  url          = "http://dx.doi.org/10.21105/joss.00055",

  textVersion  =
  paste("Dirk Eddelbuettel, Wush Wu (2016).",
        "RcppCNPy: Read-Write Support for NumPy Files in R.",
        "Journal of Open Source Software, 1(55), September 2016.",
        "URL http://dx.doi.org/10.21105/joss.00055")
)

Month, volume, issue are probably wrong. I hand-formated the text variant. do you want changes?

I had a doi entry in there, but then R chokes over it when running citation("RcppCNPy"). As shown above, it works (but may need tweaks; and I did manual line breaks below to fit the width)

R> citation("RcppCNPy")

To cite RcppCNPy in publications use:

  Dirk Eddelbuettel, Wush Wu (2016). RcppCNPy: Read-Write Support for NumPy Files in R. 
  Journal of Open Source Software, 1(55), September 2016. 
  URL http://dx.doi.org/10.21105/joss.00055

A BibTeX entry for LaTeX users is

  @Article{,
    title = {{RcppCNPy}: Read-Write Support for NumPy Files in R},
    author = {Dirk Eddelbuettel and Wush Wu},
    journal = {Journal of Open Source Software},
    publisher = {The Open Journal},
    year = {2016},
    volume = {1},
    number = {55},
    month = {September},
    url = {http://dx.doi.org/10.21105/joss.00055},
  }

R> 
@eddelbuettel

Last but not least you would have to turn the publication on so that I can refer to it:

DOI Not Found


10.21105/joss.00055


This DOI cannot be found in the DOI System. Possible reasons are:

The DOI is incorrect in your source. Search for the item by name, title, or other metadata using a search engine.
The DOI was copied incorrectly. Check to see that the string includes all the characters before and after the slash and no sentence punctuation marks.
The DOI has not been activated yet. Please try again later, and report the problem if the error continues.
@eddelbuettel

Which would block CRAN admission:

edd@max:~/git$ RD CMD check --as-cran RcppCNPy_0.2.6.tar.gz      ## RD launches R-devel 
* using log directory β€˜/home/edd/git/RcppCNPy.Rcheck’
* using R Under development (unstable) (2016-09-20 r71314)
* using platform: x86_64-pc-linux-gnu (64-bit)
* using session charset: UTF-8
* using option β€˜--as-cran’
* checking for file β€˜RcppCNPy/DESCRIPTION’ ... OK
* checking extension type ... Package
* this is package β€˜RcppCNPy’ version β€˜0.2.6’
* checking CRAN incoming feasibility ... NOTE
Maintainer: β€˜Dirk Eddelbuettel <edd@debian.org>’

Found the following (possibly) invalid URLs:
  URL: http://dx.doi.org/10.21105/joss.00055
    From: inst/CITATION
    Status: 404
    Message: Not Found
* checking package namespace information ... OK
* checking package dependencies ... OK
[....]
@arfon
Collaborator
arfon commented Sep 25, 2016 edited

OK, the DOI is registered now: http://dx.doi.org/10.21105/joss.00055 but it won't resolve to the JOSS site properly yet as we've not finished the submission here.

Quoting from the submitted Crossref metadata:

      <journal_issue>
        <publication_date media_type="online">
          <month>09</month>
          <year>2016</year>
        </publication_date>
        <journal_volume>
          <volume>1</volume>
        </journal_volume>
        <issue>5</issue>
      </journal_issue>

Also, please merge this PR into your changes before making the 0.2.6 release. The current paper metadata is malformed.

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

Well, close, but no cigar:

[...]
* this is package β€˜RcppCNPy’ version β€˜0.2.6’
* checking CRAN incoming feasibility ... NOTE
Maintainer: β€˜Dirk Eddelbuettel <edd@debian.org>’

Found the following (possibly) invalid URLs:
  URL: http://dx.doi.org/10.21105/joss.00055
    From: inst/CITATION
    Status: 500
    Message: Internal Server Error
* checking package namespace information ... OK
* checking package dependencies ... OK
[...]
@eddelbuettel

Volume and issue updated in this commit to branch and now yields:

R> citation("RcppCNPy")

To cite RcppCNPy in publications use:

  Dirk Eddelbuettel, Wush Wu (2016). RcppCNPy: Read-Write Support for NumPy Files in R. 
  Journal of Open Source Software, 1(5), September 2016. 
  URL http://dx.doi.org/10.21105/joss.00055

A BibTeX entry for LaTeX users is

  @Article{,
    title = {{RcppCNPy}: Read-Write Support for NumPy Files in R},
    author = {Dirk Eddelbuettel and Wush Wu},
    journal = {Journal of Open Source Software},
    publisher = {The Open Journal},
    year = {2016},
    volume = {1},
    issue = {5},
    month = {09},
    url = {http://dx.doi.org/10.21105/joss.00055},
  }

R> 
@arfon
Collaborator
arfon commented Sep 25, 2016

Well, close, but no cigar:

Can you produce the archive (Zenodo/figshare) of the 0.2.6 release before uploading to CRAN? That way we won't run into this:

Found the following (possibly) invalid URLs:
  URL: http://dx.doi.org/10.21105/joss.00055
    From: inst/CITATION
    Status: 500
    Message: Internal Server Error
* checking package namespace information ... OK
* checking package dependencies ... OK
@arfon
Collaborator
arfon commented Sep 25, 2016

Also, please merge this PR into your changes before making the 0.2.6 release. The current paper metadata is malformed.

Also, eddelbuettel/rcppcnpy#10 isn't required anymore. It looks like you fixed this in your feature/joss branch.

@eddelbuettel
eddelbuettel commented Sep 25, 2016 edited

Not really. I could prepare a candidate tarball, but CRAN is at least as picky as you, and there may be revisions leading to changes in the tarball invalidating the Zenodo doi. So I would prefer to wait for the CRAN acceptance of 0.2.6, and that is where we get circular.

The easiest, really, would be to point to the exiting Zenodo doi for 0.2.5. Version 0.2.6 will go out -- the existing PR is alive and well -- as will (in all likelihood) versions 0.2.7, 0.2.8 and so on. So there is always a delta. Why not seal things now?

@arfon
Collaborator
arfon commented Sep 25, 2016

Ok I give up. Let's do that.

If ever there was a case for versioned DOIs this is it. πŸ˜‚

@eddelbuettel

Ok I give up. Let's do that.

Woot!

If ever there was a case for versioned DOIs this is it. πŸ˜‚

I have been thinking about this too. There is a conflict between software changing and updating as it is never final -- so a DOI for the master branch or its HEAD ? -- as it would not make real sense to create, ie, six DOIs a year for the six Rcpp releases I make. Yet the one literature reference (the original JSS paper) is really stale. I'd love a citable reference for 'current Rcpp' (or any other changing software package). Not clear. And of course in conflict with the idea of "perfectly reproducible research" and snapshots (but that is a whole other discussion as there are some issues too...

Off for a run now, at last. Let me mull this over one more time. But thanks for moving on along...

@arfon
Collaborator
arfon commented Sep 25, 2016

but CRAN is at least as picky as you

Also, for what it's worth, I don't think it's unreasonable (or that picky) to ask authors to prepare an archive of the software that includes the changes as a result of the JOSS review but this is clearly a problem for folks (like yourself) who are submitting to CRAN.

/ cc @karthik for any ideas how to make this easier in the future.

@arfon
Collaborator
arfon commented Sep 25, 2016

Off for a run now, at last. Let me mull this over one more time. But thanks for moving on along...

OK thanks. I'll move forward with accepting.

@arfon
Collaborator
arfon commented Sep 25, 2016

@mdnunez - many thanks for the review!

@eddelbuettel - your paper is now accepted into JOSS. Your DOI is http://dx.doi.org/10.21105/joss.00055 πŸŽ‰ πŸš€ πŸ’₯

@arfon arfon closed this Sep 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment