Submitting author: @gauteh (Gaute Hope)
Repository: https://github.com/gauteh/ibcao_py
Version: v1.1
Editor: @arfon
Reviewer: @dvalters
Archive: 10.5281/zenodo.580775
Status badge code:
HTML: <a href="http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5"><img src="http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5/status.svg"></a>
Markdown: [](http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5)
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.)
paper.md
file include a list of authors with their affiliations?Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @dvalters 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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿
To fix this do the following two things:
For a list of things I can do to help you, just type:
@whedon commands
@dvalters - please carry out your review in this issue by updating the checklist above and giving feedback in this issue. The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines
Any questions/concerns please let me know.
Arfon Smith writes on april 24, 2017 22:57:
@dvalters - please carry out your review in this issue by updating the checklist above and giving feedback in this issue. The reviewer guidelines are available here: http://joss.theoj.org/about#reviewer_guidelines
@dvalters: is your review done? I do not get any notifications on changes
to checkboxes.
Best regards, Gaute
Hi @gauteh, my review is below:
I was able to download and install the software, although really it is just a script rather than a package that can be installed using a setup script or package manager. (Though I notice you are working on a setup.py
in the master branch? If you could include this in the release it would improve this software by automating the installation.) The dependencies aren't listed explicitly (such as in a requirements.txt
file). It's implied of course that you need to install Cartopy, Matplotlib, and the associated dependencies with those packages. Stating the dependencies explicitly would improve the software package.
The test command in the README.md produced the figure as indicated:
.
It might be nice to see this as an example in your accompanying paper to highlight the quality of figures this software can produce.
The unit tests provided in the /tests
folder ran successfully, though with warnings about unclosed files which may want to be addressed. (I raised an issue here: gauteh/ibcao_py#1, though it's not essential to fix this for acceptance.) The unit testing shell script is not documented anywhere though. It was a positive to see unit testing being employed. I was a bit puzzled by what some of the test output plots were meant to represent, and couldn't tell if they were correct or not. For example:
The documentation is sparse. The only documentation I could find was the README.md file, which was really just a brief set up guide and an example, rather than detailed documentation of the API. I would say this is the main issue holding back the software package from being accepted.
There are no community guidelines provided (see the checkbox for details of what is suggested)
The paper is very brief (only a single short paragraph). I understand the JOSS papers are only meant be short introductions to the software, but even so, it could be expanded on a little bit further. Who are the target audience? Oceanographers, presumably, but are there others? Earth Scientists in general?
You mention the test script 'showcases some of the possibilities', but you could briefly list the main ones here to improve the paper for the target audience. Does it do anything other than project data to one specific projection system? Here is a chance to advertise your software!
References are encouraged to have a DOI where available. Matplotlib is missing a DOI in your .bib
file, but there is one provided here that you could include: https://matplotlib.org/citing.html. I couldn't find a DOI for Cartopy, but the format of the reference should list the author field as Met Office
, as per this bibtex example: http://scitools.org.uk/cartopy/docs/v0.15/citation.html
Renaming it LICENCE (or LICENSE, depending on your spelling preference) instead of COPYING may make it more clear that this is the licence file. (Although either is fine, just a suggestion.)
The software provides useful and novel functionality to a scientific community. I think though it needs major revisions, mainly because of the lack of proper documentation for the API. (The user shouldn't have to read through all the unit tests to figure out what the software can do - sell it to them through good documentation and the paper!) I think once the documentation and the paper have been revised, it can be accepted, and I will tick off the remaining checkboxes.
Hi Declan, thanks for the review, the extensive testing and the very
useful comments. I have created a new release: v1.1 which hopefully
fixes the issues you have raised.
Please see my comments inline:
Declan Valters writes on mai 4, 2017 0:21:
Hi @gauteh, my review is below:
Software functionality and installation
I was able to download and install the software, although really it is just a script rather than a package that can be installed using a setup script or package manager. (Though I notice you are working on one in the master branch? If you could include this in the release it would improve this software by automating the installation.) The dependencies aren't listed explicitly (such as in a
requirements.txt
file). It's implied of course that you need to install Cartopy, Matplotlib, and the associated dependencies with those packages. Stating the dependencies explicitly would improve the software package.
A requirements.txt file have been included.
The test command in the README.md produced the figure as indicated:
.
It might be nice to see this as an example in your accompanying paper to highlight the quality of figures this software can produce.
The figure is now included in the paper with a small example.
The unit tests provided in the
/tests
folder ran successfully, though with warnings about unclosed files which may want to be addressed. (I raised an issue here: gauteh/ibcao_py#1, though it's not essential to fix this for acceptance.) The unit testing shell script is not documented anywhere though. It was a positive to see unit testing being employed. I was a bit puzzled by what some of the test output plots where meant to represent, and couldn't tell if they were correct or not. For example:
See your bug-report for the unclosed-files warnings, it is a trade-off
between loading the entire map into memory and making it easier to close
the file. Since keeping the file open is not a big deal unless 'Too many
files' are open I think this is the best choice.
I have removed the link to the test scripts and rather included a
demonstration jupyter-notebook (linked from the README) as well as a
documentation page on readthedocs for the API, also linked from the
README.
Documentation
The documentation is sparse. The only documentation I could find was the README.md file, which was really just a brief set up guide and an example, rather than detailed documentation of the API. I would say this is the main issue holding back the software package from being accepted.
A demonstration notebook file and API docuemntation has been added
(linked from README).
There are no community guidelines provided (see the checkbox for details of what is suggested)
A section on contributions have been added.
Paper
The paper is very brief (only a single short paragraph). I understand the JOSS papers are only meant be short introductions to the software, but even so, it could be expanded on a little bit further. Who are the target audience? Oceanographers, presumably, but are there others? Earth Scientists in general?
You mention the test script 'showcases some of the possibilities', but you could briefly list the main ones here to improve the paper for the target audience. Does it do anything other than project data to one specific projection system?
I have elaborated on what the package does and what the target audience
may be. Better examples have been included in the documentation.
References
References are encouraged to have a DOI where available. Matplotlib is missing a DOI in your
.bib
file, but there is one provided here that you could include: https://matplotlib.org/citing.html. I couldn't find a DOI for Cartopy, but the format of the reference should list the author field asMet Office
, as per this bibtex example: http://scitools.org.uk/cartopy/docs/v0.15/citation.html
These have been updated, thanks.
Licence file
Renaming it LICENCE (or LICENSE, depending on your spelling preference) instead of COPYING may make it more clear that this is the licence file.
Renamed.
Summary
The software provides useful and novel functionality to a scientific community. I think though it needs major revisions, mainly because of the lack of proper documentation for the API. (The user shouldn't have to read through all the unit tests to figure out what the software can do - sell it to them through good documentation and the paper!) I think once the documentation and the paper have been revised, it can be accepted, and I will tick off the remaining checkboxes.
Please have a look at the comments above and the revised version to see
if your comments have been adequately addressed.
Best regards,
Gaute Hope
Hi Gaute,
Thanks for addressing the comments in the review - I am happy that the software and paper meet the requirements. @arfon I think it is good to go now!
@gauteh - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.
Arfon Smith writes on mai 17, 2017 17:22:
@gauteh - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.
Hi Arfon,
the DOI is: 10.5281/zenodo.580775 (https://zenodo.org/record/580775)
@whedon set 10.5281/zenodo.580775 as archive
OK. 10.5281/zenodo.580775 is the archive.
@dvalters - many thanks for your review here ✨
@gauteh - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00250 ⚡️ 🚀 💥
@arfon: great! many thanks!
@dvalters: Thanks for the thorough review!
Arfon Smith writes on mai 22, 2017 18:52:
@dvalters - many thanks for your review here ✨
@gauteh - your paper is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00250 ⚡️ 🚀 💥