Phase 1: Foundation Layer - Centralized Config, Exceptions, and Tests#26
Conversation
This commit establishes the shared infrastructure for the murgtools refactoring effort: Task 1.1 - Centralized Configuration (config.py): - Move all hardcoded THREDDS URLs to single module - Add STAC, Argus, and NCEP endpoint URLs - Add get_thredds_server() helper for auto-selecting server based on network - Add network config constants (timeouts, IP prefixes) Task 1.2 - Custom Exception Hierarchy (exceptions.py): - Add MurgToolsError base exception - Add InvalidGaugeError with valid_gauges context - Add DataNotFoundError, NetworkError, InvalidParameterError - Update getDataFRF.py to use InvalidGaugeError instead of NameError Task 1.3 - Datetime Rounding Utility (sblib.py): - Add roundDatetimeToInterval() function for 30-min Argus rounding - Replace duplicated rounding code in getArgusImagery and findArgusImagery Task 1.4 - Endpoint Verification Tests (test_config.py): - Add unit tests for config values and helper functions - Add integration tests that verify endpoints are reachable (HEAD requests) - Add data download tests that actually retrieve and validate data: - THREDDS: Opens NetCDF file and verifies variables - STAC: POSTs search query and verifies JSON response structure - Argus: Downloads actual image and verifies numpy array - NCEP: Verifies directory listing is accessible All 102 tests pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Scanned THREDDS server and added support for newly discovered gauges: Wave gauges added: - sig940-400, sig940-600 (Nortek Signature profilers) - awac-jpier-11m (Jetty pier AWAC) - waverider-17m-1D, waverider-20m-1d (new waveriders) Current meters added: - sig769-300, sig940-300, sig940-400, sig940-600 (Signature profilers) - awac-jpier-11m, awac-5m Also fixed: - Paros naming: now supports both paros940-200 and paros-200-940m conventions - Updated waveGaugeList, directionalWaveGaugeList, currentsGaugeList - Removed skip decorator from test_can_reach_chl_thredds - Changed getCurrents to raise InvalidGaugeError instead of AssertionError Closes #21 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Two critical bugs fixed:
1. NCML indexing bug in getWaveData():
- When querying recent data from large NCML aggregation files, getnc()
would return a subset of records with a relative offset
- getWaveData() used these relative indices directly on the full netCDF,
resulting in reading data from years earlier than requested
- Fix: Capture indexRef from getnc() and compute absolute indices for
netCDF file access while keeping relative indices for allEpoch access
2. Satellite imagery bounds calculation:
- getSatelliteImagery() used STAC item['bbox'] which represents the full
tile extent, not the actual GeoTIFF extent
- This caused crop coordinates to be shifted by several kilometers
- Fix: Extract actual extent from GeoTIFF tags (ModelTiepointTag and
ModelPixelScaleTag), convert from UTM to lat/lon, then use for cropping
Also includes:
- Regression tests for NCML indexing to verify timestamps match query range
- Minor fix: Remove x-axis label from top plot in test_wave_and_imagery.py
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request establishes a foundation layer for the murgtools package by centralizing configuration, introducing custom exceptions, and adding comprehensive test coverage. The changes address issue #21 by updating gauge URL endpoints and discovering new gauges available on the THREDDS server.
Changes:
- Created centralized configuration module (
murgtools/config.py) containing all THREDDS URLs, STAC endpoints, and other configuration constants - Implemented custom exception hierarchy with
MurgToolsErroras base class and specific exceptions for common error scenarios - Added
roundDatetimeToInterval()utility function for standardized datetime rounding (particularly for Argus 30-minute intervals) - Integrated configuration module across existing codebase, replacing hardcoded URLs
- Added support for 10 newly discovered gauges (signature profilers, AWAC jetty pier, new waveriders, paros sensors)
- Created 27 comprehensive tests for configuration values, endpoint reachability, and actual data downloads
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| murgtools/config.py | New centralized configuration module with THREDDS URLs, STAC endpoints, network detection helpers |
| murgtools/exceptions.py | New custom exception hierarchy for better error handling |
| murgtools/getdata/getDataFRF.py | Integrated config module, custom exceptions, added 10 new gauge definitions |
| murgtools/getdata/getOutsideData.py | Replaced hardcoded URLs with config constants |
| murgtools/utils/sblib.py | Added roundDatetimeToInterval() function for datetime rounding |
| murgtools/utils/gridTools.py | Replaced hardcoded URLs with config constants |
| murgtools/utils/Thredds_checker.py | Replaced hardcoded URLs with config constants |
| tests/test_config.py | New comprehensive test suite with 27 tests for config values and endpoint verification |
| tests/test_getDataFRF.py | Added 550+ lines of tests for new gauge support and existing functionality |
| tests/test_integration.py | Removed redundant skip decorator (test already marked slow at class level) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
…ll ncfile accesses Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
Move inline imports to module level in getOutsideData.py
Fix missing ncfileindex computation in getWaveSpecModel causing AttributeError
Move inline imports to module level per PEP 8
Fix inconsistent indexing in getWaveSpecModel and remove unused test variables
Fix NCML indexing bug and satellite imagery bounds
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: SBFRF <8375832+SBFRF@users.noreply.github.com>
Add parameter validation to roundDatetimeToInterval
[WIP] Address feedback on Phase 1 Foundation Layer PR
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from murgtools.getdata.getDataFRF import getObs | ||
| obs = getObs(d1, d2) | ||
|
|
||
| with pytest.warns(UserWarning, match="getWaveSpec is depreciated"): |
There was a problem hiding this comment.
Spelling error in comment. The word should be spelled "deprecated" not "depreciated". "Depreciated" refers to financial depreciation, while "deprecated" means something is discouraged or obsolete in programming contexts.
| with pytest.warns(UserWarning, match="getWaveSpec is depreciated"): | |
| with pytest.warns(UserWarning, match="getWaveSpec is deprecated"): |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
murgtools/config.pywith all THREDDS URLs, STAC endpoints, Argus base URL, NCEP URL, and helper functions for server selectionmurgtools/exceptions.pywith exception hierarchy (MurgToolsError,InvalidGaugeError,DataNotFoundError,NetworkError,InvalidParameterError,InvalidTimeRangeError)roundDatetimeToInterval()to sblib.py for Argus 30-minute roundingtests/test_config.pywith 27 tests including actual data download verification from THREDDS, STAC, Argus, and NCEP serversTest plan
pytest tests/ -v)Related Issues
Closes #21 - URL endpoints are now centralized in config.py and new gauges discovered on THREDDS have been added
🤖 Generated with Claude Code