openjournals / joss-reviews

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

[REVIEW]: SSN2: The next generation of spatial stream network modeling in R #6389

Open editorialbot opened 4 months ago

editorialbot commented 4 months ago

Submitting author: !--author-handle-->@michaeldumelle<!--end-author-handle-- (Michael Dumelle) Repository: https://github.com/USEPA/SSN2 Branch with paper.md (empty if default branch): develop Version: v0.2.0(JOSS) Editor: !--editor-->@mikemahoney218<!--end-editor-- Reviewers: @fernandomayer, @k-doering-NOAA, @fawda123 Archive: 10.5281/zenodo.12770259

Status

status

Status badge code:

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

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

@fernandomayer & @k-doering-NOAA, 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 @mikemahoney218 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 @k-doering-NOAA

📝 Checklist for @fawda123

📝 Checklist for @fernandomayer

fawda123 commented 2 weeks ago

@michaeldumelle I'm sure the community is divided on this, but you might consider leveraging chatGPT or other generative AI to help with the unit tests. It's saved me a lot of time and I've found it an extremely useful tool for creating tests that actually evaluate functionality. Just copy/paste a function with the Roxygen comments and ask it to write unit tests.

michaeldumelle commented 1 week ago

@fawda123 thanks so much for the suggestion and I will keep this in mind! I have been advised to hold off on this sort of approach on EPA repositories until EPA establishes generative AI guidelines. After the review, I may reach out to you on this for more information about your workflow if that is okay with you.

@mikemahoney218, I have just two more functions to update unit tests for and then will incorporate some edits from co-authors on the manuscript. Almost there!

michaeldumelle commented 1 week ago

@mikemahoney218 I have completed my resubmission! A few things to note:

Please let me know if you have any questions/comments/concerns regarding any of my review responses or resubmission!

A huge thanks to @fawda123 , @k-doering-NOAA , @fernandomayer , and @mikemahoney218 for all the helpful feedback!

mikemahoney218 commented 1 week ago

Thanks @michaeldumelle !

@fawda123 and @fernandomayer -- I see that you've both checked off everything on your checklists. Can I ask you to confirm that these changes from @michaeldumelle have satisfied all of your initial comments?

Thanks again everyone 😄

fernandomayer commented 1 week ago

Yes, I confirm my comments were all satisfiedly addressed.

mikemahoney218 commented 3 days ago

Thank you so much, @fernandomayer !

@fawda123, can I ask you to confirm that these changes from @michaeldumelle have satisfied all of your initial comments?

fawda123 commented 3 days ago

@editorialbot generate pdf

editorialbot commented 3 days ago

:warning: An error happened when generating the pdf.

mikemahoney218 commented 3 days ago

@editorialbot set develop as branch

editorialbot commented 3 days ago

Done! branch is now develop

mikemahoney218 commented 3 days ago

@editorialbot generate pdf

mikemahoney218 commented 3 days ago

Sorry, that one's on me

editorialbot commented 3 days ago

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

fawda123 commented 3 days ago

@michaeldumelle great job addressing my comments and those from @k-doering-NOAA and @fernandomayer. I'm very pleased with the revised draft and updates on the dev branch. In particular, the code testing looks a lot better and I sincerely appreciate the time you invested making these more comprehensive. It's tedious work but definitely provides peace of mind. The updated draft is also looking good and I especially like the final figure that was added. There are a few very minor text edits that I've requested addressed in https://github.com/USEPA/SSN2/issues/21, but otherwise it's looking great! It's good to go once these final edits are made!

michaeldumelle commented 3 days ago

Thanks for the kind words @fawda123, and I agree the testing infrastructure is now much, much better! I really appreciate the text edits that you suggested in USEPA/SSN2#21, and they are now incorporated in the draft.

@mikemahoney218 : I am ready to proceed on my end!

mikemahoney218 commented 3 days ago

@editorialbot generate pdf

mikemahoney218 commented 3 days ago

@editorialbot check references

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

OK DOIs

- 10.1007/978-1-4614-7618-4 is OK
- 10.18637/jss.v067.i01 is OK
- 10.1002/9781119115151 is OK
- 10.1002/2015wr018349 is OK
- 10.1371/journal.pone.0282524 is OK
- 10.1016/j.jtherbio.2021.103028 is OK
- 10.1111/rec.13626 is OK
- 10.5281/zenodo.4679423 is OK
- 10.1111/1752-1688.12372 is OK
- 10.1002/2017WR020969 is OK
- 10.1139/cjfas-2016-0247 is OK
- 10.1371/journal.pone.0239237 is OK
- 10.18637/jss.v063.i19 is OK
- 10.1086/710340 is OK
- 10.1016/j.scitotenv.2017.08.151 is OK
- 10.1016/j.cageo.2004.03.012 is OK
- 10.32614/RJ-2018-009 is OK
- 10.1890/08-1668.1 is OK
- 10.18637/jss.v056.i02 is OK
- 10.1111/j.1523-1739.2012.01897.x is OK
- 10.1038/s41598-019-43132-7 is OK
- 10.1007/s10021-018-0311-8 is OK
- 10.1111/1752-1688.12543 is OK
- 10.1111/1365-2664.13997 is OK
- 10.1016/j.cageo.2014.02.009 is OK
- 10.1198/jasa.2009.ap08248 is OK
- 10.18637/jss.v056.i03 is OK
- 10.1007/978-0-387-98141-3 is OK

MISSING DOIs

- No DOI given, and none found for title: rgeos: Interface to Geometry Engine - Open Source ...
- No DOI given, and none found for title: maptools: Tools for Handling Spatial Objects
- No DOI given, and none found for title: rgdal: Bindings for the ’Geospatial’ Data Abstract...
- No DOI given, and none found for title: Catchment-scale stream network spatio-temporal mod...
- No DOI given, and none found for title: Applications of spatial statistical network models...
- No DOI given, and none found for title: Tidy modeling with R
- No DOI given, and none found for title: NHDPlus Version 2: User Guide
- No DOI given, and none found for title: National Stream Internet protocol and user guide
- No DOI given, and none found for title: Upcoming Changes to Popular R Packages for Spatial...
- No DOI given, and none found for title: fields: Tools for spatial data
- No DOI given, and none found for title: SSNbler: Assemble SSN objects in R
- No DOI given, and none found for title: Mixed-effects models in S and S-PLUS
- 10.32614/cran.package.geor may be a valid DOI for title: geoR: Analysis of Geostatistical Data
- 10.32614/cran.package.broom may be a valid DOI for title: broom: Convert Statistical Objects into Tidy Tibbl...
- No DOI given, and none found for title: Modeling Big, Heterogeneous, Non-Gaussian Spatial ...
- No DOI given, and none found for title: Welcome to the Tidyverse
- No DOI given, and none found for title: Spatial Linear Models for Environmental Data

INVALID DOIs

- None
editorialbot commented 3 days ago

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

mikemahoney218 commented 3 days ago

@editorialbot post-review checklist

editorialbot commented 3 days ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

mikemahoney218 commented 3 days ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

mikemahoney218 commented 3 days ago

Awesome! Thank you so much @fawda123 , @fernandomayer , and @k-doering-NOAA for your thoughtful reviews here.

@michaeldumelle you'll see in the post above a checklist with steps for you to work through ("Additional Author Tasks"). At this point could you please work through those steps -- making sure to post the version number of your software release and your archive DOI in this thread, and that the archive has the same metadata (authors, license, and title) as your JOSS publication.

I'll then go ahead with my review of the paper (mostly copy-editing) and then can move forward with recommending acceptance of the submission.

michaeldumelle commented 3 days ago

Thanks @mikemahoney218 ! A question about the release tag -- which of these three approaches do you recommend?:

  1. Generating a release from the current development branch
  2. Merging develop into main and generating a release from the main branch
  3. Merging develop into main, submitting to CRAN, and generating a release from the main branch

For what it's worth, I do plan to submit to CRAN soon, but will need to check-in with collaborators, which may take a few days.

mikemahoney218 commented 3 days ago

Either (1) or (2) seem fine. The primary purpose of the release is so that we can all point to a version and say "this is the one JOSS accepted"; it doesn't need to correspond to anything users would notice. Looking at recent papers, I don't think we even publish the version number anywhere on our website.

michaeldumelle commented 3 days ago

Thanks @mikemahoney218 . How is this release tag? If you approve, I'll use the same one for Zenodo/figshare/etc.

mikemahoney218 commented 3 days ago

Perfect!

michaeldumelle commented 3 days ago

I will ask collaborators to double-check affiliations/ORCIDs and respond back when I hear from all of them! I will edit the files in the release tag as necessary.

michaeldumelle commented 2 days ago

@mikemahoney218 I also wanted to check whether it was okay to explicitly thank reviewers/editor in the acknowledgements?

mikemahoney218 commented 2 days ago

@michaeldumelle yes, that's okay! No obligation, but certainly no prohibition either 😄

I'll do my checks tomorrow, by the way.

michaeldumelle commented 2 days ago

@mikemahoney218 I will wait until I hear back from collaborators about double-checking affiliations and then archive the JOSS materials. If I don't hear from everyone by tomorrow, does that get in the way of your workflow?

mikemahoney218 commented 2 days ago

I won't be able to check your Zenodo until you've got the archive up (of course), which will be the last thing I need to handle before I hand it off to the EiC -- but what I meant to say is that I'll do my checks of the paper for copyediting issues, DOIs and so on in references, and so on, which is usually the thing that involves the most back-and-forth.

mikemahoney218 commented 1 day ago

@editorialbot set v0.2.0(JOSS) as version

editorialbot commented 1 day ago

Done! version is now v0.2.0(JOSS)

mikemahoney218 commented 1 day ago

A few paper comments, all about citation formatting:

Otherwise, once you get the archive out I'll be able to recommend acceptance

michaeldumelle commented 1 day ago

Thanks @mikemahoney218 I'll get these and a few other affiliation/ORCID updates from coauthors done this week (hopefully today). Then I will push, update the package files in the release tag, archive on Zenodo, and finally give you a notice!

michaeldumelle commented 1 day ago

@mikemahoney218 can you help me with DOIs from archived packages (e.g., rgeos)? Do they follow the same structure as 10.32614/CRAN.package.package_name? Should I include/omit them in this case?

mikemahoney218 commented 1 day ago

The archived packages don't have DOIs, so they're good to go -- but packages like fields do have a DOI on CRAN, even if their default citation file doesn't provide it, and I think it'd be best to include those.

michaeldumelle commented 1 day ago

@mikemahoney218 I just pushed these editorial changes to the development branch. Can you verify that these edits satisfy your requests? Then I will work on archiving!

mikemahoney218 commented 1 day ago

@editorialbot generate pdf

editorialbot commented 1 day ago

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

mikemahoney218 commented 1 day ago

@editorialbot check references

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

OK DOIs

- 10.1007/978-1-4614-7618-4 is OK
- 10.18637/jss.v067.i01 is OK
- 10.1016/j.fishres.2022.106583 is OK
- 10.1002/9781119115151 is OK
- 10.1002/2015wr018349 is OK
- 10.1371/journal.pone.0282524 is OK
- 10.1016/j.jtherbio.2021.103028 is OK
- 10.1111/rec.13626 is OK
- 10.5281/zenodo.4679423 is OK
- 10.1111/1752-1688.12372 is OK
- 10.1002/wat2.1023 is OK
- 10.1002/2017WR020969 is OK
- 10.1139/cjfas-2016-0247 is OK
- 10.1371/journal.pone.0239237 is OK
- 10.18637/jss.v063.i19 is OK
- 10.1086/710340 is OK
- 10.1016/j.scitotenv.2017.08.151 is OK
- 10.32614/CRAN.package.fields is OK
- 10.1016/j.cageo.2004.03.012 is OK
- 10.32614/RJ-2018-009 is OK
- 10.1890/08-1668.1 is OK
- 10.18637/jss.v056.i02 is OK
- 10.1111/j.1523-1739.2012.01897.x is OK
- 10.32614/CRAN.package.geoR is OK
- 10.1038/s41598-019-43132-7 is OK
- 10.1007/s10021-018-0311-8 is OK
- 10.32614/CRAN.package.broom is OK
- 10.18637/jss.v108.i10 is OK
- 10.1111/1752-1688.12543 is OK
- 10.1111/1365-2664.13997 is OK
- 10.1016/j.cageo.2014.02.009 is OK
- 10.1198/jasa.2009.ap08248 is OK
- 10.18637/jss.v056.i03 is OK
- 10.1007/978-0-387-98141-3 is OK
- 10.21105/joss.01686 is OK

MISSING DOIs

- No DOI given, and none found for title: rgeos: Interface to Geometry Engine - Open Source ...
- No DOI given, and none found for title: maptools: Tools for Handling Spatial Objects
- No DOI given, and none found for title: rgdal: Bindings for the ’Geospatial’ Data Abstract...
- No DOI given, and none found for title: Tidy modeling with R
- No DOI given, and none found for title: NHDPlus Version 2: User Guide
- No DOI given, and none found for title: National Stream Internet protocol and user guide
- No DOI given, and none found for title: Upcoming Changes to Popular R Packages for Spatial...
- No DOI given, and none found for title: SSNbler: Assemble SSN objects in R
- No DOI given, and none found for title: Mixed-effects models in S and S-PLUS
- No DOI given, and none found for title: Spatial Linear Models for Environmental Data

INVALID DOIs

- None
mikemahoney218 commented 1 day ago

Sorry, caught one more -- for Wickham 2019 can you put brackets around {Tidyverse}? It's capitalized in the paper title.

Otherwise, looks good to me! Note that changes in the paper don't require changes in the Zenodo archive -- JOSS makes the authoritative archive of the paper (and mints a DOI for it), while the Zenodo archive serves as the record of the software at time of acceptance. So it's fine to keep updating the paper, if needed, once the archive is made, because the "real" paper archive will be made later on 😄

michaeldumelle commented 1 day ago

@mikemahoney218 thanks for catching this final one and letting me know about the archiving. I will plan to get to this by the end of the week (co-authors have the remainder of the day to check for minor errors, update affiliations, etc.).

michaeldumelle commented 16 hours ago

Hi @mikemahoney218, I have:

I am ready to move forward on my end; please let me know if there is anything else you need from me at this time and thank you!

mikemahoney218 commented 8 hours ago

@editorialbot set 10.5281/zenodo.12770259 as archive

editorialbot commented 8 hours ago

Done! archive is now 10.5281/zenodo.12770259

mikemahoney218 commented 8 hours ago

@editorialbot recommend-accept

editorialbot commented 8 hours ago
Attempting dry run of processing paper acceptance...