Simplify defunct stubs and fix O(n^2) pagination concats#225
Open
thodson-usgs wants to merge 1 commit intoDOI-USGS:mainfrom
Open
Simplify defunct stubs and fix O(n^2) pagination concats#225thodson-usgs wants to merge 1 commit intoDOI-USGS:mainfrom
thodson-usgs wants to merge 1 commit intoDOI-USGS:mainfrom
Conversation
- nwis.py: reduce 5 defunct function stubs (get_qwdata, get_discharge_measurements, get_gwlevels, get_pmcodes, get_water_use) to minimal **kwargs signatures since they only raise NameError - nwis.py: fix O(n^2) pd.concat in _read_json by collecting per-site DataFrames in a list and concatenating once; also drop an unnecessary str() + pd.read_json round-trip since the source is already parsed - nwis.py: restore missing `elif service == "peaks"` dispatch branch in get_record (lost in an earlier lint/format pass) - waterdata/utils.py: fix O(n^2) pd.concat in get_stats_data pagination loop using the same collect-then-concat pattern - samples.py: replace the 230-line get_usgs_samples pass-through that duplicated 22 parameters verbatim with a **kwargs wrapper - demos/hydroshare/*Peaks,Ratings,Statistics,WaterUse*.ipynb: fix SyntaxError from a joined `from dataretrieval import nwisfrom ...` import line; comment out two WaterUse cells that referenced a variable from a now-defunct get_water_use call - nb-clean metadata on NWIS_demo_1 and WaterData_demo notebooks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
A few targeted simplifications and bug fixes turned up by a code review of recent commits.
Cleanup
nwis.py: Reduce 5 defunct function stubs (get_qwdata,get_discharge_measurements,get_gwlevels,get_pmcodes,get_water_use) to minimal**kwargssignatures — they only raiseNameError, so the full typed parameter lists were dead weight.samples.py: Replace the 230-lineget_usgs_samplespass-through (which duplicated 22 parameters and their docstrings verbatim fromwaterdata.get_samples) with a small**kwargswrapper.Performance fixes
nwis.py::_read_json: Fix O(n²)pd.concatin a per-site loop — now collects DataFrames in a list and concats once. Also drops an unnecessarystr(record_json).replace("'", '"')→StringIO→pd.read_jsonround-trip, sincerecord_jsonis already parsed Python data andpd.DataFrame(record_json)works directly.waterdata/utils.py::get_stats_data: Same collect-then-concat fix for the pagination loop — previously the concat was inside thewhile next_token:loop.Bug fixes
nwis.py::get_record: Restore the missingelif service == "peaks":dispatch branch. The docstring has long advertised"peaks"as a supported service but the branch was dropped in an old lint/format pass (#164), soget_record(service="peaks")has been raisingTypeError: peaks service not yet implementeduntil now.Peaks,Ratings,Statistics,WaterUseunderdemos/hydroshare/): Fix aSyntaxErrorfrom an import line that got joined without a newline (from dataretrieval import nwisfrom dataretrieval import waterdata), apparently introduced by ruff-format commit#219.WaterUse_Examples.ipynb: Comment out two cells that still referenced apennsylvaniavariable whose assignment had been commented out as# [Defunct].NWIS_demo_1.ipynb,WaterData_demo.ipynb:nb-cleanpass (stripped staleversionmetadata only).Test plan
pytest tests/— 116 passed (excluding the untrackedngwmn_test.py)jupyter nbconvert --executeagainst live USGS APIs — 16/16 pass after the fixes (11/16 before)_read_jsonandget_stats_datapaths are still exercised by existing tests (tests/nwis_test.py,tests/waterservices_test.py,tests/waterdata_test.py)🤖 Generated with Claude Code