search

openjournals / joss-reviews

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

[REVIEW]: adaptest: Data-Adaptive Statistics for High-Dimensional Testing in R #161

Closed whedon closed 6 years ago

whedon commented 7 years ago

Submitting author: @wilsoncai1992 (Weixin Cai) Repository: https://github.com/wilsoncai1992/adaptest Version: 1.1.0 Editor: @karthik Reviewer: @kellieotto Archive: 10.5281/zenodo.1466019

Status

status

Status badge code:

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

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) 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 questions

Conflict of interest

General checks

Functionality

Documentation

Software paper

whedon commented 7 years ago

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @gavinsimpson it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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

@whedon commands
arfon commented 7 years ago

👋 friendly reminder on this review @gavinsimpson.

gavinsimpson commented 7 years ago

If you intend to submit this to CRAN, change the Title field to read:

Data-Adaptive Test Statistics for Multiple Testing in High Dimensions

i.e. in what CRAN calls Title Case.

gavinsimpson commented 7 years ago

At this point, the main issue preventing finalisation of this review is the general lack of examples and documentation that I can run to explore the functionality of the package.

I have filed a number of issues for the outstanding elements of the review check list. Once examples have been added to user-facing functions and the Vignette the authors indicate they are writing, I can tackle the final outstanding review topic of Functionality.

Please ping me when the filed issues have been addressed or responded to.

karthik commented 7 years ago

@wilsoncai1992 Checking in. Do you need any help from us to move this along?

nhejazi commented 7 years ago

We've had a bit of a delay in working out the requested changes, but will be working towards having them done by the end of the month. @karthik Thanks for the offer of help -- we'll update on this thread when the changes have all been made.

karthik commented 6 years ago

@nhejazi Just checking in and adding a paused tag this you are ready to proceed.

nhejazi commented 6 years ago

@karthik Thanks much for checking in. After the initial phase of this review, we began making extensive changes to the software package, both to address review items and for reasons related to the scope of the large research project of which this package is a part. The package has been renamed adaptest, and work on the revised version is nearly complete. Our intention is to complete the outstanding review items, submit the package for review to the Bioconductor project, and to continue with this review at that time (we anticipate by end of January 2018). Would this be acceptable?

wilsoncai1992 commented 6 years ago

Dear @karthik, thanks for your patience with this review. We're very happy to be able to report that we've just recently completed a total re-haul of the R package and have been working with the team at Bioconductor. As of 30 March, the package has been accepted into the upcoming Bioconductor 3.7 release, and we feel now would be a good time to continue with this review. Please let us know what next steps make sense in this situation, as we've made extensive changes to the codebase and many of the items from the earlier review have either been addressed or made irrelevant. Should we start a new review?

wilsoncai1992 commented 6 years ago

For reference, this is BIOC review https://github.com/Bioconductor/Contributions/issues/628#issuecomment-373484320

nhejazi commented 6 years ago

@karthik Just checking in — would it make more sense for us to open a new submission issue for this package given the significant changes to it? (Re-naming, Bioconductor acceptance.) If not, please let us know whether you might need anything further from us in order to proceed with the review. Thanks for your help with this.

arfon commented 6 years ago

👋@karthik - what would you like to do here?

gavinsimpson commented 6 years ago

FYI; I'm available next week to pick up the review again if you decide to proceed, but after that I am away for a month and unlikely to have time to do a review for 5-6 weeks from now because of conferences/vacation & work commitments when I get back.

karthik commented 6 years ago

@nhejazi @arfon Sorry, I was on travel + vacation these past 3 weeks and didn't have a chance to follow up on editorial duties. I don't believe there is a need to restart a new submission. @gavinsimpson If you're able to pick up this thread again, please proceed and we'd be grateful if you could complete this before you're away. 🙏

nhejazi commented 6 years ago

@karthik -- No problem at all; glad to have your help in the editorial component of this submission now that you're back. To facilitate using this same issue for completion of the review, I've updated the submission comment at the top of the issue to reflect the new name and links to the package/repo (adaptest instead of data.adapt.multi.test) as well as bumped the appropriate version number (from 0.2.0 to 1.1.0).

@gavinsimpson -- We would be grateful to have your review before you begin your travels. If you have the time to pick up this review again, you'll likely find that many of the previous problems (e.g., lack of unit tests) have been resolved as a consequence of the vetting process used by Bioconductor, though I will gladly dedicate time to making any further changes you point out this/next week.

nhejazi commented 6 years ago

In order to ease the process of continuing this review, we've updated the master branch of adaptest to include an updated copy of the JOSS paper for this package (in the paper subdirectory). Please refer to this branch if you have a chance to continue this review @gavinsimpson.

nhejazi commented 6 years ago

@gavinsimpson -- Just following up on your potential re-review of this R package. Since it's been ~5-6 weeks since the last comment on this thread, is there a chance you might have time to complete this review of adaptest in the near future?

gavinsimpson commented 6 years ago

Hi, yes, I was a little optimists at being able to do this inbetween my trips to Europe. I'm back in the office next week and will complete the review then.

On Tue, Aug 7, 2018, 4:03 PM Nima Hejazi, notifications@github.com wrote:

@gavinsimpson https://github.com/gavinsimpson -- Just following up on your potential re-review of this R package. Since it's been ~5-6 weeks since the last comment on this thread, is there a chance you might have time to complete this review of adaptest in the near future?

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

karthik commented 6 years ago

Thank you @gavinsimpson!

wilsoncai1992 commented 6 years ago

Thank you Gavin!

On Tue, Aug 7, 2018 at 2:08 PM Gavin Simpson notifications@github.com wrote:

Hi, yes, I was a little optimists at being able to do this inbetween my trips to Europe. I'm back in the office next week and will complete the review then.

On Tue, Aug 7, 2018, 4:03 PM Nima Hejazi, notifications@github.com wrote:

@gavinsimpson https://github.com/gavinsimpson -- Just following up on your potential re-review of this R package. Since it's been ~5-6 weeks since the last comment on this thread, is there a chance you might have time to complete this review of adaptest in the near future?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/openjournals/joss-reviews/issues/161#issuecomment-411201854 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAfaiNXQ3ol_2tujEZaB2Vakp5WAX7y1ks5uOgC0gaJpZM4LguZL

.

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

nhejazi commented 6 years ago

Hello @gavinsimpson, just another quick follow-up about the review of this R package. We're hoping to complete the review / finalize publication of this software paper in JOSS by the end of the month --- does it appear that your schedule will allow for your review to be completed within this time frame? Thank you in advance for your re-review.

arfon commented 6 years ago

:wave: @karthik - I think we might need to find another reviewer here?

nhejazi commented 6 years ago

@arfon - thanks for following up on this. If at all possible, it would be best if we could find a new reviewer and finalize this submission within the next few weeks. Since this R package is part of Bioconductor and their next release (on their biannual schedule) is at the end of this month, having the review of this paper completed co-incident with that release would allow interested/prospective users to make the most of the software and underlying statistical methodology.

arfon commented 6 years ago

@nhejazi - could you take a look a this list of potential reviewers and identify a few people who would be good candidates to review this submission?

nhejazi commented 6 years ago

@arfon -- based on a look through the list -- perhaps one of @road2stat, @malcolmbarrett, @mschubert, or @tgerke might be interested in and have time to complete this review soon?

arfon commented 6 years ago

@road2stat, @malcolmbarrett - would either of you be willing/able to complete the review of this package for us? @gavinsimpson has made a great start but we could do with some assistance to get across the line here.

Basically we'd be asking you to work through the checklist at the top of this issue and posting your feedback into this review thread.

Thanks in advance for considering!

nanxstats commented 6 years ago

@arfon -sorry I'm not an expert in multiple testing or the S4 classes the package builds on. Maybe next time!

karthik commented 6 years ago

@arfon @nhejazi I will try to locate another reviewer asap.

karthik commented 6 years ago

I've dashed off a couple of requests. Will update thread in a day or two.

karthik commented 6 years ago

Happy to report that @kellieotto can review.

karthik commented 6 years ago

I'll manually edit the issue text and uncheck the boxes for Kellie to work through.

malcolmbarrett commented 6 years ago

Hi folks, sounds like this got sorted, which is good since I wouldn't have been able to review for a few weeks, but do let me know if you end up needing help with the review in the future.

kellieotto commented 6 years ago

@karthik @wilsoncai1992 @nhejazi I made a first pass and put all my comments in issues to the repo. They should be pretty easy fixes.

wilsoncai1992 commented 6 years ago

Thank you @kellieotto @karthik for the review! We will quickly fix the suggestions!

nhejazi commented 6 years ago

Thanks for the quick turnaround on your review @kellieotto -- much appreciated! We'll start the process of addressing the points you've brought up in each of the issues you've opened.

kellieotto commented 6 years ago

@nhejazi @wilsoncai1992 thanks for such quick fixes! Looks great. @karthik I'm ready to recommend this to be accepted.

nhejazi commented 6 years ago

Great! Many thanks for quickly working through this review with us, @kellieotto!

Many thanks also to @karthik and @arfon both for searching for a new reviewer and ensuring one was found in time for this review to completed for the upcoming Bioconductor release.

wilsoncai1992 commented 6 years ago

Thank you @karthik @arfon for your continued support on this submission. Thank you @kellieotto for your timely and helpful feedback!

karthik commented 6 years ago

@nhejazi Please archive your package on Zenodo and update this thread with a doi and I can move forward with acceptance.

@kellieotto We are very grateful for a speedy review! 🙏

nhejazi commented 6 years ago

@karthik --- Done! The DOI is 10.5281/zenodo.1466019, and the Zenodo release link is here.

Once again, thanks much for all your help in this process @kellieotto, @karthik, @arfon --- it's much appreciated!

karthik commented 6 years ago

@whedon set 10.5281/zenodo.1466019 as archive

whedon commented 6 years ago

OK. 10.5281/zenodo.1466019 is the archive.

karthik commented 6 years ago

Thanks @nhejazi! Your paper is accepted. 🎉
@arfon this is ready to process.

arfon commented 6 years ago

@whedon accept

whedon commented 6 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 6 years ago

PDF failed to compile for issue #161 with the following error:

% Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 16 0 16 0 0 94 0 --:--:-- --:--:-- --:--:-- 95 Could not find bibliography file: manuscript.bib Error running filter pandoc-citeproc: Filter returned error status 1 Looks like we failed to compile the PDF

arfon commented 6 years ago

@nhejazi - could you updated your paper.md file to point to paper.bib so we can compile the paper please?

wilsoncai1992 commented 6 years ago

Dear @arfon, I have fixed the md to point to the new bib

arfon commented 6 years ago

@whedon generate pdf

whedon commented 6 years ago 
Attempting PDF compilation. Reticulating splines etc...