-
Notifications
You must be signed in to change notification settings - Fork 6
Snotel new api #136
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
base: main
Are you sure you want to change the base?
Snotel new api #136
Conversation
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.
Pull Request Overview
This PR replaces the old SOAP/Zeep-based SNOTEL client with a REST-based implementation using requests, updates tests to use JSON fixtures and requests.get mocks, and reorganizes the sensor/variable modules and import paths accordingly.
- Switch SNOTEL client from Zeep/SOAP to REST API (
requests) - Add JSON fixtures under
tests/data/snotel_mocksand aread_jsonhelper - Refactor variable definitions and module imports; remove Zeep dependency
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/utils.py | Add read_json helper to load JSON fixtures |
| tests/test_snotel.py | Refactor tests to mock requests.get and drive from JSON files |
| tests/data/snotel_mocks/semimonthly_swe.json | New semimonthly SWE mock |
| tests/data/snotel_mocks/hourly_swe.json | New hourly SWE mock |
| tests/data/snotel_mocks/hourly_soil.json | New hourly soil-temperature mock |
| tests/data/snotel_mocks/hourly_precip.json | New hourly precipitation mock |
| tests/data/snotel_mocks/daily.json | New daily data mock |
| setup.py | Remove zeep dependency |
| metloom/variables.py | Simplify variable exports; import new SnotelVariables |
| metloom/sensors.py | Introduce shared SensorDescription and VariableBase |
| metloom/pointdata/base.py | Update imports to use sensors.py |
| metloom/pointdata/snotel/snotel_client.py | Implement REST-based SNOTEL client with requests |
| metloom/pointdata/snotel/snotel.py | Refactor SnotelPointData to use REST endpoints and cached metadata |
metloom/pointdata/snotel/snotel.py
Outdated
| # if we wanted to. We would need to be careful of the meas height | ||
| if len(data) == 1: | ||
| result_map[variable] = data[0] | ||
| elif len(data["data"]) > 1: |
Copilot
AI
Jul 8, 2025
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.
The code checks len(data["data"]) but data is a list, so data["data"] will raise a TypeError. It should check len(data) > 1 to detect multiple results.
| elif len(data["data"]) > 1: | |
| elif len(data) > 1: |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.