Joss-reviews: [REVIEW]: Diff_classifier: Parallelization of multi-particle tracking video analyses

Created on 30 Sep 2018  Β·  54Comments  Β·  Source: openjournals/joss-reviews

Submitting author: @ccurtis7 (Chad Curtis)
Repository: https://github.com/ccurtis7/diff_classifier
Version: v0.1-beta
Editor: @cMadan
Reviewer: @stsievert
Archive: 10.5281/zenodo.2631862

Status

status

Status badge code:

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

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 instructions & questions

@stsievert, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @cMadan know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @stsievert

Conflict of interest

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Version: v0.1-beta
  • [x] Authorship: Has the submitting author (@ccurtis7) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

  • [x] Authors: Does the paper.md file include a list of authors with their affiliations?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
accepted published recommend-accept review

Most helpful comment

@stsievert I hope you had a good winter break! I noticed that you have checked everything except installation and performance. I believe I have addressed all the issues you brought up.

Let me know if I can do anything else to finish up!

All 54 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @stsievert 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 a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all 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
Attempting PDF compilation. Reticulating splines etc...

@stsievert, let me know if you have any questions!

@stsievert, I wanted to find out how the review is coming along, and if there is anything you need from me. Thanks!

Thanks for the reminder @ccurtis7. I've started it, and will try to finish it this week. My apologies for the delay.

@ccurtis7 could you add DOIs for first 4 papers in papers.bib? i.e., the Chenouard:2014 reference should have a DOI of https://doi.org/10.1038/nmeth.2808.

I forgot to mention that I went and updated the papers.bib file as requested.

@stsievert I hope you had a good winter break! I noticed that you have checked everything except installation and performance. I believe I have addressed all the issues you brought up.

Let me know if I can do anything else to finish up!

Thanks for the ping @ccurtis7, and my apologies for the excessive delay.

I think this paper is coming to an acceptable state. Past the issues/PRs I've filed, I think the only blocker is verification of the CloudKnot parallelization claims. I'll try to do that this weekend.

@ccurtis7 and @stsievert, how are things progressing? Thanks!

Thanks for checking in. I’ve found a couple issues in CloudKnot, specifically on some dependencies (see https://github.com/ccurtis7/diff_classifier/issues/28 for some more detail). @ccurtis7 has resolved that issue, and I need to check the notebook again.

I don’t think I would have accepted the version of diff_classifier as submitted. I think now it’s closer to being ready.

@stsievert, looks like things are moving along then. Thank you for the update!

Hey @cMadan, @stsievert has really given some great help on improving the quality of the package, and I've tried to be as prompt as possible in addressing issues. As he mentioned, I think there's only one last double check of the parallelization demo notebook, and it should be good to go.

I think diff_classifier is almost ready for acceptance. Here's a quick summary of the changes since the v0.1-alpha tag:

  • Clean dependencies on Fiji, an image processing library (https://github.com/ccurtis7/diff_classifier/issues/22, https://github.com/ccurtis7/diff_classifier/issues/30)
  • Clean confusion on Python 2/3 decadency on remote/local machines with Cloudknot (resolved in https://github.com/ccurtis7/diff_classifier/issues/28#issuecomment-459855740)
  • Cleaning usage with pip: https://github.com/ccurtis7/diff_classifier/pull/18

Here are all the commits since the v0.1-alpha tag: https://github.com/ccurtis7/diff_classifier/compare/V0.1-alpha...master

@stsievert, thanks for the detailed update! It's good to see things moving along :)

This review is complete! This software package is ready for acceptance. One question:

Significant changes have been made since the initial submission (some changes are in https://github.com/openjournals/joss-reviews/issues/989#issuecomment-469061459). I feel there should be a new release for this – I wouldn't have accepted 0.1-alpha, but I'm willing to accept the master branch now. How should this be resolved?

@stsievert, perfect, thank you for confirming! Thanks for highlighting this difference :). After I do a final pass of the submission, I will ask @ccurtis7 to mint a new release and archive the code (@ccurtis7, wait until I do a final pass though!) and this will then be the version that is accepted.

@ccurtis7, sit tight for now and I will let you know how the last steps proceed shortly.

Thanks @cMadan! And thanks @stsievert for all your hard work through the review!

@ccurtis7, everything look good and I'm almost ready to accept. Can you mint a new release that includes the changes from the review process? I also need a DOI for an archived version of the code (i.e., from Zenodo or figshare).

@cMadan, I have minted a new release, v0.1-beta, and the DOI is 10.5281/zenodo.2631862

@whedon set v0.1-beta as version

OK. v0.1-beta is the version.

@whedon set 10.5281/zenodo.2631862 as archive

OK. 10.5281/zenodo.2631862 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

PDF failed to compile for issue #989 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 17 0 17 0 0 233 0 --:--:-- --:--:-- --:--:-- 236
Error producing PDF.
! Missing $ inserted.

$
l.356 T

Looks like we failed to compile the PDF

```Reference check summary:

OK DOIs

  • 10.1049/ip-rsn:20045064 is OK
  • 10.1038/nmeth.2808 is OK
  • 10.1016/j.ymeth.2016.09.016 is OK
  • 10.1016/j.jconrel.2015.10.021 is OK
  • 10.5281/zenodo.1413740 is OK
  • 10.25080/Majora-4af1f417-001 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

@ccurtis7, it looks like a recent change broke the paper.md (or bib)

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Hmmm, it appears that whedon has changed how it treats Latex characters in the title. Previously, I had to type diff\_classifier for it to render correctly, but now I no longer have to warn it with a \. I can just type diff_classifier. I have updated the file in my master branch accordingly.

Do I have to generate another release, or is the paper.md file in the master branch OK @cMadan?

@ccurtis7, yes, I know you previously had the \, but you're right that this functionality has changed now. whedon is it's own software package that's being improved over time, so I suppose these things can happen. There's no need to generate a new release for this.

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

  • 10.1049/ip-rsn:20045064 is OK
  • 10.1038/nmeth.2808 is OK
  • 10.1016/j.ymeth.2016.09.016 is OK
  • 10.1016/j.jconrel.2015.10.021 is OK
  • 10.5281/zenodo.2631862 is OK
  • 10.25080/Majora-4af1f417-001 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/610

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/610, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

@openjournals/joss-eics, I think we're all set to accept here!

@ccurtis7 β€” Since we will have the Zenodo archive DOI on the front page of the paper, as part of the metadata shown in the margin note, we don't include a citation to it or mention it in the paper. Can you delete that?

And while you're at it, please edit:

  • "principle component analysis" >> principal
  • add comms after "e.g."

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

I have made the requested changes @labarba, thank you.

@whedon accept

Attempting dry run of processing paper acceptance...

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/611

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/611, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

```Reference check summary:

OK DOIs

  • 10.1049/ip-rsn:20045064 is OK
  • 10.1038/nmeth.2808 is OK
  • 10.1016/j.ymeth.2016.09.016 is OK
  • 10.1016/j.jconrel.2015.10.021 is OK
  • 10.25080/Majora-4af1f417-001 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

@whedon accept deposit=true

Doing it live! Attempting automated processing of paper acceptance...

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/612
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.00989
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! πŸŽ‰πŸŒˆπŸ¦„πŸ’ƒπŸ‘»πŸ€˜

    Any issues? notify your editorial technical team...

Congratulations, @ccurtis7, your JOSS paper is published!

Sincere thanks to the editor: @cMadan, and reviewer: @stsievert β€” Merci, gracias, danke πŸ™

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.00989/status.svg)](https://doi.org/10.21105/joss.00989)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.00989">
  <img src="http://joss.theoj.org/papers/10.21105/joss.00989/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.00989/status.svg
   :target: https://doi.org/10.21105/joss.00989

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Was this page helpful?
0 / 5 - 0 ratings