New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: biotmle: Targeted Learning for Biomarker Discovery #295

Closed
whedon opened this Issue Jun 14, 2017 · 22 comments

Comments

Projects
None yet
5 participants
@whedon
Collaborator

whedon commented Jun 14, 2017

Submitting author: @nhejazi (Nima Hejazi)
Repository: https://github.com/nhejazi/biotmle
Version: v0.1.0
Editor: @karthik
Reviewer: @NelleV
Archive: 10.5281/zenodo.834849

Status

status

Status badge code:

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

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

  • 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 (v0.1.0)?
  • Authorship: Has the submitting author (nhejazi) made major contributions to the software?

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

  • 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

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Jun 14, 2017

Collaborator

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

⭐️ Important ⭐️

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
Collaborator

whedon commented Jun 14, 2017

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

⭐️ Important ⭐️

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
@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jun 16, 2017

Collaborator

To the reviewer (@NelleV) -- please make sure to evaluate only the develop branch of this project, as it contains several important changes that have not yet been moved to master (this is because master is currently in line with the version of this package currently available on Bioconductor). Thanks!

Collaborator

nhejazi commented Jun 16, 2017

To the reviewer (@NelleV) -- please make sure to evaluate only the develop branch of this project, as it contains several important changes that have not yet been moved to master (this is because master is currently in line with the version of this package currently available on Bioconductor). Thanks!

@NelleV

This comment has been minimized.

Show comment
Hide comment
@NelleV

NelleV Jun 16, 2017

Collaborator

I'll keep this in mind.

Collaborator

NelleV commented Jun 16, 2017

I'll keep this in mind.

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 7, 2017

Collaborator

Best to consult the master branch now. develop has been modified to contain some functionality that is statistically questionable and still needs to be thought about.

Sorry about the weird organizational flip-flop.

Collaborator

nhejazi commented Jul 7, 2017

Best to consult the master branch now. develop has been modified to contain some functionality that is statistically questionable and still needs to be thought about.

Sorry about the weird organizational flip-flop.

@NelleV

This comment has been minimized.

Show comment
Hide comment
@NelleV

NelleV Jul 13, 2017

Collaborator

@whedon commands

Collaborator

NelleV commented Jul 13, 2017

@whedon commands

@whedon

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Jul 13, 2017

Collaborator

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

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# 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).

Collaborator

whedon commented Jul 13, 2017

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

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# 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).

@NelleV

This comment has been minimized.

Show comment
Hide comment
@NelleV

NelleV Jul 13, 2017

Collaborator

Hi @nhejazi

I've done a first pass at reviewing the paper (sorry it took so long), and I have a few comments:

There are still a couple of things I need to look at, but I'll get back to you soon.

Collaborator

NelleV commented Jul 13, 2017

Hi @nhejazi

I've done a first pass at reviewing the paper (sorry it took so long), and I have a few comments:

There are still a couple of things I need to look at, but I'll get back to you soon.

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 13, 2017

Collaborator

Hi @NelleV ---

Thanks much for taking the time to go through our biotmle package. I've gone ahead and made the suggested changes as of 1d40f60. In regard to package versioning, I've put the current version (1.1.2) of the GitHub package in the codemeta.json file, as the current status of the package reflects changes that will be present in the development version on Bioconductor, which will become the stable release when Bioconductor 3.6 becomes the standard in the Fall.

Collaborator

nhejazi commented Jul 13, 2017

Hi @NelleV ---

Thanks much for taking the time to go through our biotmle package. I've gone ahead and made the suggested changes as of 1d40f60. In regard to package versioning, I've put the current version (1.1.2) of the GitHub package in the codemeta.json file, as the current status of the package reflects changes that will be present in the development version on Bioconductor, which will become the stable release when Bioconductor 3.6 becomes the standard in the Fall.

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Jul 19, 2017

Collaborator

👋 @NelleV I know you're just back from SciPy and are catching up, but once you are caught up, can you please let me know if you're ready to sign off? 🙏

Collaborator

karthik commented Jul 19, 2017

👋 @NelleV I know you're just back from SciPy and are catching up, but once you are caught up, can you please let me know if you're ready to sign off? 🙏

@NelleV

This comment has been minimized.

Show comment
Hide comment
@NelleV

NelleV Jul 25, 2017

Collaborator

LGTM!

Collaborator

NelleV commented Jul 25, 2017

LGTM!

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Jul 26, 2017

Collaborator

Awesome! Thanks @NelleV! We appreciate your time to review this submission. 🙏

@nhejazi Can you please archive this submission on Zenodo and update the thread with the DOI so I can move forward with the acceptance?

Collaborator

karthik commented Jul 26, 2017

Awesome! Thanks @NelleV! We appreciate your time to review this submission. 🙏

@nhejazi Can you please archive this submission on Zenodo and update the thread with the DOI so I can move forward with the acceptance?

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 26, 2017

Collaborator

Thanks very much for taking the time to review this package @NelleV! 🙏
@karthik -- thanks very much for your help throughout the review process!

I've archived v1.1.2 of the package and generated a DOI via Zenodo: DOI: 10.5281/zenodo.834849. Please let me know if there's anything more on my end.

Collaborator

nhejazi commented Jul 26, 2017

Thanks very much for taking the time to review this package @NelleV! 🙏
@karthik -- thanks very much for your help throughout the review process!

I've archived v1.1.2 of the package and generated a DOI via Zenodo: DOI: 10.5281/zenodo.834849. Please let me know if there's anything more on my end.

@arfon arfon added the accepted label Jul 26, 2017

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Jul 26, 2017

Member

@whedon set 10.5281/zenodo.834849 as archive

Member

arfon commented Jul 26, 2017

@whedon set 10.5281/zenodo.834849 as archive

@whedon

This comment has been minimized.

Show comment
Hide comment
@whedon

whedon Jul 26, 2017

Collaborator

OK. 10.5281/zenodo.834849 is the archive.

Collaborator

whedon commented Jul 26, 2017

OK. 10.5281/zenodo.834849 is the archive.

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Jul 26, 2017

Member

@nhejazi - it looks like there are two paper files (paper.md and paper.Rmd). The later looks to be more closely aligned with our expected format. Could you confirm that the paper.Rmd should be the file we should be looking at here? If so, please delete the paper.md file.

Member

arfon commented Jul 26, 2017

@nhejazi - it looks like there are two paper files (paper.md and paper.Rmd). The later looks to be more closely aligned with our expected format. Could you confirm that the paper.Rmd should be the file we should be looking at here? If so, please delete the paper.md file.

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 26, 2017

Collaborator

Confirming that paper.Rmd is the appropriate (reproducible) file to be using here. paper.md and (now) unnecessary supporting files have been removed as of the latest commit on master.

Collaborator

nhejazi commented Jul 26, 2017

Confirming that paper.Rmd is the appropriate (reproducible) file to be using here. paper.md and (now) unnecessary supporting files have been removed as of the latest commit on master.

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Jul 26, 2017

Member

OK thanks @nhejazi. Can you clarify why this is .Rmd and not .md - our submission tooling is heavily automated and expects to find a file named paper.md not paper.Rmd 😁

Member

arfon commented Jul 26, 2017

OK thanks @nhejazi. Can you clarify why this is .Rmd and not .md - our submission tooling is heavily automated and expects to find a file named paper.md not paper.Rmd 😁

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 26, 2017

Collaborator

Oh, based on a different JOSS submission that I was involved in, the reviewer asked for an .Rmd to ensure reproducibility of results. That submission ended up getting stalled on our end, but I went ahead with applying the suggestion to this submission. I should be able to pretty easily convert it to .md if you'd like.

Collaborator

nhejazi commented Jul 26, 2017

Oh, based on a different JOSS submission that I was involved in, the reviewer asked for an .Rmd to ensure reproducibility of results. That submission ended up getting stalled on our end, but I went ahead with applying the suggestion to this submission. I should be able to pretty easily convert it to .md if you'd like.

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Jul 26, 2017

Member

I should be able to pretty easily convert it to .md if you'd like.

👍 Yes please.

Member

arfon commented Jul 26, 2017

I should be able to pretty easily convert it to .md if you'd like.

👍 Yes please.

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 26, 2017

Collaborator

Done! .Rmd -> .md, and everything looks close enough to what I had intended.

Collaborator

nhejazi commented Jul 26, 2017

Done! .Rmd -> .md, and everything looks close enough to what I had intended.

@arfon

This comment has been minimized.

Show comment
Hide comment
@arfon

arfon Jul 26, 2017

Member

@NelleV - many thanks for your review here and to @karthik for editing this one

@nhejazi - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00295 ⚡️ 🚀 💥

Member

arfon commented Jul 26, 2017

@NelleV - many thanks for your review here and to @karthik for editing this one

@nhejazi - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00295 ⚡️ 🚀 💥

@arfon arfon closed this Jul 26, 2017

@nhejazi

This comment has been minimized.

Show comment
Hide comment
@nhejazi

nhejazi Jul 26, 2017

Collaborator

Awesome!

Thanks again @NelleV, @karthik, @arfon!

Collaborator

nhejazi commented Jul 26, 2017

Awesome!

Thanks again @NelleV, @karthik, @arfon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment