Joss-reviews: [REVIEW]: IBCAO_py: A matplotlib library for using the International Bathymetric Chart of the Arctic Ocean with cartopy and matplotlib

Created on 24 Apr 2017  ·  12Comments  ·  Source: openjournals/joss-reviews

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

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5"><img src="http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/b3adbbf7df62f4e427f19e99e70163f5/status.svg)](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.)

Reviewer questions

Conflict of interest

  • [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

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 (v1.1)?
  • [x] Authorship: Has the submitting author (@gauteh) made major contributions to the software?

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: Have any performance claims of the software been confirmed?

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

All 12 comments

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:

  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

@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:

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 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:
figure_1-5.

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:

depth_rectbivariate

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.

There are no community guidelines provided (see the checkbox for details of what is suggested)

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? Here is a chance to advertise your software!

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 as Met Office, as per this bibtex example: http://scitools.org.uk/cartopy/docs/v0.15/citation.html

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. (Although either is fine, just a suggestion.)

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.

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:
figure_1-5.

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:

depth_rectbivariate

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 as Met 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)

  • gaute

@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 ⚡️ 🚀 💥

Was this page helpful?
0 / 5 - 0 ratings