docs: copy-edit docstrings and comments for clarity and accuracy#316
Merged
Merged
Conversation
9b9c05f to
932db26
Compare
A review pass over the package's docstrings and inline comments for spelling,
grammar, clarity, and accuracy against the implementation.
Spelling & grammar:
- Typos ("Times series" -> "Time series", "datetme", "conatining", "indeces"
-> "indices", "ESPSG" -> "EPSG", "NDAP" -> "NADP") plus articles,
subject-verb agreement, duplicated words, and punctuation. codespell-clean
("gage" is the intentional USGS spelling).
R-language cruft -> Python:
- bbox `c(xmin,ymin,xmax,ymax)` -> `[xmin, ymin, xmax, ymax]`; "(numeric)
vector"/"a vector of requested columns" -> list wording; `limit : numeric`
-> `int`; `thresholds : numeric` -> `number`.
Accuracy (docs that disagreed with the code):
- Broken `Returns` cross-references to a nonexistent
`dataretrieval.utils.Metadata` -> the real classes (`nwis.NWIS_Metadata`,
`wqp.WQP_Metadata`, `utils.BaseMetadata`).
- get_monitoring_locations: three params typed `float` -> string/iterable.
- nwis.query_waterdata / query_waterservices: `service` examples listed values
the functions reject -> the accepted sets; removed a duplicated `ssl_check`
block and a `bBox` documented as a HUC.
- waterdata/utils: _switch_properties_id docstring described a dict (copied from
_switch_arg_id); _handle_stats_nesting / _combine_chunk_frames notes
corrected to match the code.
- get_time_series_metadata: replaced the stale `properties` list (copied from
get_daily) with the collection's real queryables.
- Added missing pieces: `nadp` module deprecation note; `Raises` on
`utils.query` and `streamstats.get_watershed` (plus its `format` param and
dual return type); `get_channel` params; `get_stats_por` /
`get_stats_date_range` `Returns` sections.
- Dropped bogus `Returns` blocks from `__init__` methods (they return None).
Code fixes surfaced by the review:
- Removed the non-functional `time` parameter from `get_monitoring_locations`
and `get_time_series_metadata`. Their OGC collections have no `time`
queryable (verified via the collections' queryables and a live 400), so
`time=...` only produced an opaque server error; with no `**kwargs`, removal
now yields a clear `TypeError`. The observation getters' `time` is valid and
untouched.
- Fixed a malformed URL in the nwis seriesCatalogOutput deprecation warning
(`waterdata.usgs.gov.nwis/qwdata` -> `waterdata.usgs.gov/nwis/qwdata`).
ruff check + ruff format clean; mypy --strict clean; tests pass; live-API
behavior verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
932db26 to
fb5a79a
Compare
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.
What
A copy-edit and accuracy review pass over the package's docstrings and inline
comments — spelling, grammar, clarity, and (most importantly) making the prose
match what the code actually does. Docstrings/comments only; no executable
code, signatures, or doctest example code changed.
Notable accuracy fixes (docs that disagreed with the code)
waterdata/api.get_monitoring_locations— three params documented asfloatare actuallystring or iterable of strings.nwis.query_waterservices— a duplicatedssl_checkblock;bBoxwasdescribed as a "7-digit HUC" (it's a bounding box).
waterdata/utils._switch_properties_id— docstring described a dict(copy-pasted from
_switch_arg_id); the function takes alist[str] | None.Also refreshed
_get_resp_data/_get_argsnotes for the now-always-materialized
idcolumn and numeric-param listifying.nwis— getterReturnsreferenced a nonexistentutils.Metadata;corrected to
nwis.NWIS_Metadata. Removed a spurious(df, metadata)entryfrom
_read_json(it returns a DataFrame)."Return"→"Returns",".. note:"→".. note::".Additions where docs were silent
nadpmodule docstring.Raisessections onutils.queryandstreamstats.get_watershed; theformatparam + dual return type onget_watershed.get_channel's docstring (bbox,last_modified,limit).Returnsblocks from__init__methods (they returnNone).Spelling/grammar
"Times series"→"Time series", "datetme"→"datetime", "conatining",
"indeces"→"indices", "ESPSG"→"EPSG", "NDAP"→"NADP", plus a/an and spacing.
Noticed but left for maintainer judgment (out of scope for a copy-edit)
get_time_series_metadata'spropertiesoption list looks copy-pasted fromget_dailyand is likely stale — couldn't verify the correct set.get_stats_por/get_stats_date_rangehave noReturnssection.nwis.get_inforuntimewarnings.warn(...):waterdata.usgs.gov.nwis/qwdata(should be/nwis/) — in executable code, soleft for a separate fix.
bboxdocstrings still use R-stylec(xmin, ymin, ...)notation.Verification
ruff check+ruff format --checkclean; package imports clean; docs-only sono runtime/test impact. Scope: source-code docstrings/comments only (the
docs/RST and notebooks were not part of this pass).
🤖 Generated with Claude Code
Update — follow-up commits
Second copy-edit pass caught more misses: 9
Returnscross-references to anonexistent
utils.Metadatainwqp.py->wqp.WQP_Metadata;query_waterdata/query_waterservicesexamples that listed services thefunctions reject; wrong
id/empty-frame behavior notes in_handle_stats_nesting/_combine_chunk_frames; another bogus__init__Returns; anadpparam-order mismatch;streamstatsbranding.Resolved three of the flagged items (incl. two small code fixes):
timeparameter fromget_monitoring_locations—the collection has no
timeproperty (verified via its OGC queryables), sotime=...only ever produced an opaque400 InvalidQuery. Removal yields aclear client-side
TypeErrorinstead. Verified against the live API.nwisseriesCatalogOutputdeprecationwarning (
usgs.gov.nwis/qwdata->usgs.gov/nwis/qwdata).bboxdocstrings (c(xmin,ymin,xmax,ymax)->[xmin, ymin, xmax, ymax]).This PR is now mostly docstrings/comments plus those two small, verified code
fixes.
ruffclean; 471 tests pass.Still open for maintainer judgment
get_time_series_metadata'spropertiesoption list looks copy-pasted fromget_dailyand is likely stale (needs the API schema to correct).get_stats_por/get_stats_date_rangelackReturnssections.time-param copy-paste; onlyget_monitoring_locationswas verified here.