From 01951136991de877e3af90b2b09c8d91f7272c37 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 1 Jul 2026 09:07:12 -0500 Subject: [PATCH 1/5] feat(waterdata): add chunk_granularity to control OGC chunk fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OGC getters chunk a multi-value request only as far as the server's ~8 KB URL limit forces — the fewest sub-requests. But because every sub-request paginates, splitting a large result further is usually quota-neutral, so that conservative default can be needlessly coarse: ten states pulled as one under-limit request page just as many times as ten per-state requests would. Add `waterdata.chunk_granularity(level)`, a context manager that lets a caller who knows their pull is large opt into a finer split — trading the same pages for more, smaller sub-requests (smoother progress, more even concurrency, a smaller unit of retry/resume). The level is "low", "medium", or "high" (typed as `GranularityLevel`, a Literal, so a type checker rejects anything else; an invalid string raises ValueError at the `with`). Each level caps how many sub-chunks a multi-value argument is split into, derived from the default fan-out concurrency (`API_USGS_CONCURRENT`): high = the full width, medium a quarter, low a sixteenth (32 / 8 / 2 by default). Capping the aggressive end at the concurrency width bounds the blast radius so an accidental "high" on a huge list can't explode into thousands of sub-requests. There is no "off" level — not entering the block is off. It is a scoped `with` block, not an env var, because the library can't tell in advance whether a query is large (a short-window query might fit one page, where extra chunks only burn quota). Implementation: a soft `ChunkPlan._refine` pass runs after the hard byte pass; it only ever splits further, so the url_limit invariant holds and it never raises. The resolved per-axis cap is read from a contextvar (Ambient) set by the context manager at plan-construction time. Exported (with the `GranularityLevel` type) from `dataretrieval.waterdata` and the top-level `dataretrieval` package. Co-Authored-By: Claude Opus 4.8 (1M context) --- NEWS.md | 2 + dataretrieval/__init__.py | 8 + dataretrieval/ogc/chunking.py | 147 ++++++++++++++++- dataretrieval/ogc/planning.py | 97 +++++++++-- dataretrieval/waterdata/__init__.py | 3 + docs/source/userguide/errors.rst | 35 ++++ tests/utils_test.py | 18 ++ tests/waterdata_chunking_test.py | 248 ++++++++++++++++++++++++++++ 8 files changed, 542 insertions(+), 16 deletions(-) diff --git a/NEWS.md b/NEWS.md index a86a0314..c2d4b259 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**07/01/2026:** Added `waterdata.chunk_granularity(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further is usually quota-neutral, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.chunk_granularity("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.GranularityLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps how many sub-chunks a multi-value argument is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32. That ceiling is a granularity constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.chunk_granularity`. + **06/23/2026:** **Breaking change (1.2.0):** the minimum supported Python is now **3.10** (`requires-python = ">=3.10"`). 3.9 support was already effectively broken — the `waterdata` module's dependencies (`anyio`, the test stack) require 3.10+, and the `waterdata` test modules already skipped on <3.10. `anyio` is now declared as a direct dependency (it is imported directly by `waterdata`), and the CI/ruff/mypy targets move to 3.10. Also fully removed the deprecated `variable_info` metadata property: the `NWIS_Metadata` override only warned and returned `None` (it relied on the defunct `get_pmcodes`), and the `BaseMetadata` abstract is gone too since nothing implemented it — accessing `.variable_info` now raises `AttributeError`. `site_info` is unaffected. **06/23/2026:** **Breaking change (1.2.0):** removed the `nadp` module and the deprecated `samples` module ahead of the 1.2.0 release. `nadp` was deprecated on 05/01/2026 — NADP is not a USGS data source, so retrieve NADP data directly from https://nadp.slh.wisc.edu/. The `samples.get_usgs_samples` shim (a deprecated forward to the modern getter) is gone; use `waterdata.get_samples()` instead. `import dataretrieval.nadp` / `import dataretrieval.samples` now raise `ModuleNotFoundError`. diff --git a/dataretrieval/__init__.py b/dataretrieval/__init__.py index c9df1c45..ec79a724 100644 --- a/dataretrieval/__init__.py +++ b/dataretrieval/__init__.py @@ -44,6 +44,11 @@ URLTooLong, ) +# Chunk-granularity control (a context manager) and its level type. Defined with +# the chunker in ``dataretrieval.ogc.chunking``; surfaced here for a stable +# public path ``from dataretrieval import chunk_granularity``. +from dataretrieval.ogc.chunking import GranularityLevel, chunk_granularity + # Resumable chunk-interruption exceptions. They are defined in # ``dataretrieval.ogc.interruptions`` rather than ``dataretrieval.exceptions`` # because they carry pandas/httpx state and a resumable ``ChunkedCall`` handle, @@ -93,5 +98,8 @@ "ChunkInterrupted", "QuotaExhausted", "ServiceInterrupted", + # chunk-granularity control (defined in ogc.chunking) + "chunk_granularity", + "GranularityLevel", "__version__", ] diff --git a/dataretrieval/ogc/chunking.py b/dataretrieval/ogc/chunking.py index 00d2766c..5bbcec56 100644 --- a/dataretrieval/ogc/chunking.py +++ b/dataretrieval/ogc/chunking.py @@ -9,6 +9,12 @@ cartesian product of chunks. Requests that already fit get a trivial single-step plan — ``ChunkedCall`` has one code path either way. +Granularity: the planner is conservative by default — it splits only as far as +the byte limit forces. A caller who knows their result is large can opt into a +finer split via the ``chunk_granularity`` context manager +(``"low"`` / ``"medium"`` / ``"high"``); the resolved cap drives +:meth:`ChunkPlan._refine`. See ``chunk_granularity`` for the why and the when. + This module owns the *execution* half — the event loop and bounded concurrency that drive a plan to completion (``ChunkedCall``) plus the public ``multi_value_chunked`` decorator. The neighboring concerns live in @@ -69,8 +75,9 @@ import functools import os from collections.abc import Callable, Iterator +from contextlib import contextmanager from contextvars import copy_context -from typing import Any, cast +from typing import Any, Literal, cast, get_args import httpx import pandas as pd @@ -172,6 +179,130 @@ def get_active_client() -> httpx.AsyncClient | None: return _chunked_client.get() +# Chunk-granularity dial: opt-in to fan a query out *more finely* than the byte +# limit alone requires. Scoped to a ``with chunk_granularity(...):`` block (a +# ContextVar), deliberately NOT an env var (see :func:`chunk_granularity` for +# why). The ambient holds the resolved cap on sub-chunks per axis; ``0`` (the +# default, outside any block) means "chunk only as much as the byte limit needs". +_granularity: Ambient[int] = Ambient("ogc_chunk_granularity", 0) + +#: The three accepted granularity levels, as a typing ``Literal`` so a type +#: checker rejects any other value at the call site. +GranularityLevel = Literal["low", "medium", "high"] + +#: Valid levels derived from the type, so ``GranularityLevel`` stays the single +#: source of truth for what ``_resolve_granularity`` accepts (mirrors the +#: ``get_args``-based ``_VALID_ON_TIE`` / ``_VALID_FILE_TYPES`` in sibling +#: modules). +_VALID_LEVELS: tuple[GranularityLevel, ...] = get_args(GranularityLevel) + +# Granularity's own ceiling on sub-chunks per axis — deliberately NOT tied to the +# concurrency default: fan-out *volume* (how many sub-requests a query becomes) +# is orthogonal to how many run at once (``API_USGS_CONCURRENT``). 32 is a sane +# single-axis ceiling that also bounds the blast radius of an accidental +# ``"high"`` on a very long list; the milder levels are a quarter and a +# sixteenth of it (the three are spaced 4x apart). +_GRANULARITY_MAX_CHUNKS = 32 +_GRANULARITY_LEVELS: dict[str, int] = { + "low": _GRANULARITY_MAX_CHUNKS // 16, # 2 + "medium": _GRANULARITY_MAX_CHUNKS // 4, # 8 + "high": _GRANULARITY_MAX_CHUNKS, # 32 +} + + +def _resolve_granularity(level: GranularityLevel) -> int: + """ + Map a granularity level name to its per-axis sub-chunk cap. + + Parameters + ---------- + level : {"low", "medium", "high"} + The user-supplied level. + + Returns + ------- + int + The maximum sub-chunks per multi-value axis for that level + (``2`` / ``8`` / ``32``). + + Raises + ------ + ValueError + If ``level`` is anything other than ``"low"``, ``"medium"``, or + ``"high"`` — raised eagerly (at the ``with`` statement) so a typo fails + loudly rather than silently doing nothing. + """ + if level not in _VALID_LEVELS: + raise ValueError( + f"chunk_granularity level must be one of {_VALID_LEVELS}; got {level!r}." + ) + return _GRANULARITY_LEVELS[level] + + +@contextmanager +def chunk_granularity(level: GranularityLevel) -> Iterator[None]: + """ + Scope how finely the OGC getters chunk multi-value requests. + + By default the Water Data / NGWMN getters chunk a request only as much as + the server's ~8 KB URL-byte limit forces — the fewest sub-requests that + fit. That is the safe default, but it can be *needlessly* conservative: + because every sub-request paginates, splitting a large result further is + usually quota-neutral (ten states pulled as one under-limit request page + just as many times as ten per-state requests would). This context manager + lets a caller who *knows* their pull is large ask for that finer split — + trading the same pages for more, smaller sub-requests, which gives smoother + progress, more even concurrency, and a smaller unit of retry/resume. + + Because the library can't tell in advance whether a query is large (ten + states over a short window might fit in a single page, where extra chunks + would only burn quota), this is a *deliberate* per-call knob rather than an + automatic behavior or a process-wide environment variable — scoping it to a + ``with`` block keeps an aggressive setting from leaking into unrelated calls + and accidentally spending quota. Outside any block the getters use the + conservative default; there is no "off" level because *not* entering the + block is off. Only the OGC getters (Water Data, NGWMN) read this; wrapping a + legacy NWIS call in the block is a harmless no-op. + + Parameters + ---------- + level : {"low", "medium", "high"} + How aggressively to chunk within the block. Each level caps how many + sub-chunks a single multi-value argument is split into — ``2`` / ``8`` / + ``32`` for ``"low"`` / ``"medium"`` / ``"high"`` — or one value per + sub-request once the argument has fewer values than the cap. The ceiling + is fixed (it is *not* tied to ``API_USGS_CONCURRENT``: how finely a query + splits is orthogonal to how many sub-requests run at once); capping + ``"high"`` at ``32`` keeps an accidental aggressive level on a very long + list from exploding into thousands of sub-requests. (With several + multi-value arguments the per-argument counts still multiply.) + + Yields + ------ + None + + Raises + ------ + ValueError + If ``level`` isn't ``"low"``, ``"medium"``, or ``"high"`` — raised on + ``with`` entry, before any request is issued. + + Examples + -------- + >>> from dataretrieval import waterdata + >>> with waterdata.chunk_granularity("high"): + ... df, md = waterdata.get_daily( + ... monitoring_location_id=many_sites, parameter_code="00060" + ... ) # doctest: +SKIP + + See Also + -------- + ChunkPlan._refine : the planning-side effect of the level. + """ + with _granularity(_resolve_granularity(level)): + yield + + class ChunkedCall: """ Stateful handle for a chunked call. @@ -591,8 +722,9 @@ def multi_value_chunked( ``async def fetch(args) -> (df, response)``, and drives it to completion via :meth:`ChunkedCall.resume`. The plan splits multi-value list params and the cql-text filter so each sub-request URL fits the - byte limit; an already-fitting request is a one-step plan. See the - module docstring for the concurrency model. + byte limit; an already-fitting request is a one-step plan, unless an + active :func:`chunk_granularity` block asks the plan to fan out more + finely. See the module docstring for the concurrency model. Parameters ---------- @@ -636,7 +768,14 @@ def wrapper( finalize: _Finalize = _passthrough_result, ) -> tuple[pd.DataFrame, Any]: limit = _OGC_URL_BYTE_LIMIT if url_limit is None else url_limit - plan = ChunkPlan(args, build_request, limit) + # Read the granularity dial from the ambient set by + # ``chunk_granularity`` (0 = off outside any such block; otherwise the + # per-axis sub-chunk cap). It only affects *planning*, done here up + # front, so a later resume — which re-issues the already-planned + # sub-requests — needs no snapshot. + plan = ChunkPlan( + args, build_request, limit, max_chunks_per_axis=_granularity.get() + ) retry_policy = RetryPolicy.from_env() # The concurrency cap is resolved inside ``resume()`` from # ``API_USGS_CONCURRENT``; ``1`` is a sequential gather, diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index 68c337ae..700a0e85 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -288,6 +288,21 @@ def _extract_axes(args: dict[str, Any]) -> list[_Axis]: return axes +def _split_at(chunks: list[list[str]], idx: int) -> None: + """Replace ``chunks[idx]`` in place with its two contiguous halves. + + The single primitive both planning passes use to fan an axis out. It + preserves the partition invariants every consumer relies on: *coverage* + (each atom survives, exactly once) and *contiguous, deterministic order* + (resume and :meth:`ChunkPlan.iter_sub_args` depend on it). Kept in one + place so those invariants can't drift between :meth:`ChunkPlan._plan` + (byte-driven) and :meth:`ChunkPlan._refine` (granularity-driven). + """ + chunk = chunks[idx] + mid = len(chunk) // 2 + chunks[idx : idx + 1] = [chunk[:mid], chunk[mid:]] + + class ChunkPlan: """ Strategy for issuing one user-level request as a sequence of @@ -312,7 +327,17 @@ class ChunkPlan: Factory that turns a kwargs dict into a sized httpx request, e.g. ``_construct_api_requests``. url_limit : int - Byte budget for the request (URL + body). + Byte budget for the request (URL + body) — a hard ceiling every + sub-request must fit. + max_chunks_per_axis : int, optional + Soft cap on sub-chunks per multi-value axis (default ``0`` = off). + ``0`` chunks only as much as ``url_limit`` requires — the most + conservative plan, fewest sub-requests. A positive cap fans each axis + out into up to ``min(len(atoms), max_chunks_per_axis)`` pieces (never + fewer than the byte budget already forces), so a large multi-page pull + is issued as more, smaller sub-requests. Set from the resolved + :func:`~dataretrieval.ogc.chunking.chunk_granularity` level; see + :meth:`_refine`. Attributes ---------- @@ -344,6 +369,7 @@ def __init__( args: dict[str, Any], build_request: Callable[..., httpx.Request], url_limit: int, + max_chunks_per_axis: int = 0, ) -> None: self.args = args self.axes: list[_Axis] = [] @@ -352,10 +378,10 @@ def __init__( axes = _extract_axes(args) if not axes: - # No chunkable axis: nothing to split. If the single request fits, - # run it verbatim (the common passthrough). ``_safe_request_bytes`` - # treats an un-constructable URL (httpx.InvalidURL, > 64 KB) as over - # budget. + # No chunkable axis: nothing to split, and ``granularity`` has + # nothing to act on either. If the single request fits, run it + # verbatim (the common passthrough). ``_safe_request_bytes`` treats + # an un-constructable URL (httpx.InvalidURL, > 64 KB) as over budget. if _safe_request_bytes(build_request, args, url_limit) <= url_limit: return # Over budget. A filter the chunker doesn't manage — cql-json — is @@ -388,14 +414,29 @@ def __init__( except httpx.InvalidURL: initial_request = None + fits = False if initial_request is not None: self.canonical_url = str(initial_request.url) - if _request_bytes(initial_request) <= url_limit: - return + fits = _request_bytes(initial_request) <= url_limit + + # A request that already fits and hasn't opted into finer chunking is + # the common passthrough: leave ``axes``/``chunks`` empty so + # ``total == 1`` and ``iter_sub_args`` yields the original args + # verbatim. Only when ``max_chunks_per_axis`` asks for extra fan-out do + # we set the axes up to be refined below. + if fits and max_chunks_per_axis <= 0: + return self.axes = axes self.chunks = {axis.arg_key: [list(axis.atoms)] for axis in axes} - self._plan(build_request, url_limit) + if not fits: + # Hard pass: greedy-halve until every worst-case sub-request fits + # the byte budget (may raise ``Unchunkable``). + self._plan(build_request, url_limit) + # Soft pass: optionally split further than the byte budget requires. + # Purely additive — never re-raises, and the byte budget stays + # satisfied; a no-op at ``max_chunks_per_axis <= 0``. + self._refine(max_chunks_per_axis) if self.canonical_url is None: # Original URL was un-constructable (httpx.InvalidURL); fall @@ -447,10 +488,42 @@ def _plan( f"sub-request). Reduce input sizes, shorten or simplify " f"the filter, or split the call manually." ) - axis_chunks = self.chunks[biggest_axis.arg_key] - chunk = axis_chunks[biggest_idx] - mid = len(chunk) // 2 - axis_chunks[biggest_idx : biggest_idx + 1] = [chunk[:mid], chunk[mid:]] + _split_at(self.chunks[biggest_axis.arg_key], biggest_idx) + + def _refine(self, max_chunks_per_axis: int) -> None: + """ + Fan each axis out more finely than the byte budget alone requires — + the granularity dial (see + :func:`~dataretrieval.ogc.chunking.chunk_granularity` for why a caller + would want this). + + Each axis is split until it holds at least + ``min(len(atoms), max_chunks_per_axis)`` chunks — up to the cap, or one + atom per chunk for a shorter axis. Purely additive — only ever *splits* + existing chunks, so the byte pass's work and the ``url_limit`` invariant + are both preserved (an axis the byte pass already split past the cap is + left alone), and it never raises. A no-op at ``max_chunks_per_axis <= 0``. + + Parameters + ---------- + max_chunks_per_axis : int + Soft cap on sub-chunks per axis (``0`` = off), the resolved + granularity level. A shorter axis simply saturates at one atom per + chunk. + """ + if max_chunks_per_axis <= 0: + return + for axis in self.axes: + chunks = self.chunks[axis.arg_key] + target = min(len(axis.atoms), max_chunks_per_axis) + # ``target <= len(atoms)`` guarantees a splittable chunk each pass, so + # this reaches exactly ``target`` and terminates. Split the chunk with + # the most *atoms* (``_plan`` splits by *bytes*): here we even out + # cardinality for smooth fan-out, not URL size. Ties take the lowest + # index, keeping the split deterministic. + while len(chunks) < target: + idx, _ = max(enumerate(chunks), key=lambda kv: len(kv[1])) + _split_at(chunks, idx) def _worst_case_args(self) -> dict[str, Any]: """ diff --git a/dataretrieval/waterdata/__init__.py b/dataretrieval/waterdata/__init__.py index 99b6e178..d70a3d74 100644 --- a/dataretrieval/waterdata/__init__.py +++ b/dataretrieval/waterdata/__init__.py @@ -9,6 +9,7 @@ from __future__ import annotations +from dataretrieval.ogc.chunking import GranularityLevel, chunk_granularity from dataretrieval.ogc.filters import FILTER_LANG # Public API exports @@ -50,6 +51,8 @@ "PROFILE_LOOKUP", "SERVICES", "WATERDATA_SERVICES", + "GranularityLevel", + "chunk_granularity", "get_channel", "get_codes", "get_combined_metadata", diff --git a/docs/source/userguide/errors.rst b/docs/source/userguide/errors.rst index e2dc3ef1..10b1a2c7 100644 --- a/docs/source/userguide/errors.rst +++ b/docs/source/userguide/errors.rst @@ -96,6 +96,41 @@ condition clears -- only the unfinished sub-requests are re-issued. except ChunkInterrupted as again: exc = again +Chunk a large request more finely +================================= + +By default the getters split an over-large request only as much as the +server's ~8 KB URL limit forces -- the fewest sub-requests. Because each +sub-request paginates, splitting a large result further is usually +quota-neutral (ten states pulled as one under-limit request page just as many +times as ten per-state requests would), so if you *know* your pull is large +you can ask for a finer split with ``chunk_granularity`` -- trading the same +pages for more, smaller sub-requests, which gives smoother progress, more even +concurrency, and a smaller unit of retry/resume. It is a scoped ``with`` +block, so an aggressive setting can't leak into unrelated calls and +accidentally spend quota: + +.. code-block:: python + + from dataretrieval import waterdata + + with waterdata.chunk_granularity("high"): + df, md = waterdata.get_daily( + monitoring_location_id=many_sites, parameter_code="00060" + ) + +The level is one of ``"low"``, ``"medium"``, or ``"high"`` (an invalid value +raises ``ValueError`` at the ``with``). Each caps how many sub-chunks a +multi-value argument is split into -- ``2`` / ``8`` / ``32`` for +``"low"`` / ``"medium"`` / ``"high"``. That ceiling is fixed, deliberately +independent of the fan-out concurrency (``API_USGS_CONCURRENT``): how finely a +query splits is orthogonal to how many sub-requests run at once. Capping the +aggressive end at 32 is a guardrail -- an accidental ``"high"`` on a very long +list can't explode into thousands of sub-requests. There is no "off" level: +simply don't enter the block unless you already expect a large, multi-page +result -- on a query that would have fit in a single page, extra chunks only +burn quota. + The full taxonomy ================= diff --git a/tests/utils_test.py b/tests/utils_test.py index b3201419..8ca4630d 100644 --- a/tests/utils_test.py +++ b/tests/utils_test.py @@ -208,6 +208,24 @@ def test_chunk_interruptions_exported_at_top_level(self): dataretrieval.ChunkInterrupted, dataretrieval.DataRetrievalError ) + def test_chunk_granularity_exported_at_top_level_and_waterdata(self): + """The ``chunk_granularity`` context manager and its ``GranularityLevel`` + type are reachable both from the top level (``from dataretrieval import + chunk_granularity``) and from the user-facing ``dataretrieval.waterdata`` + namespace, and both resolve to the single objects defined in + ``dataretrieval.ogc.chunking``.""" + import dataretrieval + from dataretrieval import waterdata + from dataretrieval.ogc import chunking + + assert dataretrieval.chunk_granularity is chunking.chunk_granularity + assert waterdata.chunk_granularity is chunking.chunk_granularity + assert dataretrieval.GranularityLevel is chunking.GranularityLevel + assert waterdata.GranularityLevel is chunking.GranularityLevel + for name in ("chunk_granularity", "GranularityLevel"): + assert name in dataretrieval.__all__ + assert name in waterdata.__all__ + class Test_BaseMetadata: """Tests of BaseMetadata""" diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index 5a946946..6e0a3ec1 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -40,8 +40,13 @@ from dataretrieval.ogc import chunking as _chunking from dataretrieval.ogc import retry as _retry_mod from dataretrieval.ogc.chunking import ( + _GRANULARITY_LEVELS, + _GRANULARITY_MAX_CHUNKS, ChunkedCall, _chunked_client, + _granularity, + _resolve_granularity, + chunk_granularity, get_active_client, multi_value_chunked, ) @@ -2105,3 +2110,246 @@ async def fetch(args): assert "finalized" in df.columns assert md[0] == "METADATA" assert calls["finalize"] >= 1 + + +# --------------------------------------------------------------------------- +# Chunk granularity: the opt-in dial (``"low"`` / ``"medium"`` / ``"high"``) to +# fan a query out MORE finely than the byte limit alone requires +# (``ChunkPlan._refine`` + the ``chunk_granularity`` context manager). +# ``_fake_build``'s base is 200 bytes, so a handful of short atoms sits far +# under ``url_limit=8000`` — the byte pass passes it through untouched, and any +# splitting below is the granularity cap alone. ``ChunkPlan`` takes the resolved +# integer cap (``max_chunks_per_axis``) directly; ``chunk_granularity`` / +# ``_resolve_granularity`` map the level names onto it. +# --------------------------------------------------------------------------- + + +def test_zero_cap_preserves_passthrough(): + """``max_chunks_per_axis=0`` (the default) must not perturb the existing + plan: a multi-value request that fits the byte limit is still the trivial + passthrough (no axes, ``total == 1``), byte-for-byte the pre-feature + behavior.""" + args = {"monitoring_location_id": ["A", "B", "C", "D"]} + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=0) + assert plan.axes == [] + assert plan.total == 1 + assert list(plan.iter_sub_args()) == [args] + + +@pytest.mark.parametrize( + ("max_chunks_per_axis", "expected_pieces"), + [(0, 1), (2, 2), (8, 8), (16, 10), (32, 10)], +) +def test_cap_ramps_then_saturates(max_chunks_per_axis, expected_pieces): + """A single 10-atom axis that fits the byte limit splits into + ``min(10, cap)`` pieces: 1 (off), 2, 8, then saturating at 10 (one atom per + chunk) once the cap overshoots the atom count. Monotonic and bounded, and + whenever it splits the partition is a cover — every atom exactly once. (The + cap-0 passthrough has no axis to cover; see the passthrough test.)""" + atoms = [f"S{i:02d}" for i in range(10)] + plan = ChunkPlan( + {"monitoring_location_id": atoms}, + _fake_build, + url_limit=8000, + max_chunks_per_axis=max_chunks_per_axis, + ) + assert plan.total == expected_pieces + if max_chunks_per_axis: + flattened = [ + a for chunk in plan.chunks["monitoring_location_id"] for a in chunk + ] + assert sorted(flattened) == sorted(atoms) + + +def test_cap_bounds_fan_out_for_a_long_axis(): + """The cap is a quota guardrail: at the ``"high"`` cap a 100-atom axis + fans into ``cap`` pieces — NOT 100 singletons — so an accidental + ``chunk_granularity("high")`` on a huge list can't detonate into hundreds + of sub-requests. Every atom is still covered exactly once.""" + high = _GRANULARITY_LEVELS["high"] + atoms = [f"X{i:03d}" for i in range(100)] + plan = ChunkPlan( + {"monitoring_location_id": atoms}, + _fake_build, + url_limit=8000, + max_chunks_per_axis=high, + ) + assert plan.total == high + flattened = [a for chunk in plan.chunks["monitoring_location_id"] for a in chunk] + assert sorted(flattened) == sorted(atoms) + + +def test_cap_below_byte_split_does_not_reduce_fan_out(): + """The cap is purely additive — it can only split further, never coarsen. + A request the byte budget already fans into K>2 chunks is untouched by a + cap of 2 (below K), so the byte-driven plan is preserved.""" + # Heavy axis of four 30-char atoms; a limit tight enough that the byte pass + # must drive every atom into its own sub-request (4 pieces > the cap of 2). + args = {"monitoring_location_id": ["X" * 30, "Y" * 30, "Z" * 30, "W" * 30]} + baseline = ChunkPlan(args, _fake_build, url_limit=250, max_chunks_per_axis=0) + assert baseline.total > 2 # byte pass alone already fanned out past 2 + refined = ChunkPlan(args, _fake_build, url_limit=250, max_chunks_per_axis=2) + # cap 2 < baseline pieces → refine is a no-op here. + assert refined.total == baseline.total + + +def test_cap_never_exceeds_the_byte_budget(): + """Refining on top of an over-budget request keeps the hard invariant: + every sub-request still fits ``url_limit`` (splitting only ever shrinks + a chunk), and the fan-out is at least what the byte pass required.""" + args = {"monitoring_location_id": ["X" * 30, "Y" * 30, "Z" * 30, "W" * 30]} + limit = 310 + byte_only = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks_per_axis=0) + plan = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks_per_axis=32) + assert plan.total >= byte_only.total + for sub in plan.iter_sub_args(): + assert _safe_request_bytes(_fake_build, sub, limit) <= limit + + +def test_cap_refines_the_filter_axis(): + """The dial treats the cql-text ``filter`` axis like any other: an + under-budget filter of N top-level OR-clauses is split along that axis + into ``min(N, cap)`` pieces.""" + clauses = [f"p='{i}'" for i in range(8)] + args = {"filter": " OR ".join(clauses)} + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=4) + assert len(plan.chunks["filter"]) == 4 # min(8, 4) + assert plan.total == 4 + + +def test_cap_multiplies_across_axes(): + """With more than one multi-value axis the per-axis caps multiply — + documented behavior the caller opts into. Two 6-atom axes at a cap of 4 + yield a 4x4 cartesian product.""" + args = { + "monitoring_location_id": [f"L{i}" for i in range(6)], + "parameter_code": [f"{i:05d}" for i in range(6)], + } + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=4) + assert len(plan.chunks["monitoring_location_id"]) == 4 + assert len(plan.chunks["parameter_code"]) == 4 + assert plan.total == 16 + + +def test_cap_does_not_mask_unchunkable(): + """A request with nothing to split that still busts the byte limit must + raise ``Unchunkable`` regardless of the cap — the soft pass has no axis to + act on and must not swallow the hard failure.""" + args = {"monitoring_location_id": "one-huge-scalar"} + with pytest.raises(Unchunkable): + ChunkPlan(args, _fake_build, url_limit=10, max_chunks_per_axis=32) + + +@pytest.mark.parametrize("level", ["low", "medium", "high"]) +def test_resolve_granularity_maps_each_level_to_its_cap(level): + """Each level name resolves to its per-axis sub-chunk cap from the table + (a positive int).""" + assert _resolve_granularity(level) == _GRANULARITY_LEVELS[level] + assert _resolve_granularity(level) >= 1 + + +def test_granularity_levels_ordered_and_spaced(): + """The three caps hold the properties callers rely on: strictly increasing, + ``"high"`` saturating the declared ceiling, ``"low"`` still a real split + (>= 2), and each level spaced 4x from the next. The ceiling is a granularity + constant, deliberately independent of the concurrency default.""" + low = _GRANULARITY_LEVELS["low"] + medium = _GRANULARITY_LEVELS["medium"] + high = _GRANULARITY_LEVELS["high"] + assert low < medium < high + assert high == _GRANULARITY_MAX_CHUNKS + assert low >= 2 + assert medium == high // 4 + assert low == medium // 4 + + +@pytest.mark.parametrize( + "bad", + [ + "off", # a dead keyword form + "LOW", # wrong case — exact match only + " low ", # stray whitespace + 5, # the old integer levels are gone + None, # None not accepted + ["low"], # unhashable → the ``not in`` check still rejects it cleanly + ], +) +def test_resolve_granularity_rejects_everything_but_the_three_levels(bad): + """Only the three exact level strings are accepted; every other value — a + representative of each rejected shape (dead keyword, wrong case, whitespace, + old int, ``None``, unhashable) — raises ``ValueError`` so a typo fails + loudly.""" + with pytest.raises(ValueError, match="chunk_granularity level must be"): + _resolve_granularity(bad) + + +def test_chunk_granularity_scopes_and_restores_the_ambient(): + """The context manager resolves the level to its cap, publishes it on the + ambient for the block, and restores the previous value on exit — including + proper nesting.""" + assert _granularity.get() == 0 + with chunk_granularity("high"): + assert _granularity.get() == _GRANULARITY_LEVELS["high"] + with chunk_granularity("low"): + assert _granularity.get() == _GRANULARITY_LEVELS["low"] + assert _granularity.get() == _GRANULARITY_LEVELS["high"] # outer restored + assert _granularity.get() == 0 # default (off) outside any block + + +def test_chunk_granularity_validates_on_entry(): + """An invalid level raises at ``with`` entry — before any request is + issued — and leaves the ambient untouched.""" + with pytest.raises(ValueError, match="chunk_granularity level must be"): + with chunk_granularity("aggressive"): + pass + assert _granularity.get() == 0 + + +def test_chunk_granularity_high_drives_end_to_end_fan_out(): + """End-to-end: the same fitting request passes through as a single call by + default, but fans into several sub-requests inside a + ``chunk_granularity("high")`` block — and the combined result still + recovers every atom exactly once.""" + sites = [f"S{i:02d}" for i in range(8)] + + calls: list[tuple[str, ...]] = [] + + @multi_value_chunked(build_request=_fake_build, url_limit=8000) + async def fetch(args): + chunk = tuple(args["monitoring_location_id"]) + calls.append(chunk) + return pd.DataFrame({"site": list(chunk)}), _ok_response() + + # Default: comfortably under the byte limit → one passthrough call. + df_plain, _ = fetch({"monitoring_location_id": sites}) + assert len(calls) == 1 + assert sorted(df_plain["site"]) == sorted(sites) + + calls.clear() + with chunk_granularity("high"): + df_fine, _ = fetch({"monitoring_location_id": sites}) + # 8 atoms at the "high" cap (>= 8) → 8 singleton sub-requests. + assert len(calls) == 8 + assert all(len(chunk) == 1 for chunk in calls) + # Union across chunks recovers the original set, once each. + assert sorted(a for chunk in calls for a in chunk) == sorted(sites) + assert sorted(df_fine["site"]) == sorted(sites) + + +def test_chunk_granularity_low_is_a_gentle_split(): + """``"low"`` is the mildest opt-in: an under-limit request fans into just + the ``"low"`` cap's worth of pieces, not singletons.""" + low = _GRANULARITY_LEVELS["low"] + sites = [f"S{i:02d}" for i in range(8)] + calls: list[int] = [] + + @multi_value_chunked(build_request=_fake_build, url_limit=8000) + async def fetch(args): + calls.append(len(args["monitoring_location_id"])) + return pd.DataFrame(), _ok_response() + + with chunk_granularity("low"): + fetch({"monitoring_location_id": sites}) + # low cap → that many sub-requests, together covering all 8 sites. + assert len(calls) == low + assert sum(calls) == 8 From e87feb684b1a27c631569685bf937dbf69ab2795 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 8 Jul 2026 15:51:38 -0500 Subject: [PATCH 2/5] feat(waterdata): cap chunk_granularity by total sub-requests; document + benchmark Cap chunk_granularity on the plan's TOTAL sub-request count (2/8/32) rather than per multi-value axis, so several multi-value arguments can't multiply past the ceiling. ChunkPlan._refine now splits the largest splittable chunk across every axis round-robin; behavior is identical for the common single-axis query. Add a 'Speeding up large downloads' usage section to the README (Water Data API) with a measured cold-cache benchmark: parallelizing a large paginated pull's sub-requests gave ~6x (production page size, cold) up to ~12x (latency-bound) on 271 Ohio discharge sites. Temper the 'quota-neutral' claim in the docstring, NEWS, and user guide: a finer split is only ~quota-neutral when each sub-request still spans many pages; otherwise each chunk's partial final page adds some requests. Co-Authored-By: Claude Opus 4.8 (1M context) --- NEWS.md | 2 +- README.md | 48 +++++++++++++++++++++++ dataretrieval/ogc/chunking.py | 61 ++++++++++++++++++------------ dataretrieval/ogc/planning.py | 65 +++++++++++++++++++------------- docs/source/userguide/errors.rst | 24 +++++++----- tests/waterdata_chunking_test.py | 44 ++++++++++++++++----- 6 files changed, 173 insertions(+), 71 deletions(-) diff --git a/NEWS.md b/NEWS.md index c2d4b259..545f5080 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -**07/01/2026:** Added `waterdata.chunk_granularity(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further is usually quota-neutral, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.chunk_granularity("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.GranularityLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps how many sub-chunks a multi-value argument is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32. That ceiling is a granularity constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.chunk_granularity`. +**07/01/2026:** Added `waterdata.chunk_granularity(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further costs little or no extra quota when each sub-request still spans many pages, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.chunk_granularity("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.GranularityLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps the *total* number of sub-requests the call is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32 overall, across every multi-value argument combined, not per argument. That ceiling is a granularity constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list, or a call with several multi-value arguments, can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.chunk_granularity`. **06/23/2026:** **Breaking change (1.2.0):** the minimum supported Python is now **3.10** (`requires-python = ">=3.10"`). 3.9 support was already effectively broken — the `waterdata` module's dependencies (`anyio`, the test stack) require 3.10+, and the `waterdata` test modules already skipped on <3.10. `anyio` is now declared as a direct dependency (it is imported directly by `waterdata`), and the CI/ruff/mypy targets move to 3.10. Also fully removed the deprecated `variable_info` metadata property: the `NWIS_Metadata` override only warned and returned `None` (it relied on the defunct `get_pmcodes`), and the `BaseMetadata` abstract is gone too since nothing implemented it — accessing `.variable_info` now raises `AttributeError`. `site_info` is unaffected. diff --git a/README.md b/README.md index d904e6d6..e37c263b 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,54 @@ df, metadata = waterdata.get_continuous( print(f"Retrieved {len(df)} continuous gage height measurements") ``` +#### Speeding up large downloads with `chunk_granularity` + +By default the getters split a multi-value request only as far as the server's +~8 KB URL limit forces — the fewest sub-requests. For a **large, paginated** +pull that is needlessly conservative: every sub-request pages through its own +results, so dividing the query into more, smaller sub-requests lets those pages +be fetched **in parallel**. `chunk_granularity` opts a single call into that +finer split. It pays off only when the result is large enough to span many +pages *and* the query has a multi-value argument to divide (such as a list of +monitoring locations); on a small query — or one with nothing to split — it just +adds requests, so it is a deliberate, scoped `with` block, never the default. + +```python +from dataretrieval import waterdata + +# All stream gages in Ohio, then 20 years of their daily discharge — large +# enough to span many pages, so it profits from a finer split. +sites, _ = waterdata.get_monitoring_locations(state="Ohio", site_type_code="ST") + +with waterdata.chunk_granularity("high"): # "low" | "medium" | "high" + df, md = waterdata.get_daily( + monitoring_location_id=sites["monitoring_location_id"].tolist(), + parameter_code="00060", # discharge + time="2004-01-01/2023-12-31", + ) +``` + +`"high"` fans a call out into up to 32 sub-requests, `"medium"` up to 8, and +`"low"` up to 2 — a fixed ceiling, so an accidental setting on a huge list can't +explode into thousands of requests. + +Benchmark — 271 Ohio discharge sites (`get_daily`, `parameter_code="00060"`), +cold cache, each level run against its own time window so results are not +cache-served, with the page size fixed so every level fetches roughly the same +number of pages (isolating the effect of parallelism): + +| level | parallelism | pages | wall-clock | speedup | +| -------------- | ----------- | ----- | ------------------------- | ------- | +| `default` | 1 | 32 | 23.9 s / 27.4 s (2 runs) | 1× | +| `"medium"` (8) | 8 | 37 | 4.1 s / 4.3 s | ~6× | +| `"high"` (32) | 32 | 44 | 2.0 s | ~12× | + +The gain comes from overlapping each sub-request's per-page latency and +server-side work — a genuinely large pull at the default page size shows a +similar multi-fold speedup. The extra sub-requests each cost one request against +your hourly [rate limit](https://api.waterdata.usgs.gov/signup/), so reserve the +aggressive levels for pulls you know are large. + Visit the [API Reference](https://doi-usgs.github.io/dataretrieval-python/reference/waterdata.html) for more information and examples on available services and input parameters. diff --git a/dataretrieval/ogc/chunking.py b/dataretrieval/ogc/chunking.py index 5bbcec56..1334f018 100644 --- a/dataretrieval/ogc/chunking.py +++ b/dataretrieval/ogc/chunking.py @@ -182,8 +182,9 @@ def get_active_client() -> httpx.AsyncClient | None: # Chunk-granularity dial: opt-in to fan a query out *more finely* than the byte # limit alone requires. Scoped to a ``with chunk_granularity(...):`` block (a # ContextVar), deliberately NOT an env var (see :func:`chunk_granularity` for -# why). The ambient holds the resolved cap on sub-chunks per axis; ``0`` (the -# default, outside any block) means "chunk only as much as the byte limit needs". +# why). The ambient holds the resolved cap on the plan's total sub-request +# count; ``0`` (the default, outside any block) means "chunk only as much as +# the byte limit needs". _granularity: Ambient[int] = Ambient("ogc_chunk_granularity", 0) #: The three accepted granularity levels, as a typing ``Literal`` so a type @@ -196,12 +197,14 @@ def get_active_client() -> httpx.AsyncClient | None: #: modules). _VALID_LEVELS: tuple[GranularityLevel, ...] = get_args(GranularityLevel) -# Granularity's own ceiling on sub-chunks per axis — deliberately NOT tied to the -# concurrency default: fan-out *volume* (how many sub-requests a query becomes) -# is orthogonal to how many run at once (``API_USGS_CONCURRENT``). 32 is a sane -# single-axis ceiling that also bounds the blast radius of an accidental -# ``"high"`` on a very long list; the milder levels are a quarter and a -# sixteenth of it (the three are spaced 4x apart). +# Granularity's own ceiling on the plan's total sub-request count — +# deliberately NOT tied to the concurrency default: fan-out *volume* (how many +# sub-requests a query becomes) is orthogonal to how many run at once +# (``API_USGS_CONCURRENT``). 32 is a sane ceiling on the whole call (across +# every axis, not per axis — see ``ChunkPlan._refine``) that bounds the blast +# radius of an accidental ``"high"`` on a very long list or several multi-value +# arguments at once; the milder levels are a quarter and a sixteenth of it (the +# three are spaced 4x apart). _GRANULARITY_MAX_CHUNKS = 32 _GRANULARITY_LEVELS: dict[str, int] = { "low": _GRANULARITY_MAX_CHUNKS // 16, # 2 @@ -212,7 +215,7 @@ def get_active_client() -> httpx.AsyncClient | None: def _resolve_granularity(level: GranularityLevel) -> int: """ - Map a granularity level name to its per-axis sub-chunk cap. + Map a granularity level name to its total sub-request cap. Parameters ---------- @@ -222,7 +225,7 @@ def _resolve_granularity(level: GranularityLevel) -> int: Returns ------- int - The maximum sub-chunks per multi-value axis for that level + The maximum total sub-requests for the whole call at that level (``2`` / ``8`` / ``32``). Raises @@ -247,12 +250,16 @@ def chunk_granularity(level: GranularityLevel) -> Iterator[None]: By default the Water Data / NGWMN getters chunk a request only as much as the server's ~8 KB URL-byte limit forces — the fewest sub-requests that fit. That is the safe default, but it can be *needlessly* conservative: - because every sub-request paginates, splitting a large result further is - usually quota-neutral (ten states pulled as one under-limit request page - just as many times as ten per-state requests would). This context manager - lets a caller who *knows* their pull is large ask for that finer split — - trading the same pages for more, smaller sub-requests, which gives smoother - progress, more even concurrency, and a smaller unit of retry/resume. + because every sub-request paginates, splitting a large result further costs + little or no extra quota *as long as each sub-request still spans many + pages* — rows-per-chunk far exceeding the page size (ten states pulled as + one request then page nearly as many times as ten per-state requests + would). When a split leaves each sub-request only a page or two, its partial + final page is extra, so finer chunks do add some requests. This context + manager lets a caller who *knows* their pull is large ask for that finer + split — trading roughly the same pages for more, smaller sub-requests, which + gives smoother progress, more even concurrency, and a smaller unit of + retry/resume. Because the library can't tell in advance whether a query is large (ten states over a short window might fit in a single page, where extra chunks @@ -267,15 +274,19 @@ def chunk_granularity(level: GranularityLevel) -> Iterator[None]: Parameters ---------- level : {"low", "medium", "high"} - How aggressively to chunk within the block. Each level caps how many - sub-chunks a single multi-value argument is split into — ``2`` / ``8`` / - ``32`` for ``"low"`` / ``"medium"`` / ``"high"`` — or one value per - sub-request once the argument has fewer values than the cap. The ceiling - is fixed (it is *not* tied to ``API_USGS_CONCURRENT``: how finely a query - splits is orthogonal to how many sub-requests run at once); capping - ``"high"`` at ``32`` keeps an accidental aggressive level on a very long - list from exploding into thousands of sub-requests. (With several - multi-value arguments the per-argument counts still multiply.) + How aggressively to chunk within the block. Each level caps the + *total* number of sub-requests the call is split into — ``2`` / ``8`` + / ``32`` for ``"low"`` / ``"medium"`` / ``"high"`` — or one + sub-request per remaining atom once that's fewer than the cap. The + cap applies to the whole call, not per multi-value argument: with + several multi-value arguments the axes are refined together against + the same shared ceiling (their cartesian product does not multiply + past it). The ceiling is fixed (it is *not* tied to + ``API_USGS_CONCURRENT``: how finely a query splits is orthogonal to + how many sub-requests run at once); capping ``"high"`` at ``32`` keeps + an accidental aggressive level on a very long list — or several + multi-value arguments at once — from exploding into thousands of + sub-requests. Yields ------ diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index 700a0e85..58bdad65 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -330,12 +330,13 @@ class ChunkPlan: Byte budget for the request (URL + body) — a hard ceiling every sub-request must fit. max_chunks_per_axis : int, optional - Soft cap on sub-chunks per multi-value axis (default ``0`` = off). + Soft cap on the plan's total sub-request count (default ``0`` = off). ``0`` chunks only as much as ``url_limit`` requires — the most - conservative plan, fewest sub-requests. A positive cap fans each axis - out into up to ``min(len(atoms), max_chunks_per_axis)`` pieces (never - fewer than the byte budget already forces), so a large multi-page pull - is issued as more, smaller sub-requests. Set from the resolved + conservative plan, fewest sub-requests. A positive cap fans the plan + out to up to ``max_chunks_per_axis`` sub-requests overall (the + cartesian product across axes, never fewer than the byte budget + already forces) — capped as a whole, not per axis, so several + multi-value axes can't multiply past the cap. Set from the resolved :func:`~dataretrieval.ogc.chunking.chunk_granularity` level; see :meth:`_refine`. @@ -492,38 +493,50 @@ def _plan( def _refine(self, max_chunks_per_axis: int) -> None: """ - Fan each axis out more finely than the byte budget alone requires — + Fan the plan out more finely than the byte budget alone requires — the granularity dial (see :func:`~dataretrieval.ogc.chunking.chunk_granularity` for why a caller would want this). - Each axis is split until it holds at least - ``min(len(atoms), max_chunks_per_axis)`` chunks — up to the cap, or one - atom per chunk for a shorter axis. Purely additive — only ever *splits* - existing chunks, so the byte pass's work and the ``url_limit`` invariant - are both preserved (an axis the byte pass already split past the cap is - left alone), and it never raises. A no-op at ``max_chunks_per_axis <= 0``. + Caps the plan's *total* sub-request count (:attr:`total`, the + cartesian product across all axes) at ``max_chunks_per_axis``, not + each axis independently — with several multi-value axes, a cap of 32 + still means at most 32 sub-requests overall, not ``32 ** n_axes``. + Each split picks the single largest splittable chunk across *every* + axis (ties broken by axis-extraction order, then lowest index), so + growth is distributed round-robin rather than one axis saturating + before another is touched. Purely additive — only ever *splits* + existing chunks, so the byte pass's work and the ``url_limit`` + invariant are both preserved, and it never raises. A no-op at + ``max_chunks_per_axis <= 0``. Parameters ---------- max_chunks_per_axis : int - Soft cap on sub-chunks per axis (``0`` = off), the resolved - granularity level. A shorter axis simply saturates at one atom per - chunk. + Soft cap on the plan's total sub-request count (``0`` = off), + the resolved granularity level. Despite the name — kept for the + public dial's "sub-chunks per multi-value argument" framing, + which matches this cap exactly in the common single-axis case — + multi-axis plans are capped on the product, not per axis. """ if max_chunks_per_axis <= 0: return - for axis in self.axes: - chunks = self.chunks[axis.arg_key] - target = min(len(axis.atoms), max_chunks_per_axis) - # ``target <= len(atoms)`` guarantees a splittable chunk each pass, so - # this reaches exactly ``target`` and terminates. Split the chunk with - # the most *atoms* (``_plan`` splits by *bytes*): here we even out - # cardinality for smooth fan-out, not URL size. Ties take the lowest - # index, keeping the split deterministic. - while len(chunks) < target: - idx, _ = max(enumerate(chunks), key=lambda kv: len(kv[1])) - _split_at(chunks, idx) + while self.total < max_chunks_per_axis: + # Largest splittable chunk across every axis; a chunk of size 1 + # can't be split further. ``max`` with a stable input order + # breaks ties by axis order, then lowest index within an axis. + candidate: tuple[_Axis, int] | None = None + candidate_size = -1 + for axis in self.axes: + for idx, chunk in enumerate(self.chunks[axis.arg_key]): + if len(chunk) <= 1: + continue + if len(chunk) > candidate_size: + candidate, candidate_size = (axis, idx), len(chunk) + if candidate is None: + return # every axis saturated at one atom per chunk + axis, idx = candidate + _split_at(self.chunks[axis.arg_key], idx) def _worst_case_args(self) -> dict[str, Any]: """ diff --git a/docs/source/userguide/errors.rst b/docs/source/userguide/errors.rst index 10b1a2c7..0c4429c0 100644 --- a/docs/source/userguide/errors.rst +++ b/docs/source/userguide/errors.rst @@ -101,12 +101,14 @@ Chunk a large request more finely By default the getters split an over-large request only as much as the server's ~8 KB URL limit forces -- the fewest sub-requests. Because each -sub-request paginates, splitting a large result further is usually -quota-neutral (ten states pulled as one under-limit request page just as many -times as ten per-state requests would), so if you *know* your pull is large -you can ask for a finer split with ``chunk_granularity`` -- trading the same -pages for more, smaller sub-requests, which gives smoother progress, more even -concurrency, and a smaller unit of retry/resume. It is a scoped ``with`` +sub-request paginates, splitting a large result further costs little or no +extra quota *as long as each sub-request still spans many pages* (ten states +pulled as one request then page nearly as many times as ten per-state requests +would; a split that leaves each sub-request only a page or two adds its partial +final page). So if you *know* your pull is large you can ask for a finer split +with ``chunk_granularity`` -- trading roughly the same pages for more, smaller +sub-requests, which gives smoother progress, more even concurrency, and a +smaller unit of retry/resume. It is a scoped ``with`` block, so an aggressive setting can't leak into unrelated calls and accidentally spend quota: @@ -120,13 +122,15 @@ accidentally spend quota: ) The level is one of ``"low"``, ``"medium"``, or ``"high"`` (an invalid value -raises ``ValueError`` at the ``with``). Each caps how many sub-chunks a -multi-value argument is split into -- ``2`` / ``8`` / ``32`` for -``"low"`` / ``"medium"`` / ``"high"``. That ceiling is fixed, deliberately +raises ``ValueError`` at the ``with``). Each caps the *total* number of +sub-requests the call is split into -- ``2`` / ``8`` / ``32`` for +``"low"`` / ``"medium"`` / ``"high"``, across every multi-value argument +combined, not per argument. That ceiling is fixed, deliberately independent of the fan-out concurrency (``API_USGS_CONCURRENT``): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 is a guardrail -- an accidental ``"high"`` on a very long -list can't explode into thousands of sub-requests. There is no "off" level: +list, or a call with several multi-value arguments, can't explode into +thousands of sub-requests. There is no "off" level: simply don't enter the block unless you already expect a large, multi-page result -- on a query that would have fit in a single page, extra chunks only burn quota. diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index 6e0a3ec1..055db44b 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -2120,7 +2120,9 @@ async def fetch(args): # under ``url_limit=8000`` — the byte pass passes it through untouched, and any # splitting below is the granularity cap alone. ``ChunkPlan`` takes the resolved # integer cap (``max_chunks_per_axis``) directly; ``chunk_granularity`` / -# ``_resolve_granularity`` map the level names onto it. +# ``_resolve_granularity`` map the level names onto it. The cap bounds the +# plan's *total* sub-request count (the cartesian product across axes), not +# each axis independently — see ``test_cap_caps_the_total_across_axes``. # --------------------------------------------------------------------------- @@ -2217,18 +2219,42 @@ def test_cap_refines_the_filter_axis(): assert plan.total == 4 -def test_cap_multiplies_across_axes(): - """With more than one multi-value axis the per-axis caps multiply — - documented behavior the caller opts into. Two 6-atom axes at a cap of 4 - yield a 4x4 cartesian product.""" +def test_cap_caps_the_total_across_axes(): + """With more than one multi-value axis the cap bounds the *total* + sub-request count (the cartesian product), not each axis independently — + the blast-radius guardrail the dial exists for. Two 6-atom axes at a cap + of 4 top out at 4 sub-requests total, not 4x4=16; growth is distributed + round-robin across axes rather than one axis alone climbing to the cap.""" args = { "monitoring_location_id": [f"L{i}" for i in range(6)], "parameter_code": [f"{i:05d}" for i in range(6)], } plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=4) - assert len(plan.chunks["monitoring_location_id"]) == 4 - assert len(plan.chunks["parameter_code"]) == 4 - assert plan.total == 16 + assert plan.total == 4 + # Every atom on every axis is still covered exactly once. + for key, atoms in ( + ("monitoring_location_id", args["monitoring_location_id"]), + ("parameter_code", args["parameter_code"]), + ): + flattened = [a for chunk in plan.chunks[key] for a in chunk] + assert sorted(flattened) == sorted(atoms) + + +def test_cap_bounds_fan_out_across_many_axes(): + """The guardrail holds regardless of axis count: three multi-value axes + at the ``"high"`` cap still top out at ``high`` sub-requests total, not + ``high ** 3`` — the property the single-axis-only cap in the original + implementation did not guarantee.""" + high = _GRANULARITY_LEVELS["high"] + # Three chunkable axes (two list axes + the filter OR-axis), each with 10 + # atoms — under the old per-axis cap this would have been high**3. + args = { + "monitoring_location_id": [f"L{i}" for i in range(10)], + "parameter_code": [f"{i:05d}" for i in range(10)], + "filter": " OR ".join(f"p='{i}'" for i in range(10)), + } + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=high) + assert plan.total <= high def test_cap_does_not_mask_unchunkable(): @@ -2242,7 +2268,7 @@ def test_cap_does_not_mask_unchunkable(): @pytest.mark.parametrize("level", ["low", "medium", "high"]) def test_resolve_granularity_maps_each_level_to_its_cap(level): - """Each level name resolves to its per-axis sub-chunk cap from the table + """Each level name resolves to its total sub-request cap from the table (a positive int).""" assert _resolve_granularity(level) == _GRANULARITY_LEVELS[level] assert _resolve_granularity(level) >= 1 From 2b2f5f0bc3b3d0c0b941bb366a9eef76871e9ada Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 8 Jul 2026 18:09:24 -0500 Subject: [PATCH 3/5] refactor(waterdata): rename chunk_granularity -> parallel_chunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the public context manager chunk_granularity to parallel_chunks and the GranularityLevel type to ParallelChunksLevel (both exported from dataretrieval and dataretrieval.waterdata), plus internals (_resolve_level, _MAX_PARALLEL_CHUNKS, _LEVEL_CAPS, the _parallel_chunks ambient). 'parallel_chunks' names what the knob does — fan a query into more, parallel sub-requests — without colliding with API_USGS_CONCURRENT (the separate in-flight cap). Docstrings, NEWS, user guide, README, and tests updated to match. Pure rename; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- NEWS.md | 2 +- README.md | 6 +- dataretrieval/__init__.py | 12 ++-- dataretrieval/ogc/chunking.py | 58 +++++++++---------- dataretrieval/ogc/planning.py | 12 ++-- dataretrieval/waterdata/__init__.py | 6 +- docs/source/userguide/errors.rst | 4 +- tests/utils_test.py | 16 +++--- tests/waterdata_chunking_test.py | 89 +++++++++++++++-------------- 9 files changed, 103 insertions(+), 102 deletions(-) diff --git a/NEWS.md b/NEWS.md index 545f5080..e3d6f32b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -**07/01/2026:** Added `waterdata.chunk_granularity(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further costs little or no extra quota when each sub-request still spans many pages, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.chunk_granularity("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.GranularityLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps the *total* number of sub-requests the call is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32 overall, across every multi-value argument combined, not per argument. That ceiling is a granularity constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list, or a call with several multi-value arguments, can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.chunk_granularity`. +**07/01/2026:** Added `waterdata.parallel_chunks(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further costs little or no extra quota when each sub-request still spans many pages, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.parallel_chunks("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.ParallelChunksLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps the *total* number of sub-requests the call is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32 overall, across every multi-value argument combined, not per argument. That ceiling is a parallel_chunks constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list, or a call with several multi-value arguments, can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.parallel_chunks`. **06/23/2026:** **Breaking change (1.2.0):** the minimum supported Python is now **3.10** (`requires-python = ">=3.10"`). 3.9 support was already effectively broken — the `waterdata` module's dependencies (`anyio`, the test stack) require 3.10+, and the `waterdata` test modules already skipped on <3.10. `anyio` is now declared as a direct dependency (it is imported directly by `waterdata`), and the CI/ruff/mypy targets move to 3.10. Also fully removed the deprecated `variable_info` metadata property: the `NWIS_Metadata` override only warned and returned `None` (it relied on the defunct `get_pmcodes`), and the `BaseMetadata` abstract is gone too since nothing implemented it — accessing `.variable_info` now raises `AttributeError`. `site_info` is unaffected. diff --git a/README.md b/README.md index e37c263b..71f3b329 100644 --- a/README.md +++ b/README.md @@ -105,13 +105,13 @@ df, metadata = waterdata.get_continuous( print(f"Retrieved {len(df)} continuous gage height measurements") ``` -#### Speeding up large downloads with `chunk_granularity` +#### Speeding up large downloads with `parallel_chunks` By default the getters split a multi-value request only as far as the server's ~8 KB URL limit forces — the fewest sub-requests. For a **large, paginated** pull that is needlessly conservative: every sub-request pages through its own results, so dividing the query into more, smaller sub-requests lets those pages -be fetched **in parallel**. `chunk_granularity` opts a single call into that +be fetched **in parallel**. `parallel_chunks` opts a single call into that finer split. It pays off only when the result is large enough to span many pages *and* the query has a multi-value argument to divide (such as a list of monitoring locations); on a small query — or one with nothing to split — it just @@ -124,7 +124,7 @@ from dataretrieval import waterdata # enough to span many pages, so it profits from a finer split. sites, _ = waterdata.get_monitoring_locations(state="Ohio", site_type_code="ST") -with waterdata.chunk_granularity("high"): # "low" | "medium" | "high" +with waterdata.parallel_chunks("high"): # "low" | "medium" | "high" df, md = waterdata.get_daily( monitoring_location_id=sites["monitoring_location_id"].tolist(), parameter_code="00060", # discharge diff --git a/dataretrieval/__init__.py b/dataretrieval/__init__.py index ec79a724..567be6ab 100644 --- a/dataretrieval/__init__.py +++ b/dataretrieval/__init__.py @@ -44,10 +44,10 @@ URLTooLong, ) -# Chunk-granularity control (a context manager) and its level type. Defined with +# Parallel-chunks control (a context manager) and its level type. Defined with # the chunker in ``dataretrieval.ogc.chunking``; surfaced here for a stable -# public path ``from dataretrieval import chunk_granularity``. -from dataretrieval.ogc.chunking import GranularityLevel, chunk_granularity +# public path ``from dataretrieval import parallel_chunks``. +from dataretrieval.ogc.chunking import ParallelChunksLevel, parallel_chunks # Resumable chunk-interruption exceptions. They are defined in # ``dataretrieval.ogc.interruptions`` rather than ``dataretrieval.exceptions`` @@ -98,8 +98,8 @@ "ChunkInterrupted", "QuotaExhausted", "ServiceInterrupted", - # chunk-granularity control (defined in ogc.chunking) - "chunk_granularity", - "GranularityLevel", + # parallel-chunks control (defined in ogc.chunking) + "parallel_chunks", + "ParallelChunksLevel", "__version__", ] diff --git a/dataretrieval/ogc/chunking.py b/dataretrieval/ogc/chunking.py index 1334f018..3d0e9aff 100644 --- a/dataretrieval/ogc/chunking.py +++ b/dataretrieval/ogc/chunking.py @@ -9,11 +9,11 @@ cartesian product of chunks. Requests that already fit get a trivial single-step plan — ``ChunkedCall`` has one code path either way. -Granularity: the planner is conservative by default — it splits only as far as +Parallel chunks: the planner is conservative by default — it splits only as far as the byte limit forces. A caller who knows their result is large can opt into a -finer split via the ``chunk_granularity`` context manager +finer split via the ``parallel_chunks`` context manager (``"low"`` / ``"medium"`` / ``"high"``); the resolved cap drives -:meth:`ChunkPlan._refine`. See ``chunk_granularity`` for the why and the when. +:meth:`ChunkPlan._refine`. See ``parallel_chunks`` for the why and the when. This module owns the *execution* half — the event loop and bounded concurrency that drive a plan to completion (``ChunkedCall``) plus the @@ -179,25 +179,25 @@ def get_active_client() -> httpx.AsyncClient | None: return _chunked_client.get() -# Chunk-granularity dial: opt-in to fan a query out *more finely* than the byte -# limit alone requires. Scoped to a ``with chunk_granularity(...):`` block (a -# ContextVar), deliberately NOT an env var (see :func:`chunk_granularity` for +# Parallel-chunks dial: opt-in to fan a query out *more finely* than the byte +# limit alone requires. Scoped to a ``with parallel_chunks(...):`` block (a +# ContextVar), deliberately NOT an env var (see :func:`parallel_chunks` for # why). The ambient holds the resolved cap on the plan's total sub-request # count; ``0`` (the default, outside any block) means "chunk only as much as # the byte limit needs". -_granularity: Ambient[int] = Ambient("ogc_chunk_granularity", 0) +_parallel_chunks: Ambient[int] = Ambient("ogc_parallel_chunks", 0) -#: The three accepted granularity levels, as a typing ``Literal`` so a type +#: The three accepted parallel_chunks levels, as a typing ``Literal`` so a type #: checker rejects any other value at the call site. -GranularityLevel = Literal["low", "medium", "high"] +ParallelChunksLevel = Literal["low", "medium", "high"] -#: Valid levels derived from the type, so ``GranularityLevel`` stays the single -#: source of truth for what ``_resolve_granularity`` accepts (mirrors the +#: Valid levels derived from the type, so ``ParallelChunksLevel`` stays the single +#: source of truth for what ``_resolve_level`` accepts (mirrors the #: ``get_args``-based ``_VALID_ON_TIE`` / ``_VALID_FILE_TYPES`` in sibling #: modules). -_VALID_LEVELS: tuple[GranularityLevel, ...] = get_args(GranularityLevel) +_VALID_LEVELS: tuple[ParallelChunksLevel, ...] = get_args(ParallelChunksLevel) -# Granularity's own ceiling on the plan's total sub-request count — +# The parallel_chunks ceiling on the plan's total sub-request count — # deliberately NOT tied to the concurrency default: fan-out *volume* (how many # sub-requests a query becomes) is orthogonal to how many run at once # (``API_USGS_CONCURRENT``). 32 is a sane ceiling on the whole call (across @@ -205,17 +205,17 @@ def get_active_client() -> httpx.AsyncClient | None: # radius of an accidental ``"high"`` on a very long list or several multi-value # arguments at once; the milder levels are a quarter and a sixteenth of it (the # three are spaced 4x apart). -_GRANULARITY_MAX_CHUNKS = 32 -_GRANULARITY_LEVELS: dict[str, int] = { - "low": _GRANULARITY_MAX_CHUNKS // 16, # 2 - "medium": _GRANULARITY_MAX_CHUNKS // 4, # 8 - "high": _GRANULARITY_MAX_CHUNKS, # 32 +_MAX_PARALLEL_CHUNKS = 32 +_LEVEL_CAPS: dict[str, int] = { + "low": _MAX_PARALLEL_CHUNKS // 16, # 2 + "medium": _MAX_PARALLEL_CHUNKS // 4, # 8 + "high": _MAX_PARALLEL_CHUNKS, # 32 } -def _resolve_granularity(level: GranularityLevel) -> int: +def _resolve_level(level: ParallelChunksLevel) -> int: """ - Map a granularity level name to its total sub-request cap. + Map a parallel_chunks level name to its total sub-request cap. Parameters ---------- @@ -237,13 +237,13 @@ def _resolve_granularity(level: GranularityLevel) -> int: """ if level not in _VALID_LEVELS: raise ValueError( - f"chunk_granularity level must be one of {_VALID_LEVELS}; got {level!r}." + f"parallel_chunks level must be one of {_VALID_LEVELS}; got {level!r}." ) - return _GRANULARITY_LEVELS[level] + return _LEVEL_CAPS[level] @contextmanager -def chunk_granularity(level: GranularityLevel) -> Iterator[None]: +def parallel_chunks(level: ParallelChunksLevel) -> Iterator[None]: """ Scope how finely the OGC getters chunk multi-value requests. @@ -301,7 +301,7 @@ def chunk_granularity(level: GranularityLevel) -> Iterator[None]: Examples -------- >>> from dataretrieval import waterdata - >>> with waterdata.chunk_granularity("high"): + >>> with waterdata.parallel_chunks("high"): ... df, md = waterdata.get_daily( ... monitoring_location_id=many_sites, parameter_code="00060" ... ) # doctest: +SKIP @@ -310,7 +310,7 @@ def chunk_granularity(level: GranularityLevel) -> Iterator[None]: -------- ChunkPlan._refine : the planning-side effect of the level. """ - with _granularity(_resolve_granularity(level)): + with _parallel_chunks(_resolve_level(level)): yield @@ -734,7 +734,7 @@ def multi_value_chunked( completion via :meth:`ChunkedCall.resume`. The plan splits multi-value list params and the cql-text filter so each sub-request URL fits the byte limit; an already-fitting request is a one-step plan, unless an - active :func:`chunk_granularity` block asks the plan to fan out more + active :func:`parallel_chunks` block asks the plan to fan out more finely. See the module docstring for the concurrency model. Parameters @@ -779,13 +779,13 @@ def wrapper( finalize: _Finalize = _passthrough_result, ) -> tuple[pd.DataFrame, Any]: limit = _OGC_URL_BYTE_LIMIT if url_limit is None else url_limit - # Read the granularity dial from the ambient set by - # ``chunk_granularity`` (0 = off outside any such block; otherwise the + # Read the parallel_chunks dial from the ambient set by + # ``parallel_chunks`` (0 = off outside any such block; otherwise the # per-axis sub-chunk cap). It only affects *planning*, done here up # front, so a later resume — which re-issues the already-planned # sub-requests — needs no snapshot. plan = ChunkPlan( - args, build_request, limit, max_chunks_per_axis=_granularity.get() + args, build_request, limit, max_chunks_per_axis=_parallel_chunks.get() ) retry_policy = RetryPolicy.from_env() # The concurrency cap is resolved inside ``resume()`` from diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index 58bdad65..1e8bb9d3 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -296,7 +296,7 @@ def _split_at(chunks: list[list[str]], idx: int) -> None: (each atom survives, exactly once) and *contiguous, deterministic order* (resume and :meth:`ChunkPlan.iter_sub_args` depend on it). Kept in one place so those invariants can't drift between :meth:`ChunkPlan._plan` - (byte-driven) and :meth:`ChunkPlan._refine` (granularity-driven). + (byte-driven) and :meth:`ChunkPlan._refine` (fan-out-driven). """ chunk = chunks[idx] mid = len(chunk) // 2 @@ -337,7 +337,7 @@ class ChunkPlan: cartesian product across axes, never fewer than the byte budget already forces) — capped as a whole, not per axis, so several multi-value axes can't multiply past the cap. Set from the resolved - :func:`~dataretrieval.ogc.chunking.chunk_granularity` level; see + :func:`~dataretrieval.ogc.chunking.parallel_chunks` level; see :meth:`_refine`. Attributes @@ -379,7 +379,7 @@ def __init__( axes = _extract_axes(args) if not axes: - # No chunkable axis: nothing to split, and ``granularity`` has + # No chunkable axis: nothing to split, and ``parallel_chunks`` has # nothing to act on either. If the single request fits, run it # verbatim (the common passthrough). ``_safe_request_bytes`` treats # an un-constructable URL (httpx.InvalidURL, > 64 KB) as over budget. @@ -494,8 +494,8 @@ def _plan( def _refine(self, max_chunks_per_axis: int) -> None: """ Fan the plan out more finely than the byte budget alone requires — - the granularity dial (see - :func:`~dataretrieval.ogc.chunking.chunk_granularity` for why a caller + the parallel_chunks dial (see + :func:`~dataretrieval.ogc.chunking.parallel_chunks` for why a caller would want this). Caps the plan's *total* sub-request count (:attr:`total`, the @@ -514,7 +514,7 @@ def _refine(self, max_chunks_per_axis: int) -> None: ---------- max_chunks_per_axis : int Soft cap on the plan's total sub-request count (``0`` = off), - the resolved granularity level. Despite the name — kept for the + the resolved parallel_chunks level. Despite the name — kept for the public dial's "sub-chunks per multi-value argument" framing, which matches this cap exactly in the common single-axis case — multi-axis plans are capped on the product, not per axis. diff --git a/dataretrieval/waterdata/__init__.py b/dataretrieval/waterdata/__init__.py index d70a3d74..7a987442 100644 --- a/dataretrieval/waterdata/__init__.py +++ b/dataretrieval/waterdata/__init__.py @@ -9,7 +9,7 @@ from __future__ import annotations -from dataretrieval.ogc.chunking import GranularityLevel, chunk_granularity +from dataretrieval.ogc.chunking import ParallelChunksLevel, parallel_chunks from dataretrieval.ogc.filters import FILTER_LANG # Public API exports @@ -51,8 +51,8 @@ "PROFILE_LOOKUP", "SERVICES", "WATERDATA_SERVICES", - "GranularityLevel", - "chunk_granularity", + "ParallelChunksLevel", + "parallel_chunks", "get_channel", "get_codes", "get_combined_metadata", diff --git a/docs/source/userguide/errors.rst b/docs/source/userguide/errors.rst index 0c4429c0..7af7bada 100644 --- a/docs/source/userguide/errors.rst +++ b/docs/source/userguide/errors.rst @@ -106,7 +106,7 @@ extra quota *as long as each sub-request still spans many pages* (ten states pulled as one request then page nearly as many times as ten per-state requests would; a split that leaves each sub-request only a page or two adds its partial final page). So if you *know* your pull is large you can ask for a finer split -with ``chunk_granularity`` -- trading roughly the same pages for more, smaller +with ``parallel_chunks`` -- trading roughly the same pages for more, smaller sub-requests, which gives smoother progress, more even concurrency, and a smaller unit of retry/resume. It is a scoped ``with`` block, so an aggressive setting can't leak into unrelated calls and @@ -116,7 +116,7 @@ accidentally spend quota: from dataretrieval import waterdata - with waterdata.chunk_granularity("high"): + with waterdata.parallel_chunks("high"): df, md = waterdata.get_daily( monitoring_location_id=many_sites, parameter_code="00060" ) diff --git a/tests/utils_test.py b/tests/utils_test.py index 8ca4630d..dca00218 100644 --- a/tests/utils_test.py +++ b/tests/utils_test.py @@ -208,21 +208,21 @@ def test_chunk_interruptions_exported_at_top_level(self): dataretrieval.ChunkInterrupted, dataretrieval.DataRetrievalError ) - def test_chunk_granularity_exported_at_top_level_and_waterdata(self): - """The ``chunk_granularity`` context manager and its ``GranularityLevel`` + def test_parallel_chunks_exported_at_top_level_and_waterdata(self): + """The ``parallel_chunks`` context manager and its ``ParallelChunksLevel`` type are reachable both from the top level (``from dataretrieval import - chunk_granularity``) and from the user-facing ``dataretrieval.waterdata`` + parallel_chunks``) and from the user-facing ``dataretrieval.waterdata`` namespace, and both resolve to the single objects defined in ``dataretrieval.ogc.chunking``.""" import dataretrieval from dataretrieval import waterdata from dataretrieval.ogc import chunking - assert dataretrieval.chunk_granularity is chunking.chunk_granularity - assert waterdata.chunk_granularity is chunking.chunk_granularity - assert dataretrieval.GranularityLevel is chunking.GranularityLevel - assert waterdata.GranularityLevel is chunking.GranularityLevel - for name in ("chunk_granularity", "GranularityLevel"): + assert dataretrieval.parallel_chunks is chunking.parallel_chunks + assert waterdata.parallel_chunks is chunking.parallel_chunks + assert dataretrieval.ParallelChunksLevel is chunking.ParallelChunksLevel + assert waterdata.ParallelChunksLevel is chunking.ParallelChunksLevel + for name in ("parallel_chunks", "ParallelChunksLevel"): assert name in dataretrieval.__all__ assert name in waterdata.__all__ diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index 055db44b..98cb965b 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -40,15 +40,15 @@ from dataretrieval.ogc import chunking as _chunking from dataretrieval.ogc import retry as _retry_mod from dataretrieval.ogc.chunking import ( - _GRANULARITY_LEVELS, - _GRANULARITY_MAX_CHUNKS, + _LEVEL_CAPS, + _MAX_PARALLEL_CHUNKS, ChunkedCall, _chunked_client, - _granularity, - _resolve_granularity, - chunk_granularity, + _parallel_chunks, + _resolve_level, get_active_client, multi_value_chunked, + parallel_chunks, ) from dataretrieval.ogc.interruptions import ( ChunkInterrupted, @@ -2113,14 +2113,14 @@ async def fetch(args): # --------------------------------------------------------------------------- -# Chunk granularity: the opt-in dial (``"low"`` / ``"medium"`` / ``"high"``) to +# Parallel chunks: the opt-in dial (``"low"`` / ``"medium"`` / ``"high"``) to # fan a query out MORE finely than the byte limit alone requires -# (``ChunkPlan._refine`` + the ``chunk_granularity`` context manager). +# (``ChunkPlan._refine`` + the ``parallel_chunks`` context manager). # ``_fake_build``'s base is 200 bytes, so a handful of short atoms sits far # under ``url_limit=8000`` — the byte pass passes it through untouched, and any -# splitting below is the granularity cap alone. ``ChunkPlan`` takes the resolved -# integer cap (``max_chunks_per_axis``) directly; ``chunk_granularity`` / -# ``_resolve_granularity`` map the level names onto it. The cap bounds the +# splitting below is the parallel_chunks cap alone. ``ChunkPlan`` takes the resolved +# integer cap (``max_chunks_per_axis``) directly; ``parallel_chunks`` / +# ``_resolve_level`` map the level names onto it. The cap bounds the # plan's *total* sub-request count (the cartesian product across axes), not # each axis independently — see ``test_cap_caps_the_total_across_axes``. # --------------------------------------------------------------------------- @@ -2166,9 +2166,9 @@ def test_cap_ramps_then_saturates(max_chunks_per_axis, expected_pieces): def test_cap_bounds_fan_out_for_a_long_axis(): """The cap is a quota guardrail: at the ``"high"`` cap a 100-atom axis fans into ``cap`` pieces — NOT 100 singletons — so an accidental - ``chunk_granularity("high")`` on a huge list can't detonate into hundreds + ``parallel_chunks("high")`` on a huge list can't detonate into hundreds of sub-requests. Every atom is still covered exactly once.""" - high = _GRANULARITY_LEVELS["high"] + high = _LEVEL_CAPS["high"] atoms = [f"X{i:03d}" for i in range(100)] plan = ChunkPlan( {"monitoring_location_id": atoms}, @@ -2245,7 +2245,7 @@ def test_cap_bounds_fan_out_across_many_axes(): at the ``"high"`` cap still top out at ``high`` sub-requests total, not ``high ** 3`` — the property the single-axis-only cap in the original implementation did not guarantee.""" - high = _GRANULARITY_LEVELS["high"] + high = _LEVEL_CAPS["high"] # Three chunkable axes (two list axes + the filter OR-axis), each with 10 # atoms — under the old per-axis cap this would have been high**3. args = { @@ -2267,23 +2267,24 @@ def test_cap_does_not_mask_unchunkable(): @pytest.mark.parametrize("level", ["low", "medium", "high"]) -def test_resolve_granularity_maps_each_level_to_its_cap(level): +def test_resolve_level_maps_each_level_to_its_cap(level): """Each level name resolves to its total sub-request cap from the table (a positive int).""" - assert _resolve_granularity(level) == _GRANULARITY_LEVELS[level] - assert _resolve_granularity(level) >= 1 + assert _resolve_level(level) == _LEVEL_CAPS[level] + assert _resolve_level(level) >= 1 -def test_granularity_levels_ordered_and_spaced(): +def test_parallel_chunks_levels_ordered_and_spaced(): """The three caps hold the properties callers rely on: strictly increasing, ``"high"`` saturating the declared ceiling, ``"low"`` still a real split - (>= 2), and each level spaced 4x from the next. The ceiling is a granularity - constant, deliberately independent of the concurrency default.""" - low = _GRANULARITY_LEVELS["low"] - medium = _GRANULARITY_LEVELS["medium"] - high = _GRANULARITY_LEVELS["high"] + (>= 2), and each level spaced 4x from the next. The ceiling is a + parallel_chunks constant, deliberately independent of the concurrency + default.""" + low = _LEVEL_CAPS["low"] + medium = _LEVEL_CAPS["medium"] + high = _LEVEL_CAPS["high"] assert low < medium < high - assert high == _GRANULARITY_MAX_CHUNKS + assert high == _MAX_PARALLEL_CHUNKS assert low >= 2 assert medium == high // 4 assert low == medium // 4 @@ -2300,41 +2301,41 @@ def test_granularity_levels_ordered_and_spaced(): ["low"], # unhashable → the ``not in`` check still rejects it cleanly ], ) -def test_resolve_granularity_rejects_everything_but_the_three_levels(bad): +def test_resolve_level_rejects_everything_but_the_three_levels(bad): """Only the three exact level strings are accepted; every other value — a representative of each rejected shape (dead keyword, wrong case, whitespace, old int, ``None``, unhashable) — raises ``ValueError`` so a typo fails loudly.""" - with pytest.raises(ValueError, match="chunk_granularity level must be"): - _resolve_granularity(bad) + with pytest.raises(ValueError, match="parallel_chunks level must be"): + _resolve_level(bad) -def test_chunk_granularity_scopes_and_restores_the_ambient(): +def test_parallel_chunks_scopes_and_restores_the_ambient(): """The context manager resolves the level to its cap, publishes it on the ambient for the block, and restores the previous value on exit — including proper nesting.""" - assert _granularity.get() == 0 - with chunk_granularity("high"): - assert _granularity.get() == _GRANULARITY_LEVELS["high"] - with chunk_granularity("low"): - assert _granularity.get() == _GRANULARITY_LEVELS["low"] - assert _granularity.get() == _GRANULARITY_LEVELS["high"] # outer restored - assert _granularity.get() == 0 # default (off) outside any block + assert _parallel_chunks.get() == 0 + with parallel_chunks("high"): + assert _parallel_chunks.get() == _LEVEL_CAPS["high"] + with parallel_chunks("low"): + assert _parallel_chunks.get() == _LEVEL_CAPS["low"] + assert _parallel_chunks.get() == _LEVEL_CAPS["high"] # outer restored + assert _parallel_chunks.get() == 0 # default (off) outside any block -def test_chunk_granularity_validates_on_entry(): +def test_parallel_chunks_validates_on_entry(): """An invalid level raises at ``with`` entry — before any request is issued — and leaves the ambient untouched.""" - with pytest.raises(ValueError, match="chunk_granularity level must be"): - with chunk_granularity("aggressive"): + with pytest.raises(ValueError, match="parallel_chunks level must be"): + with parallel_chunks("aggressive"): pass - assert _granularity.get() == 0 + assert _parallel_chunks.get() == 0 -def test_chunk_granularity_high_drives_end_to_end_fan_out(): +def test_parallel_chunks_high_drives_end_to_end_fan_out(): """End-to-end: the same fitting request passes through as a single call by default, but fans into several sub-requests inside a - ``chunk_granularity("high")`` block — and the combined result still + ``parallel_chunks("high")`` block — and the combined result still recovers every atom exactly once.""" sites = [f"S{i:02d}" for i in range(8)] @@ -2352,7 +2353,7 @@ async def fetch(args): assert sorted(df_plain["site"]) == sorted(sites) calls.clear() - with chunk_granularity("high"): + with parallel_chunks("high"): df_fine, _ = fetch({"monitoring_location_id": sites}) # 8 atoms at the "high" cap (>= 8) → 8 singleton sub-requests. assert len(calls) == 8 @@ -2362,10 +2363,10 @@ async def fetch(args): assert sorted(df_fine["site"]) == sorted(sites) -def test_chunk_granularity_low_is_a_gentle_split(): +def test_parallel_chunks_low_is_a_gentle_split(): """``"low"`` is the mildest opt-in: an under-limit request fans into just the ``"low"`` cap's worth of pieces, not singletons.""" - low = _GRANULARITY_LEVELS["low"] + low = _LEVEL_CAPS["low"] sites = [f"S{i:02d}" for i in range(8)] calls: list[int] = [] @@ -2374,7 +2375,7 @@ async def fetch(args): calls.append(len(args["monitoring_location_id"])) return pd.DataFrame(), _ok_response() - with chunk_granularity("low"): + with parallel_chunks("low"): fetch({"monitoring_location_id": sites}) # low cap → that many sub-requests, together covering all 8 sites. assert len(calls) == low From 115a2302033efadd147e59b27e70f3104ada23ff Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 8 Jul 2026 19:03:11 -0500 Subject: [PATCH 4/5] refactor(waterdata): make parallel_chunks take an integer, not low/medium/high Replace the three-tier "low"/"medium"/"high" enum with a plain positive integer: parallel_chunks(n) fans a call out into n sub-requests. More expressive (any n, not just 2/8/32), precise, and mirrors the int-valued API_USGS_CONCURRENT. Removes ParallelChunksLevel, the _LEVEL_CAPS/_MAX_PARALLEL_CHUNKS constants, and _resolve_level; validation is now an inline positive-int check (rejects 0, negatives, floats, bool, str). n is bounded below by the byte-limit minimum and above by the number of values to split; n=1 is an explicit no-op. Docstrings, NEWS, user guide, README, exports, and tests updated; 2/8/32 remain as documented examples. Co-Authored-By: Claude Opus 4.8 (1M context) --- NEWS.md | 2 +- README.md | 40 +++---- dataretrieval/__init__.py | 9 +- dataretrieval/ogc/chunking.py | 132 ++++++++--------------- dataretrieval/ogc/planning.py | 14 +-- dataretrieval/waterdata/__init__.py | 3 +- docs/source/userguide/errors.rst | 30 +++--- tests/utils_test.py | 16 ++- tests/waterdata_chunking_test.py | 156 ++++++++++++---------------- 9 files changed, 166 insertions(+), 236 deletions(-) diff --git a/NEWS.md b/NEWS.md index e3d6f32b..aee2ef07 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -**07/01/2026:** Added `waterdata.parallel_chunks(...)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further costs little or no extra quota when each sub-request still spans many pages, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.parallel_chunks("high"): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. The level is one of `"low"`, `"medium"`, or `"high"` — typed as `waterdata.ParallelChunksLevel` (a `typing.Literal`), so a type checker rejects any other value and an invalid string raises `ValueError` at the `with`. Each level caps the *total* number of sub-requests the call is split into — `"low"` / `"medium"` / `"high"` cap at 2 / 8 / 32 overall, across every multi-value argument combined, not per argument. That ceiling is a parallel_chunks constant, deliberately independent of the fan-out concurrency (`API_USGS_CONCURRENT`): how finely a query splits is orthogonal to how many sub-requests run at once. Capping the aggressive end at 32 bounds the blast radius — an accidental `"high"` on a huge list, or a call with several multi-value arguments, can't explode into thousands of sub-requests. There is no "off" level: not entering the block *is* off. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.parallel_chunks`. +**07/01/2026:** Added `waterdata.parallel_chunks(n)` — a context manager to control how finely the OGC `waterdata` getters split multi-value requests. By default the getters chunk only as much as the server's ~8 KB URL limit forces (the fewest sub-requests). But because every sub-request paginates, splitting a large result further costs little or no extra quota when each sub-request still spans many pages, so a caller who *knows* their pull is large can opt into a finer split for smoother progress, more even concurrency, and a smaller unit of retry/resume: `with waterdata.parallel_chunks(32): df, md = waterdata.get_daily(monitoring_location_id=many_sites)`. `n` is a positive integer (e.g. `2`, `8`, `32`) — the number of sub-requests to fan the call out into; a non-integer or non-positive value raises `ValueError` at the `with`. It caps the *total* sub-request count across every multi-value argument combined (not per argument), bounded below by what the byte limit already forces and above by how many values there are to split, so several multi-value arguments can't multiply past it and `n=1` asks for no extra fan-out. Each sub-request costs a request against your hourly rate limit, and because how many run *at once* is capped separately by `API_USGS_CONCURRENT` (default 32) an `n` beyond that adds quota without adding parallelism, so the useful range is roughly `2` up to `API_USGS_CONCURRENT`. It is deliberately a scoped `with` block rather than an automatic behavior or an environment variable, since the library can't tell in advance whether a query is large — a short-window query might fit in a single page, where extra chunks would only burn quota. Also exported at the top level as `dataretrieval.parallel_chunks`. **06/23/2026:** **Breaking change (1.2.0):** the minimum supported Python is now **3.10** (`requires-python = ">=3.10"`). 3.9 support was already effectively broken — the `waterdata` module's dependencies (`anyio`, the test stack) require 3.10+, and the `waterdata` test modules already skipped on <3.10. `anyio` is now declared as a direct dependency (it is imported directly by `waterdata`), and the CI/ruff/mypy targets move to 3.10. Also fully removed the deprecated `variable_info` metadata property: the `NWIS_Metadata` override only warned and returned `None` (it relied on the defunct `get_pmcodes`), and the `BaseMetadata` abstract is gone too since nothing implemented it — accessing `.variable_info` now raises `AttributeError`. `site_info` is unaffected. diff --git a/README.md b/README.md index 71f3b329..73aa273e 100644 --- a/README.md +++ b/README.md @@ -111,11 +111,12 @@ By default the getters split a multi-value request only as far as the server's ~8 KB URL limit forces — the fewest sub-requests. For a **large, paginated** pull that is needlessly conservative: every sub-request pages through its own results, so dividing the query into more, smaller sub-requests lets those pages -be fetched **in parallel**. `parallel_chunks` opts a single call into that -finer split. It pays off only when the result is large enough to span many -pages *and* the query has a multi-value argument to divide (such as a list of -monitoring locations); on a small query — or one with nothing to split — it just -adds requests, so it is a deliberate, scoped `with` block, never the default. +be fetched **in parallel**. `parallel_chunks(n)` opts a single call into that +finer split, fanning it out into `n` sub-requests. It pays off only when the +result is large enough to span many pages *and* the query has a multi-value +argument to divide (such as a list of monitoring locations); on a small query — +or one with nothing to split — it just adds requests, so it is a deliberate, +scoped `with` block, never the default. ```python from dataretrieval import waterdata @@ -124,7 +125,7 @@ from dataretrieval import waterdata # enough to span many pages, so it profits from a finer split. sites, _ = waterdata.get_monitoring_locations(state="Ohio", site_type_code="ST") -with waterdata.parallel_chunks("high"): # "low" | "medium" | "high" +with waterdata.parallel_chunks(32): # fan out into 32 sub-requests df, md = waterdata.get_daily( monitoring_location_id=sites["monitoring_location_id"].tolist(), parameter_code="00060", # discharge @@ -132,26 +133,27 @@ with waterdata.parallel_chunks("high"): # "low" | "medium" | "high" ) ``` -`"high"` fans a call out into up to 32 sub-requests, `"medium"` up to 8, and -`"low"` up to 2 — a fixed ceiling, so an accidental setting on a huge list can't -explode into thousands of requests. +`n` is the number of sub-requests to fan the call out into. It is capped by how +many values there are to split, and each sub-request costs a request against +your hourly [rate limit](https://api.waterdata.usgs.gov/signup/); since how many +run *at once* is capped separately by `API_USGS_CONCURRENT` (default 32), the +useful range is roughly `2` up to that value. Benchmark — 271 Ohio discharge sites (`get_daily`, `parameter_code="00060"`), -cold cache, each level run against its own time window so results are not -cache-served, with the page size fixed so every level fetches roughly the same +cold cache, each `n` run against its own time window so results are not +cache-served, with the page size fixed so every run fetches roughly the same number of pages (isolating the effect of parallelism): -| level | parallelism | pages | wall-clock | speedup | -| -------------- | ----------- | ----- | ------------------------- | ------- | -| `default` | 1 | 32 | 23.9 s / 27.4 s (2 runs) | 1× | -| `"medium"` (8) | 8 | 37 | 4.1 s / 4.3 s | ~6× | -| `"high"` (32) | 32 | 44 | 2.0 s | ~12× | +| `n` | parallelism | pages | wall-clock | speedup | +| ---- | ----------- | ----- | ------------------------- | ------- | +| off | 1 | 32 | 23.9 s / 27.4 s (2 runs) | 1× | +| `8` | 8 | 37 | 4.1 s / 4.3 s | ~6× | +| `32` | 32 | 44 | 2.0 s | ~12× | The gain comes from overlapping each sub-request's per-page latency and server-side work — a genuinely large pull at the default page size shows a -similar multi-fold speedup. The extra sub-requests each cost one request against -your hourly [rate limit](https://api.waterdata.usgs.gov/signup/), so reserve the -aggressive levels for pulls you know are large. +similar multi-fold speedup. The extra sub-requests each cost quota, so reserve a +large `n` for pulls you know are large. Visit the [API Reference](https://doi-usgs.github.io/dataretrieval-python/reference/waterdata.html) diff --git a/dataretrieval/__init__.py b/dataretrieval/__init__.py index 567be6ab..469fe0f5 100644 --- a/dataretrieval/__init__.py +++ b/dataretrieval/__init__.py @@ -44,10 +44,10 @@ URLTooLong, ) -# Parallel-chunks control (a context manager) and its level type. Defined with -# the chunker in ``dataretrieval.ogc.chunking``; surfaced here for a stable -# public path ``from dataretrieval import parallel_chunks``. -from dataretrieval.ogc.chunking import ParallelChunksLevel, parallel_chunks +# Parallel-chunks control (a context manager). Defined with the chunker in +# ``dataretrieval.ogc.chunking``; surfaced here for a stable public path +# ``from dataretrieval import parallel_chunks``. +from dataretrieval.ogc.chunking import parallel_chunks # Resumable chunk-interruption exceptions. They are defined in # ``dataretrieval.ogc.interruptions`` rather than ``dataretrieval.exceptions`` @@ -100,6 +100,5 @@ "ServiceInterrupted", # parallel-chunks control (defined in ogc.chunking) "parallel_chunks", - "ParallelChunksLevel", "__version__", ] diff --git a/dataretrieval/ogc/chunking.py b/dataretrieval/ogc/chunking.py index 3d0e9aff..1a3b37b6 100644 --- a/dataretrieval/ogc/chunking.py +++ b/dataretrieval/ogc/chunking.py @@ -11,9 +11,9 @@ Parallel chunks: the planner is conservative by default — it splits only as far as the byte limit forces. A caller who knows their result is large can opt into a -finer split via the ``parallel_chunks`` context manager -(``"low"`` / ``"medium"`` / ``"high"``); the resolved cap drives -:meth:`ChunkPlan._refine`. See ``parallel_chunks`` for the why and the when. +finer split via the ``parallel_chunks(n)`` context manager, which fans the query +out into ``n`` parallel sub-requests; ``n`` drives :meth:`ChunkPlan._refine`. See +``parallel_chunks`` for the why and the when. This module owns the *execution* half — the event loop and bounded concurrency that drive a plan to completion (``ChunkedCall``) plus the @@ -77,7 +77,7 @@ from collections.abc import Callable, Iterator from contextlib import contextmanager from contextvars import copy_context -from typing import Any, Literal, cast, get_args +from typing import Any, cast import httpx import pandas as pd @@ -182,70 +182,16 @@ def get_active_client() -> httpx.AsyncClient | None: # Parallel-chunks dial: opt-in to fan a query out *more finely* than the byte # limit alone requires. Scoped to a ``with parallel_chunks(...):`` block (a # ContextVar), deliberately NOT an env var (see :func:`parallel_chunks` for -# why). The ambient holds the resolved cap on the plan's total sub-request -# count; ``0`` (the default, outside any block) means "chunk only as much as -# the byte limit needs". +# why). The ambient holds ``n`` — the requested cap on the plan's total +# sub-request count; ``0`` (the default, outside any block) means "chunk only as +# much as the byte limit needs". _parallel_chunks: Ambient[int] = Ambient("ogc_parallel_chunks", 0) -#: The three accepted parallel_chunks levels, as a typing ``Literal`` so a type -#: checker rejects any other value at the call site. -ParallelChunksLevel = Literal["low", "medium", "high"] - -#: Valid levels derived from the type, so ``ParallelChunksLevel`` stays the single -#: source of truth for what ``_resolve_level`` accepts (mirrors the -#: ``get_args``-based ``_VALID_ON_TIE`` / ``_VALID_FILE_TYPES`` in sibling -#: modules). -_VALID_LEVELS: tuple[ParallelChunksLevel, ...] = get_args(ParallelChunksLevel) - -# The parallel_chunks ceiling on the plan's total sub-request count — -# deliberately NOT tied to the concurrency default: fan-out *volume* (how many -# sub-requests a query becomes) is orthogonal to how many run at once -# (``API_USGS_CONCURRENT``). 32 is a sane ceiling on the whole call (across -# every axis, not per axis — see ``ChunkPlan._refine``) that bounds the blast -# radius of an accidental ``"high"`` on a very long list or several multi-value -# arguments at once; the milder levels are a quarter and a sixteenth of it (the -# three are spaced 4x apart). -_MAX_PARALLEL_CHUNKS = 32 -_LEVEL_CAPS: dict[str, int] = { - "low": _MAX_PARALLEL_CHUNKS // 16, # 2 - "medium": _MAX_PARALLEL_CHUNKS // 4, # 8 - "high": _MAX_PARALLEL_CHUNKS, # 32 -} - - -def _resolve_level(level: ParallelChunksLevel) -> int: - """ - Map a parallel_chunks level name to its total sub-request cap. - - Parameters - ---------- - level : {"low", "medium", "high"} - The user-supplied level. - - Returns - ------- - int - The maximum total sub-requests for the whole call at that level - (``2`` / ``8`` / ``32``). - - Raises - ------ - ValueError - If ``level`` is anything other than ``"low"``, ``"medium"``, or - ``"high"`` — raised eagerly (at the ``with`` statement) so a typo fails - loudly rather than silently doing nothing. - """ - if level not in _VALID_LEVELS: - raise ValueError( - f"parallel_chunks level must be one of {_VALID_LEVELS}; got {level!r}." - ) - return _LEVEL_CAPS[level] - @contextmanager -def parallel_chunks(level: ParallelChunksLevel) -> Iterator[None]: +def parallel_chunks(n: int) -> Iterator[None]: """ - Scope how finely the OGC getters chunk multi-value requests. + Fan the OGC getters' multi-value requests out into ``n`` parallel sub-requests. By default the Water Data / NGWMN getters chunk a request only as much as the server's ~8 KB URL-byte limit forces — the fewest sub-requests that @@ -267,26 +213,27 @@ def parallel_chunks(level: ParallelChunksLevel) -> Iterator[None]: automatic behavior or a process-wide environment variable — scoping it to a ``with`` block keeps an aggressive setting from leaking into unrelated calls and accidentally spending quota. Outside any block the getters use the - conservative default; there is no "off" level because *not* entering the - block is off. Only the OGC getters (Water Data, NGWMN) read this; wrapping a - legacy NWIS call in the block is a harmless no-op. + conservative default. Only the OGC getters (Water Data, NGWMN) read this; + wrapping a legacy NWIS call in the block is a harmless no-op. Parameters ---------- - level : {"low", "medium", "high"} - How aggressively to chunk within the block. Each level caps the - *total* number of sub-requests the call is split into — ``2`` / ``8`` - / ``32`` for ``"low"`` / ``"medium"`` / ``"high"`` — or one - sub-request per remaining atom once that's fewer than the cap. The - cap applies to the whole call, not per multi-value argument: with - several multi-value arguments the axes are refined together against - the same shared ceiling (their cartesian product does not multiply - past it). The ceiling is fixed (it is *not* tied to - ``API_USGS_CONCURRENT``: how finely a query splits is orthogonal to - how many sub-requests run at once); capping ``"high"`` at ``32`` keeps - an accidental aggressive level on a very long list — or several - multi-value arguments at once — from exploding into thousands of - sub-requests. + n : int + The number of sub-requests to fan the whole call out into — a positive + integer such as ``2``, ``8``, or ``32``. It caps the plan's *total* + sub-request count (the cartesian product across every multi-value + argument combined, not per argument), so several multi-value arguments + cannot multiply past it. The actual count is bounded below by what the + ~8 KB URL limit already forces and above by the number of values there + are to split, so an ``n`` larger than the input allows simply yields one + sub-request per value; ``n=1`` asks for no extra fan-out. + + Each sub-request fetches at least one page, so it costs at least one + request against your hourly rate limit — a larger ``n`` spends more + quota. And because how many sub-requests run *at once* is capped + separately by ``API_USGS_CONCURRENT`` (default 32), an ``n`` beyond that + adds quota without adding parallelism; the useful range is roughly ``2`` + up to ``API_USGS_CONCURRENT``. Yields ------ @@ -295,22 +242,29 @@ def parallel_chunks(level: ParallelChunksLevel) -> Iterator[None]: Raises ------ ValueError - If ``level`` isn't ``"low"``, ``"medium"``, or ``"high"`` — raised on - ``with`` entry, before any request is issued. + If ``n`` is not a positive integer — raised on ``with`` entry, before + any request is issued, so a bad value fails loudly rather than silently + doing nothing. Examples -------- >>> from dataretrieval import waterdata - >>> with waterdata.parallel_chunks("high"): + >>> with waterdata.parallel_chunks(32): ... df, md = waterdata.get_daily( ... monitoring_location_id=many_sites, parameter_code="00060" ... ) # doctest: +SKIP See Also -------- - ChunkPlan._refine : the planning-side effect of the level. + ChunkPlan._refine : the planning-side effect of ``n``. """ - with _parallel_chunks(_resolve_level(level)): + # ``bool`` is an ``int`` subclass but nonsensical here; reject it explicitly. + if not isinstance(n, int) or isinstance(n, bool) or n < 1: + raise ValueError( + f"parallel_chunks(n): n must be a positive integer (e.g. 2, 8, 32); " + f"got {n!r}." + ) + with _parallel_chunks(n): yield @@ -779,11 +733,11 @@ def wrapper( finalize: _Finalize = _passthrough_result, ) -> tuple[pd.DataFrame, Any]: limit = _OGC_URL_BYTE_LIMIT if url_limit is None else url_limit - # Read the parallel_chunks dial from the ambient set by + # Read the parallel_chunks dial ``n`` from the ambient set by # ``parallel_chunks`` (0 = off outside any such block; otherwise the - # per-axis sub-chunk cap). It only affects *planning*, done here up - # front, so a later resume — which re-issues the already-planned - # sub-requests — needs no snapshot. + # requested total sub-request cap). It only affects *planning*, done + # here up front, so a later resume — which re-issues the + # already-planned sub-requests — needs no snapshot. plan = ChunkPlan( args, build_request, limit, max_chunks_per_axis=_parallel_chunks.get() ) diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index 1e8bb9d3..ebadcb36 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -336,8 +336,8 @@ class ChunkPlan: out to up to ``max_chunks_per_axis`` sub-requests overall (the cartesian product across axes, never fewer than the byte budget already forces) — capped as a whole, not per axis, so several - multi-value axes can't multiply past the cap. Set from the resolved - :func:`~dataretrieval.ogc.chunking.parallel_chunks` level; see + multi-value axes can't multiply past the cap. Set from the + :func:`~dataretrieval.ogc.chunking.parallel_chunks` ``n``; see :meth:`_refine`. Attributes @@ -513,11 +513,11 @@ def _refine(self, max_chunks_per_axis: int) -> None: Parameters ---------- max_chunks_per_axis : int - Soft cap on the plan's total sub-request count (``0`` = off), - the resolved parallel_chunks level. Despite the name — kept for the - public dial's "sub-chunks per multi-value argument" framing, - which matches this cap exactly in the common single-axis case — - multi-axis plans are capped on the product, not per axis. + Soft cap on the plan's total sub-request count (``0`` = off) — the + ``parallel_chunks(n)`` value. The parameter name is legacy: the cap + is on the whole plan (the cartesian product across axes), which + matches "sub-chunks per argument" only in the common single-axis + case — multi-axis plans are capped on the product, not per axis. """ if max_chunks_per_axis <= 0: return diff --git a/dataretrieval/waterdata/__init__.py b/dataretrieval/waterdata/__init__.py index 7a987442..eb231469 100644 --- a/dataretrieval/waterdata/__init__.py +++ b/dataretrieval/waterdata/__init__.py @@ -9,7 +9,7 @@ from __future__ import annotations -from dataretrieval.ogc.chunking import ParallelChunksLevel, parallel_chunks +from dataretrieval.ogc.chunking import parallel_chunks from dataretrieval.ogc.filters import FILTER_LANG # Public API exports @@ -51,7 +51,6 @@ "PROFILE_LOOKUP", "SERVICES", "WATERDATA_SERVICES", - "ParallelChunksLevel", "parallel_chunks", "get_channel", "get_codes", diff --git a/docs/source/userguide/errors.rst b/docs/source/userguide/errors.rst index 7af7bada..28da515f 100644 --- a/docs/source/userguide/errors.rst +++ b/docs/source/userguide/errors.rst @@ -106,7 +106,7 @@ extra quota *as long as each sub-request still spans many pages* (ten states pulled as one request then page nearly as many times as ten per-state requests would; a split that leaves each sub-request only a page or two adds its partial final page). So if you *know* your pull is large you can ask for a finer split -with ``parallel_chunks`` -- trading roughly the same pages for more, smaller +with ``parallel_chunks(n)`` -- trading roughly the same pages for more, smaller sub-requests, which gives smoother progress, more even concurrency, and a smaller unit of retry/resume. It is a scoped ``with`` block, so an aggressive setting can't leak into unrelated calls and @@ -116,24 +116,24 @@ accidentally spend quota: from dataretrieval import waterdata - with waterdata.parallel_chunks("high"): + with waterdata.parallel_chunks(32): df, md = waterdata.get_daily( monitoring_location_id=many_sites, parameter_code="00060" ) -The level is one of ``"low"``, ``"medium"``, or ``"high"`` (an invalid value -raises ``ValueError`` at the ``with``). Each caps the *total* number of -sub-requests the call is split into -- ``2`` / ``8`` / ``32`` for -``"low"`` / ``"medium"`` / ``"high"``, across every multi-value argument -combined, not per argument. That ceiling is fixed, deliberately -independent of the fan-out concurrency (``API_USGS_CONCURRENT``): how finely a -query splits is orthogonal to how many sub-requests run at once. Capping the -aggressive end at 32 is a guardrail -- an accidental ``"high"`` on a very long -list, or a call with several multi-value arguments, can't explode into -thousands of sub-requests. There is no "off" level: -simply don't enter the block unless you already expect a large, multi-page -result -- on a query that would have fit in a single page, extra chunks only -burn quota. +``n`` is a positive integer (e.g. ``2``, ``8``, ``32``) -- the number of +sub-requests to fan the call out into; a non-integer or non-positive value +raises ``ValueError`` at the ``with``. It caps the *total* sub-request count +across every multi-value argument combined (not per argument), bounded below by +what the byte limit already forces and above by how many values there are to +split, so several multi-value arguments can't multiply past it and ``n=1`` asks +for no extra fan-out. Each sub-request costs a request against your hourly rate +limit, and because how many run *at once* is capped separately by +``API_USGS_CONCURRENT`` (default 32) an ``n`` beyond that adds quota without +adding parallelism -- the useful range is roughly ``2`` up to +``API_USGS_CONCURRENT``. There is no "off" level: simply don't enter the block +unless you already expect a large, multi-page result -- on a query that would +have fit in a single page, extra chunks only burn quota. The full taxonomy ================= diff --git a/tests/utils_test.py b/tests/utils_test.py index dca00218..30950294 100644 --- a/tests/utils_test.py +++ b/tests/utils_test.py @@ -209,22 +209,18 @@ def test_chunk_interruptions_exported_at_top_level(self): ) def test_parallel_chunks_exported_at_top_level_and_waterdata(self): - """The ``parallel_chunks`` context manager and its ``ParallelChunksLevel`` - type are reachable both from the top level (``from dataretrieval import - parallel_chunks``) and from the user-facing ``dataretrieval.waterdata`` - namespace, and both resolve to the single objects defined in - ``dataretrieval.ogc.chunking``.""" + """The ``parallel_chunks`` context manager is reachable both from the top + level (``from dataretrieval import parallel_chunks``) and from the + user-facing ``dataretrieval.waterdata`` namespace, and both resolve to + the single object defined in ``dataretrieval.ogc.chunking``.""" import dataretrieval from dataretrieval import waterdata from dataretrieval.ogc import chunking assert dataretrieval.parallel_chunks is chunking.parallel_chunks assert waterdata.parallel_chunks is chunking.parallel_chunks - assert dataretrieval.ParallelChunksLevel is chunking.ParallelChunksLevel - assert waterdata.ParallelChunksLevel is chunking.ParallelChunksLevel - for name in ("parallel_chunks", "ParallelChunksLevel"): - assert name in dataretrieval.__all__ - assert name in waterdata.__all__ + assert "parallel_chunks" in dataretrieval.__all__ + assert "parallel_chunks" in waterdata.__all__ class Test_BaseMetadata: diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index 98cb965b..1ab57378 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -40,12 +40,9 @@ from dataretrieval.ogc import chunking as _chunking from dataretrieval.ogc import retry as _retry_mod from dataretrieval.ogc.chunking import ( - _LEVEL_CAPS, - _MAX_PARALLEL_CHUNKS, ChunkedCall, _chunked_client, _parallel_chunks, - _resolve_level, get_active_client, multi_value_chunked, parallel_chunks, @@ -2113,16 +2110,15 @@ async def fetch(args): # --------------------------------------------------------------------------- -# Parallel chunks: the opt-in dial (``"low"`` / ``"medium"`` / ``"high"``) to -# fan a query out MORE finely than the byte limit alone requires -# (``ChunkPlan._refine`` + the ``parallel_chunks`` context manager). -# ``_fake_build``'s base is 200 bytes, so a handful of short atoms sits far -# under ``url_limit=8000`` — the byte pass passes it through untouched, and any -# splitting below is the parallel_chunks cap alone. ``ChunkPlan`` takes the resolved -# integer cap (``max_chunks_per_axis``) directly; ``parallel_chunks`` / -# ``_resolve_level`` map the level names onto it. The cap bounds the -# plan's *total* sub-request count (the cartesian product across axes), not -# each axis independently — see ``test_cap_caps_the_total_across_axes``. +# Parallel chunks: the opt-in dial ``parallel_chunks(n)`` to fan a query out +# MORE finely than the byte limit alone requires (``ChunkPlan._refine`` + the +# ``parallel_chunks`` context manager). ``_fake_build``'s base is 200 bytes, so +# a handful of short atoms sits far under ``url_limit=8000`` — the byte pass +# passes it through untouched, and any splitting below is the ``n`` cap alone. +# ``ChunkPlan`` takes the integer cap (``max_chunks_per_axis``) directly; +# ``parallel_chunks(n)`` publishes ``n`` onto it. The cap bounds the plan's +# *total* sub-request count (the cartesian product across axes), not each axis +# independently — see ``test_cap_caps_the_total_across_axes``. # --------------------------------------------------------------------------- @@ -2164,11 +2160,11 @@ def test_cap_ramps_then_saturates(max_chunks_per_axis, expected_pieces): def test_cap_bounds_fan_out_for_a_long_axis(): - """The cap is a quota guardrail: at the ``"high"`` cap a 100-atom axis - fans into ``cap`` pieces — NOT 100 singletons — so an accidental - ``parallel_chunks("high")`` on a huge list can't detonate into hundreds - of sub-requests. Every atom is still covered exactly once.""" - high = _LEVEL_CAPS["high"] + """The cap holds fan-out to ``n``: at ``n=32`` a 100-atom axis fans into + ``n`` pieces — NOT 100 singletons — so ``parallel_chunks(32)`` on a huge + list can't detonate into hundreds of sub-requests. Every atom is still + covered exactly once.""" + high = 32 atoms = [f"X{i:03d}" for i in range(100)] plan = ChunkPlan( {"monitoring_location_id": atoms}, @@ -2242,10 +2238,10 @@ def test_cap_caps_the_total_across_axes(): def test_cap_bounds_fan_out_across_many_axes(): """The guardrail holds regardless of axis count: three multi-value axes - at the ``"high"`` cap still top out at ``high`` sub-requests total, not - ``high ** 3`` — the property the single-axis-only cap in the original - implementation did not guarantee.""" - high = _LEVEL_CAPS["high"] + at ``n=32`` still top out at ``n`` sub-requests total, not ``n ** 3`` — + the property the single-axis-only cap in the original implementation did + not guarantee.""" + high = 32 # Three chunkable axes (two list axes + the filter OR-axis), each with 10 # atoms — under the old per-axis cap this would have been high**3. args = { @@ -2266,77 +2262,62 @@ def test_cap_does_not_mask_unchunkable(): ChunkPlan(args, _fake_build, url_limit=10, max_chunks_per_axis=32) -@pytest.mark.parametrize("level", ["low", "medium", "high"]) -def test_resolve_level_maps_each_level_to_its_cap(level): - """Each level name resolves to its total sub-request cap from the table - (a positive int).""" - assert _resolve_level(level) == _LEVEL_CAPS[level] - assert _resolve_level(level) >= 1 - - -def test_parallel_chunks_levels_ordered_and_spaced(): - """The three caps hold the properties callers rely on: strictly increasing, - ``"high"`` saturating the declared ceiling, ``"low"`` still a real split - (>= 2), and each level spaced 4x from the next. The ceiling is a - parallel_chunks constant, deliberately independent of the concurrency - default.""" - low = _LEVEL_CAPS["low"] - medium = _LEVEL_CAPS["medium"] - high = _LEVEL_CAPS["high"] - assert low < medium < high - assert high == _MAX_PARALLEL_CHUNKS - assert low >= 2 - assert medium == high // 4 - assert low == medium // 4 +def test_parallel_chunks_publishes_n_on_the_ambient(): + """The context manager publishes ``n`` on the ambient for the block and + restores the previous value on exit — including proper nesting.""" + assert _parallel_chunks.get() == 0 + with parallel_chunks(32): + assert _parallel_chunks.get() == 32 + with parallel_chunks(2): + assert _parallel_chunks.get() == 2 + assert _parallel_chunks.get() == 32 # outer restored + assert _parallel_chunks.get() == 0 # default (off) outside any block @pytest.mark.parametrize( "bad", [ - "off", # a dead keyword form - "LOW", # wrong case — exact match only - " low ", # stray whitespace - 5, # the old integer levels are gone + 0, # not positive + -1, # negative + 1.5, # a float, not an int + "8", # a string, even a numeric one + "high", # the old level names are gone None, # None not accepted - ["low"], # unhashable → the ``not in`` check still rejects it cleanly + True, # bool is an int subclass but nonsensical here + ["8"], # a list ], ) -def test_resolve_level_rejects_everything_but_the_three_levels(bad): - """Only the three exact level strings are accepted; every other value — a - representative of each rejected shape (dead keyword, wrong case, whitespace, - old int, ``None``, unhashable) — raises ``ValueError`` so a typo fails - loudly.""" - with pytest.raises(ValueError, match="parallel_chunks level must be"): - _resolve_level(bad) - - -def test_parallel_chunks_scopes_and_restores_the_ambient(): - """The context manager resolves the level to its cap, publishes it on the - ambient for the block, and restores the previous value on exit — including - proper nesting.""" +def test_parallel_chunks_rejects_non_positive_int(bad): + """``n`` must be a positive integer; every other shape — zero, negative, a + float, a string (including a numeric one and the old level names), ``None``, + a ``bool``, a list — raises ``ValueError`` at ``with`` entry, before any + request, and leaves the ambient untouched.""" + with pytest.raises(ValueError, match="must be a positive integer"): + with parallel_chunks(bad): + pass assert _parallel_chunks.get() == 0 - with parallel_chunks("high"): - assert _parallel_chunks.get() == _LEVEL_CAPS["high"] - with parallel_chunks("low"): - assert _parallel_chunks.get() == _LEVEL_CAPS["low"] - assert _parallel_chunks.get() == _LEVEL_CAPS["high"] # outer restored - assert _parallel_chunks.get() == 0 # default (off) outside any block -def test_parallel_chunks_validates_on_entry(): - """An invalid level raises at ``with`` entry — before any request is - issued — and leaves the ambient untouched.""" - with pytest.raises(ValueError, match="parallel_chunks level must be"): - with parallel_chunks("aggressive"): - pass - assert _parallel_chunks.get() == 0 +def test_parallel_chunks_n1_is_no_extra_fan_out(): + """``n=1`` is the explicit no-op: an under-limit request stays a single + passthrough call, exactly as if the block weren't entered.""" + sites = [f"S{i:02d}" for i in range(8)] + calls: list[int] = [] + + @multi_value_chunked(build_request=_fake_build, url_limit=8000) + async def fetch(args): + calls.append(len(args["monitoring_location_id"])) + return pd.DataFrame(), _ok_response() + + with parallel_chunks(1): + fetch({"monitoring_location_id": sites}) + assert calls == [8] # one passthrough call carrying all sites -def test_parallel_chunks_high_drives_end_to_end_fan_out(): +def test_parallel_chunks_drives_end_to_end_fan_out(): """End-to-end: the same fitting request passes through as a single call by - default, but fans into several sub-requests inside a - ``parallel_chunks("high")`` block — and the combined result still - recovers every atom exactly once.""" + default, but fans into ``n`` sub-requests inside a ``parallel_chunks(n)`` + block — and the combined result still recovers every atom exactly once.""" sites = [f"S{i:02d}" for i in range(8)] calls: list[tuple[str, ...]] = [] @@ -2353,9 +2334,9 @@ async def fetch(args): assert sorted(df_plain["site"]) == sorted(sites) calls.clear() - with parallel_chunks("high"): + with parallel_chunks(8): df_fine, _ = fetch({"monitoring_location_id": sites}) - # 8 atoms at the "high" cap (>= 8) → 8 singleton sub-requests. + # 8 atoms at n=8 → 8 singleton sub-requests. assert len(calls) == 8 assert all(len(chunk) == 1 for chunk in calls) # Union across chunks recovers the original set, once each. @@ -2363,10 +2344,10 @@ async def fetch(args): assert sorted(df_fine["site"]) == sorted(sites) -def test_parallel_chunks_low_is_a_gentle_split(): - """``"low"`` is the mildest opt-in: an under-limit request fans into just - the ``"low"`` cap's worth of pieces, not singletons.""" - low = _LEVEL_CAPS["low"] +@pytest.mark.parametrize("n", [2, 3, 8]) +def test_parallel_chunks_supports_arbitrary_n(n): + """An arbitrary ``n`` (not only 2/8/32) fans an under-limit request into + exactly ``n`` sub-requests, together covering every site once.""" sites = [f"S{i:02d}" for i in range(8)] calls: list[int] = [] @@ -2375,8 +2356,7 @@ async def fetch(args): calls.append(len(args["monitoring_location_id"])) return pd.DataFrame(), _ok_response() - with parallel_chunks("low"): + with parallel_chunks(n): fetch({"monitoring_location_id": sites}) - # low cap → that many sub-requests, together covering all 8 sites. - assert len(calls) == low + assert len(calls) == n assert sum(calls) == 8 From c4d336d96a0bed774d511669957f05877683802c Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 8 Jul 2026 19:14:21 -0500 Subject: [PATCH 5/5] refactor(waterdata): tidy parallel_chunks (rename param, align validation, dedup test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /simplify cleanup: (1) rename ChunkPlan's max_chunks_per_axis -> max_chunks — the cap is on the plan's TOTAL sub-request count, not per axis, so the old name needed apologetic docstrings; dropped them. (2) Validate parallel_chunks(n) with numbers.Integral (matching the max_rows guard in engine.py), so numpy integers are accepted like the sibling validator. (3) Fold the n=1 no-op test into the parametrized arbitrary-n test, removing a duplicated fetch fixture. Co-Authored-By: Claude Opus 4.8 (1M context) --- dataretrieval/ogc/chunking.py | 8 +++-- dataretrieval/ogc/planning.py | 33 ++++++++++---------- tests/waterdata_chunking_test.py | 53 ++++++++++++-------------------- 3 files changed, 40 insertions(+), 54 deletions(-) diff --git a/dataretrieval/ogc/chunking.py b/dataretrieval/ogc/chunking.py index 1a3b37b6..c6aa2357 100644 --- a/dataretrieval/ogc/chunking.py +++ b/dataretrieval/ogc/chunking.py @@ -73,6 +73,7 @@ import asyncio import functools +import numbers import os from collections.abc import Callable, Iterator from contextlib import contextmanager @@ -258,8 +259,9 @@ def parallel_chunks(n: int) -> Iterator[None]: -------- ChunkPlan._refine : the planning-side effect of ``n``. """ - # ``bool`` is an ``int`` subclass but nonsensical here; reject it explicitly. - if not isinstance(n, int) or isinstance(n, bool) or n < 1: + # Accept any integer type (incl. numpy ints, mirroring ``max_rows``); ``bool`` + # is an ``Integral`` subclass but nonsensical here, so reject it explicitly. + if not isinstance(n, numbers.Integral) or isinstance(n, bool) or n < 1: raise ValueError( f"parallel_chunks(n): n must be a positive integer (e.g. 2, 8, 32); " f"got {n!r}." @@ -739,7 +741,7 @@ def wrapper( # here up front, so a later resume — which re-issues the # already-planned sub-requests — needs no snapshot. plan = ChunkPlan( - args, build_request, limit, max_chunks_per_axis=_parallel_chunks.get() + args, build_request, limit, max_chunks=_parallel_chunks.get() ) retry_policy = RetryPolicy.from_env() # The concurrency cap is resolved inside ``resume()`` from diff --git a/dataretrieval/ogc/planning.py b/dataretrieval/ogc/planning.py index ebadcb36..a32e48c6 100644 --- a/dataretrieval/ogc/planning.py +++ b/dataretrieval/ogc/planning.py @@ -329,11 +329,11 @@ class ChunkPlan: url_limit : int Byte budget for the request (URL + body) — a hard ceiling every sub-request must fit. - max_chunks_per_axis : int, optional + max_chunks : int, optional Soft cap on the plan's total sub-request count (default ``0`` = off). ``0`` chunks only as much as ``url_limit`` requires — the most conservative plan, fewest sub-requests. A positive cap fans the plan - out to up to ``max_chunks_per_axis`` sub-requests overall (the + out to up to ``max_chunks`` sub-requests overall (the cartesian product across axes, never fewer than the byte budget already forces) — capped as a whole, not per axis, so several multi-value axes can't multiply past the cap. Set from the @@ -370,7 +370,7 @@ def __init__( args: dict[str, Any], build_request: Callable[..., httpx.Request], url_limit: int, - max_chunks_per_axis: int = 0, + max_chunks: int = 0, ) -> None: self.args = args self.axes: list[_Axis] = [] @@ -423,9 +423,9 @@ def __init__( # A request that already fits and hasn't opted into finer chunking is # the common passthrough: leave ``axes``/``chunks`` empty so # ``total == 1`` and ``iter_sub_args`` yields the original args - # verbatim. Only when ``max_chunks_per_axis`` asks for extra fan-out do + # verbatim. Only when ``max_chunks`` asks for extra fan-out do # we set the axes up to be refined below. - if fits and max_chunks_per_axis <= 0: + if fits and max_chunks <= 0: return self.axes = axes @@ -436,8 +436,8 @@ def __init__( self._plan(build_request, url_limit) # Soft pass: optionally split further than the byte budget requires. # Purely additive — never re-raises, and the byte budget stays - # satisfied; a no-op at ``max_chunks_per_axis <= 0``. - self._refine(max_chunks_per_axis) + # satisfied; a no-op at ``max_chunks <= 0``. + self._refine(max_chunks) if self.canonical_url is None: # Original URL was un-constructable (httpx.InvalidURL); fall @@ -491,7 +491,7 @@ def _plan( ) _split_at(self.chunks[biggest_axis.arg_key], biggest_idx) - def _refine(self, max_chunks_per_axis: int) -> None: + def _refine(self, max_chunks: int) -> None: """ Fan the plan out more finely than the byte budget alone requires — the parallel_chunks dial (see @@ -499,7 +499,7 @@ def _refine(self, max_chunks_per_axis: int) -> None: would want this). Caps the plan's *total* sub-request count (:attr:`total`, the - cartesian product across all axes) at ``max_chunks_per_axis``, not + cartesian product across all axes) at ``max_chunks``, not each axis independently — with several multi-value axes, a cap of 32 still means at most 32 sub-requests overall, not ``32 ** n_axes``. Each split picks the single largest splittable chunk across *every* @@ -508,20 +508,19 @@ def _refine(self, max_chunks_per_axis: int) -> None: before another is touched. Purely additive — only ever *splits* existing chunks, so the byte pass's work and the ``url_limit`` invariant are both preserved, and it never raises. A no-op at - ``max_chunks_per_axis <= 0``. + ``max_chunks <= 0``. Parameters ---------- - max_chunks_per_axis : int + max_chunks : int Soft cap on the plan's total sub-request count (``0`` = off) — the - ``parallel_chunks(n)`` value. The parameter name is legacy: the cap - is on the whole plan (the cartesian product across axes), which - matches "sub-chunks per argument" only in the common single-axis - case — multi-axis plans are capped on the product, not per axis. + ``parallel_chunks(n)`` value. The cap is on the whole plan (the + cartesian product across axes), not per axis, so several multi-value + axes can't multiply past it. """ - if max_chunks_per_axis <= 0: + if max_chunks <= 0: return - while self.total < max_chunks_per_axis: + while self.total < max_chunks: # Largest splittable chunk across every axis; a chunk of size 1 # can't be split further. ``max`` with a stable input order # breaks ties by axis order, then lowest index within an axis. diff --git a/tests/waterdata_chunking_test.py b/tests/waterdata_chunking_test.py index 1ab57378..850b8709 100644 --- a/tests/waterdata_chunking_test.py +++ b/tests/waterdata_chunking_test.py @@ -2115,7 +2115,7 @@ async def fetch(args): # ``parallel_chunks`` context manager). ``_fake_build``'s base is 200 bytes, so # a handful of short atoms sits far under ``url_limit=8000`` — the byte pass # passes it through untouched, and any splitting below is the ``n`` cap alone. -# ``ChunkPlan`` takes the integer cap (``max_chunks_per_axis``) directly; +# ``ChunkPlan`` takes the integer cap (``max_chunks``) directly; # ``parallel_chunks(n)`` publishes ``n`` onto it. The cap bounds the plan's # *total* sub-request count (the cartesian product across axes), not each axis # independently — see ``test_cap_caps_the_total_across_axes``. @@ -2123,22 +2123,22 @@ async def fetch(args): def test_zero_cap_preserves_passthrough(): - """``max_chunks_per_axis=0`` (the default) must not perturb the existing + """``max_chunks=0`` (the default) must not perturb the existing plan: a multi-value request that fits the byte limit is still the trivial passthrough (no axes, ``total == 1``), byte-for-byte the pre-feature behavior.""" args = {"monitoring_location_id": ["A", "B", "C", "D"]} - plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=0) + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks=0) assert plan.axes == [] assert plan.total == 1 assert list(plan.iter_sub_args()) == [args] @pytest.mark.parametrize( - ("max_chunks_per_axis", "expected_pieces"), + ("max_chunks", "expected_pieces"), [(0, 1), (2, 2), (8, 8), (16, 10), (32, 10)], ) -def test_cap_ramps_then_saturates(max_chunks_per_axis, expected_pieces): +def test_cap_ramps_then_saturates(max_chunks, expected_pieces): """A single 10-atom axis that fits the byte limit splits into ``min(10, cap)`` pieces: 1 (off), 2, 8, then saturating at 10 (one atom per chunk) once the cap overshoots the atom count. Monotonic and bounded, and @@ -2149,10 +2149,10 @@ def test_cap_ramps_then_saturates(max_chunks_per_axis, expected_pieces): {"monitoring_location_id": atoms}, _fake_build, url_limit=8000, - max_chunks_per_axis=max_chunks_per_axis, + max_chunks=max_chunks, ) assert plan.total == expected_pieces - if max_chunks_per_axis: + if max_chunks: flattened = [ a for chunk in plan.chunks["monitoring_location_id"] for a in chunk ] @@ -2170,7 +2170,7 @@ def test_cap_bounds_fan_out_for_a_long_axis(): {"monitoring_location_id": atoms}, _fake_build, url_limit=8000, - max_chunks_per_axis=high, + max_chunks=high, ) assert plan.total == high flattened = [a for chunk in plan.chunks["monitoring_location_id"] for a in chunk] @@ -2184,9 +2184,9 @@ def test_cap_below_byte_split_does_not_reduce_fan_out(): # Heavy axis of four 30-char atoms; a limit tight enough that the byte pass # must drive every atom into its own sub-request (4 pieces > the cap of 2). args = {"monitoring_location_id": ["X" * 30, "Y" * 30, "Z" * 30, "W" * 30]} - baseline = ChunkPlan(args, _fake_build, url_limit=250, max_chunks_per_axis=0) + baseline = ChunkPlan(args, _fake_build, url_limit=250, max_chunks=0) assert baseline.total > 2 # byte pass alone already fanned out past 2 - refined = ChunkPlan(args, _fake_build, url_limit=250, max_chunks_per_axis=2) + refined = ChunkPlan(args, _fake_build, url_limit=250, max_chunks=2) # cap 2 < baseline pieces → refine is a no-op here. assert refined.total == baseline.total @@ -2197,8 +2197,8 @@ def test_cap_never_exceeds_the_byte_budget(): a chunk), and the fan-out is at least what the byte pass required.""" args = {"monitoring_location_id": ["X" * 30, "Y" * 30, "Z" * 30, "W" * 30]} limit = 310 - byte_only = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks_per_axis=0) - plan = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks_per_axis=32) + byte_only = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks=0) + plan = ChunkPlan(args, _fake_build, url_limit=limit, max_chunks=32) assert plan.total >= byte_only.total for sub in plan.iter_sub_args(): assert _safe_request_bytes(_fake_build, sub, limit) <= limit @@ -2210,7 +2210,7 @@ def test_cap_refines_the_filter_axis(): into ``min(N, cap)`` pieces.""" clauses = [f"p='{i}'" for i in range(8)] args = {"filter": " OR ".join(clauses)} - plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=4) + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks=4) assert len(plan.chunks["filter"]) == 4 # min(8, 4) assert plan.total == 4 @@ -2225,7 +2225,7 @@ def test_cap_caps_the_total_across_axes(): "monitoring_location_id": [f"L{i}" for i in range(6)], "parameter_code": [f"{i:05d}" for i in range(6)], } - plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=4) + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks=4) assert plan.total == 4 # Every atom on every axis is still covered exactly once. for key, atoms in ( @@ -2249,7 +2249,7 @@ def test_cap_bounds_fan_out_across_many_axes(): "parameter_code": [f"{i:05d}" for i in range(10)], "filter": " OR ".join(f"p='{i}'" for i in range(10)), } - plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks_per_axis=high) + plan = ChunkPlan(args, _fake_build, url_limit=8000, max_chunks=high) assert plan.total <= high @@ -2259,7 +2259,7 @@ def test_cap_does_not_mask_unchunkable(): act on and must not swallow the hard failure.""" args = {"monitoring_location_id": "one-huge-scalar"} with pytest.raises(Unchunkable): - ChunkPlan(args, _fake_build, url_limit=10, max_chunks_per_axis=32) + ChunkPlan(args, _fake_build, url_limit=10, max_chunks=32) def test_parallel_chunks_publishes_n_on_the_ambient(): @@ -2298,22 +2298,6 @@ def test_parallel_chunks_rejects_non_positive_int(bad): assert _parallel_chunks.get() == 0 -def test_parallel_chunks_n1_is_no_extra_fan_out(): - """``n=1`` is the explicit no-op: an under-limit request stays a single - passthrough call, exactly as if the block weren't entered.""" - sites = [f"S{i:02d}" for i in range(8)] - calls: list[int] = [] - - @multi_value_chunked(build_request=_fake_build, url_limit=8000) - async def fetch(args): - calls.append(len(args["monitoring_location_id"])) - return pd.DataFrame(), _ok_response() - - with parallel_chunks(1): - fetch({"monitoring_location_id": sites}) - assert calls == [8] # one passthrough call carrying all sites - - def test_parallel_chunks_drives_end_to_end_fan_out(): """End-to-end: the same fitting request passes through as a single call by default, but fans into ``n`` sub-requests inside a ``parallel_chunks(n)`` @@ -2344,10 +2328,11 @@ async def fetch(args): assert sorted(df_fine["site"]) == sorted(sites) -@pytest.mark.parametrize("n", [2, 3, 8]) +@pytest.mark.parametrize("n", [1, 2, 3, 8]) def test_parallel_chunks_supports_arbitrary_n(n): """An arbitrary ``n`` (not only 2/8/32) fans an under-limit request into - exactly ``n`` sub-requests, together covering every site once.""" + exactly ``n`` sub-requests, together covering every site once — including + ``n=1``, the explicit no-op that stays a single passthrough call.""" sites = [f"S{i:02d}" for i in range(8)] calls: list[int] = []