joss-reviews icon indicating copy to clipboard operation
joss-reviews copied to clipboard

[REVIEW]: TessPy: a python package for geographical tessellation

Open editorialbot opened this issue 3 years ago • 24 comments

Submitting author: @siavash-saki (Siavash Saki) Repository: https://github.com/siavash-saki/tesspy Branch with paper.md (empty if default branch): Version: v0.1.0 Editor: @martinfleis Reviewers: @jGaboardi, @BenjMy Archive: Pending

Status

status

Status badge code:

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

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) by leaving comments 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

@jGaboardi, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @martinfleis know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @jGaboardi

📝 Checklist for @BenjMy

editorialbot avatar Jul 28 '22 14:07 editorialbot

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

editorialbot avatar Jul 28 '22 14:07 editorialbot

Software report:

github.com/AlDanial/cloc v 1.88  T=1.35 s (52.4 files/s, 130725.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
JSON                             3              0              0         141583
JavaScript                      13           2405           2470           9222
HTML                            19            885             57           8162
SVG                              1              0              0           2671
Python                           9            379            517           1232
CSS                              4            191             35            759
Jupyter Notebook                 9              0           5203            620
Markdown                         3             63              0            153
TeX                              1             18              0            150
reStructuredText                 6             72             44             79
DOS Batch                        1              8              1             26
YAML                             1              1              4             18
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            71           4026           8338         164684
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

editorialbot avatar Jul 28 '22 14:07 editorialbot

Wordcount for paper.md is 1110

editorialbot avatar Jul 28 '22 14:07 editorialbot

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.tra.2016.01.014 is OK
- 10.3390/rs12020229 is OK
- 10.21105/joss.00215 is OK
- 10.21105/joss.01807 is OK
- 10.48718/7jjr-1c66 is OK
- 10.5281/zenodo.3483425 is OK
- 10.1080/10095020.2016.1146440 is OK
- 10.1016/s0022-1694(00)00144-x is OK
- 10.1016/j.compenvurbsys.2007.11.002 is OK
- 10.1145/2030112.2030126 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot avatar Jul 28 '22 14:07 editorialbot

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot avatar Jul 28 '22 14:07 editorialbot

@editorialbot add @BenjMy as reviewer

martinfleis avatar Jul 28 '22 14:07 martinfleis

@BenjMy added to the reviewers list!

editorialbot avatar Jul 28 '22 14:07 editorialbot

👋🏼 @siavash-saki, @jgaboardi, @benjmy, this is the review thread for the paper. All of our communications will happen here from now on.

All reviewers should create checklists with the JOSS requirements using the command @editorialbot generate my checklist. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues (and small pull requests if needed) on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/4620 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks (but considering the summer we can wait a bit longer), feel free to start whenever it works for you. Please let me know if any of you require significantly more time. We can also use editorialbot to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@martinfleis) if you have any questions/concerns.

martinfleis avatar Jul 28 '22 14:07 martinfleis

Review checklist for @jGaboardi

Conflict of interest

  • [x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the https://github.com/siavash-saki/tesspy?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@siavash-saki) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • [x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

jGaboardi avatar Jul 28 '22 15:07 jGaboardi

Review checklist for @BenjMy

Conflict of interest

  • [x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the https://github.com/siavash-saki/tesspy?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@siavash-saki) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • [x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

BenjMy avatar Jul 29 '22 07:07 BenjMy

@siavash-saki @martinfleis

I have completed an initial pass for tesspy.

This package has the potential to make an immediate impact in the scientific & spatial/geographic communities. There are thoughtful and well-paced tutorial notebooks that provide high-level introductions and more detailed examples for the various tessellation methods offered in tesspy. I am excited both for the potential of the package in the long run and possibly to use it in the immediate future. (Also, very cool logo).

Below are some of my initials thoughts from the checklist, etc. that I will begin creating issues for and linking back to this comment.

Checklist topics

  • General Checks
    • Contribution and authorship
      • Unclear what role Tobias Hagen has played in the code development (no commits).
      • The repo only 2 PRs; Is there a previous version that is private?
  • Functionality
    • Installation
      • Install proceeds as planned (on conda-forge). However, tests can't be run out of the box. There needs to testing requirements. What do the authors use to run testing? Maybe pytest?
    • Functionality
      • Tests do not pass locally for me (see below). I need to see all tests passing before digging deeper into functionality.
  • Documentation
    • A statement of need
      • The authors define the problem being solved in the docs/README but don't identify the target audience.
    • Functionality documentation
      • siavash-saki/tesspy#8
    • Automated tests
      • There are tests but they are failing for me locally. I installed following the conda instructions.
      • See siavash-saki/tesspy#6 for more information.
    • Community guidelines?
      • siavash-saki/tesspy#7
  • Software paper
    • Overall it is good start, but reads a bit choppy. There are frequent examples of short sentences that can be combined for readability and clarity. To reiterate, the paper is a good place as a starting point, but should be fleshed out and smoothed.
    • siavash-saki/tesspy#12
    • siavash-saki/tesspy#13
  • Misc. consider implementing these before making more substantive changes
    • siavash-saki/tesspy#4
    • siavash-saki/tesspy#5

jGaboardi avatar Jul 31 '22 02:07 jGaboardi

@jGaboardi

Thank you for carefully reviewing our code and manuscript. We are pleased by your enthusiasm for tesspy and thank you for the insightful comments. We could definitely improve tesspy according to the issues that you opened.

I can answer some of your general questions here:

  • Unclear what role Tobias Hagen has played in the code development (no commits).

Tobias Hagen is the project manager of the research project. He has advised and supervised the other two authors along the entire process of code development. His contribution to the TessPy package is raising the research question, pointing out the need for code development, supporting in finding conceptual solutions, reviewing and commenting on ideas and tasks, testing the code on real-world data and checking the usefulness of the code in concrete applications.

  • The repo only 2 PRs; Is there a previous version that is private?

The initial version was developed for some parts locally. After that, there were a lot of commits directly to the master branch. But we recently followed the best practice to create, review, and merge Prs.

  • Install proceeds as planned (on conda-forge). However, tests can't be run out of the box. There needs to testing requirements. What do the authors use to run testing? Maybe pytest?

We have used pytest.

  • Tests do not pass locally for me (see below). I need to see all tests passing before digging deeper into functionality.

We will review the tests and make the necessary changes to ensure they all pass.

Regarding Documentation, Software paper, and Misc: We will work on the issues in the next days and incorporate the points you mentioned.

siavash-saki avatar Aug 02 '22 15:08 siavash-saki

@siavash-saki @martinfleis

I have completed an initial pass for tesspy (using the dev environment, on ubuntu 20.04).

Coming from the Geosciences community, I didn't know about Tesselation and appreciated the opportunity to read and review this paper and the code. I found the paper a little bit technical but it reads quite well (see my minor comments below). As for the code, I found some minor issues also listed below. As the authors state in the manuscript that tesspy may be used to "find the most suitable method" my main concern for both code/article is the lack of metrics/explanation on how to evaluate the quality of the Tesselation results.

I thank in advance the authors for their response and hope to see the manuscript published in JOSS soon.

Checklist topics

  • Functionality

    • Installation As the Conda package is not up to date I had to go through a dev environment in order to be able to run the examples:

      • How do you plan to release on Conda? If not using CI, I recommend adding instructions on how to install the development version using setup.py in the README directly (not only in the documentation)
      • Also there is a need to add instructions on how to install notebook examples dependencies and how to run them using jupyter-notebooks. This was not very intuitive to me in particular because there are both yaml and requirements.txt files in the example folder.
      • Notebooks installation returns errors due to the wrong syntax in requirement.txt (check this also for the tutorials_env.yaml ): see https://github.com/siavash-saki/tesspy/issues/37
    • Functionality

      • No issue during the execution of the notebooks except for 01_Getting_Started due to the already raised issue https://github.com/siavash-saki/tesspy/issues/30#issue-1331166797
      • I successfully tested tesspy for my hometown city. It was really interesting and easy to get results. I appreciated that data are automatically fetched using API from OpenStreetMap.
  • Documentation

    • Functionality documentation As the authors state in the manuscript that tesspy may be used to "find the most suitable method", I wonder if the code includes a way to evaluate this. Is there existing metrics for that purpose? as it is done for meshing (i.e. aspect ratio, skewness, orthogonality, and smoothness).
    • Automated tests I run the tests locally, and had the same report than the authors using their Github action tests_package.yml i.e. 27 passed and 22 warnings (not attributed to tesspy) with a significant coverage.
  • Software paper

    • I have a couple of questions/ minor issues regarding the paper.

      • L.44: Is there a proxy to evaluate the quality of the different tesselation (or unit) with respect to a given topology?
      • Fig. 3 right: I don't understand what the 'spatial lag' analysis consists in. Can you say a word about it in the text? is it the result spatial autocorrelation of fig3 left?
      • L.59/60: You state "to easily find the most suitable method" based on what? see my comment for L.44.
    • State of the field

      • How do you position tesspycompare to this package
      • Should you also include references to other Open-Source packages such as Tess in R (?)

BenjMy avatar Aug 11 '22 08:08 BenjMy

@BenjMy

Thank you very much for putting time and effort into reviewing tesspy. Each and every comment of yours is genuinely improving this package, as we've seen during the past few days.

I created issues for the tasks you mentioned. They will be addressed in the next few days. I can also answer some of your general questions here:

  • Installation:
    • We currently use CI to release on PyPI but manually publish on conda. The v0.1.1 came just a few hours ago on conda-forge. But it would be a good idea to also consider CI for releasing on conda. Anyway, an instruction about setting up a dev version on Readme is a good idea. I'll do it.
    • An instruction is added to the "01_Getting_Started" notebook, which explains how to install requirements and how to run example notebooks.
    • The confusion about the syntax error is also explained and added to the "01_Getting_Started" notebook.
  • Functionality:
    • Import error is fixed.
  • Paper:
    • About your question: Based on what we can find the best tessellation method? This is a very complex and interesting research question. Unfortunately, there is not a short answer to this. There isn't any global metric for all use cases to demonstrate the best tessellation. We are actually publishing a paper with the same research question in a few weeks. In that paper, we choose a specific use case and dive deep into comparing different tessellation methods and their effects on the spatial analyses. For example, we show how different methods alter the underlying distribution of features and how it could affect our final results. However, I'm afraid that would be too technical in the context of tesspy. Tesspy aims to provide a simple and easy-to-use spatial framework to explore different tessellation variations and discover what works bests using experiments or try-and-error. In addition, the more advanced users can do more specific statistical analyses based on their dataset and use cases to find the proper tessellation method.
    • I'll add a few words about the fig3 and spatial lag.

siavash-saki avatar Aug 12 '22 14:08 siavash-saki

The v0.1.1 came just a few hours ago on conda-forge. But it would be a good idea to also consider CI for releasing on conda

conda-forge has a bot that does that already, it just usually needs a couple of hours to pick a new release from PyPI.

martinfleis avatar Aug 12 '22 15:08 martinfleis

@martinfleis Oh thanks for the hint. Good to know :)

siavash-saki avatar Aug 12 '22 15:08 siavash-saki

@siavash-saki @martinfleis as a heads up, I will be mostly off the grid from 08/19-09/05. If there are any edits/PRs to review before then I can do them tomorrow or Thursday, otherwise it will be after I'm back in September.

jGaboardi avatar Aug 16 '22 23:08 jGaboardi

@jGaboardi Thanks for the info. We actually addressed all the JOSS-related issues. I'd appreciate it if you could look over the revised paper.

siavash-saki avatar Aug 17 '22 09:08 siavash-saki

@editorialbot commands

jGaboardi avatar Aug 17 '22 15:08 jGaboardi

Hello @jGaboardi, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Check the references of the paper for missing DOIs
@editorialbot check references

# Perform checks on the repository
@editorialbot check repository

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Generates the pdf paper
@editorialbot generate pdf

# Get a link to the complete list of reviewers
@editorialbot list reviewers

editorialbot avatar Aug 17 '22 15:08 editorialbot

@editorialbot check references

jGaboardi avatar Aug 17 '22 15:08 jGaboardi

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.tra.2016.01.014 is OK
- 10.3390/rs12020229 is OK
- 10.21105/joss.00215 is OK
- 10.1016/j.compenvurbsys.2017.05.004 is OK
- 10.5281/zenodo.5139815 is OK
- 10.21105/joss.01807 is OK
- 10.48718/7jjr-1c66 is OK
- 10.21105/joss.00205 is OK
- 10.5281/zenodo.3483425 is OK
- 10.5281/zenodo.6326050 is OK
- 10.1080/10095020.2016.1146440 is OK
- 10.1016/s0022-1694(00)00144-x is OK
- 10.5281/zenodo.6946292 is OK
- 10.52324/001c.8285 is OK
- 10.1111/gean.12276 is OK
- 10.21105/joss.03092 is OK
- 10.32614/RJ-2018-009 is OK
- 10.18637/jss.v012.i06 is OK
- 10.25436/E2Z59N is OK
- 10.1016/j.compenvurbsys.2007.11.002 is OK
- 10.1145/2030112.2030126 is OK

MISSING DOIs

- Errored finding suggestions for "Shapely/shapely: Manipulation and analysis of geom...", please try later

INVALID DOIs

- https://doi.org/10.1016/j.softx.2018.12.005 is INVALID because of 'https://doi.org/' prefix

editorialbot avatar Aug 17 '22 15:08 editorialbot

@editorialbot generate pdf

jGaboardi avatar Aug 17 '22 15:08 jGaboardi

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot avatar Aug 17 '22 15:08 editorialbot

@editorialbot check references

jGaboardi avatar Aug 18 '22 12:08 jGaboardi

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.tra.2016.01.014 is OK
- 10.3390/rs12020229 is OK
- 10.1016/j.softx.2018.12.005 is OK
- 10.21105/joss.00215 is OK
- 10.1016/j.compenvurbsys.2017.05.004 is OK
- 10.5281/zenodo.5139815 is OK
- 10.21105/joss.01807 is OK
- 10.48718/7jjr-1c66 is OK
- 10.21105/joss.00205 is OK
- 10.5281/zenodo.3483425 is OK
- 10.5281/zenodo.6326050 is OK
- 10.1080/10095020.2016.1146440 is OK
- 10.1016/s0022-1694(00)00144-x is OK
- 10.5281/zenodo.6946292 is OK
- 10.52324/001c.8285 is OK
- 10.1111/gean.12276 is OK
- 10.21105/joss.03092 is OK
- 10.32614/RJ-2018-009 is OK
- 10.18637/jss.v012.i06 is OK
- 10.25436/E2Z59N is OK
- 10.1016/j.compenvurbsys.2007.11.002 is OK
- 10.1145/2030112.2030126 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot avatar Aug 18 '22 12:08 editorialbot

@editorialbot generate pdf

jGaboardi avatar Aug 18 '22 12:08 jGaboardi

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot avatar Aug 18 '22 12:08 editorialbot

@martinfleis The authors have addressed my concerns and have met all criteria in the checklist. @siavash-saki Well done.

jGaboardi avatar Aug 18 '22 12:08 jGaboardi

@siavash-saki Thanks for your answers to my questions.

@martinfleis I've read the revised manuscript and I'm satisfied with the changes. All the criteria from the checklist are met. For me it is ready for publication.

All the best to TessPy and the dev team!

BenjMy avatar Aug 18 '22 15:08 BenjMy