[REVIEW]: AutoClassWrapper: a Python wrapper for AutoClass C classification

[whedon]

Submitting author: @pierrepo (Pierre Poulain)
Repository: https://github.com/pierrepo/autoclasswrapper
Version: 1.5.1
Editor: @trallard
Reviewer: @rpetit3, @lowandrew
Archive: 10.5281/zenodo.3339286

Status

Status badge code:

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

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

@rpetit3 & @lowandrew, 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 @trallard know.

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

Review checklist for @rpetit3

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

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: 1.5.1
• Authorship: Has the submitting author (@pierrepo) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• 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)?

Review checklist for @lowandrew

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

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: 1.5.1
• Authorship: Has the submitting author (@pierrepo) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• 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]

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @rpetit3, 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 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:

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

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

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

👉 Check article proof 📄 👈

[trallard]

Hi @rpetit3, @lowandrew!

Here is the official review issue. You will notice that each of you has a checklist with the items you need to go through.
I suggest using this issue to follow the review process and carry on any discussions.
If there are particular issues or something that needs more in-depth/longer conversations please open an issue on the software repository and reference it here so I can follow up.

If there are any issues you need help with or need clarification of the process at any point please ping me here 🙋🏻‍♀️

[rpetit3]

@pierrepo - Great work Pierre! AutoClassWrapper was easy to install and get started with. I think your work offers a valuable path to making AutoClass C more accessible to naive users (such as myself!). Another important aspect, is that AutoClassWrapper allows the user to easily integrate AutoClass C into reproducible Jupyter/IPython notebooks.

Here are a few suggestions

1. Missing installation instruction. On the main readme, AutoClass C isn't listed as a dependency. I think since it is required, it would be useful to either include the steps from the demo notebook for installing AutoClass C in the README.md file, or at minimum note that it is required in the README. I created an issue (Add AutoClass C as a dependency pierrepo/autoclasswrapper#12) for this.

Important For now this has prevented me from checking the Installation instructions box above.

2. A statement of need. Currently the Background, describes AutoClass C and its uses, then ends with AutoClass@IJM, a webserver, making AutoClass C more accessible. It doesn't say anything about a need for something like AutoClassWrapper, a wrapper for running AutoClass C locally. That is not to say there is not a need for AutoClassWrapper, there definitely is! I found only one other AutoClass C related package, written in R (see: https://github.com/wikiselev/autoclass-c). This to me suggests a need. In order to highlight this need, I think a sentence highlighting the lack of packages/wrappers for making AutoClass C more accessible locally is all that would be needed.

Important For now this as prevented me from checking the A statement of need box above.

3. Author Ordering. At the moment, Jean-Michel Camadro is listed as the first author. This may have been intentional, but the citation at the bottom of the PDF reads Camdro et al.. I realize in most cases the actual citation would actually read Camadro & Poulain. But I thought it was worth pointing out since, based on the GitHub commits, you are the only code contributor. Please keep in mind, this is not in any way meant to negate any contributions Jean-Michel made. I just wanted to make sure it wasn't an unintended effect of the PDF creation.

4. I ran into a user-error, related to a non functioning AutoClass C installation, while going through the demo. I've gone into more details at Check AutoClass C is working pierrepo/autoclasswrapper#13. This is just a feature suggestion, and should not affect publication.

5. I wonder if the last sentence of the background could be shortened?

Original:
To this aim, AutoClass@IJM (Achcar, Camadro, & Mestivier, 2009), a web server that utilizes
AutoClass C, has been published few years ago to make Bayesian classification more 
accessible (see for instance results from (Simpson et al., 2011, Léger, Garcia, Ounissi, 
Lelandais, & Camadro (2015), Elliott, Tanaka, Schwark, & Andrade (2018))).


Alternative:
To this aim, AutoClass@IJM (Achcar, Camadro, & Mestivier, 2009), a web server that utilizes
AutoClass C, has made Bayesian classification more accessible (Simpson et al., 2011, Léger, 
Garcia, Ounissi, Lelandais, & Camadro (2015), Elliott, Tanaka, Schwark, & Andrade (2018))).

Again, great job with AutoClassWrapper, Pierre! Please feel free to discuss any of these suggestions!

Tania @trallard - Just a heads up, I will be starting paternity leave sometime in the near future. During this time, I might be a little slow to respond, but will try my best!

[trallard]

Thanks for the review and the heads-up @rpetit3

@lowandrew is there anything we can help you with for this review?

[lowandrew]

@trallard
Sorry for the delay on reviewing - I'll make sure I get around to this sometime early this week, I'll let you know if I need any assistance.

[lowandrew]

@pierrepo @trallard I overall quite like the package and find it works fairly well, but I would like to see more added in the way of documentation/real world examples, as I found myself troubleshooting via trial and error more than I would have liked when playing around with the package.

1. As noted by @rpetit3, the installation instructions aren't quite complete - it's only in going through the demo notebook that the install instructions for Autoclass-C itself are found. Ideally I'd like to see the the binaries for Autoclass-C included in the python distribution so that everything is installed in one shot via PyPi, but adding instructions to the Install section of the README/readthedocs would also be acceptable.

2. While there is a demo jupyter notebook (which is great!), it only provided one toy example. Ideally I'd like to see some more fleshed out examples, as there appears to be lots of functionality in Autoclass that isn't covered by the toy example (something demonstrating what happens with missing data/discrete values, for example, would be excellent). It'd also be nice to see something pulled from a real-world dataset, even if it was downsampled heavily to make it more manageable. My other issue with the tutorial is that it showed what to do, but I was often missing out on why certain steps had to happen. As an example of that, I ran into problems when making my own dataset because I had it all in one dataframe and didn't call the merge_dataframes() method see (merge_dataframes() necessary even when only one dataframe is present? pierrepo/autoclasswrapper#14). I was eventually able to get things to work, but more explanation of what each step does and why each step is necessary would be very welcome.

3. While the API does have documentation that appears to be fairly complete, I didn't find it to be particularly user friendly. As an example, see API documentation - clarify required parameters. pierrepo/autoclasswrapper#15. Throughout the API documentation, it isn't clear what parameters are required and which are optional, and when required parameters are missed errors don't always get thrown immediately, and so I was left having to reverse engineer at what point I'd missed something in a function call. Getting the API doc to be more clear throughout would be very helpful to users like myself that aren't familiar with Autoclass.

Overall though, I found that the wrapper made it fairly simple to get Autoclass up and running - with some more tutorial/documentation I think it makes for an excellent package. Sorry for the delay in getting the review done!

[trallard]

Thanks for the thorough review @lowandrew!

@pierrepo It seems from both reviews that adding clear installation instructions should be tackled for the review to move forward as well as expanding a bit the docs/examples

[pierrepo]

Dear @rpetit3 & @lowandrew thank you for your reviews. I appreciate very much the time you spent to test and assess AutoClassWrapper.
You both noticed a lack in the documentation and I will try to improve this point.
Of course, I will also answer all the questions you raised.
If you agree I'll come back to you before mid-June (ping @trallard ).

[kyleniemeyer]

@whedon remind @pierrepo in 2 weeks

[whedon]

Reminder set for @pierrepo in 2 weeks

[whedon]

👋 @pierrepo, please update us on how things are progressing here.

[kyleniemeyer]

Hi @pierrepo, just checking in. Have you had a chance to work on this?

[pierrepo]

Hello @kyleniemeyer
I'm on it. I've just fixed 2 issues opened by @rpetit3 (#12 and #13)
I will now handle those opened by @lowandrew (https://github.com/pierrepo/autoclasswrapper/issues).

[pierrepo]

Dear @rpetit3 and @lowandrew (cc @trallard),

once again, thank you both for your kind and constructive reviews.

Here are my comments and answers to your reviews.

---

@rpetit3 's review

1. Missing installation instruction. On the main readme, AutoClass C isn't listed as a dependency

AutoClass C installation steps are now provided in:

• the main README.md file,
• the documentation,
• both demo notebooks.

2. A statement of need. Currently the Background, describes AutoClass C and its uses, then ends with AutoClass@IJM, a webserver, making AutoClass C more accessible. It doesn't say anything about a need for something like AutoClassWrapper, a wrapper for running AutoClass C locally.

Thanks for pointing this to me. This was so obvious to me that I totally forgot!

I added the following sentence at the end of the Background section :

Albeit its proven efficiency and versatility, AutoClass C is not easy to configure and run locally for the end user. As far as we know of, there is only a R wrapper developed by M. Spivakov but with a limited interface.

3. Author Ordering.

Jean-Michel Camadro was a key part in this project although he did not release any commit. Alphabetical order was chosen for author order. We both agreed this order is fine.

4. I ran into a user-error, related to a non functioning AutoClass C installation, while going through the demo

Thank you for pointing this error to me. It is now fixed (your issue has been closed).

5. I wonder if the last sentence of the background could be shortened?

Yes. Thank you! Nice suggestion.

---

@lowandrew 's review

1. As noted by @rpetit3, the installation instructions aren't quite complete

I'm very sorry for those very incomplete installation instructions regarding AutoClass C. As suggested also by @rpetit3, AutoClass C installation steps are now provided in README, documentation and demo notebooks.

2. Ideally I'd like to see some more fleshed out examples

I've created a second notebook with the analysis of the iris flowers dataset. I modified this perfect dataset to add discrete and missing values.

2. (continued) My other issue with the tutorial is that it showed what to do, but I was often missing out on why certain steps had to happen. As an example of that, I ran into problems when making my own dataset because I had it all in one dataframe and didn't call the merge_dataframes()

In the online documentation, I have added more explanations on what successive commands are doing and what are the most interesting options. I hope this helps.

The issue you raised about the .merge_dataframe() is closed.

3. Throughout the API documentation, it isn't clear what parameters are required and which are optional.

All non mandatory parameters are now clearly labeled as optional in the API documentation. Sorry for the confusion this brought to you.

---

I've also created a convenient Binder link to ease quick overviews.

[rpetit3]

Dear @pierrepo,

This is great! Thank you very much for considering my suggestions and making changes you saw fit. It is safe to say you have my approval for your manuscript.

Cheers!
Robert

@trallard - With the changes mentioned above, @pierrepo's manuscript has my approval for publication.

[lowandrew]

@pierrepo

This looks good to me too! The new examples and documentation are excellent.

@trallard - this has my approval as well.

[trallard]

🎉Thanks, for the review @rpetit3 and @lowandrew

@pierrepo Can you please do the following before moving to publication?

• Create a new release of the package and report the version here
• Generate a new DOI in Zenodo and paste it here
• Do a last check on the paper for missing references, typos and the such?

Once completed we can move this forward

[trallard]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

👉 Check article proof 📄 👈

[pierrepo]

Hi @trallard Thanks!

Create a new release of the package and report the version here

Release 1.5.1 has been created and is available here

Generate a new DOI in Zenodo and paste it here

The specific DOI for this release is 10.5281/zenodo.3339286
Please note there is also a generic DOI (for all releases) : 10.5281/zenodo.2527058

Do a last check on the paper for missing references, typos and the such?

Sounds good to me 😄

[trallard]

@whedon check references

[whedon]

Attempting to check references...

[whedon]

Check final proof 👉 openjournals/joss-papers#862

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#862, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

[trallard]

@openjournals/joss-eics as the editor of this submission I would like to recommend this for acceptance at JOSS. I am now passing this onto you for the next steps

[kyleniemeyer]

Hi @pierrepo, can you add your coauthor to the Zenodo archive? You should be able to just edit the metadata.

[kyleniemeyer]

@pierrepo also, please merge the small PR I just submitted, which fixes some references in the paper: pierrepo/autoclasswrapper#17

[kyleniemeyer]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

👉 Check article proof 📄 👈

[pierrepo]

Dear @trallard thank you for taking care of this manuscript until there 👏

@kyleniemeyer PR has been accepted and coauthor has been added in the Zenodo archive:
https://zenodo.org/record/3339286

[ooo]

👋 Hey @pierrepo...

Letting you know, @trallard is currently OOO until Monday, August 19th 2019. ❤️

[kyleniemeyer]

@whedon set 10.5281/zenodo.3339286 as archive

[whedon]

OK. 10.5281/zenodo.3339286 is the archive.

[kyleniemeyer]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

Check final proof 👉 openjournals/joss-papers#865

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#865, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

[kyleniemeyer]

@whedon accept deposit=true

[whedon]

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

[whedon]

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

[whedon]

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

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.01390 joss-papers#866
1. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01390
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

[pierrepo]

Everything seems OK.
Thanks 🎉

[kyleniemeyer]

@pierrepo congrats on your paper's acceptance in JOSS! Thanks to @rpetit3 and @lowandrew for reviewing, and @trallard for editing!

[ooo]

👋 Hey @kyleniemeyer...

Letting you know, @trallard is currently OOO until Monday, August 19th 2019. ❤️

[whedon]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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.01390/status.svg)](https://doi.org/10.21105/joss.01390)

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

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

This is how it will look in your documentation:

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:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.salsalabs.org/donate-to-joss

[pierrepo]

Dear @rpetit3 @lowandrew @trallard and @kyleniemeyer
Thank you very much for having helped me to improve this paper and eventually accepted it for publication in JOSS. This was a nice and smooth publishing experience. 👏 🎉

[ooo]

👋 Hey @pierrepo...

Letting you know, @trallard is currently OOO until Monday, August 19th 2019. ❤️