search

openjournals / joss-reviews

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

[REVIEW]: Integrated hydrologic model development and postprocessing for GSFLOW using pyGSFLOW #3852

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: !--author-handle-->@jlarsen-usgs<!--end-author-handle-- (Joshua Larsen) Repository: https://github.com/pygsflow/pygsflow.git Branch with paper.md (empty if default branch): Version: 1.0.2 Editor: !--editor-->@crvernon<!--end-editor-- Reviewers: @thurber, @mdbartos, @mdbartos Archive: 10.5281/zenodo.6468426

: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

Status badge code:

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

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

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

  1. Make sure you're logged in to your GitHub account
  2. 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 @thurber

✨ 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

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @mdbartos

✨ 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

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @thurber, @mdbartos 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:

  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

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

@whedon generate pdf
whedon commented 3 years ago

Wordcount for paper.md is 1529

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1111/gwat.12413 is OK
- 10.5066/F7BK19FH is OK
- 10.1016/j.jhydrol.2016.03.026 is OK
- 10.1016/j.envsoft.2018.07.020 is OK
- 10.3133/tm6A16 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.3233/978-1-61499-649-1-87 is OK
- 10.3133/tm6A55 is OK
- 10.13140/2.1.2741.9202 is OK
- 10.3133/tm6B7 is OK
- 10.5194/gmd-11-4755-2018 is OK
- 10.3133/tm6A37 is OK
- 10.3133/tm6A45 is OK
- 10.1016/J.ENVSOFT.2019.01.006 is OK
- 10.3133/sir20145052 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.26 s (351.2 files/s, 121230.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          60           1985           2648           6741
JSON                             2              7              0           1506
Visual Basic                     4              1              0           1286
Jupyter Notebook                 9              0          15586            586
Markdown                         7            141              0            464
TeX                              1             16              0            231
YAML                             4             19             11            136
XML                              3              0              0             42
reStructuredText                 1              2              0              6
-------------------------------------------------------------------------------
SUM:                            91           2171          18245          10998
-------------------------------------------------------------------------------

Statistical information for the repository '9d3c4197ae169fff697d827c' was
gathered on 2021/10/26.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Ayman Alzraiee                  10           741             51            4.29
Joshua Larsen                  145         13802           3673           94.68
ayman_alzraiee                   3             0             35            0.19
aymanalz                         3           140             11            0.82
jlarsen-usgs                     1             2              2            0.02

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Ayman Alzraiee              479           64.6         28.5                9.19
Joshua Larsen             10812           78.3         11.4                6.45
aymanalz                     83           59.3          9.5                6.02
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:

crvernon commented 3 years ago

:wave: @jlarsen-usgs @thurber @mdbartos - 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/pygsflow/pygsflow repository. This will help everyone have a single point of reference.

crvernon commented 3 years ago

:mega: Mid-week rally! Looks like @thurber has started providing feedback to @jlarsen-usgs. Let's keep things rolling towards a successful review!

:wave: @mdbartos - please update me to your progress thus far and don't hesitate to reach out if you have any questions.

👏 Keep up the good work!

whedon commented 3 years ago

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

whedon commented 3 years ago

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

jlarsen-usgs commented 3 years ago

review from @thurber is linked to

pygsflow/pygsflow#12

crvernon commented 2 years ago

:mega: Mid-week rally! Looks like issues are cycling with @thurber and @jlarsen-usgs. Let's keep things rolling towards a successful review!

:wave: @mdbartos - Please let me know if you are unable to participate in this review at this time. If you are still willing, could you reply here with an expected time of completion? Thanks!

mdbartos commented 2 years ago

@crvernon Can I get permissions to edit my checklist? Thx.

mdbartos commented 2 years ago

Could I get another invite? I still don't have permissions to edit the checklist.

crvernon commented 2 years ago

@whedon add @mdbartos as reviewer

whedon commented 2 years ago

OK, @mdbartos is now a reviewer

crvernon commented 2 years ago

Ok, @mdbartos you should have just received another invite to review this submission. After you accept, you will have access to the checklist. Let me know if you have any further questions.

crvernon commented 2 years ago

@mdbartos were you able to confirm and then access the checklist?

mdbartos commented 2 years ago

Yes, it is working now. Thank you.

thurber commented 2 years ago

hi @crvernon & @jlarsen-usgs, I've completed my review checklist and am satisfied with the resolutions of pygsflow/pygsflow#12. There's one unchecked box remaining, but i don't believe it to be critical path. Nice work!

jlarsen-usgs commented 2 years ago

Thanks @thurber!

I have been waiting to update the text fully until all of the review comments are in (journal reviews and internal USGS). The unchecked box in pygsflow/pygsflow#12 will be addressed in the final version.

crvernon commented 2 years ago

Thanks @thurber !

@mdbartos do you have an expected date that you could have your review completed by? Thanks!

mdbartos commented 2 years ago

I should be able to get through the checklist items this weekend, although there are some issues with the installation depending on which branch/python version I am using.

To be clear, should I be reviewing the develop branch of the current github repo?

mdbartos commented 2 years ago

Greetings, I've completed as much of my review as I can as of right now.

Currently I cannot install the software from the develop branch.

I can install from pypi and from the master branch. However, when I install this way, the unit tests are currently failing.

These issues need to be resolved before I can move further.

Thanks, MDB

jlarsen-usgs commented 2 years ago

@mdbartos

Can you be a little more specific about the current issues you are facing?

What tests fail for you on the master branch? I just checked out a fresh copy of that branch and ran the tests locally and they all pass (Both branches undergo a CI step too, where all tests are run on linux and windows (no MacOS support, as GSFLOW is not currently compiled for MacOS), with multiple versions of python).

What error do you get when trying to install from the development branch? FloPy had significiant changes made to it in their develop branch for the upcoming 3.3.5 release (3.3.5 is not backward compatible). pyGSFLOW's develop branch had to adapt to stay compatible with the upcoming FloPy release. As such, the requirements for installing this branch include Python 3.7 or higher and flopy 3.3.5 (installed from https://github.com/modflowpy/flopy/tree/develop). My guess is that this is the issue you're facing, and I should update the Readme.md to be more specific about installing from the develop branch.

mdbartos commented 2 years ago

Welp, I was trying to install on a mac. So now it makes sense why the unit tests were failing on master.

Which branch does the JOSS release correspond to? Assuming it's develop, would it not make more sense to wait until an official release of flopy 3.3.5 is uploaded to pypi before publishing? Ideally, the installation procedure should be seamless for the end user. Especially if you are formally publishing a piece of software, you don't want users to be manually installing dependencies that are still in development, right?

jlarsen-usgs commented 2 years ago

@mdbartos

Instead of waiting until the next release of FloPy (which is most likely going to be sometime in late February), I went ahead and made some changes to develop that provide support for both FloPy 3.3.4 (current release) and FloPy's 3.3.5 develop version.

crvernon commented 2 years ago

:mega: Mid-week rally! Looks like we are getting close to completing the review! @mdbartos and @jlarsen-usgs please post an update here on the status of what is remaining. Thanks!

mdbartos commented 2 years ago

Greetings,

Got behind due to end-of-the semester madness, but I'm planning to install and run unit tests on my linux box and windows laptop today. If all goes well that should take care of the major outstanding issues.

MDB

mdbartos commented 2 years ago

Greetings,

I think I've finished the major part of my review. My only comments are the following:

Aside from that, I think this is good to go.

Cheers, MDB

crvernon commented 2 years ago

✅ Thanks @mdbartos !

@jlarsen-usgs once you have addressed the reviewer comments let me know and I will confirm that all is in order and make suggestions for any edits that I think are necessary to satisfy journal requirements.

jlarsen-usgs commented 2 years ago

Thanks for your review @mdbartos

I'll be working through the issues that you mentioned in the coming week or so!

crvernon commented 2 years ago

:mega: Mid-week rally!

It would be good to get this one wrapped up soon @jlarsen-usgs so we can free up our reviewers. How are things looking on your side of things for finishing up the remaining issues? It looks like @mdbartos has two boxes left to check off and @thurber has approved all.

Thanks!

jlarsen-usgs commented 2 years ago

Hi @crvernon

I've advanced it to the last step in the USGS Bureau Approval process that goes in conjunction with the journal review. Once I get the approval there I'll be able to check the remaining issues and have the work on my end wrapped up. Thanks for checking in, the final step on my end shouldn't take very long (< 1 week).

crvernon commented 2 years ago

Sounds great @jlarsen-usgs ! Let me know when you are clear and I'll make my final pass and recommendation.

crvernon commented 2 years ago

@jlarsen-usgs - have you gotten the approval and remaining issues straightened out yet? I would like to move this review forward.

Thanks!

jlarsen-usgs commented 2 years ago

@crvernon - Thanks for checking in, I've hit a minor snag and am going through a second USGS software security review. Once this is done, I should be able to more forward. I sincerely appologise for the delays on my end.

mdbartos commented 2 years ago

You can check off my review. Everything looks good to me.

crvernon commented 2 years ago

@jlarsen-usgs - just checking in to see if you are able to move forward with this review. Thanks!

jlarsen-usgs commented 2 years ago

@crvernon We're getting close to having the USGS review process complete and I'm currently expecting this to be able to move forward sometime next week. I'll let you know as soon as things wrap up on our end so we can move forward here. Thanks again for your patience!

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

jlarsen-usgs commented 2 years ago

@crvernon Just got the go ahead to submit the updated paper and start moving forward. The USGS code review has been completed and the final USGS approval for publication on the paper should be complete early next week. Once again I appologize for how long this has taken on my end! Thanks for being understanding.

jlarsen-usgs commented 2 years ago

@crvernon We've gotten the final approval from USGS to go ahead and publish, so we can finally start moving forward and wrap this up.

crvernon commented 2 years ago

@editorialbot generate pdf

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

crvernon commented 2 years ago

@whedon generate pdf

editorialbot commented 2 years ago

My name is now @editorialbot

crvernon commented 2 years ago

@editorialbot check references

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

OK DOIs

- 10.1111/gwat.12413 is OK
- 10.5066/F7BK19FH is OK
- 10.1016/j.jhydrol.2016.03.026 is OK
- 10.1016/j.envsoft.2018.07.020 is OK
- 10.3133/tm6A16 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.3233/978-1-61499-649-1-87 is OK
- 10.5066/P9NPZ5AD is OK
- 10.3133/tm6A55 is OK
- 10.13140/2.1.2741.9202 is OK
- 10.3133/tm6B7 is OK
- 10.5194/gmd-11-4755-2018 is OK
- 10.3133/tm6A37 is OK
- 10.3133/tm6A45 is OK
- 10.5066/F7P55KJN is OK
- 10.1016/J.ENVSOFT.2019.01.006 is OK
- 10.3133/sir20145052 is OK

MISSING DOIs

- None

INVALID DOIs

- None