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 badge code:
HTML: <a href="http://joss.theoj.org/papers/452707998bc50b787c0ba41cf544b1c3"><img src="http://joss.theoj.org/papers/452707998bc50b787c0ba41cf544b1c3/status.svg"></a>
Markdown: [](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.)
@djmitche & @ gsagnol, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
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 โจ
paper.md
file include a list of authors with their affiliations?paper.md
file include a list of authors with their affiliations?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:
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:
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_ZeroStructDo 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.
@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:
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
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:
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:
[](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:
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! ๐๐๐
Most helpful comment
๐๐๐ This was the 500th accepted paper in JOSS! ๐๐๐