[REVIEW]: corner.py: Scatterplot matrices in Python

whedon commented 8 years ago

Submitting author: @dfm (Daniel Foreman-Mackey) Repository: https://github.com/dfm/corner.py Version: v2.0.0 Editor: @arfon Reviewer: @tacaswell
Archive: http://dx.doi.org/10.5281/zenodo.53155

Status

Status badge code:

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

Reviewer questions

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Version: Does the release version given match the GitHub release (v2.0.0)?
[x] Archive: Does the software archive resolve?
Functionality
[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: Have the performance claims of the software been confirmed?
Documentation
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the functionality of the software documented to a satisfactory level (e.g. API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
[x] 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.00024.pdf

[x] Authors: Does the paper.md file include a list of authors with their affiliations?
[x] 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)?

arfon commented 8 years ago

/ 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! 🚀

tacaswell commented 8 years ago

:hand: I will take a look at this over the weekend

tacaswell commented 8 years ago

I actually have 1 commit into this project (completely forgot about that until my name showed up in the author list on zenodo). It is an almost trivial change: https://github.com/dfm/corner.py/commit/8b87b92739974ce48f84f0b43e2c208472b8035f

I suspect that I am being pedantic about conflict of interest here, but is this a problem?

arfon commented 8 years ago

Thanks for bringing this to my attention. Given that the commit is very small I think it's ok to proceed with the review.

genomematt commented 8 years ago

@arfon I think for COI we should say bug fix patches are ok (need to be declared) but not feature additions. This is clearly a bug fix. With reviewers who are active and projects that are useful bug fixes may be common, and the last thing we want is to inhibit our reviewers from fixing bugs.

arfon commented 8 years ago

@genomematt - I've added a small clarification here: https://github.com/openjournals/joss/commit/dad06058f66a9681508c907fb44b27f43e37c2c9

Perhaps we should add something to the reviewer guidelines too?

tacaswell commented 8 years ago

@dfm quantile is exported in the __all__, but is not in the built docs nor is it's docstring numpydoc format. Was this intended to be part of the public API?

This paper makes no performance claims, how does that get handled?

The mpl paper has a DOI http://dx.doi.org/10.1109/MCSE.2007.55

The citation to the ADS search results seems strange to me, I think it is ok to assert an 'also known as' with out citation.

tacaswell commented 8 years ago

I recommend publication on revision of my minor comments.

arfon commented 8 years ago

This paper makes no performance claims, how does that get handled?

In that case you can just check the box.

dfm commented 8 years ago

@tacaswell: Thanks for the suggestions. I've added some tests and documentation for the quantiles function. I added the DOI for the matplotlib paper and removed the ADS citation.

Thanks!

arfon commented 8 years ago

Thanks for the review @tacaswell 🎉 🚀 :boom:

@dfm your DOI will be http://dx.doi.org/10.21105/joss.00024 once the Crossref queue is processed (looks a little backed up right now).

dfm commented 8 years ago

@arfon: what are your thoughts on submitting a paper from joss to arxiv? On Wed, Jun 8, 2016 at 12:05 PM Arfon Smith notifications@github.com wrote:

Closed #24 https://github.com/openjournals/joss-reviews/issues/24.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/24#event-686027361, or mute the thread https://github.com/notifications/unsubscribe/AAVYSsARCgrBHONi_QdMMZ1cgzsml0Beks5qJugWgaJpZM4IoH4j .

arfon commented 8 years ago

@arfon: what are your thoughts on submitting a paper from joss to arxiv?

Fine by me. My only request would be that you update the arXiv metadata to point to the JOSS DOI.

dfm commented 8 years ago

Done. One request for the future: I had to download the pandoc latex template and generate the .tex file to submit to arxiv (they spot PDFs that were generated by LaTeX) so it would be great to make the tex available somewhere!

danielskatz commented 8 years ago

I notice that "References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?" never got checked in the review. There are references listed, and while one has a DOI, the Matplotlib paper also should (specifically, 10.1109/MCSE.2007.55). I wonder if we have a process problem here? It seems like the review shouldn't be finalized and the paper shouldn't be published until all the boxes are checked. Also for the third reference, I don't think Astrophysics Data System is a proper name that should be written as "System, Astrophysics Data".

This is based on what I see right now at http://joss.theoj.org/papers/10.21105/joss.00024 as well as in the PDF at https://github.com/openjournals/joss-papers/blob/master/joss.00024/10.21105.joss.00024.pdf . I also notice that these doesn't seem to match the comments above, which seem to indicate that there may be later version that already has these fixes.

dfm commented 8 years ago

These changes were made here: https://github.com/dfm/corner.py/blob/master/paper/paper.bib ... I'm not sure if there was something else I needed to do.

danielskatz commented 8 years ago

It looks like there was a paper regeneration step that didn't get done. @arfon ?

arfon commented 8 years ago

It looks like there was a paper regeneration step that didn't get done. @arfon ?

Darn it, yes, looks like human error (on my part). Will update.

dfm commented 8 years ago

It looks like you have updated it now. Should we close this issue again or are there outstanding problems?

danielskatz commented 8 years ago

agreed

openjournals / joss-reviews