search

openjournals / joss-reviews

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

[REVIEW]: gamut: A Geospatial Analysis of Multisector Urban Teleconnections #3383

Closed whedon closed 2 years ago

whedon commented 3 years ago

Submitting author: @KristianNelson (Kristian Nelson) Repository: https://github.com/IMMM-SFA/gamut Version: v1.0.1 Editor: @KristinaRiemer Reviewers: @fsanchez-trigueros, @jsta Archive: 10.5281/zenodo.5590217

: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/a2e0dc1979792f5c73ba9d8e65a717dd"><img src="https://joss.theoj.org/papers/a2e0dc1979792f5c73ba9d8e65a717dd/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/a2e0dc1979792f5c73ba9d8e65a717dd/status.svg)](https://joss.theoj.org/papers/a2e0dc1979792f5c73ba9d8e65a717dd)

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

@fsanchez-trigueros, 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 @KristinaRiemer 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 @fsanchez-trigueros

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

@fsanchez-trigueros, 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 @KristinaRiemer 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 @jsta

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. @fsanchez-trigueros 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 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.04 s (543.8 files/s, 70545.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
R                               15            354            415           1188
Markdown                         2             50              0            264
TeX                              1             30              0            234
Rmd                              2             46             85             96
YAML                             2             17              0             75
-------------------------------------------------------------------------------
SUM:                            22            497            500           1857
-------------------------------------------------------------------------------

Statistical information for the repository '814f195a697ec60010763591' was
gathered on 2021/06/18.
No commited files with the specified extensions were found.
whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.envsoft.2015.11.007 is OK
- 10.32614/RJ-2018-009 is OK

MISSING DOIs

- 10.32614/rj-2011-002 may be a valid DOI for title: testthat: Get Started with Testing
- 10.1201/9781315373461-1 may be a valid DOI for title: knitr: A Comprehensive Tool for Reproducible Research in R
- 10.1201/9781315373461-1 may be a valid DOI for title: knitr: A Comprehensive Tool for Reproducible Research in R

INVALID DOIs

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

danielskatz commented 3 years ago

@whedon add @jsta as reviewer

whedon commented 3 years ago

OK, @jsta is now a reviewer

whedon commented 3 years ago

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

jsta commented 3 years ago

I'm not listed as a reviewer on the sidebar of the article proof.

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

danielskatz commented 3 years ago

I'm not listed as a reviewer on the sidebar of the article proof.

Now you are. When the PDF you were looking at was generated, you weren't assigned as a reviewer in the issue, and now that you are, I've regenerated the PDF and it shows you.

jsta commented 3 years ago

Here is my review @KristianNelson @KristinaRiemer :

Checklist

Documentation

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

I don't see guidelines for 1 and 2. Often R packages use a BugReports field in the DESCRIPTION for 2 and/or a CONTRIBUTING file for 1.

Software Paper

Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?

I feel like the aim of the paper is a bit muddled. Much of the text and the title suggest that the aim of the paper is to report the results of an analysis but I think JOSS papers are typically focused on describing software tools (not reporting on an analysis). Perhaps the paper could include more text describing how gamut works beyond using "watershed delineations ... to mask ... geospatial land use layers"? How does this happen? What kind of decisions (if any) does gamut make behind the scenes to accomplish this task? Do some datasets require special handling? Are there any pieces of gamut as a software package that might be useful in other MSD projects?

Do the authors describe how this software compares to other commonly-used packages?

Are there any other software packages for MSD analysis? If not, might be good to say this in the paper.

Curiosity questions and nice to haves (not required)

How are Los Angeles and other large cities that draw water from outside their watershed handled by gamut?

I wish the function documentation was a little more informative. https://immm-sfa.github.io/gamut/reference/index.html just repeats the function name in function descriptions.

How can we make a map of a given watershed like in the paper figure? Is there a function for that?

I think some of the clisymbols and crayon functionality is dependent on RStudio? If so, might be good to list that as a suggested IDE in some capacity. I'm not seeing them when I use radian and VSCode.

Issues

https://github.com/IMMM-SFA/gamut/issues/112, https://github.com/IMMM-SFA/gamut/issues/111

KristianNelson commented 3 years ago

@jsta Thank you for your feedback! I will add in those necessary guidelines and I do agree we need to clear up the aim of the paper so that it focuses more on how the package works. You make some really good points that I will consider in the edits.

To answer your first question, we utilize the Urban Water Blueprint to know where cities are getting their drinking water. Los Angeles has multiple watersheds all throughout California, but also pulls in water from the Colorado River Basin using the Colorado River Aqueduct which starts at Lake Havasu. We analyze the hydrological statistics upstream of Lake Havasu and include them in the results for Los Angeles. It is important to note that gamut also considers the contribution of watersheds to each city. So watersheds that contribute less to a cities overall drinking water supply have less of an impact on the final statistics.

For question 2, I will look into making the function documentation more informative for users.

We did have a function called plot_watershed where a specific city could be named and it would produce a map of their watersheds. We removed it prior to submission, but we can definitely place it back in if you think that is a helpful addition to the package.

Thank you again for the feedback! It is greatly appreciated.

KristinaRiemer commented 3 years ago

Hi @KristianNelson, thanks again for your JOSS submission! This is to notify you that I'm going to be on vacation July 14 - August 8. Hopefully you and the submission reviewers @fsanchez-trigueros and @jsta will be able to make progress on this submission's review in my absence. If anything urgent comes up during that time period, feel free to contact EiC @arfon. If you have any questions right now, please let me know.

KristinaRiemer commented 3 years ago

/ooo July 14 until August 8

KristinaRiemer commented 3 years ago

Hi everyone! @KristianNelson have you had a chance to add contribution guidelines or update the paper using the suggestions from @jsta yet? And @fsanchez-trigueros have you had a chance to start your review of this yet? Please let me know how I can help or if there are any questions!

KristianNelson commented 3 years ago

@KristinaRiemer I am currently working on a branch that is addressing @jsta first round of edits! This will include fixes to code errors, paper edits, and a CONTRIBUTING.md file for bug reports and contributing guidelines. I will merge this one and make a new branch for any further reviews.

KristinaRiemer commented 3 years ago

@KristianNelson I hope the updates are going well! I have pinged @fsanchez-trigueros via email about their review.

fsanchez-trigueros commented 3 years ago

Here is my review @KristianNelson @KristinaRiemer:

Overall comment

Thanks to its tools and the data sets (https://zenodo.org/record/4662993/files/Geospatial_Input_Datasets.zip), gamut seems useful to streamline current environmental and socioeconomic analysis of main CONUS cities based on watershed location, but the paper may still need revisions to contextualize its basic terminology, describe the state of the art in MSD and urban teleconnections, illustrate real case studies that demonstrate the power of the method and the gamut tools, clarify the possibility of further software and data expansions (for now it can only be used with a limited set of geodata in CONUS, how might it be updated to be more versatile and useable in other regions or with custom data?), or comment on its capacity for the open-source community to participate in its development

Checklist

In the comments below, I bundle checklist questions first (in bold) and then list comments that relate to the bundle:

Installation: Does installation proceed as outlined in the documentation?

Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

devtools::install_github('IMMM-SFA/gamut')

Downloading GitHub repo IMMM-SFA/gamut@HEAD Error in utils::download.file(url, path, method = method, quiet = quiet, : download from 'https://api.github.com/repos/IMMM-SFA/gamut/tarball/HEAD' failed

So I saved the gz file on my machine from https://api.github.com/repos/IMMM-SFA/gamut/tarball/HEAD, then I run install_local() with my local copy of the source code:

install_local('C:\Users\fsanc\Downloads\IMMM-SFA-gamut-v0.1.0-340-g0db7d24.tar.gz')

I know how to troubleshoot this in R but potential users may not, and I didn’t get to find instructions in the gamut materials to help users navigate this potential issue when unable to install remotely.

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

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?

State of the field: Do the authors describe how this software compares to other commonly-used packages?

Other observations and recommendations

KristinaRiemer commented 3 years ago

@KristianNelson when you get a chance to look over @fsanchez-trigueros review, feel free to ask questions or provide feedback.

KristianNelson commented 3 years ago

@fsanchez-trigueros thank you for your comments.

I'll try to answer the questions and respond to comments in order.

Generalization:

This package does have potential to be applied elsewhere other than CONUS, and I agree that it would be a huge enhancement which would attract more users. The challenge is that the gamut package is rather specific to the data that it pulls in. For example, the crop cover raster has specific values for each cover type, which is then reclassified in the package to GCAM values. This reclassification would have to change if values from non-CONUS crop cover rasters are different.

If a new data package was created for non-CONUS areas, there would be a couple options to make the change successful:

  1. Set up the new data package so that it aligns with the data in the CONUS package. This would timely but would help keep the simplicity of the code in gamut
  2. Lets say data was compiled for Canada, but the data isn't set up the same as the original CONUS data. A "domain" variable in the count_watershed_teleconnections function could be set up, which could be changed to "Canada" or "CONUS". Based on this variable, different sets of code could be ran which aligns with either area.

Installation: Does installation proceed as outlined in the documentation?

Installation does proceed as outlined. I have not run into the remote installation issue that you encountered, so I appreciate that feedback and I will be sure to provide other installation options so that users can go through different routes if they have issues.

I will also look into the Rtools dependency and make sure that I provide some explanation for users that run into that problem. I'm wondering if it has anything to do with my version of R since I haven't ran into that issue.

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

There is a paper currently being reviewed that uses the gamut package to analyze water reuse in urban cities. When this is published I would like to reference this paper to show how gamut can be applied to the real world.

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?

The list of references is complete and uses the .bib format required by JOSS. The in text citations use proper syntax. If any additional references are used I will be sure to continue using the correct formatting.

State of the field: Do the authors describe how this software compares to other commonly-used packages?

This package is similar to other multi-sectoral packages in the sense that it combines several datasets regarding a number of sectors in order to provide useful data within certain domains. An example of this is the cerf package which combines multiple datasets to show power plant suitability across the CONUS.

Who coined the urban teleconnections? What publications talk about it?

"Urban teleconnections" was originally coined by Seto et. al in this journal article: https://www.researchgate.net/publication/224879342_Urban_Land_Teleconnections_and_Sustainability

I will be sure to put a reference to that and some context for the reader so that they can get some background on the importance of observing urban teleconnections. I do see where there needs to be more background to attract potential users, so I will extend the literature review to provide more depth on this space.

In regards to the ZIP file size, I will look into other options that can provide easier access for users. We knew we wanted to have the data package accessible through an online open-source repository, and Zenodo has always been my first option. There is the option of splitting the data package up into different zip files which would decrease the file size and lessen the computational burden. For example, within the main folder there are subfolders called "water", "land", and "energy" which could be separated. This would only require a small change in the code to make sure the package retrieves them correctly.

KristinaRiemer commented 3 years ago

@KristianNelson could you please update us here when you've made your proposed changes to the documentation and paper?

KristianNelson commented 3 years ago

@KristinaRiemer Yes I will. I will be pushing up a development branch into main that has the proposed changes and I will link here. I should have this by today or tomorrow.

KristianNelson commented 2 years ago

There are some issues occurring with the vignette build that we are currently working on. Once those are resolve I will send an update.

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

KristianNelson commented 2 years ago

@whedon check references

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

OK DOIs

- 10.1016/j.envsoft.2015.11.007 is OK
- 10.32614/RJ-2018-009 is OK
- 10.1073/pnas.1117622109 is OK
- 10.1021/es402792s is OK

MISSING DOIs

- 10.32614/rj-2011-002 may be a valid DOI for title: testthat: Get Started with Testing
- 10.1201/9781315373461-1 may be a valid DOI for title: knitr: A Comprehensive Tool for Reproducible Research in R
- 10.1201/9781315373461-1 may be a valid DOI for title: knitr: A Comprehensive Tool for Reproducible Research in R

INVALID DOIs

- None
KristianNelson commented 2 years ago

@whedon generate pdf

KristianNelson commented 2 years ago

@whedon check references

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

OK DOIs

- 10.1016/j.envsoft.2015.11.007 is OK
- 10.32614/RJ-2018-009 is OK
- 10.32614/RJ-2011-002 is OK
- 10.1201/9781315373461-1 is OK
- 10.1073/pnas.1117622109 is OK
- 10.1021/es402792s is OK

MISSING DOIs

- None

INVALID DOIs

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

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

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

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

KristianNelson commented 2 years ago

:wave: @KristinaRiemer @fsanchez-trigueros @jsta - at this point I believe I have addressed all of your issues and comments. The changes are now represented on the main branch. Please let me know if you have any further comments. Thanks for the great feedback!

fsanchez-trigueros commented 2 years ago

@KristianNelson Thanks for the update! @KristinaRiemer Do we need to fill out a new form at this time?

jsta commented 2 years ago

Looks good to me. @fsanchez-trigueros I think we just need to verify and check off any missing items from the original form.

KristinaRiemer commented 2 years ago

Thanks for making all of those changes @KristianNelson! @jsta is correct about the next steps, @fsanchez-trigueros. Check off the remaining checkboxes in your review checklist if you're satisfied with them, and then let me know via a comment here if you're content with this submission.

fsanchez-trigueros commented 2 years ago

@KristinaRiemer @jsta @KristianNelson, it sounds good. I'll work on it in the next couple of days.

KristianNelson commented 2 years ago

Thank you @fsanchez-trigueros!

fsanchez-trigueros commented 2 years ago

@KristinaRiemer @KristianNelson, I have completed my form.

Thanks for inviting me to review this contribution to JOSS. I look forward to knowing more about this research!

KristianNelson commented 2 years ago

Great! Thank you @fsanchez-trigueros and @jsta. Your help has been really appreciated!

KristinaRiemer commented 2 years ago

Awesome, thank you so much for your reviews @jsta and @fsanchez-trigueros!

@KristianNelson the next steps are to review the paper and archive the code, and then recommend acceptance to the EiCs. I'm first going to review the paper to check for minor edits needed in the text and references, and it is helpful for you @KristianNelson to also review it.

KristinaRiemer commented 2 years ago

@whedon generate pdf

KristinaRiemer commented 2 years ago

@whedon check references

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

OK DOIs

- 10.1016/j.envsoft.2015.11.007 is OK
- 10.32614/RJ-2018-009 is OK
- 10.32614/RJ-2011-002 is OK
- 10.1201/9781315373461-1 is OK
- 10.1073/pnas.1117622109 is OK
- 10.1021/es402792s is OK

MISSING DOIs

- None

INVALID DOIs

- None