-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optionally include additional metadata with nwis_client get
requests
#214
Conversation
nwis_client get
requests
Appears 3.7 error is unrelated. Edit: 3.7 is EOL in 2023-06-27. Should probably drop it and add 3.10. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for introducing this, @jarq6c! The code looks good to me, I really only have a question and I think we can get this on its way.
For the added optional columns, what are your thoughts on using camelcase and expanding abbreviations in their names? My thinking is to preserve naming conventions from the hydrotools
canonical format. Here are the transformations I am thinking:
siteTypeCd -> site_type_code
hucCd -> huc_code
countyCd -> county_code
stateCd -> state_code
siteName -> site_name
srs
Thoughts?
Agreed and updated column names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes, @jarq6c! This looks good to me!
This PR adds an
include_expanded_metadata
switch tonwis_client.iv.IVDataService.get
. This is a boolean switch that adds the following columns to the resulting dataframe:['siteName', 'srs', 'latitude', 'longitude', 'siteTypeCd', 'hucCd', 'countyCd', 'stateCd']
. These data were always present in the raw json, but were previously ignored. This addition should benefit any geospatial applications.Additions
include_expanded_metadata
parameter. Default is set toFalse
.Removals
infer_datetime
option perpandas``UserWarning
from datetime conversions.Changes
Testing
get_raw
andget
using the new parameter.Notes
Todos
Checklist