Joss-reviews: [REVIEW]: Py-school-match: Matching algorithms to assign students to schools

Created on 30 Nov 2018  ยท  64Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @igarizio (Iacopo Garizio)
Repository: https://github.com/igarizio/py-school-match
Version: 0.2.0
Editor: @arokem
Reviewer: @jmhernan
Archive: 10.5281/zenodo.2554632

Status

status

Status badge code:

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

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

@jmhernan, 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 @arokem know.

โœจ Please try and complete your review in the next two weeks โœจ

Review checklist for @jmhernan

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: Does the release version given match the GitHub release (0.2.0)?
  • [x] Authorship: Has the submitting author (@igarizio) 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

@igarizio : in the meanwhile, a couple of comments from me:

  • The last sentence of the first paragraph should probably end with a question mark.
  • I believe that "Along the years" (paragraph 2), should be "Over the years".
  • "assignation" => "assignment" wherever it appears.
  • "without the need of developing case-by-case solutions" => "without the need to develop case-by-case solutions".

I would prefer that the related literature section, that is currently a list of bullet points be integrated into your introductory paragraph, as references to specific points made there, or as extensions to that paragraph.

All 64 comments

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

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@arokem I'm having trouble assessing this bullet:

  • [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

The dependency for this software is graph-tool which is C++ library wrapped in python. The installation was not straight forward on my end (I could not get things to work on my conda python environment. I was able to get it to work with homebrew after some stackoveflowing) Should more information be included or is a link to the graph-tool folks enough as is provided?

@igarizio Great work on this piece of software. Here are some of my comments so far:

  • [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

I don't see this on any of the documentation. On your software paper you do mention that this software is intended to help researchers compare assignment algorithms. Is your audience only researchers or also practitioners interested in using some of these algoriths for assignment?

  • [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

You provide an easy to follow and implement example, however is this how researchers will use this software? How would someone use your software to compare the effectiveness of these assignment algorithms?

  • [ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?

Coud you provide some instructions on how a user could check whether or not things installed correctly? Documented manual steps that can be followed to check the expected functionality of the software (e.g. a sample input file to assert behaviour)

I think that a link to this page: https://git.skewed.de/count0/graph-tool/wikis/installation-instructions would be helpful, considering installation can be a bit arcane.

@jmhernan Thank you for your comments! I will work on them as soon as posible.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@jmhernan I think I fixed all the comments. Here are the details:

  • [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

I changed the wording of the paper (a little) and added a small introduction in the documentation page.
Answering your question, I think my software focuses only in researchers, because it is not fully optimized for each particular case available (something that practitioners may find really important).

  • [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

I added a new example in the documentation page showing how to run and compare multiple algorithms.

  • [ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?

I added a section in the documentation showing how to run the test suite.

  • [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

I added the link to graph-tool's installation instructions in the README and in the docs.

Hey @jmhernan -- have you had a chance to take a look at these revisions? Thanks!

@igarizio thank you for making those changes. Tested things again and everything ran as documented.

Great, thank you @jmhernan!
What should I do now?

@jmhernan : just to be clear -- is your recommendation that we accept the paper as it is?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@igarizio : in the meanwhile, a couple of comments from me:

  • The last sentence of the first paragraph should probably end with a question mark.
  • I believe that "Along the years" (paragraph 2), should be "Over the years".
  • "assignation" => "assignment" wherever it appears.
  • "without the need of developing case-by-case solutions" => "without the need to develop case-by-case solutions".

I would prefer that the related literature section, that is currently a list of bullet points be integrated into your introductory paragraph, as references to specific points made there, or as extensions to that paragraph.

@arokem sorry for the delay, I thought the checklist would trigger something after everything was checked off. The author addressed my initial revisions and after your revisions, I do recommend this paper moves forward.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@arokem @jmhernan thank you for your feedback!
I think I have now fixed the issues in the paper (I have rephrased some parts of it). Please let me know if they sound good to you.

@igarizio: looks good.

Could you please create an archive for the current state of the software (e.g., using zenodo) and post the doi that you get here?

Great!
Here is the DOI: 10.5281/zenodo.2554632

@whedon set 10.5281/zenodo.2554632 as archive

OK. 10.5281/zenodo.2554632 is the archive.

Congratulations @igarizio! Your paper is now ready to be accepted.

Thanks @jmhernan for the review!

Please stand by for the EIC or an Associate EIC to drop by and finalize this.

cc: @arfon, @danielskatz, @labarba, @kyleniemeyer

@whedon accept

Attempting dry run of processing paper acceptance...

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

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/471, 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

MISSING DOIs

INVALID DOIs

  • None
    ```

One last thing @igarizio - could you please check to see if the references above under MISSING DOIS are indeed the correct DOIs for some of your references? If they are, then please add the DOI field to your bibtex.

Sure @arfon, I will check it right away.

@arfon I added the missing DOIs.

@whedon accept

Attempting dry run of processing paper acceptance...

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

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

@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/473
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01111
  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...

@jmhernan - many thanks for your review and to @arokem for editing this submission โœจ

@igarizio - your paper is now accepted into JOSS :zap::rocket::boom:

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

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

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

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:

Thanks again @jmhernan for the review, @arokem for the feedback and @arfon for the final process :smiley:.

Was this page helpful?
0 / 5 - 0 ratings