Rename camelCase methods to snake_case with deprecation warnings (Phase 3+4)#61
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the API transition to PEP 8 naming by renaming public camelCase functions/methods to snake_case in murgtools.getdata.getDataFRF, while preserving backward compatibility via deprecated aliases and updating imports/exports and tests accordingly.
Changes:
- Added a
_deprecated_aliasdecorator and introduced deprecated camelCase alias wrappers for renamed APIs. - Renamed module-level Argus helpers and many
getObspublic methods to snake_case equivalents. - Updated package exports (
__init__.py) and adjusted tests to patch the new snake_case function names.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
murgtools/getdata/getDataFRF.py |
Introduces _deprecated_alias, renames public APIs to snake_case, and adds deprecated alias wrappers for backward compatibility. |
murgtools/getdata/__init__.py |
Re-exports new snake_case APIs while keeping camelCase aliases available for compatibility. |
murgtools/__init__.py |
Updates top-level re-exports to include selected renamed functions and their compatibility aliases. |
tests/test_getDataFRF.py |
Updates mocks/patch targets to the new snake_case Argus imagery function. |
Comments suppressed due to low confidence (1)
murgtools/getdata/getDataFRF.py:866
get_wave_specwarns using the old camelCase name (and misspells “deprecated”) and then callsself.getWaveData(...), which is now a deprecated alias. That means calling the new API emits an extra deprecation warning and points users at the wrong replacement. Update the warning text/category and callget_wave_datadirectly.
def get_wave_spec(self, gaugenumber=0, roundto=30, removeBadDataFlag=4, **kwargs):
warnings.warn("WARNING: getWaveSpec is depreciated, update to use getWaveData, spec=True")
returnAB = kwargs.get('returnAB', False)
specOnly = kwargs.get('specOnly', False)
# if specOnly:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+516
to
+520
| # Deprecated alias for backward compatibility | ||
| @_deprecated_alias('remove_duplicates_from_dictionary') | ||
| def removeDuplicatesFromDictionary(inputDict): | ||
| """Deprecated: Use remove_duplicates_from_dictionary instead.""" | ||
| return remove_duplicates_from_dictionary(inputDict) |
Comment on lines
+19
to
+23
| # New snake_case names (preferred) | ||
| "remove_duplicates_from_dictionary", | ||
| "get_argus_imagery", | ||
| "thread_get_argus_imagery", | ||
| # Deprecated camelCase aliases (backward compatibility) |
SBFRF
force-pushed
the
feature/issue-51-52-snake-case-rename
branch
from
July 17, 2026 13:26
47a0191 to
b0a1921
Compare
…ues #51, #52) - Add _deprecated_alias decorator for emitting DeprecationWarnings - Rename module-level functions: - removeDuplicatesFromDictionary -> remove_duplicates_from_dictionary - getArgusImagery -> get_argus_imagery - threadGetArgusImagery -> thread_get_argus_imagery - findArgusImagery -> find_argus_imagery - Rename getObs class methods to snake_case: - getWaveData -> get_wave_data - getWaveSpec -> get_wave_spec - getWaveGaugeLoc -> get_wave_gauge_loc - getCurrents -> get_currents - getWind -> get_wind - getWL -> get_wl - getGaugeWL -> get_gauge_wl - getBathyTransectFromNC -> get_bathy_transect_from_nc - getBathyTransectProfNum -> get_bathy_transect_prof_num - getBathyGridFromNC -> get_bathy_grid_from_nc - getBathyDuckLoc -> get_bathy_duck_loc - getBathyRegionalDEM -> get_bathy_regional_dem - getBathyGridcBathy -> get_bathy_grid_cbathy - getLidarRunup -> get_lidar_runup - getLidarWaveProf -> get_lidar_wave_prof - getLidarTopo -> get_lidar_topo - getCTD -> get_ctd - getALT -> get_alt - getArgus -> get_argus - _waveGaugeURLlookup -> _wave_gauge_url_lookup - _wlGageURLlookup -> _wl_gauge_url_lookup - Add deprecated aliases for all renamed functions/methods - Update __init__.py exports to include both old and new names - Update tests to patch snake_case function names Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SBFRF
force-pushed
the
feature/issue-51-52-snake-case-rename
branch
from
July 17, 2026 13:29
b0a1921 to
0c51abc
Compare
Comment on lines
834
to
835
| def get_wave_spec(self, gaugenumber=0, roundto=30, removeBadDataFlag=4, **kwargs): | ||
| warnings.warn("WARNING: getWaveSpec is depreciated, update to use getWaveData, spec=True") |
Comment on lines
+19
to
+27
| # New snake_case names (preferred) | ||
| "remove_duplicates_from_dictionary", | ||
| "get_argus_imagery", | ||
| "thread_get_argus_imagery", | ||
| # Deprecated camelCase aliases (backward compatibility) | ||
| "removeDuplicatesFromDictionary", | ||
| "forecastData", | ||
| "getSatelliteImagery", | ||
| "getArgusImagery", | ||
| "threadGetArgusImagery", | ||
| # Other exports |
Comment on lines
+2780
to
+2784
| def __getattr__(self, name): | ||
| """Handle deprecated method names with warnings.""" | ||
| if name in self._DEPRECATED_METHODS: | ||
| new_name = self._DEPRECATED_METHODS[name] | ||
| warnings.warn( |
- Fix internal calls to use snake_case names (remove_duplicates_from_dictionary)
to avoid triggering deprecation warnings internally
- Fix get_wave_spec: correct spelling ("deprecated" not "depreciated"),
use DeprecationWarning, reference new method names, call get_wave_data directly
- Add missing find_argus_imagery/findArgusImagery to top-level exports
- Update tests to match new warning type and method names
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines
+835
to
+839
| warnings.warn( | ||
| "get_wave_spec is deprecated, use get_wave_data(..., spec=True) instead", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) |
Comment on lines
+2778
to
+2788
| def __getattr__(self, name): | ||
| """Handle deprecated method names with warnings.""" | ||
| if name in self._DEPRECATED_METHODS: | ||
| new_name = self._DEPRECATED_METHODS[name] | ||
| warnings.warn( | ||
| f"{name} is deprecated, use {new_name} instead", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) | ||
| return getattr(self, new_name) | ||
| raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'") |
Comment on lines
+1019
to
+1021
| with pytest.warns(DeprecationWarning, match="get_wave_spec is deprecated"): | ||
| with patch.object(obs, 'get_wave_data', return_value={'test': 'data'}): | ||
| obs.get_wave_spec() |
Comment on lines
+1034
to
+1037
| import warnings | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore") | ||
| result = obs.getWaveSpec(gaugenumber='waverider-26m', roundto=30) | ||
| result = obs.get_wave_spec(gaugenumber='waverider-26m', roundto=30) |
- Fix __getattr__ to warn on call, not on attribute access (hasattr/introspection no longer triggers warnings) - Handle getWaveSpec specially to avoid double-warning: now skips get_wave_spec and goes directly to get_wave_data - Remove warning from get_wave_spec (new snake_case API shouldn't warn) - Update tests to verify: - getWaveSpec emits warning pointing to get_wave_data - get_wave_spec does NOT emit warning - hasattr doesn't trigger deprecation warnings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SBFRF
force-pushed
the
feature/issue-51-52-snake-case-rename
branch
from
July 17, 2026 13:58
5a9d829 to
1efee67
Compare
Comment on lines
+2750
to
+2757
| # ========================================================================= | ||
| # Deprecated method name mapping (for backward compatibility) | ||
| # ========================================================================= | ||
| _DEPRECATED_METHODS = { | ||
| 'getWaveData': 'get_wave_data', | ||
| 'getWaveSpec': 'get_wave_spec', | ||
| 'getWaveGaugeLoc': 'get_wave_gauge_loc', | ||
| 'getCurrents': 'get_currents', |
Comment on lines
+2750
to
+2761
| # ========================================================================= | ||
| # Deprecated method name mapping (for backward compatibility) | ||
| # ========================================================================= | ||
| _DEPRECATED_METHODS = { | ||
| 'getWaveData': 'get_wave_data', | ||
| 'getWaveSpec': 'get_wave_spec', | ||
| 'getWaveGaugeLoc': 'get_wave_gauge_loc', | ||
| 'getCurrents': 'get_currents', | ||
| 'getWind': 'get_wind', | ||
| 'getWL': 'get_wl', | ||
| 'getGaugeWL': 'get_gauge_wl', | ||
| 'getBathyTransectFromNC': 'get_bathy_transect_from_nc', |
Comment on lines
+613
to
616
| def get_wave_data(self, gaugenumber=0, roundto=30, removeBadDataFlag=4, force_refresh=False, **kwargs): | ||
| """This function pulls down the data from the thredds server and puts the data into a dictionary. | ||
|
|
||
| TODO: Set optional date input from function arguments to change self.start self.end |
Comment on lines
+3911
to
3914
| def thread_get_argus_imagery(dateOfInterest, filename=None, imageType="timex", verbose=True): | ||
| """Retrieve Argus imagery in a background thread (non-blocking). | ||
|
|
||
| Spawns a daemon thread to download the image and returns immediately |
Comment on lines
3954
to
3958
| """Find available Argus imagery with configurable search strategy. | ||
|
|
||
| Wrapper around getArgusImagery that searches for available imagery | ||
| Wrapper around get_argus_imagery that searches for available imagery | ||
| when the exact requested time is not available. | ||
|
|
- Update internal calls in conditions_plot.py and getPlotData.py to use snake_case methods (get_wave_data, get_wave_spec, get_currents) - Update docstrings to reference snake_case names: - _getWaveData_impl: references get_wave_data - get_wave_data: references _wave_gauge_url_lookup - thread_get_argus_imagery: example uses thread_get_argus_imagery - find_argus_imagery: returns/example use snake_case names Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines
+2792
to
+2796
| def _getWaveSpec_wrapper(*args, **kwargs): | ||
| warnings.warn( | ||
| "getWaveSpec is deprecated, use get_wave_data(..., spec=True) instead", | ||
| DeprecationWarning, | ||
| stacklevel=2 |
Comment on lines
+1007
to
1021
| """Tests for the getObs.get_wave_spec method and deprecated getWaveSpec alias.""" | ||
|
|
||
| @patch('murgtools.getdata.getDataFRF.nc.date2num') | ||
| def test_getWaveSpec_emits_deprecation_warning(self, mock_date2num): | ||
| """Test that getWaveSpec emits a deprecation warning.""" | ||
| """Test that deprecated getWaveSpec emits a warning pointing to get_wave_data.""" | ||
| mock_date2num.return_value = 1577836800.0 | ||
| d1 = DT.datetime(2020, 1, 1) | ||
| d2 = DT.datetime(2020, 1, 2) | ||
|
|
||
| from murgtools.getdata.getDataFRF import getObs | ||
| obs = getObs(d1, d2) | ||
|
|
||
| with pytest.warns(UserWarning, match="getWaveSpec is depreciated"): | ||
| with patch.object(obs, 'getWaveData', return_value={'test': 'data'}): | ||
| with pytest.warns(DeprecationWarning, match="getWaveSpec is deprecated.*get_wave_data"): | ||
| with patch.object(obs, 'get_wave_data', return_value={'test': 'data'}): | ||
| obs.getWaveSpec() |
- Remove special case for getWaveSpec in __getattr__ - getWaveSpec now warns to use get_wave_spec (consistent with Phase 3 rename) - Update test to expect get_wave_spec in warning message Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines
+2753
to
+2758
| _DEPRECATED_METHODS = { | ||
| 'getWaveData': 'get_wave_data', | ||
| 'getWaveSpec': 'get_wave_spec', | ||
| 'getWaveGaugeLoc': 'get_wave_gauge_loc', | ||
| 'getCurrents': 'get_currents', | ||
| 'getWind': 'get_wind', |
…iases - Replace __getattr__ with explicit deprecated wrapper methods - This enables class-level access (getObs.getWaveData) for: - Mocking/monkeypatching at class level - Documentation/introspection tools - IDE autocompletion - All 21 deprecated methods now exist as real class methods - Each emits DeprecationWarning when called, then delegates to snake_case Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getObsclass and module-level functions to snake_case@_deprecated_aliasdecoratorDeprecationWarningwhen calledChanges
removeDuplicatesFromDictionary,getArgusImagery,threadGetArgusImagery,findArgusImagerygetWaveData→get_wave_data)__init__.pyfiles to include both old and new namesBackward Compatibility
All old method names are preserved as deprecated aliases. Users will see warnings like:
Test plan
Closes #51
Closes #52
🤖 Generated with Claude Code