nismod / cdcam Goto Github PK

Assuming:

• you have a PyPI account, access set up on the local machine, and publish permissions for cdcam on PyPI
• you have twine installed (pip install twine)

Steps to release:

• check that you're on latest master, and all tests have passed locally and on travis
• note a list of fixes and features since the previous release
• create a git tag git tag -a v1.2.3
• push the tag git push --tags
• create distributions python setup.py sdist bdist_wheel
• upload distributions python -m twine upload dist/*

Hello @tomalrussell, I have finished reviewing your software paper submission to JOSS according to the checklist in openjournals/joss-reviews#1911. Here are my comments.

General checks

License

Copyright (c) 2019 Edward Oughton

Should you be included in here as well, @tomalrussell?

Functionality

I installed the software and performed the tests according to the instructions provided in README.md and had no issues. I also ran the example project using the data from Zenodo and there were no problems.

Code

There were a few typos in the docstrings in model.py.

• urban
  

  cdcam/src/cdcam/model.py

  Line 81 in e5ee9b1

  Settlement type (rban, suburban and rural)

• District
  

  cdcam/src/cdcam/model.py

  Line 286 in e5ee9b1

  The Local Authority Disrict which this area is within.

• population
  

  cdcam/src/cdcam/model.py

  Line 600 in e5ee9b1

  The current populaion density requiring the lookup.

The repository URL provided in setup.py seems outdated. It still redirects to the current repository though.

The code looks good otherwise!

Documentation

The documentation does not clearly state the need for this software and the target audience. This should be similar to the statement of need in the paper. An example of how to use the software to solve real-world problems should also be provided here.

The installation instructions provided in the documentation is not as detailed as the one provided in README.md. Information about installing the package using conda could be included in the documentation. The review checklist asks for a clearly-stated list of dependencies, handled ideally with an automated package management solution.

The documentation does not contain community guidelines for third parties wishing to contribute to the software, report issues or problems with the software, and seek support. A CONTRIBUTING file should be included in the repository and documentation, and a link to the guidelines should be provided in README.md.

Software paper

The last sentence of the 'Applications' section states the following, but no citations have been provided.

Increasingly, the modelling capability reported here is being applied internationally, including to provide the analytics for the World Bank's Flagship 5G report.

The DOI fields in paper.bib should contain just the identifier and not the full URL, i.e., doi = {10.1016/j.techfore.2018.03.016}.

The paper is well written and can be understood easily by a non-specialist audience.

Hi @tomalrussell - I'm filing this issue as part of my review for your JOSS submission. The reviewer guidelines request that issues be filed at the package repository and then referenced from the review thread, so specific issues are detailed here and I will summarize in the submission thread. Below is my review response.

Overall the cdcam library looks nicely put together. My suggestions are mainly to do with documentation, and particularly with providing a working real-world example (even if substantially simplified) of analysis using the library, which I think would help new users understand how the library can be used in their own work.

Thank you for the cleanliness of the library code, it was a pleasure to review.

Code and Tests

No issues with the code and test suite.

Installation

It may be useful to include non-conda instructions for linux users who do not use anaconda. I used something like the following:

python -m venv cdcam
cdcam/bin/python cdcam/bin/pip install -r requirements.txt
cdcam/bin/python cdcam/bin/pip install -r requirements-dev.txt
cdcam/bin/python cdcam/bin/pip install cdcam
cdcam/bin/python cdcam/bin/pip install pytest pytest-cov
pytest --cov-report=term --cov=cdcam tests/

Example

Review guidelines state

The authors should include examples of how to use the software (ideally to solve real-world analysis problems).

The provided example in scripts/run.py generates a series of projections, but it would be helpful to have context in the Getting Started documentation about how these projections should be interpreted and what they contain. Currently run.py prints the results to the terminal for several categories. A discussion of what the categories are and what the outputs (i.e. "Service" and "Demand") mean would be helpful.

Alternatively, a more complete use case that demonstrates a question to be answered with the data at hand, the development of the model that is currently encapsulated in scripts/run.py, and a discussion of what the output of the model implies for the question would be a great help for new users. From the cited articles in this repository's paper I suspect there already exists an excellent example that could be stripped down and would work well.

This suggestion is different from what is currently in "Getting Started", which walks the user through the different parameters that need to be chosen. This is helpful, but I think would be more powerful and improve understanding if it was within the context of a real-world applicable use case that answers a question.

Community Guidelines

Part of review criteria indicate the need for

clear guidelines for third-parties wishing to:

• Contribute to the software
• Report issues or problems with the software
• Seek support

At the moment there is no information for these in the README or software documentation.

Minor suggestions

• Some publicly facing methods lack examples in their documentation.

I'm not sure if there is a workaround for this problem, but any model simulation using the provided database files in the documentation does not work without the 'daytime_employment.csv' file.