Skip to content

Conversation

@ehinman
Copy link
Collaborator

@ehinman ehinman commented Jul 30, 2024

This PR moves the 'gwlevels' service from the 'waterservices' umbrella to the 'waterdata' umbrella, since the 'waterservices' endpoint is going away as soon as next week! It ensures that users who use the 'waterservices' inputs like 'startDT', 'endDT', and 'sites' are still able to return gwlevel queries. I updated the tests for this function as well.

The edit to the notebook was to change the plotting syntax to avoid an error with using the index. Looks like other cells were slightly affected by spacing and such.

Question: should the user be able to use a bounding box with this query? I tried to get it to work and could not figure out the correct syntax. I'm not sure if I broke it with the URL change, or if it has always been a bit tricky to figure out.

@thodson-usgs
Copy link
Collaborator

Question: should the user be able to use a bounding box with this query? I tried to get it to work and could not figure out the correct syntax. I'm not sure if I broke it with the URL change, or if it has always been a bit tricky to figure out.

I don't know. Did you check the service's doc and check whether you could provide a bbox to through the REST API?

@ehinman
Copy link
Collaborator Author

ehinman commented Jul 30, 2024

Ok, I'll take a look. I probably did something wrong. Hey, do you have insights for why the tests are failing for gwlevels? The basic error message for all is:

requests_mock.exceptions.NoMockAddress: No mock address: GET https://nwis.waterdata.usgs.gov/nwis/gwlevel...

I'm not sure how I messed up the mocking structure. Thanks!

@thodson-usgs
Copy link
Collaborator

I'm not sure how I messed up the mocking structure. Thanks!

I don't see anything obvious. Did you verify that the url in the mock matches that in the metadata from the same query?

@ehinman
Copy link
Collaborator Author

ehinman commented Aug 1, 2024

Fixed it! Whew. I was at a loss for a day. Simple fix.

Copy link
Collaborator

@thodson-usgs thodson-usgs left a comment

Choose a reason for hiding this comment

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

One suggestion

>>> # Get groundwater levels for site 434400121275801
>>> df, md = dataretrieval.nwis.get_gwlevels(sites='434400121275801')
>>> df, md = dataretrieval.nwis.get_gwlevels(site_no='434400121275801')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you decide to break the API? sites -> site_no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, I see later you catch sites and pass them to site_no. In that case, I might keep sites as the recommended parameter.

keep API consistent

Co-authored-by: Timothy Hodson <[email protected]>
@ehinman
Copy link
Collaborator Author

ehinman commented Aug 1, 2024

Do we need lines 444-445 in nwis.py, then? I guess it's served in line 451. This is my first time messing with kwargs.

@thodson-usgs thodson-usgs self-requested a review August 2, 2024 00:42
Copy link
Collaborator

@thodson-usgs thodson-usgs left a comment

Choose a reason for hiding this comment

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

LGTM.

@thodson-usgs thodson-usgs merged commit d3865a2 into DOI-USGS:master Aug 2, 2024
@lstanish-usgs
Copy link
Contributor

@ehinman regarding bbox queries, they are kinda tricky. You enter the coordinates starting in the west and end in the north, so you move in a counterclockwise direction: Ex. -176, 34, -174, 36.

Description here: https://waterservices.usgs.gov/docs/site-service/site-service-details/

@thodson-usgs thodson-usgs mentioned this pull request Aug 26, 2024
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.

3 participants