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]: kerasR: R Interface to the Keras Deep Learning Library #296

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

Comments

Projects
None yet
5 participants
@whedon
Copy link
Collaborator

whedon commented Jun 14, 2017

Submitting author: @statsmaths (Taylor Arnold)
Repository: https://github.com/statsmaths/kerasR
Version: 0.6.1
Editor: @karthik
Reviewer: @kaneplusplus
Archive: 10.5281/zenodo.814996

Status

status

Status badge code:

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

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 (0.6.1)?
  • Authorship: Has the submitting author (@statsmaths) 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.

Copy link
Collaborator

whedon commented Jun 14, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @kaneplusplus 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
@kaneplusplus

This comment has been minimized.

Copy link
Collaborator

kaneplusplus commented Jun 19, 2017

This is a package I have begun using when applying deep learning models. This interface mirrors Python's making it very easy to transition between the two languages. Constructing a model, fitting it, and predicting is easy. The examples in the documentation were helpful and I liked that the vignette includes references for other deep learning resources.

The package should be accepted. While going through the code and documentation, I came up with the following three suggestions.

  • (suggestion) The installation assumes that keras is installed and configured for python. Is it worth pointing to installation instructions or adding the pip install commands that can be used to set this up? This would include installing pydot for visualization.
  • (suggestion) There is code output in the vignette that simply reads output. Should this be removed?
  • (suggestion) The code coverage is slightly low. Will this be increased in the coming weeks?
@statsmaths

This comment has been minimized.

Copy link

statsmaths commented Jun 19, 2017

@kaneplusplus Thank you very much for reviewing this submission and your very helpful feedback. I just pushed version 0.7.0 to GitHub, which I believe implements your three suggestions. Namely:

  • The package now gives an explicit warning message whenever any keras-related function is called if the keras package is not available. Details instructions for using pip to install the requirements are now given. I've also made it so that there is an explicit printing of the python path being used by reticulate.
  • I removed the erroneous output line in the vignette and README file.
  • Due to an oversight on my part, many of the test files were not pushed to the repository. These are now online. Because CRAN does not have the Python keras module, and some of the tests take a while to run, there are skip_on_cran() calls in the testing suite, so you need to set the export NOT_CRAN='true' if you want to run these locally. The current coverage is now at 76%.

Please let me know if you have any further questions or suggestions.

@kaneplusplus

This comment has been minimized.

Copy link
Collaborator

kaneplusplus commented Jun 19, 2017

This is great. I have no other issues or suggestions.

@karthik

This comment has been minimized.

Copy link
Collaborator

karthik commented Jun 20, 2017

@kaneplusplus Thank you very much for the review. We really appreciate it.

@statsmaths Could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

@statsmaths

This comment has been minimized.

Copy link

statsmaths commented Jun 20, 2017

@karthik Thanks. I archived the current version of the software to Zenodo with this DOI: 10.5281/zenodo.814996

@karthik

This comment has been minimized.

Copy link
Collaborator

karthik commented Jun 20, 2017

@whedon set 10.5281/zenodo.814996 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator

whedon commented Jun 20, 2017

OK. 10.5281/zenodo.814996 is the archive.

@karthik karthik added accepted and removed review labels Jun 20, 2017

@arfon

This comment has been minimized.

Copy link
Member

arfon commented Jun 22, 2017

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

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

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