Joss-reviews: [REVIEW]: OApackage: A Python package for generation and analysis of orthogonal arrays and conference designs

Created on 26 Nov 2018  ยท  80Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @eendebakpt (Pieter Eendebak)
Repository: https://github.com/eendebakpt/oapackage
Version: 2.6.3
Editor: @danielskatz
Reviewer: @djmitche, @gsagnol
Archive: 10.5281/zenodo.2579025

Status

status

Status badge code:

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

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

@djmitche & @ gsagnol, 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 @danielskatz know.

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

Review checklist for @djmitche

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: 2.6.3
  • [x] Authorship: Has the submitting author (@eendebakpt) 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)?

Review checklist for @gsagnol

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: 2.6.3
  • [x] Authorship: Has the submitting author (@eendebakpt) 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

๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰ This was the 500th accepted paper in JOSS! ๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰

All 80 comments

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

๐Ÿ‘‹ @djmitche, @tanaken-basis - thanks for agreeing to review this submission. Please see the instructions above and feel free to ask me if you have any questions. Otherwise, I will assume that you will work through the checklist items above, and will bring up any issues with the submission with the author (via very short discussion here or via new issues in the source repo), who will resolve them.

It's worth noting that the most recent version is 2.5.2 now. That is the same in the repo and the package repository.

Any updates to the package to improve based on review comments will go to the dev branch. On a regular basis I will merge dev to master and update the PyPi packages.

@whedon generate pdf from branch feat/updates_paper_alan

Attempting PDF compilation from custom branch feat/updates_paper_alan. Reticulating splines etc...

@whedon generate pdf from branch feat/updates_paper_alan

Just a heads up @eendebakpt - compiling from a branch doesn't seem to work very well. You might have to merge to master to see new changes with @whedon.

All - @tanaken-basis has informed me by email that he needs to be taken off this review because of a personal matter. So I need to find another reviewer - @djmitche, do you have any suggestion for someone else?

@gsagnol has been added as the second reviewer - thanks!

๐Ÿ‘‹ @gsagnol - thanks for agreeing to review this submission. Please see the instructions above and feel free to ask me if you have any questions. Otherwise, I will assume that you will work through the checklist items above, and will bring up any issues with the submission with the author (via very short discussion here or via new issues in the source repo), who will resolve them.

This is my first JOSS review so no, I don't have ideas :/

@whedon list reviewers

Here's the current list of reviewers: https://bit.ly/joss-reviewers

๐Ÿ‘‹ @gsagnol - how are things going? When will you be able to start on the review (at least the first 2 checkboxes regarding COIs and the JOSS CoC?)

Hi @danielskatz , I wanted to start with the review this week.

I tried to check some of the boxes, but this will not work, although I'm logged in to github. Is google-chrome under linux not supported ? I tried under firefox, but this did not work neither.

As in the first comment in this thread, can you:

  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

That's it... I probably did not accept the invitation, but I guess the invitation link expired:
"Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account."

๐Ÿ‘‹ @arfon - can you help with this? Or tell me what I can do?

@gsagnol - I've re-made the invitation here: https://github.com/openjournals/joss-reviews/invitations

I installed the latest release from github, and oapackage.__version__ returns 2.5.2, while the check list above asks for 2.5.1. Should the checklist be updated, or should I review a former version ?

@gsagnol I don't know about JOSS policy on this matter. I submitted version 2.5.1 to JOSS, but since that moment (also based on reviewers comments) I updated the code and the paper and made a new release. I will continue to push updates based on the review to dev and once in a while merge dev to master (with an increase in version number)

@peendebak : I would like to have a look at your unit tests. I tried to run
coverage run --source='./oapackage' -m pytest,
but numpy raises some error:
ImportError: /homes/combi/sagnol/.local/lib/python2.7/site-packages/numpy/core/multiarray.so: undefined symbol: _Py_ZeroStruct

Do I need a particular version of numpy to execute the testing suite ? Or is there another way to run the unit tests ?

@peendebak : I posted an issue on the OApackage repository about confusing locations for the documentation (readthedocs.io vs. http://pietereendebak.nl)

@peendebak : I would like to have a look at your unit tests. I tried to run
coverage run --source='./oapackage' -m pytest,
but numpy raises some error:
ImportError: /homes/combi/sagnol/.local/lib/python2.7/site-packages/numpy/core/multiarray.so: undefined symbol: _Py_ZeroStruct

Do I need a particular version of numpy to execute the testing suite ? Or is there another way to run the unit tests ?

The minimal version of numpy is specified in the setup.py (1.13, although perhaps earlier versions of numpy work as well). Can you provide me with some more details of your system (e.g. platform, version of numpy installed, how did you install (pip or compile from source), send output of installation process, version of swig installed)

Can you also send me the output of the following commands?

python -c "import numpy; print(numpy); print(numpy.__version__')
python -c "import oapackage; print(oapackage)"
pytest - v

@gsagnol

@peendebak and @gsagnol - please review the "latest" and be sure that all the docs are consistent, then we will accept that version into the repository and the journal once everything is approved.

@peendebak I created an issue on the oapackage repo for the tests.

  • On the PC of my institute, I am not able to run the tests. Here, I used a local information (pip install --user)
  • On my laptop, the testing suite seems to work, but some tests fail. On my laptop I used a standard global installation without the --user flag.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@peendebak : Concerning the last "check-box", it seems that some doi's are missing in the software paper. For example,
The book "Orthogonal Arrays: Theory and Applications" of Hedayat et al. should have doi https://doi.org/10.1007/978-1-4612-1478-6

Moreover, this reference appears in two separate bib entries (once as a book, once as a website).

@peendebak : I created a number of issues in the source repository. This can be considered as my list of requests for a minor revision.

@gsagnol Thanks for the review! A couple of issues we already addressed (in particular the tests that failed) and we made a new release to pypi based on that. The other issues we will probably address after new year.

@eendebakpt - please let us know when these are done

Hi @eendebakpt - just checking on the post-holiday status of this...

@danielskatz We started addressing the remaining comments.

@eendebakpt - Is there any further update at this point?

@gsagnol @danielskatz

Thank you for the extensive review of our package. We tried to addressed the issues you raised on the OApackage issue tracker. In particular:

https://github.com/eendebakpt/oapackage/issues/92: The github site now points to https://oapackage.readthedocs.io/. The webssite with generated results in mentioned on the readthedocs documentation.
https://github.com/eendebakpt/oapackage/issues/95: Issues have been fixed in https://github.com/eendebakpt/oapackage/issues/96 (make tests python 2.7 compatible, resolve issue with multiple scanf versions)
https://github.com/eendebakpt/oapackage/issues/97: The documentation for Doptimize was improved in several PRs. The referee points out that our software does not find globally optimal solutions. We now explicitly mention in the documentation that our software does not guarantee to find the global optimum. The generation of the model matrix still needs better documentation. The compound criterium is explained in detail in the paper to which we refer.
https://github.com/eendebakpt/oapackage/issues/98: We updated the introducion text (https://github.com/eendebakpt/oapackage/issues/122)
https://github.com/eendebakpt/oapackage/issues/99, https://github.com/eendebakpt/oapackage/issues/100: We added warnings to the Doptim function to let the user know that this method does not use the strength variable. The DOPTIM_UPDATE variable was indeed not used in the example, so we removed it. See https://github.com/eendebakpt/oapackage/issues/104, https://github.com/eendebakpt/oapackage/issues/106, https://github.com/eendebakpt/oapackage/issues/107
https://github.com/eendebakpt/oapackage/issues/101: We agree with the author that the complete c++ signature is not needed in the documentation (this style of documentation was started before we could properly generate the documentation with https://breathe.readthedocs.io). We updated the documentation (https://github.com/eendebakpt/oapackage/issues/113, https://github.com/eendebakpt/oapackage/issues/114, https://github.com/eendebakpt/oapackage/issues/119)
https://github.com/eendebakpt/oapackage/issues/102: We use doctest now in the documentation which guarantees the code snippets in the documentation will run without errors. https://github.com/eendebakpt/oapackage/issues/111, https://github.com/eendebakpt/oapackage/issues/113
https://github.com/eendebakpt/oapackage/issues/103: We updated the documentation https://github.com/eendebakpt/oapackage/issues/121

We still need to improve the documentation on the model matrix and the statistics of conference designs. We will do this in the next weeks. Feel free to already look and comment on the improvements we made. A new release with the improvements has already been made on https://pypi.org/project/OApackage/

@eendebakpt
Nice to see the progress ! I will wait for the improved documentation to complete my review.

I still have a concern about eendebakpt/oapackage#97: I really think writing the formula for the criterion you optimize is necessary, for several reasons:

  1. This is a software, and my philosophy is that software users should not need to read a full scientific paper to use it. Of course, the article must be cited in the doc, but at least the formula you use in your software to evaluate the quality of a design should be in the doc.
  2. This is a journal about open source software, and you are referring to an article hosted on a commercial website.

Perhaps @danielskatz can chime in here as to the requirements of JOSS?

My understanding is that it's a bridge between the world of publishing and the world of OSS, and as such part of its purpose is to inter-reference between articles on commercial websites and open-source applications.

I do agree that the documentation should be self-sufficient in understanding how to use the software, but that the documentation need not go into detail as to the implementation. By comparison, I would not expect the documentation for GIMP to include formal models for its "magic wand" boundary detection algorithm. Including a formula in oapackage's docs, for comparison to the source code, but referring the user to the paper for the explanation, seems a good compromise.

My understanding is that it's a bridge between the world of publishing and the world of OSS, and as such part of its _purpose_ is to inter-reference between articles on commercial websites and open-source applications.

I don't quite agree. The goal of JOSS is really to give software authors a product that can be cited, and to improve the software along the way. How internally complete the documentation is versus how much it references externally (and potentially commercial) articles is not not so much as factor, as long as a potential user can access as much documentation as they need to use the software.

Including a formula in oapackage's docs, for comparison to the source code, but referring the user to the paper for the explanation, seems a good compromise.

I think this seems reasonable.

As a part of this compromise, regarding the "article hosted on a commercial website," perhaps a pre-print would be allowed to be deposited on arxiv or similar?

@danielskatz @djmitche @gsagnol We all agree that the software must be usable without references, so I will add to the documentation.

I do believe that referencing the paper is the right thing to do. To understand the ideas and motivations behind the code one needs to read the paper, and it doesn't make sense to rewrite the contents of the paper in the documentation of the oapackage.

The link is to a commercial publisher, but using google and the title of the paper as search term a PDF can be found easily.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@danielskatz @gsagnol @djmitche @alanrvazquez Thank you again for reviewing our package and providing valuable feedback. In a series of PRs we addressed the comments you had. The changes have been merged to the dev branch and there a a new release on pypi. Can you take the time to review the package once more?

:heavy_check_mark: from me

๐Ÿ‘‹ @gsagnol - please let us know your thoughts as well:
Does this now allow you to check all the boxes?
If not, what else do you think needs to be done?

It's OK for me, all boxes checked !

@gsagnol - I see some boxes that are not checked... Can you go back and make sure?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Sorry, the boxes are really checked, now.

thanks @gsagnol and @djmitche

@eendebakpt - are you happy with everything now? The paper, the software version in the repo, the software that is archived in zenodo?

@danielskatz We are happy with the paper. Could you update the software version to the latest (2.6.2). We made some small updates to the documentation, but did not change the paper.
What do you mean exactly by the software that is archived in zenodo?

@whedon set 2.6.2 as version

OK. 2.6.2 is the version.

@eendebakpt - you need to make an archive of this version of the software, in zenodo or in another repository.

See the third bullet in https://joss.readthedocs.io/en/latest/submitting.html#the-review-process

(And see https://guides.github.com/activities/citable-code/ for how to do this for Zenodo)

Once you have done so, paste the DOI of the archive here.

@whedon set 2.6.3 as version

I'm sorry @eendebakpt, I'm afraid I can't do that. That's something only editors are allowed to do.

@danielskatz To create a zenodo archive I updated the version number of the package, it is now 2.6.3

The DOI of the archive is 10.5281/zenodo.2579025

@whedon set 2.6.3 as version

OK. 2.6.3 is the version.

@whedon set 10.5281/zenodo.2579025 as archive

OK. 10.5281/zenodo.2579025 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

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

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

Thanks to @gsagnol and @djmitche for your reviews

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

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

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

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

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:

@danielskatz @gsagnol @djmitche Thanks for reviewing the package. It helped us to improve the package structure.

๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰ This was the 500th accepted paper in JOSS! ๐ŸŽ‰๐ŸŽ‰๐ŸŽ‰

Was this page helpful?
0 / 5 - 0 ratings