Skip to content

Conversation

@ehinman
Copy link
Collaborator

@ehinman ehinman commented Jan 7, 2026

Built off the PR that adds reference table functions.

Adds a demo of the waterdata functions to drpy. Addresses: #200

Also adds a few query parameters to the get_time_series_metadata() function.

@ehinman ehinman requested a review from jzemmels January 13, 2026 20:44
@ehinman ehinman marked this pull request as ready for review January 13, 2026 22:09
Copy link
Collaborator

@jzemmels jzemmels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I appreciate the consistent throughline example. I've just a few comments that you might consider, but I'm only requesting changes on the errors I ran into (the double monitoring_location_id error + the GeoPandas-dependent code).

"metadata": {},
"source": [
"## Lay of the Land\n",
"Now that your API key is configured, it's time to take a 10,000-ft view of the functions in the `waterdata` module.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to link to the specific OGC API homepage somewhere -- maybe here? https://api.waterdata.usgs.gov/ogcapi

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add to resources section.

]
}
],
"metadata": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End the vignette with a list of links/resources to learn more

@ehinman
Copy link
Collaborator Author

ehinman commented Jan 15, 2026

@jzemmels I think this is ready for a second look. I have fixed the duplicated ID column issue in the waterdata/utils.py (for some reason I can't link to that page right now, GitHub being weird). I added resources, more links, and reduced plotting duplication, inspired by your inquiry about matplotlib! I have articulated that matplotlib and geopandas are required to run the entire notebook, but the waterdata functions do not require them.

The one little hitch I've run into is that there was some discussion in the get_reference_table() MR (merged) about whether the "you don't have geopandas in your environment" logging message should be a warning or an info-level message. I had switched it from warning (prints in the console when running the function) to info (only prints if you request it), but switched it back following Tim's suggestion. Unfortunately, the way I've coded the reference table function, this means that it will warn you you don't have geopandas installed, even if you do. I need to get more creative with how this all works. I think I will address in a separate PR to limit confusion.

@jzemmels jzemmels self-requested a review January 15, 2026 22:22
Copy link
Collaborator

@jzemmels jzemmels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, these changes all look great. Other than populating the additional resources section and one minor comment, I think this is good to go. Approved.

Co-authored-by: Joe Zemmels (he/him) <[email protected]>
"* [Water Data for the Nation Home](https://waterdata.usgs.gov/)\n",
"* [Water Data for the Nation Feedback Form](https://waterdata.usgs.gov/questions-comments/?referrerUrl=https://api.waterdata.usgs.gov)\n",
"* [R dataRetrieval package](https://github.com/DOI-USGS/dataretrieval)\n",
"* [WDFN Blog Post on NWISWeb Decommissioning Timeline](https://waterdata.usgs.gov/blog/nwisweb-decommission-campaign2/)\n"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzemmels Are there any that you would add?

@ehinman
Copy link
Collaborator Author

ehinman commented Jan 15, 2026

Yep, these changes all look great. Other than populating the additional resources section and one minor comment, I think this is good to go. Approved.

Thanks so much for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants