Fix #33: Correct case sensitivity in waverider-17m-1d THREDDS path#39
Fix #33: Correct case sensitivity in waverider-17m-1d THREDDS path#39SBFRF wants to merge 2 commits into
Conversation
- Changed THREDDS path from 'waverider-17m-1D' to 'waverider-17m-1d' (uppercase D to lowercase d) - Aligns with THREDDS server conventions and matches waverider-20m-1d pattern - Updated test expectations to match corrected path - Verified with actual THREDDS server URL structure
|
@copilot review this PR |
The file had corrupted content from line 1355 which was causing syntax errors. Reverted to correct syntax from PR #38 which was already merged.
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #33 by fixing a case sensitivity inconsistency in the waverider-17m-1d gauge THREDDS URL path. The gauge name uses lowercase 'd' but the URL path incorrectly used uppercase 'D'.
Changes:
- Updated test expectations in test_getDataFRF.py to use lowercase 'd' in the waverider-17m-1d path (2 test cases)
- Attempted to update the source code in getDataFRF.py to use lowercase 'd' in the THREDDS path
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_getDataFRF.py | Updated test expectations for waverider-17m-1d gauge to use lowercase 'd' in THREDDS URL path |
| murgtools/getdata/getDataFRF.py | Attempted to fix the THREDDS URL path case sensitivity, but contains critical syntax errors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1354,7 +1354,7 @@ def _waveGaugeURLlookup(self, gaugenumber): | |||
| self.dataloc = 'oceanography/waves/awac-jpier-11m/awac-jpier-11m.ncml' | |||
| # New waveriders | |||
| elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: | |||
There was a problem hiding this comment.
Critical syntax error: This line is malformed and will cause a SyntaxError at runtime. The line contains invalid Python syntax with "d/waverider-17m-1d" appearing without proper string quotes or closing bracket. The line should be properly formatted as: elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: to match the pattern used for similar gauges (see line 1358 for waverider-20m as reference).
| elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: | |
| elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: |
| # New waveriders | ||
| elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: | ||
| self.dataloc = 'oceanography/waves/waverider-17m-1D/waverider-17m-1D.ncml' | ||
| self.dataloc = 'oceanography/waves/waverider-17m-1d/waverider-17m-1d.ncml' |
There was a problem hiding this comment.
Incomplete fix: This line still contains the uppercase 'D' in 'waverider-17m-1D', which is the exact issue this PR is meant to fix. According to the PR description, this should be changed to lowercase 'd' to be consistent with the gauge name and match the pattern used by waverider-20m-1d (line 1359). The path should be: 'oceanography/waves/waverider-17m-1d/waverider-17m-1d.ncml'
| self.dataloc = 'oceanography/waves/waverider-17m-1d/waverider-17m-1d.ncml' | |
| self.dataloc = 'oceanography/waves/waverider-17m-1d/waverider-17m-1d.ncml' |
| @@ -1354,7 +1354,7 @@ def _waveGaugeURLlookup(self, gaugenumber): | |||
| self.dataloc = 'oceanography/waves/awac-jpier-11m/awac-jpier-11m.ncml' | |||
| # New waveriders | |||
| elif str(gaugenumber).lower() in ['waverider-17m-1d', '17m-1d']: | |||
There was a problem hiding this comment.
The PR description claims that all 126 unit tests pass and the full test suite runs successfully. However, with the syntax error on line 1356 in the main code file, the Python module would fail to import entirely. This discrepancy suggests that either the tests weren't actually run against this exact code, or the diff representation is incorrect.
Issue
Fixes #33 - Case sensitivity inconsistency in waverider-17m-1d gauge URL path
Problem
The gauge name is accepted as lowercase 'waverider-17m-1d' but the THREDDS URL path used uppercase 'D' in 'waverider-17m-1D'. This is inconsistent with:
Changes
Testing