Fix WQP_Metadata.site_info: bind as a real property and forward kwargs#249
Draft
thodson-usgs wants to merge 2 commits intoDOI-USGS:mainfrom
Draft
Fix WQP_Metadata.site_info: bind as a real property and forward kwargs#249thodson-usgs wants to merge 2 commits intoDOI-USGS:mainfrom
thodson-usgs wants to merge 2 commits intoDOI-USGS:mainfrom
Conversation
`site_info` was defined inside `WQP_Metadata.__init__` as a nested
`@property`-decorated function that was never bound to the class. The
decorator returned a property descriptor that was immediately discarded
when `__init__` returned, so `md.site_info` always fell through to
`BaseMetadata.site_info` and raised `NotImplementedError`. The closure
also used `parameters[...]` (the captured `**parameters` kwargs) for the
lookup but `self._parameters` for the `in` check — inconsistent.
Compounding the issue, every helper called `WQP_Metadata(response)` with
no kwargs, so `self._parameters` was always `{}` even after fixing the
closure.
Move `site_info` to a real property on the class, look up `siteid` first
(WQP-native; the previous `sites`/`site`/`site_no` keys reflected an NWIS
copy-paste and never matched a WQP query), and thread `**kwargs` through
`WQP_Metadata(response, **kwargs)` from all nine call sites so the
property has the parameters it needs.
Also corrected the docstring: the attribute is `comment` (not `comments`,
inherited from `BaseMetadata`), and the `query_time` type is
`datetime.timedelta` (not `datetme.timedelta`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Folds the special-cased `siteid` branch into the loop over candidate keys; semantics are unchanged. Co-Authored-By: Claude Opus 4.7 (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
1 participant
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
WQP_Metadata.site_infowas defined inside__init__as a nested@property-decorated function that was never bound to the class. The decorator returned a property descriptor that was immediately discarded when__init__returned, somd.site_infoalways fell through toBaseMetadata.site_infoand raisedNotImplementedError. The closure also usedparameters[...](the captured**parameterskwargs) for the lookup butself._parametersfor theincheck — an internal inconsistency.Compounding the issue, every helper called
WQP_Metadata(response)with no kwargs, soself._parameterswas always{}even after fixing the closure.This PR:
site_infoto a real@propertyon the class.siteidfirst (WQP-native; the previoussites/site/site_nokeys reflected an NWIS copy-paste and never matched a WQP query) while keeping the legacy aliases as a fallback.**kwargsthroughWQP_Metadata(response, **kwargs)from all nine call sites so the property has the parameters it needs.comment(singular, inherited fromBaseMetadata) andquery_timeis adatetime.timedelta(wasdatetme.timedelta).Test plan
tests/wqp_test.py:site_inforesolves and returns a(DataFrame, WQP_Metadata)tuple whensiteidwas provided; returnsNonewhen no site filter was supplied.tests/wqp_test.py(13 tests) pass.API_USGS_PATset to avoid 429s on live tests).