[REVIEW]: Representation and Manipulation of Genomic Tuples in R about joss-reviews HOT 23 CLOSED

Woohoo 🎉 Thanks, @arfon, @amoeba, and @ctb. Great experience, 10/10 would do again

from joss-reviews.

from joss-reviews.

@PeteHaitch http://dx.doi.org/10.21105/joss.00020 🎉 🚀 💥

Thanks for the review @amoeba & @ctb

from joss-reviews.

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

from joss-reviews.

✋ I am reviewing this.

I'll do this tonight (after 8pm pst). if anyone is eager to do a review and wants to take this on before I get to it, please do.

from joss-reviews.

Interesting package. I'm completely out of my depth on the genetics part BioConductor part but I found the software easy to install and was able to run the code in the vignette. I was impressed by the detailed vignette included with the package (realizing its based on a previous vignette). My suggestions are essentially to improve the base README.md which should require copying and possibly slightly modifying existing prose from other sources such as the paper and vignette.

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

  Not in the README. It's unclear from either the repo subtitle or the README what the software does. If I visit the linked Bioconductor page for the stable release of the package, I can learn more. The paper also includes this information.

  Suggest making the README in this repository look something like the text at http://bioconductor.org/packages/release/bioc/html/GenomicTuples.html or include some of the text from the paper.

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

  No claims made. I checked this as complete.

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

  No claims made. I checked this as complete.

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

  No (see above).

  Suggest: Expand the README with this information.

• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

  Not in the README in the repo root. After a quick scan I don't use @example blocks in the source. I do, however, see a very detailed vignette, which is a standard way for R packages to provide detailed documentation for their package. I would say this counts but I would suggest the README be elaborated upon to include examples.

  Suggest: Including examples in the README, potentially taking from the vignette.

• 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

No on 1+2+3.

Suggest: Edit the README to include community guidelines.

Grade: (2) Minor revisions.

Please comment in this Issue if any of my comments are confusing or if you'd like to discuss.

from joss-reviews.

👍 thanks for the rapid review @amoeba.

I'm completely out of my depth on the genetics part BioConductor

@amoeba - do you think we should seek a second reviewer here to evaluate the domain-specific aspects of the code?

@PeteHaitch - would you mind taking a look at @amoeba's comments and addressing them?

from joss-reviews.

Indeed, thanks for the speedy review, @amoeba! I aim to address these comments tomorrow morning.
Briefly, however, it seems clear that JOSS would like to encourage developer's to include much of the detail in the README. I'm not against this, but for R packages, and Bioconductor packages in particular, much of this information is in the examples in the .Rd documentation files and the vignettes. I'm wondering whether it's worth developing some standard language to indicate this, as I see it being a common occurrence with R packages. This might avoid some redundancy and the risk of the README falling out-of-sync with the documentation proper.

from joss-reviews.

I think the solution is simple: explain in the README this convention, including links to the appropriate locations of examples or other documentation files.

from joss-reviews.

@arfon I think a second reviewer could be good given that I can't fully verify the software does what it sets out to do. Asking another reviewer to simply check out the functional aspects of the code would be great.

from joss-reviews.

@arfon: BTW, the package includes many examples beyond the vignette (in the .Rd files). However, I haven't used roxygen2 to create the .Rd files, hence there are no @example tags in the .R files

from joss-reviews.

@labarba That sounds good to me, I'll do that in my amended submission.

from joss-reviews.

Thanks again for the review, @amoeba. I've updated the README as follows:

• Statement of need: Included intro paragraph to explain that the package is for storing and manipulating tuples of genomic loci (also added to repo subtitle). Explained that GenomicTuples is intended to extend the functionality of the popular GenomicRanges package for use with tuples rather than ranges of genomic loci
• Example usage: I have included a quick demo in the README, a link to the rendered vignette, and instructions on how to access the extensive documentation
• Community guidelines: Added community guidelines for (1) contributing to the software, (2) reporting issues or problems, and (3) seeking support

I certainly feel your review has substantially improved the README and I believe I have addressed your concerns. I also updated the paper very slightly to improve the language

from joss-reviews.

This looks perfect. I ran through the install instructions and example code in the updated README.md and checked the URLs. It all looks great! Thanks for taking the time to consider my edits. You did a nice job addressing my suggestions.

I have checked off the remaining items in the review checklist.

Grade (1): Accept

@arfon Given that edits to the source code were made after submission, does this need a new release, archive, and corresponding DOI?

Edit: I also re-read the paper and it still meets the requirements of submission.

from joss-reviews.

@PeteHaitch - could you suggest a second reviewer to take a look at the library functionality that @amoeba wasn't comfortable reviewing?

from joss-reviews.

@ctb: That'd be great if you could take a look. I'm quite confident you'll be able to assess it and I don't think any super R expertise is required. If needs be, I can probably come up with some reviewers from the Bioconductor community.

from joss-reviews.

LGTM! I made a small pull request and would recommend that you run spellcheck on the documentation, but overall looks very complete. thx, nice work!

from joss-reviews.

Thanks, Titus. I'll run a spell check over it tomorrow and report back here. Hopefully we'll then be good to go.

from joss-reviews.

Typos fixed and pushed
@arfon Do I need to mint a new DOI? Anything else I need do?

from joss-reviews.

@PeteHaitch - yeah, if you've made changes then minting a new DOI would be great. Feel free to just drop the URL to the new DOI in this thread and I'll do the updates.

from joss-reviews.

@arfon I made a new release but I've gotten myself confused on the DOI and badge. Can you please help?

The README currently uses:

[![DOI](https://zenodo.org/badge/22085/PeteHaitch/GenomicTuples.svg)](https://zenodo.org/badge/latestdoi/22085/PeteHaitch/GenomicTuples)

to display the "latest" DOI badge and link. That image currently shows 10.5281/zenodo.51498 but when clicked the link resolves to the latest/updated/current DOI (10.5281/zenodo.53186). So I guess 2 questions:

1. Is there a lag in the badge being updated to display the latest doi?
2. Should I be using the latest doi or directly linking to to a specific DOI in my README?
   Thanks

from joss-reviews.

1. Is there a lag in the badge being updated to display the latest doi?

Not sure sorry. Zenodo is a third party service.

1. Should I be using the latest doi or directly linking to to a specific DOI in my README?

I think either is fine. The latest might make sense for people who stumble upon your repo? I'm mostly concerned that we have the correct DOI associated with this paper which is http://dx.doi.org/10.5281/zenodo.53186 right?

from joss-reviews.

I'm mostly concerned that we have the correct DOI associated with this paper which is http://dx.doi.org/10.5281/zenodo.53186 right?

Yep, that's the one. Thanks!

from joss-reviews.