docs+types: reference/build fixups and unify last_modified type#326
Merged
thodson-usgs merged 5 commits intoJun 22, 2026
Merged
Conversation
thodson-usgs
force-pushed
the
docs/reference-and-types-fixups
branch
4 times, most recently
from
f0fc17e to
9b9f158
Compare
…nf root_doc, drop dead nblink media, unify last_modified type Four small, independent fixups surfaced in a library review: - docs: add a `reference/samples.rst` page and wire it into the API reference toctree. `dataretrieval.samples` is public (in `__all__`) but had no reference page. - docs(conf): `main_doc` is not a recognized Sphinx setting (the correct name is `root_doc`); it was silently ignored. Rename so the intent is real rather than relying on the `index` default. - docs(examples): drop the `extra-media: ../../../demos/datasets` entry from `peak_streamflow_trends.nblink`. That directory does not exist and the notebook reads no local files, so nbsphinx_link warned/failed copying missing media on every docs build. - types(waterdata): widen `get_daily`/`get_continuous` `last_modified` from `str | None` to `str | Iterable[str] | None`, matching the other eight OGC getters. `last_modified` is routed through `_format_api_dates`, which accepts a single interval string or a two-element [start, end] range list, so the narrow annotation rejected a valid input shape and was inconsistent with the parallel getters. - README: add an NGWMN usage example (state -> sites -> water levels) and an NGWMN entry under Available Data Services; reformat that section as a function index -- each service led by its function name (e.g. `get_dv`) with a brief description, and each subsection tagged with its module. - README: document `get_ratings` (usage example + index entry) and note its `dict`-of-rating-tables return shape; pass a Series to a getter directly in the NGWMN example (drop the redundant `.tolist()`). mypy --strict passes on the changed module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
thodson-usgs
force-pushed
the
docs/reference-and-types-fixups
branch
from
9b9f158 to
8159a8e
Compare
thodson-usgs
commented
Jun 22, 2026
thodson-usgs
left a comment
thodson-usgs
left a comment
Collaborator
Author
There was a problem hiding this comment.
fix these items
thodson-usgs
added a commit
to thodson-usgs/dataretrieval-python
that referenced
this pull request
Jun 23, 2026
The Ubuntu CI on main (run 27969553989, on the DOI-USGS#326 merge) failed: a transient 503 from the live legacy NWIS site service hit the live tests in nwis_test.py. Windows passed by timing. Not a regression from DOI-USGS#326 — the live tests and the offending call date to DOI-USGS#62. Two things made the transient fatal: - nwis_test.py had no flaky-rerun marker (waterdata/ngwmn got one in DOI-USGS#325), and - TestTZ fetched its sites in the class body (`sites, _ = what_sites(...)`), i.e. at collection time, where a 503 aborts the whole module and can never be reran (rerunfailures retries failed tests, not collection errors). Changes: - Add one shared marker, `conftest.flaky_api`, with a unified transient pattern list, and apply it to every live suite: nwis_test.py / waterdata_test.py / ngwmn_test.py (module-level) and utils_test.py::Test_query (class-level — the only other live suite, found by auditing each module behind a dead proxy). Replaces three drifted inline copies. - The status pattern now matches both the OGC path's "<status>:" and the legacy query path's "HTTP <status>" message shapes, so one list serves all four. This closes a latent gap: ngwmn's old marker omitted ServiceUnavailable and the chunked QuotaExhausted/ServiceInterrupted wrappers (DOI-USGS#325 fixed waterdata but missed ngwmn), so a 503 in an ngwmn live test would have failed CI too. - Move TestTZ's class-body fetch into a class-scoped `sites` fixture so it runs at test time, where the marker can retry a transient. Everything else is mocked, module-skipped (nadp), or — the chunking ..._on_real_transport test — a deterministic local http.server, not external. Verified: nwis collection is network-free; the markers attach with the unified config; ngwmn now covers ServiceUnavailable/ServiceInterrupted; live Test_query and TestTZ pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
thodson-usgs
added a commit
that referenced
this pull request
Jun 23, 2026
#329) The Ubuntu CI on main (run 27969553989, on the #326 merge) failed: a transient 503 from the live legacy NWIS site service hit the live tests in nwis_test.py. Windows passed by timing. Not a regression from #326 — the live tests and the offending call date to #62. Two things made the transient fatal: - nwis_test.py had no flaky-rerun marker (waterdata/ngwmn got one in #325), and - TestTZ fetched its sites in the class body (`sites, _ = what_sites(...)`), i.e. at collection time, where a 503 aborts the whole module and can never be reran (rerunfailures retries failed tests, not collection errors). Changes: - Add one shared marker, `conftest.flaky_api`, with a unified transient pattern list, and apply it to every live suite: nwis_test.py / waterdata_test.py / ngwmn_test.py (module-level) and utils_test.py::Test_query (class-level — the only other live suite, found by auditing each module behind a dead proxy). Replaces three drifted inline copies. - The status pattern now matches both the OGC path's "<status>:" and the legacy query path's "HTTP <status>" message shapes, so one list serves all four. This closes a latent gap: ngwmn's old marker omitted ServiceUnavailable and the chunked QuotaExhausted/ServiceInterrupted wrappers (#325 fixed waterdata but missed ngwmn), so a 503 in an ngwmn live test would have failed CI too. - Move TestTZ's class-body fetch into a class-scoped `sites` fixture so it runs at test time, where the marker can retry a transient. Everything else is mocked, module-skipped (nadp), or — the chunking ..._on_real_transport test — a deterministic local http.server, not external. Verified: nwis collection is network-free; the markers attach with the unified config; ngwmn now covers ServiceUnavailable/ServiceInterrupted; live Test_query and TestTZ pass. Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd Co-authored-by: Claude Opus 4.8 (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.
Four small, independent fixups surfaced in a documentation/design review. Each is verified and low-risk; grouped here as a fast first pass. Larger structural items (api.py docstring de-duplication, the legacy-module docstring sweep) are deliberately left for separate PRs.
Documentation
samplesreference page.dataretrieval.samplesis public (in__all__and the package docstring) but had no API-reference page. Addsdocs/source/reference/samples.rstand wires it into the reference toctree.conf.py:main_doc→root_doc.main_docis not a recognized Sphinx config variable, so it was silently ignored (it only worked because Sphinx defaultsroot_doctoindexanyway). Renamed so the setting is real.extra-mediapath.peak_streamflow_trends.nblinkdeclaredextra-media: ../../../demos/datasets, but that directory does not exist and the notebook reads no local files — nbsphinx_link warned/failed copying missing media on every build. Removed the entry.Types
last_modifiedacross the OGC getters.get_daily/get_continuousdeclaredlast_modified: str | None, while the other eight getters usestr | Iterable[str] | None.last_modifiedis routed through_format_api_dates, which accepts a single interval string or a two-element[start, end]range list, so the narrow annotation rejected a valid input shape and was inconsistent with the parallel getters. Widened the two outliers to match.mypy --strictpasses on the changed module.Notes
reference/index.rstandapi.py); the conflicts are trivial toctree/signature-line edits. This branch is offmain.make htmlhere because nbsphinx executes the demo notebooks live against USGS web services. Validated instead: thesamplesautomodule target imports, the rst is well-formed, the nblink is valid JSON, andmypy --strictpasses.🤖 Generated with Claude Code