[REVIEW]: GemGIS - Spatial Data Processing for Geomodeling

whedon commented 3 years ago

Submitting author: !--author-handle-->@AlexanderJuestel@crvernon<!--end-editor-- Reviewers: @omshinde, @kanishkan91 Archive: 10.5281/zenodo.6511767

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@omshinde & @kanishkan91, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @crvernon 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 ✨

Review checklist for @omshinde

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

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] Contribution and authorship: Has the submitting author (@AlexanderJuestel) 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 and who the target audience is?
[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?

Review checklist for @kanishkan91

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

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

[x] I confirm that I read and will adhere to the JOSS code of conduct.

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] Contribution and authorship: Has the submitting author (@AlexanderJuestel) 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 and who the target audience is?
[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?

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @omshinde, @kanishkan91 it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

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

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

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

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

@whedon generate pdf

crvernon commented 3 years ago

:wave: @omshinde @kanishkan91 @AlexanderJuestel - the review takes place in this issue.

crvernon commented 3 years ago

Also, please don't forget to add a link to this review issue in any issues or pull requests you may generate in the https://github.com/cgre-aachen/gemgis repository. This will help everyone have a single point of reference.

AlexanderJuestel commented 3 years ago

@crvernon, thanks for assisting with this! @omshinde, @kanishkan91, I am looking forward to go through this review with you guys!

AlexanderJuestel commented 3 years ago

@crvernon is there anything that I can do in the meantime or do we have to wait for @omshinde and @kanishkan91 to start the review?

crvernon commented 3 years ago

Thanks for checking in @AlexanderJuestel - at this point we will wait on the first round of feedback to come in as issues/comments. You can address these as they come up to expedite the process. I'll periodically send out check-in comments that open up lines for us to communicate any blockers to progress.

kanishkan91 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

kanishkan91 commented 3 years ago

@AlexanderJuestel I've started the review. I had some very minor questions for you related to the repository to start with (mostly related to the initial checks),

Firstly, there are two pull requests that are open here. Do these need to be merged in? Just wanted to confirm.
I looked at the recent commit history. It looked like the recent commits were mostly from yourself and @RichardScottOZ. Could you describe the roles that the other co-authors played in the development of the software repository?
Also, in the pydocs, the main developers from the pydocs file - https://gemgis.readthedocs.io/en/latest/getting_started/authors.html don't all appear as co-authors in the paper. Is this intentional? This being a software paper, ideally all major contributors to software development that are mentioned in the docs should be co-authors.
The license you use is GNU LESSER GENERAL PUBLIC LICENSE. My understanding is limited here, but a GPL license would impose some copy left restrictions on the users (they would have to ensure that they release anything that uses GemGIS as a freely available open source software package). Is this intentional? I have seen packages use BSD2 in its place which offers more flexibility to the end user. @crvernon Do you have any thoughts on this?

Once again, I just wanted to post these here since they are related to the initial checks.

AlexanderJuestel commented 3 years ago

@kanishkan91, thanks for getting started on the review. Regarding your comments:

I merged one of the PRs but would like to keep the second one open as we may change some of the tutorial notebooks during the review process here in combination with updating the documentation.
@RichardScottOZ is a very active user providing always great feedback on typos and grammatical errors in the docstrings or notebooks. However, he has not played an active role in the development of the software yet.
Good point. As for @RichardScottOz, @Japhiolite provided great feedback on the code and the documentation, mostly offline, but did not provide any functionality for the software. I would add 'Marius Pischke' and 'Miguel de la Varga' as authors to the documentation and change the status of @RichardScottOz and @Japhiolite.

AlexanderJuestel commented 3 years ago

@kanishkan91 I just fixed the authors in the docs. The current build of the documentation is failing though, so changes are only visible locally right now but let's tackle one thing at a time :)

kanishkan91 commented 3 years ago

@AlexanderJuestel Thanks. For my point no 2 from above, could you describe the roles the different co-authors played in the development of the software? This does not need to be a part of the paper or repo but having those descriptions here in the review thread would suffice.

AlexanderJuestel commented 3 years ago

@kanishkan91, I am the main developer of the software and added most of the functionality of the package. @Endarthur contributed with helpful tips with respect to the implementation of certain functionality. He basically helped me to make things run. Marius Pischke developed with me functionality within the framework of his master thesis at RWTH Aachen University, which I then implemented in GemGIS as he does not have a Github account. @Leguark is the main developer for the GemPy package (https://github.com/cgre-aachen/gempy). Without his developments, GemGIS would not have a purpose. He also contributed minor things to GemGIS as well. @flohorovicic is/was my supervisor who highly encouraged me to develop GemGIS. @Japhiolite and @RichardScottOz mainly provided feedback on the documentation and notebooks and also provided feedback on other communication channels.

Would this description be sufficient or do you need any more information or statements of the respective persons?

kanishkan91 commented 3 years ago

@AlexanderJuestel This is sufficient. Thanks.

crvernon commented 3 years ago

:mega: Mid-week rally! Just checking in to see how things are going.

:open_hands: @kanishkan91 thanks for kicking off this review!

@omshinde how are things going? The author @AlexanderJuestel has been very responsive if you have any questions.

Have a great and productive rest of the week!

whedon commented 3 years ago

:wave: @omshinde, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @kanishkan91, please update us on how your review is going (this is an automated reminder).

omshinde commented 3 years ago

@crvernon Thanks for the reminder. I just finished my other JOSS review and would be able to progress fast with GemGIS submission now. I am very excited and looking forward.

crvernon commented 2 years ago

:mega: Hope all is progressing smoothly.

:wave: @kanishkan91 and @omshinde could you update me on your progress? Thanks!

omshinde commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

kanishkan91 commented 2 years ago

@crvernon I am going through the review. I hope to complete it by this weekend or latest by the 6th of October. Let me know what you think.

crvernon commented 2 years ago

Sounds great @kanishkan91 - thanks for the update!

crvernon commented 2 years ago

:wave: @omshinde - I'm just checking to see how thing are progressing with your review. Don't hesitate to reach out if you have any questions!

❗ As a reminder @kanishkan91 and @omshinde - don't forget to:

reference the link to this review thread in any issues you create on https://github.com/cgre-aachen/gemgis
check off your review checklist items at the top of this thread as you make progress

🤝 These things help me ensure that we are on track for a timely and comprehensive review. Keep up the good work!

omshinde commented 2 years ago

Hi @crvernon Thanks for the reminder. I have commenced my review and I plan to complete it over the weekend. Kind regards.

kanishkan91 commented 2 years ago

Hi @crvernon. My apologies. I had to complete some sudden deadlines. I just jumped back to the review today. Will try to complete this week.

kanishkan91 commented 2 years ago

@AlexanderJuestel I am going through this review and am adding comments as I go through. I hope to be done by Sunday night. But there is one particular feature that I think would highlight the utility of what you have built much more. Also considering the functionality already built, I think this would be low hanging fruit. I have created an issue already but I add the text from the issue below since I think we should start a discussion on this sooner rather than later.

gemgis is clearly a powerful tool that creates data objects as would be required by GemPy. In particular I like how a user can not only make use of gemgis to create a data object but can also make use of this software to do meaningful post processing of GemPy outputs. But, these functionalities can be very effectively illustrated using a dashboard (I would recommend Dash -https://dash.plotly.com/).

In particular Example 1 can be presented using a dashboard as opposed to the text heavy example as it stands right now. As the repo currently stands it is very much usable but not entirely digestible for novice users (the ones who would most benefit from something like this given that they would find GemPy intimidating.) Having that dashboard would effectively illustrate the utility of this software (which is very high). But more importantly, it would help a novice user understand the functionality much better and would help them effectively use this software. The coding for this would not be too heavy I think given that your functions are already extracting a bunch of results related to this.

In particular, it is important to highlight that this is not just a data processing or post processing software. It does bridge the gap between regular exploratory software like QGIS and modeling software like GemPy (as you mention in the paper). A dashboard would communicate this more effectively.

@crvernon, @omshinde - Please feel free to weigh in on this.

AlexanderJuestel commented 2 years ago

@kanishkan91 thanks for the comments so far! I hope to get back to them at the beginning of next week!

kanishkan91 commented 2 years ago

@AlexanderJuestel I just finished the review of the repository. I have added all my comments for the same. I will also summarize my review below. I will post some additional comments on the paper (I will be going through the same shortly).

Summary- gemgis is clearly a powerful software that bridges the gap between the nebulous world of GIS data processing (traditionally performed using ArcGis and QGIS) and the complex world of geomodelling (performed using GemPy). Thus it creates an effective environment for GIS processing and modelling. The software writing is mostly rock solid and most of my comments are related to better documentation and the testing framework.

But, I request some new features be added both to the documentation and the software before publication. This is because as the repository currently stands, it gives the impression that this is a software that is concerned with data processing and post processing. But it is actually much more than that. This software creates an environment for GIS data processing and geomodelling that will be very beneficial to the community. I think this needs to be highlighted more.

One of the bigger changes I request is the creation of an interactive dashboard (in python dash) that allows the user to see utility of the functions. Example 1 already has all the components that would be required. For example, show the various plots as outputs and show the various parameters that the plot uses as input boxes. The user can thus see the power of the software by changing parameter values. The functions in this software are already built for this purpose. So creating the dashboard would be an inexpensive (effort wise) way of highlighting the utility of this software. For now the dashboard can be invoked using a function but in the future (not as a part of this publication!) this can also be hosted on a remote server (heroku or AWS). This is my biggest comment related to this repository.

Summary of other comments

I recommend that the testing framework also be implemented for github actions by adding the relevant yml files. This would be a good practice with future development in mind. I also recommend the addition of a Coverage.py file to this repository (see relevant comment) so that future developers can see test coverage. This will be important as more users contribute to this software.
See my comments on the readme. There are some additions that are required (contribution instructions) and a number of things that can be removed. Also add a link to a vignette (jupyter notebook with the example) in the readme, so the users don't have to fish around in the documentation.
The attributes of the gemPy data class need to be documented upfront. I assume that these will also change in the future as GemPy evolves. Knowing what the current attributes are would be very beneficial to the future developers and to the end users.
I have added some comments on the documentation itself. In my opinion, this documentation is great for very technical users but should be modified to highlight the utility of this software better.

I think those are all the comments I had. I hope these are useful. Let me know if you require clarifications on any of the above or any of the issues.

(PS - This is great piece of software with a lot of attention to detail. With the above changes or appropriate justifications, I would be more than happy to approve this for publication!)

@crvernon Let me know i you need anything else from my side.

crvernon commented 2 years ago

:wave: @kanishkan91 - thanks so much for your thoughtful review and feedback! I am sure @AlexanderJuestel will find much value in these. As far as additional features go that do not directly address the requirements of the reviewer checklist, we generally do not require these; however, the author may find that these suggestions strengthen the submission and choose to implement them. Again, thanks for caring about the quality of your review! Nice work!

AlexanderJuestel commented 2 years ago

Hello @kanishkan91,

Thank you very much for your detailed comments! You made some really good points that I would like to integrate! I will go through them when I have time these days. I would also appreciate a little discussion on some issues that you opened on how to implement features the best way.

Cheers Alex

crvernon commented 2 years ago

:mega: Mid-week rally! Looks like @AlexanderJuestel and @kanishkan91 have a good dialogue going and are keeping the review progressing.

:wave: @omshinde - Just checking in to see how things are going? Let me know if you have any questions.

❗ @kanishkan91 and @omshinde - Don't forget to keep you checklist up-to-date as you work through the review. Thanks!

:clap: Keep up the good work!

omshinde commented 2 years ago

Hi @crvernon Thanks for the reminder. I had a slow progress this week due to some inevitable academic commitments. I plan to turn in my review positively by this weekend. Regards, Rajat

omshinde commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

omshinde commented 2 years ago

@crvernon, @AlexanderJuestel

Review 1: Thanks to the authors for their contributions. Please find my queries and suggestions below regarding the submission - "GemGIS: Spatial Data Processing for Geomodeling".

Working Environment: OS - Ubuntu 20.04.2 LTS Python 3.8.10

Submission:

The authors propose GemGIS, a Python-based open-source geographic information processing library capable of preprocessing spatial data including web based services such as WMS, WFS and WCS. The preprocessed data can be stored in a data model and can be subsequently used for further processing to use with GIS software viz. QGIS, ArcGIS etc.

Functionality

The installation instructions works perfectly and are easy to follow. The tutorials cover the details and contains step-by-step instructions to use the GemGIS library.

Documentation

The examples are very well defined and comprehensive for new users interested in using GemGIS package. I have following queries/suggestions associated with this section -

Functionality documentation: The links to the API references are not working or only headings are mentioned - (GemGIS Data Object or Working with vector data). Please correct me if I am missing something here.
Automated tests: Although there is a section describing the process to check the installation in the documentation, but automated tests are not defined. https://gemgis.readthedocs.io/en/latest/getting_started/installation.html#checking-the-installation. Meanwhile I could find the description for automated tests on the project Readme so I would suggest authors to replicate this page on the documentation too.
Community guidelines: I could not find community guidelines in the documentation. But similar to tests, the authors could replicate the guidelines from the Readme as mentioned here.

Software paper

The software paper seems to be short and here, I agree to the comments of the fellow reviewer that there is a scope of adding more details to the software paper (maybe adding one of the example tutorials as a case study of the proposed package). But this should be the authors judgement to extend the paper or not, as there is no such limit/rule.

I have following suggestions for this section:

The section titled Statement of need is missing in the software paper. I would suggest to take highlighting points from the GemGIS Target Audience and Background section and add it as bullet points in the Statement of need to give a crisp understanding to the readers.
The heading References is missing from the software paper and after Acknowledgments, directly the list of referenced articles are mentioned. Please correct me if I am missing anything here.
There is a typo in the paper at line 78, GemGIS Outlook. graviational -> gravitational It is recommended to comprehensively check the paper for typos.

GemGIS is a powerful software package and I foresee immense utility for analysing and processing GIS data. I sincerely congratulate the authors for great piece of work. I would recommend the authors to address the minor issues mentioned above and enhance the software paper for including more details or examples as it would really help the users and readers.

Please let me know if any further clarifications are required from my side. Best, Rajat

AlexanderJuestel commented 2 years ago

Dear all, Thanks for your comments. I will get back to them as soon as possible! :)

crvernon commented 2 years ago

:mega: Mid-week rally! Great work everyone! It looks like all is moving along well here.

:wave: @kanishkan91 and @omshinde don't forget to update your review checkboxes as @AlexanderJuestel satisfies your review comments.

:clap: Keep up the good work!

crvernon commented 2 years ago

:mega: Mid-week rally! @AlexanderJuestel @kanishkan91 and @omshinde could you provide a short update as to how things are going? Looks like you are making good progress. Let's keep driving towards getting things in shape for publication!

:clap: Keep up the good work!

crvernon commented 2 years ago

:mega: Mid-week rally!

@kanishkan91 and @omshinde please provide a short update in this issue thread concerning the status of your review. Is there anything you are waiting on? Thanks!

@AlexanderJuestel how are things going on your side of things? Could you provide a short update here as well? I would like to keep this moving while we have good momentum. Thanks!

AlexanderJuestel commented 2 years ago

@crvernon, thanks for checking in! Currently, I am a little limited in time due to my PhD studies. In addition, we are currently working on a new GemPy release so I wanted to wait for that :)

crvernon commented 2 years ago

@kanishkan91 and @omshinde please provide a short update in this issue thread concerning the status of your review. Is there anything you are waiting on? Thanks!

omshinde commented 2 years ago

Hi @crvernon ! Thanks for the reminder. I have submitted my initial review (https://github.com/openjournals/joss-reviews/issues/3709#issuecomment-953245623) and waiting for the authors response. Once they are addressed, I would be happy to proceed with the review.

AlexanderJuestel commented 2 years ago

@crvernon @omshinde We have some crazy days here before the Christmas Holidays. I will get back to the review as soon as I can!

omshinde commented 2 years ago

@AlexanderJuestel I understand. Best wishes! :)

crvernon commented 2 years ago

Thanks for the updates @omshinde and @AlexanderJuestel !

kanishkan91 commented 2 years ago

@crvernon Apologies for the late message. I am also waiting on the author to respond to comments.

@AlexanderJuestel Let me know whenever and I can take a look again (I know its a busy time of year, so no hurry!). By the way its interesting that you mentioned that GemPy is also coming out with a release. This would be an interesting opportunity to see the documentation of the GemPy object in gemgis when its ready. You could even mention in the documentation that this object is compatible with the latest version of GemPy.

crvernon commented 2 years ago

Sounds good @kanishkan91 thanks for the update!

crvernon commented 2 years ago

:wave: @AlexanderJuestel just checking in to see how the responses to the reviewers are going? Thanks!

AlexanderJuestel commented 2 years ago

Same as before @crvernon. I will get back to the requests once I have some more time. Thanks for checking in though

openjournals / joss-reviews