openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: rTASSEL: An R interface to TASSEL for analyzing genomic diversity #4530

Closed editorialbot closed 2 years ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@btmonier<!--end-author-handle-- (Brandon Monier) Repository: https://github.com/maize-genetics/rTASSEL Branch with paper.md (empty if default branch): joss-paper Version: v0.9.27 Editor: !--editor-->@fboehm<!--end-editor-- Reviewers: @tkchafin, @tomsing1 Archive: 10.5281/zenodo.6977430

Status

status

Status badge code:

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

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@tkchafin & @tomsing1, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @fboehm know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @tomsing1

📝 Checklist for @tkchafin

editorialbot commented 2 years ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.04 s (1147.0 files/s, 204172.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               39            905           1784           3474
Rmd                              3            425            969            333
Markdown                         3             83              0            318
TeX                              1             14              0            213
YAML                             2              4              0             22
-------------------------------------------------------------------------------
SUM:                            48           1431           2753           4360
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 1260

editorialbot commented 2 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1093/bioinformatics/btm308 is OK
- 10.1093/bib/bbp050 is OK
- 10.1186/gb-2004-5-10-r80 is OK
- 10.1371/journal.pcbi.1003118 is OK
- 10.1038/ng1702 is OK
- 10.1093/bioinformatics/bts163 is OK
- 10.1534/g3.112.004259 is OK
- 10.1016/j.ajhg.2010.11.011 is OK
- 10.1534/genetics.114.171322 is OK
- 10.1016/j.ajhg.2015.01.001 is OK
- 10.1093/bioinformatics/bty633 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

fboehm commented 2 years ago

@tkchafin and @tomsing1 - please see above for instructions on generating your checklists. Once you have generated them, please check off the items once you believe that the criteria are met. For those items that you don't immediately check off the checklist, please comment here about why you aren't able to check them. The authors will then make needed revisions. Please mention me (@fboehm) whenever questions arise. Thanks again!

tomsing1 commented 2 years ago

Review checklist for @tomsing1

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

tomsing1 commented 2 years ago

Opened issue https://github.com/maize-genetics/rTASSEL/issues/3

tomsing1 commented 2 years ago

Opened issue https://github.com/maize-genetics/rTASSEL/issues/4

tomsing1 commented 2 years ago

Opened issue https://github.com/maize-genetics/rTASSEL/issues/5

tomsing1 commented 2 years ago

@fboehm : The R package includes a vignette rtassel_benchmarks.Rmd that reports the results of an extensive benchmarking exercise of rTASSEL's performance. I have neither the code, data nor the computational resources to reproduce these benchmarks.

Nevertheless, I think the value of this manuscript is clear, e.g. it is not (just) based on performance but by offering R users a robust way to interact with TASSEL. Hence I would like to tick the Performance box, taking the authors at their word. Any advice?

fboehm commented 2 years ago

hi, @tomsing1 - Thanks for such a thorough and timely review! It's fine that you don't have the resources to reproduce the benchmarking. Please do add your checkmark to the Performance box. Thanks again!

fboehm commented 2 years ago

@btmonier - you may wish to address the comments and issues opened by @tomsing1. If you have any questions about them, please feel free to ask them here.

btmonier commented 2 years ago

Thanks for the prompt review @tomsing1! Quick question for @fboehm - for correcting issues raised (in this case adding a contribution section and fleshing out the installation components), would I push those changes to the master branch of the rTASSEL repository or add this to a new branch. Thanks again.

fboehm commented 2 years ago

@btmonier - Thanks for your question. It's fine to add the changes to your master branch; you don't need to make a new branch. Also, adding the changes to the master branch will make it easier for the second reviewer to find them.

tkchafin commented 2 years ago

Review checklist for @tkchafin

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

tkchafin commented 2 years ago

@fboehm : I've completed my review. The submitted R package and accompanying manuscript are valuable contributions to the field, and adds considerable value (e.g., through integrated plotting and interaction with other R packages) beyond simply wrapping the TASSEL toolkit.

In addition to the issues pointed out by @tomsing1 (i.e., installation and contributing docs), I would suggest that the authors consider creating a containerized option, as I had to use some workarounds to get everything working on an M1 Mac. This would increase portability and save some headache for some users... But just a suggestion (I wouldn't consider this critical). I also suggested some very minor editorial changes, but again nothing critical.

Great job to the authors, and after fixing the minor issues which have been brought up in this review I would say that this has my full support for publication!

btmonier commented 2 years ago

@tomsing1, @tkchafin - thank you very much for your reviews and constructive comments! I am meeting with my collaborators next week to go over these issues. Most of these should be quickly resolved by the end of next week.

fboehm commented 2 years ago

@btmonier - How are the revisions coming along? Is there anything that I might help with?

Thanks!

btmonier commented 2 years ago

Opened issue maize-genetics/rTASSEL#3

@tomsing1 - I have added new installation instructions found in the following document:

I have also created a new installation vignette where I also cover this:

btmonier commented 2 years ago

Opened issue maize-genetics/rTASSEL#4

@tomsing1 - I have updated the pkgdown build process in which I have:

Please see the _pkgdown.yml file for further details.

btmonier commented 2 years ago

Opened issue maize-genetics/rTASSEL#5

@tomsing1 - I have added the following documentation to help with contribution and also a code of conduct guide as laid out by the Contributor Covenant Code of Conduct.

Please see the following files for further details:

I have also added links to these files on the right side bar of our website.

btmonier commented 2 years ago

maize-genetics/rTASSEL#8

@tkchafin - I have created a containerized version of rTASSEL based off the rocker/verse Docker image.

Please see the following file for further details:

btmonier commented 2 years ago

maize-genetics/rTASSEL#7

@tkchafin - I have revised the manuscript based on the comments you have given us.

@editorialbot generate pdf

tkchafin commented 2 years ago

All of my suggestions have been dealt with -- updated checklist to complete.

btmonier commented 2 years ago

maize-genetics/rTASSEL#6

@tomsing1 - We have identified the log4j issue and submitted a fix which will be released in the next TASSEL standalone build (2022-08-04). Once this goes into production, we will update the JAR files for the deployment of rTASSEL. Thanks!

btmonier commented 2 years ago

@fboehm - I have submitted my revisions with the exception of the log4j issue raised by @tomsing1. Please see prior comments for further information.

fboehm commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

fboehm commented 2 years ago

Thanks so much, @btmonier ! And thanks to @tkchafin, too, for the thorough review. @tkchafin, now that your review checklist is all checks, your review is completed. Thanks so much!

tomsing1 commented 2 years ago

Thank you for making the changes @btmonier ! That completes my checklist, @fboehm, from my perspective, the paper is ready to go!

fboehm commented 2 years ago

Great! Thanks, @tomsing1!

@btmonier - the reviewers have recommended your submission for publication. The next step is for me to proofread the paper. I'll get to this in the next few days. Thanks again!

fboehm commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

fboehm commented 2 years ago

@btmonier - the pdf looks really good - I have no corrections to offer.

fboehm commented 2 years ago

@editorialbot check references

editorialbot commented 2 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1093/bioinformatics/btm308 is OK
- 10.1093/bib/bbp050 is OK
- 10.1186/gb-2004-5-10-r80 is OK
- 10.1371/journal.pcbi.1003118 is OK
- 10.1038/ng1702 is OK
- 10.1093/bioinformatics/bts163 is OK
- 10.1534/g3.112.004259 is OK
- 10.1016/j.ajhg.2010.11.011 is OK
- 10.1534/genetics.114.171322 is OK
- 10.1016/j.ajhg.2015.01.001 is OK
- 10.1093/bioinformatics/bty633 is OK

MISSING DOIs

- None

INVALID DOIs

- None
fboehm commented 2 years ago

@btmonier - the next steps are for you to make a new release of rTASSEL and to deposit that new release as an archive, for example, with zenodo.org. Please report here the new release's version number and the doi for the archive. Thank you!

fboehm commented 2 years ago

Please let me know if you have any questions about these tasks.

btmonier commented 2 years ago

@fboehm - I have pushed a new update which is v0.9.27 and have created an archive on Zenodo with the following DOI:

https://doi.org/10.5281/zenodo.6977430

fboehm commented 2 years ago

@btmonier - Thanks! I should have specified that the archive must have the same authors list as the pdf of your manuscript. It looks like the archive currently is missing middle initials for the last three authors. Can you add these initials to the author names in the archive?

btmonier commented 2 years ago

@fboehm - I have updated the author list to include middle initials.

fboehm commented 2 years ago

Thanks, @btmonier! Can you also fix the title of the archive? It should be the same as the title of the manuscript. THanks again!

btmonier commented 2 years ago

@fboehm - Sorry about that. I have updated the title as well.

fboehm commented 2 years ago

Thanks so much, @btmonier !

fboehm commented 2 years ago

@editorialbot set 10.5281/zenodo.6977430 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.6977430

fboehm commented 2 years ago

@editorialbot set v0.9.27 as version

editorialbot commented 2 years ago

Done! version is now v0.9.27

fboehm commented 2 years ago

@editorialbot recommend-accept