openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
721 stars 38 forks source link

[REVIEW]: pDEMtools: conveniently search, download, and process ArcticDEM and REMA products #7149

Closed editorialbot closed 1 week ago

editorialbot commented 2 months ago

Submitting author: !--author-handle-->@trchudley<!--end-author-handle-- (Thomas Russell Chudley) Repository: https://github.com/trchudley/pdemtools Branch with paper.md (empty if default branch): Version: 0.8.5 Editor: !--editor-->@AdamRJensen<!--end-editor-- Reviewers: @mn5hk, @maiwinstrup, @adamrjensen Archive: 10.5281/zenodo.13936813

Status

status

Status badge code:

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

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

@mn5hk & @maiwinstrup & @PennyHow, 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 @AdamRJensen 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 @mn5hk

📝 Checklist for @maiwinstrup

📝 Checklist for @AdamRJensen

editorialbot commented 2 months ago

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 commented 2 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1080/13658810802527499 is OK
- 10.5880/icgem.2015.1 is OK
- 10.21105/joss.05298 is OK
- 10.7910/DVN/EBW8UC is OK
- 10.7910/DVN/X7NDNY is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.11080352 is OK
- 10.5067/GMEVBWFLWA7X is OK
- 10.5067/FPSU0V1MWUB6 is OK
- 10.1016/j.isprsjprs.2017.04.019 is OK
- 10.5194/tc-5-271-2011 is OK
- 10.7910/DVN/C98DVS is OK
- 10.7910/DVN/3VDC4W is OK
- 10.21105/joss.04912 is OK
- 10.5194/tc-17-15-2023 is OK
- 10.1002/esp.3290120107 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: RichDEM: Terrain Analysis Software
- No DOI given, and none found for title: rioxarray: geospatial xarray extension powered by ...
- No DOI given, and none found for title: xDEM: Analysis of digital elevation models
- No DOI given, and none found for title: PySTAC

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
editorialbot commented 2 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.04 s (1089.5 files/s, 171131.9 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          13            554            663           1542
Jupyter Notebook                 4              0           2862            338
Markdown                        11            128              0            219
TeX                              1             19              0            166
YAML                             7             30             28            155
TOML                             1              9              0             33
reStructuredText                 5             28             55             27
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            44            780           3616           2515
-------------------------------------------------------------------------------

Commit count by author:

    83  trchudley
     5  Tom Chudley
     2  Ian Howat
     1  Adrien Wehrlé
editorialbot commented 2 months ago

Paper file info:

📄 Wordcount for paper.md is 1163

✅ The paper includes a Statement of need section

editorialbot commented 2 months ago

License info:

✅ License found: MIT License (Valid open source OSI approved license)

editorialbot commented 2 months ago

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

AdamRJensen commented 2 months ago

👋🏼 @mn5hk, @maiwinstrup, @PennyHow thank you so much for being willing to take time to review this submission - personally I think it's a very important job in achieving true open science!

Back to business, this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains 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 pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#REVIEW_NUMBER 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/issues 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. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

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

mn5hk commented 2 months ago

Review checklist for @mn5hk

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

mn5hk commented 2 months ago

Dear editor @AdamRJensen ,

I am done with my review (of the latest version: v.0.8.4), and I recommend the submission be accepted without any additional modifications. Furthermore, I thank the authors for quick clarifications on my minor queries, which have been linked to this Github issue thread.

I have focused my review primarily on the quality of the code functionality and documentation. The code is well-written, with excellent documentation and some practical examples of how this code may be used. The API documentation and community guidelines are satisfactory. Lastly, I appreciate the extensive unit tests and Git workflow automation incorporated into the repository.

The manuscript is satisfactory, with good references to other alternative toolkits. I appreciate the extensive citation of relevant tools and methods throughout the manuscript, repository, and software documentation.

The software is focused on streamlining polar Digital Elevation Models (DEM) download and preprocessing steps. The examples provided in the documentation are good starting points for new users to familiarize themselves with the software and DEM preprocessing. I am of the opinion that the software can make a good contribution in simplifying such preprocessing steps and aiding further advancement of research in the field.

Lastly, as an Early Career Researcher and an aspiring scientific programmer, I thank the editor and the authors for the opportunity to review this submission. I enjoyed testing the software and hope it will be useful in my own future research activities (and many other researchers' work).

Sincerely, Amin

PennyHow commented 2 months ago

Review checklist for @PennyHow

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AdamRJensen commented 1 month ago

👋 @PennyHow @maiwinstrup I hope you'll be able to finish your reviews within the next two weeks.

If you have questions on the review process or need help getting started don't hesitate to reach out 😄

maiwinstrup commented 1 month ago

Review checklist for @maiwinstrup

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

maiwinstrup commented 4 weeks ago

Review of pDEMtools (v.0.8.4) by Chudley et al

The software enables easy download and preprocessing of the high-resolution polar DEMs data sets from stereo photogrammetry (ArcticDEM and REMA). Very nice work, and well-documented. Most of my remarks/issues are minor, and should be easy to fix. They primarily concern making the software (even) more user-friendly.

A potential bug has been submitted as a github issue, along with a few requested clarifications on the code and documentation. Further, there are a few typos in the notebooks and the API documentation (I have not corrected these).

The accompanying software paper is well-written, and explains well the merits of the product. One comment: Please explain the difference between a dynamic STAC and a ‘normal’ STAC (L31), and the abbreviation RESTful (L31).

After fixing these, and accounting for the remarks below, I recommend the acceptance of the submission.

Installation: Nice introduction in the ‘readthedocs’, ensuring easy download and install that (mostly) works out of the box. I did, however encounter some issues regarding e.g., clarification on supplementary data download and file location (issue raised on github)

I enjoyed following the examples in the notebooks, which are provided as part of the source code. The same examples are provided as part of the readthedocs. I am, however, missing a reference to the notebooks in the “readthedocs”, to ensure that users reading this document are aware that the notebooks exist.

Functionality: I have tested the main functionalities, primarily based on the examples provided in the notebooks. Everything seems to work as it should, except for a potential bug when searching for strip DEMs in areas mostly covered by ocean (issue raised on github).

The flow_aware_hillshade notebook is written in a slightly different style from the other notebooks. I suggest to revise it to better align with the others.

Disclaimer: I have only tested the code for Greenland (ArcticDEM), not for Antarctica.

Performance: No claims given

AdamRJensen commented 4 weeks ago

@editorialbot remove @PennyHow from reviewers

editorialbot commented 4 weeks ago

@PennyHow removed from the reviewers list!

PennyHow commented 4 weeks ago

Hi @AdamRJensen, sorry for my delay in this review. Settling back into office work after fieldwork has been a little bumpy this time. I will have time to review in October if you would like me to, but understand if you want to keep things moving with the two existing reviewers.

AdamRJensen commented 4 weeks ago

Hi @AdamRJensen, sorry for my delay in this review. Settling back into office work after fieldwork has been a little bumpy this time. I will have time to review in October if you would like me to, but understand if you want to keep things moving with the two existing reviewers.

No worries @PennyHow, I'd say save your efforts for another review given that we're close to having this completed.

Thanks for being willing to review though ❤️

trchudley commented 3 weeks ago

Thanks @maiwinstrup for such a detailed and thorough review! We really appreciate the time you've taken on this.

We have outlined our responses below and pushed all the changes to the github. Please note that we would appreciate some clarification for this issue https://github.com/trchudley/pdemtools/issues/21.

The updates to the documentation should filter through to the website within hours. Once @AdamRJensen and yourself are satisfied with our responses and that pdemtools is suitable for publication in JOSS, we are prepared to push the minor changes to pip and conda as a new version (0.8.5) alongside/prior to publication.

Responses

The software enables easy download and preprocessing of the high-resolution polar DEMs data sets from stereo photogrammetry (ArcticDEM and REMA). Very nice work, and well-documented. Most of my remarks/issues are minor, and should be easy to fix. They primarily concern making the software (even) more user-friendly.

We are grateful for your kind words and detailed review.

A potential bug has been submitted as a github issue, along with a few requested clarifications on the code and documentation.

Action:

Further, there are a few typos in the notebooks and the API documentation (I have not corrected these).

Action: We have gone through the notebooks and attempted to correct typos where visible.

The accompanying software paper is well-written, and explains well the merits of the product. One comment: Please explain the difference between a dynamic STAC and a ‘normal’ STAC (L31), and the abbreviation RESTful (L31).

The Spatio Temporal Asset Catalog specification is a standardised way of storing geospatial data as a network of JSON files (see here for a good intro). A 'static' catalog is simply a collection of JSON can be viewed on file servers or the cloud. A dynamic STAC can be queried over the internet in real time, by asking via HTML in a specific way (an API interface known as Representational State Transfer, or REST). Tools such as pystac allows the user to ask for, and retreive, data from the dynamic STAC.

Action: We have updated the software paper to clarify the meaning of static and dynamic, and we have removed the reference to a RESTful API as, given the target audience of our software and the intended high-level overview of the manuscript, it is not necessary to understand this complexity here.

After fixing these, and accounting for the remarks below, I recommend the acceptance of the submission.

Installation:

Nice introduction in the ‘readthedocs’, ensuring easy download and install that (mostly) works out of the box. I did, however encounter some issues regarding e.g., clarification on supplementary data download and file location (issue raised on github).

We have responded to the supplementary data download queries by updating the relevant page in the documentation, described in detail on the issue (https://github.com/trchudley/pdemtools/issues/17).

I enjoyed following the examples in the notebooks, which are provided as part of the source code. The same examples are provided as part of the readthedocs. I am, however, missing a reference to the notebooks in the “readthedocs”, to ensure that users reading this document are aware that the notebooks exist.

Action: We have added the relevant GitHub link to the top of every notebook, so that they can be found from the readthedocs (and whereever else they end up).

Functionality:

I have tested the main functionalities, primarily based on the examples provided in the notebooks. Everything seems to work as it should, except for a potential bug when searching for strip DEMs in areas mostly covered by ocean (issue raised on github).

As mentioned above, please note that we would appreciate some clarification for this issue https://github.com/trchudley/pdemtools/issues/21.

The flow_aware_hillshade notebook is written in a slightly different style from the other notebooks. I suggest to revise it to better align with the others.

We have previously discussed the flow_aware_hillshade notebook with Reviewer 1 here. In brief, it is a work-in-progress that I have made it available on the GitHub to share with a student who is working on this problem without 'publicising' it on the website. This may explain why the tone is different, why it uses some further datasets without detailed introduction, and why it introduces complex fucntions that are not wrapped within pdemtools.

Action:

trchudley commented 3 weeks ago

@editorialbot generate pdf

editorialbot commented 3 weeks ago

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

maiwinstrup commented 3 weeks ago

The authors have satisfactory responded to my comments and suggested changes, and I happily recommend the acceptance of the submission. I have enjoyed trying out pDEMtool, and foresee ways that I may be using it in the future.

trchudley commented 3 weeks ago

Thanks to both @maiwinstrup and @mn5hk, it's been a very enjoyable and helpful review process and we're very grateful for your time!

trchudley commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

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

trchudley commented 2 weeks ago

Hi @AdamRJensen , I've had one more look over the manuscript and fixed a couple typos. I think the ball is now in your court (at your convenience of course!) but please do let us know if there’s anything more we can do.

AdamRJensen commented 2 weeks ago

@editorialbot add @adamrjensen as reviewer

editorialbot commented 2 weeks ago

@adamrjensen added to the reviewers list!

AdamRJensen commented 2 weeks ago

Review checklist for @AdamRJensen

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AdamRJensen commented 2 weeks ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

AdamRJensen commented 2 weeks ago

@editorialbot check references

editorialbot commented 2 weeks ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1080/13658810802527499 is OK
- 10.5880/icgem.2015.1 is OK
- 10.21105/joss.05298 is OK
- 10.7910/DVN/EBW8UC is OK
- 10.7910/DVN/X7NDNY is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.11080352 is OK
- 10.5067/GMEVBWFLWA7X is OK
- 10.5067/FPSU0V1MWUB6 is OK
- 10.1016/j.isprsjprs.2017.04.019 is OK
- 10.5194/tc-5-271-2011 is OK
- 10.7910/DVN/C98DVS is OK
- 10.7910/DVN/3VDC4W is OK
- 10.21105/joss.04912 is OK
- 10.5194/tc-17-15-2023 is OK
- 10.1002/esp.3290120107 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: RichDEM: Terrain Analysis Software
- No DOI given, and none found for title: rioxarray: geospatial xarray extension powered by ...
- No DOI given, and none found for title: xDEM: Analysis of digital elevation models
- No DOI given, and none found for title: PySTAC

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
trchudley commented 2 weeks ago

Hi @AdamRJensen, I've responded to your last few issues. If you're OK with them (particularly my response to https://github.com/trchudley/pdemtools/issues/38), please let me know here and I will release the post-review version (0.8.5), archive the release on zenodo, and post the link here.

AdamRJensen commented 2 weeks ago

@trchudley Alright, we've made it! Thank you for your patience 🥳

Please go ahead and make a new release and upload it to Zenodo and write back with the version number and DOI

trchudley commented 2 weeks ago

Thanks @AdamRJensen for your time!

v0.8.5 of pdemtools has been released and is archived on Zenodo at https://doi.org/10.5281/zenodo.13936813

AdamRJensen commented 2 weeks ago

@editorialbot set 10.5281/zenodo.13936813 as archive

editorialbot commented 2 weeks ago

Done! archive is now 10.5281/zenodo.13936813

AdamRJensen commented 2 weeks ago

@editorialbot set 0.8.5 as version

editorialbot commented 2 weeks ago

Done! version is now 0.8.5

AdamRJensen commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

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

AdamRJensen commented 2 weeks ago

@editorialbot recommend-accept

editorialbot commented 2 weeks ago
Attempting dry run of processing paper acceptance...
editorialbot commented 2 weeks ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.1080/13658810802527499 is OK
- 10.5880/icgem.2015.1 is OK
- 10.21105/joss.05298 is OK
- 10.7910/DVN/X7NDNY is OK
- 10.7910/DVN/EBW8UC is OK
- 10.5334/jors.148 is OK
- 10.5281/zenodo.11080352 is OK
- 10.5067/GMEVBWFLWA7X is OK
- 10.5067/FPSU0V1MWUB6 is OK
- 10.1016/j.isprsjprs.2017.04.019 is OK
- 10.5194/tc-5-271-2011 is OK
- 10.7910/DVN/C98DVS is OK
- 10.7910/DVN/3VDC4W is OK
- 10.21105/joss.04912 is OK
- 10.5194/tc-17-15-2023 is OK
- 10.1002/esp.3290120107 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: RichDEM: Terrain Analysis Software
- No DOI given, and none found for title: rioxarray: geospatial xarray extension powered by ...
- No DOI given, and none found for title: xDEM: Analysis of digital elevation models
- No DOI given, and none found for title: PySTAC

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None
editorialbot commented 2 weeks ago

:wave: @openjournals/ese-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/6002, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

trchudley commented 1 week ago

@AdamRJensen - just checking in that this isn't waiting for me to take any direct action? I read it as not but just making sure...

AdamRJensen commented 1 week ago

I believe it's waiting for @openjournals/ese-eics

kthyng commented 1 week ago

Hi! I'll take over now as Track Associate Editor in Chief to do some final submission editing checks. After these checks are complete, I will publish your submission! (Sorry I've been traveling)

kthyng commented 1 week ago

Please check the capitalization in your references. You can preserve capitalization by placing {} around characters/words in your .bib file (such as "toulouse" but please check for others)

trchudley commented 1 week ago

@editorialbot generate pdf

editorialbot commented 1 week ago

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