docs(ogc): correct aggregation descriptions

Clarify rate-limit header selection, elapsed-duration aggregation, cross-chunk deduplication, GeoJSON output columns, and retry-policy wording.

Author: thodson-usgs

dataretrieval/ogc/chunking.py

@@ -438,12 +438,11 @@ def _combine_raw(self) -> tuple[pd.DataFrame, httpx.Response]:
 
         Frames concatenate in sub-args *index* order (``sorted`` keys —
         deterministic, independent of parallel completion order). The
-        aggregated response takes its headers from the most-recently-
-        *completed* sub-request: the ``track`` closure in :meth:`_run`
-        is the only writer of ``self._chunks`` and ``dict`` preserves
-        insertion order, so the chunks' natural order is completion
-        order and the last one carries the freshest
-        ``x-ratelimit-remaining``.
+        aggregated response takes its headers from the response with the
+        lowest reported ``x-ratelimit-remaining`` value. If no response
+        reports that header, it falls back to the last completed response;
+        ``self._chunks`` preserves completion order because the ``track``
+        closure in :meth:`_run` is its only writer.
 
         Returns
         -------
@@ -542,8 +541,9 @@ def resume(self) -> tuple[pd.DataFrame, Any]:
             Combined data from every successful sub-request.
         response
             The finalized aggregate — a raw :class:`httpx.Response`
-            (canonical URL, most-recently-completed sub-request's headers,
-            cumulative elapsed time) by default, or whatever
+            (canonical URL, headers from the response with the lowest reported
+            remaining quota, and summed response elapsed durations) by default,
+            or whatever
             :attr:`finalize` produces (e.g. ``BaseMetadata`` for the OGC
             getters).
 
@@ -624,8 +624,9 @@ async def _run(self, max_concurrent: int | None) -> tuple[pd.DataFrame, Any]:
             Combined data from every sub-request.
         response
             The finalized aggregate — a raw :class:`httpx.Response`
-            (canonical URL, most-recently-completed sub-request's headers,
-            cumulative elapsed time) by default, or whatever
+            (canonical URL, headers from the response with the lowest reported
+            remaining quota, and summed response elapsed durations) by default,
+            or whatever
             :attr:`finalize` produces (e.g. ``BaseMetadata`` for OGC getters).
 
         Raises

dataretrieval/ogc/combining.py

@@ -72,11 +72,11 @@ def _set_response_url(response: httpx.Response, url: str | httpx.URL) -> None:
 def _lowest_remaining(responses: list[httpx.Response]) -> httpx.Response:
     """The response reporting the lowest ``x-ratelimit-remaining``.
 
-    The rate-limit counter decreases monotonically within a window, so the
-    smallest value any sub-request saw is the most-current "quota left after
-    this call" — the right thing to surface.  Under concurrent fan-out the
-    last response *by index* need not be the one the server processed last, so
-    pick the minimum (falling back to the last response if none report it).
+    Within a rate-limit window, the counter decreases monotonically, so the
+    smallest value observed is the most conservative value to surface. Under
+    concurrent fan-out, the last response *by index* need not be the one the
+    server processed last. Fall back to the last response when none reports
+    the header.
     """
     best: httpx.Response | None = None
     best_remaining: int | None = None
@@ -115,18 +115,19 @@ def _merge_response(
 
 
 def _combine_chunk_frames(frames: list[pd.DataFrame]) -> pd.DataFrame:
-    """Concatenate per-chunk frames and deduplicate non-null feature IDs.
+    """Concatenate per-chunk frames and deduplicate IDs across chunks.
 
     Empty frames are ignored before concatenation so an empty plain
     :class:`pandas.DataFrame` cannot downgrade a real ``GeoDataFrame`` and
     strip its geometry or CRS. When every frame is empty, the first frame is
     returned to preserve its concrete type.
 
-    Deduplication is unconditional because overlap is possible on every plan
-    axis. Filter clauses can match the same feature, and list inputs can contain
-    repeated values or otherwise select overlapping records. Rows without an
-    ``id`` are preserved verbatim: pandas treats null values as duplicates, so
-    applying ``drop_duplicates`` to those rows would silently lose data.
+    When multiple non-empty frames are combined, non-null feature IDs are
+    deduplicated regardless of the plan axis. Filter clauses can match the same
+    feature, and list inputs can contain repeated values or otherwise select
+    overlapping records. Rows without an ``id`` are preserved verbatim: pandas
+    treats null values as duplicates, so deduplicating them would silently lose
+    data.
     """
     non_empty = [frame for frame in frames if not frame.empty]
     if not non_empty:
@@ -155,10 +156,10 @@ def _combine_chunk_responses(
     Fold per-sub-request responses into a single aggregated response.
 
     For a multi-response input, returns a shallow copy of
-    ``responses[0]`` with ``.headers`` set to those of the most-depleted
-    response (lowest ``x-ratelimit-remaining`` — the quota actually left
-    after the fan-out; see :func:`_lowest_remaining`), ``.elapsed`` set
-    to total wall-clock across every response, and ``.url`` set to the
+    ``responses[0]`` with ``.headers`` set to those of the response reporting
+    the lowest ``x-ratelimit-remaining`` value (the most conservative quota
+    observation; see :func:`_lowest_remaining`), ``.elapsed`` set to the sum of
+    the per-response elapsed durations, and ``.url`` set to the
     canonical original-query URL (when supplied) so ``BaseMetadata``
     reflects the user's full request rather than the first chunk.
 
@@ -169,7 +170,7 @@ def _combine_chunk_responses(
     Parameters
     ----------
     responses : list[httpx.Response]
-        One response per completed sub-request, in execution order.
+        One response per completed sub-request, in caller-provided order.
     canonical_url : str or None
         URL of the unchunked original request. ``None`` skips the URL
         override — used by the passthrough path (the fetcher's
@@ -191,8 +192,8 @@ def _combine_chunk_responses(
     if len(responses) == 1 and canonical_url is None:
         return responses[0]
 
-    # Headers come from the most-depleted response (lowest quota left after a
-    # concurrent fan-out; ``_lowest_remaining`` returns the lone response as-is
+    # Headers come from the response with the lowest reported remaining quota;
+    # ``_lowest_remaining`` returns the lone response as-is
     # for a single-element list).  ``_merge_response`` re-sums elapsed onto a
     # fresh copy, so repeated calls (e.g. via ``ChunkedCall.partial_response``
     # during resume) stay idempotent.

dataretrieval/ogc/engine.py

@@ -658,9 +658,9 @@ async def _paginate(
     response : httpx.Response
         A shallow copy of the first-page response, with ``.headers``
         rebuilt as a fresh ``httpx.Headers`` reflecting the last page and
-        ``.elapsed`` set to cumulative wall-clock. The canonical URL is
-        preserved from the first page. The original first-page response
-        is not mutated.
+        ``.elapsed`` set to the sum of the per-page response durations. The
+        canonical URL is preserved from the first page. The original first-page
+        response is not mutated.
 
     Raises
     ------

dataretrieval/ogc/retry.py

@@ -203,9 +203,9 @@ def _classify_transient(
     if isinstance(exc, RateLimited):
         return QuotaExhausted, exc.retry_after
     if isinstance(exc, TransientError):
-        # Every non-rate-limit typed transient is a service interruption. This
-        # fallback keeps future TransientError subclasses resumable after their
-        # inline retries are exhausted.
+        # Every typed transient other than a rate-limit error is a service
+        # interruption. This fallback keeps future TransientError subclasses
+        # resumable after their inline retries are exhausted.
         return ServiceInterrupted, exc.retry_after
     if isinstance(exc, (httpx.HTTPError, httpx.InvalidURL)):
         return ServiceInterrupted, None
@@ -265,8 +265,8 @@ def _retryable(exc: BaseException) -> tuple[bool, float | None]:
 
     Wrapped mid-pagination failures are not retried from page one; they instead
     escalate to a resumable :class:`ChunkInterrupted`. ``httpx.InvalidURL`` and
-    non-transport ``httpx.HTTPError`` instances are resumable but deterministic,
-    so they are classified without being retried.
+    non-transport ``httpx.HTTPError`` instances are classified as resumable but
+    excluded from automatic retry by policy.
     """
     classification = _classify_transient(exc)
     if classification is None:

dataretrieval/ogc/shaping.py

@@ -101,12 +101,13 @@ def _get_resp_data(
 
     Notes
     -----
-    The non-geopandas branch normalizes each feature's ``properties``
-    object, flattening nested dictionaries with an underscore separator, then
-    adds the top-level ``id`` and ``geometry.coordinates`` columns. The ``id``
-    column is always added so the downstream service-specific rename works even
-    when all IDs are missing; ``geometry`` is added only when coordinates are
-    present. Feature-level envelope fields are deliberately excluded.
+    The non-geopandas branch normalizes each feature's ``properties`` object,
+    flattening nested dictionaries with an underscore separator, then adds the
+    top-level ``id`` and a ``geometry`` column containing the coordinates. The
+    ``id`` column is always added so the downstream service-specific rename
+    works even when all IDs are missing; ``geometry`` is added only when
+    coordinates are present. Feature-level envelope fields are deliberately
+    excluded.
     """
     if body is None:
         body = resp.json()

dataretrieval/wateruse.py

@@ -365,8 +365,9 @@ async def _one(request: httpx.Request) -> tuple[pd.DataFrame, httpx.Response]:
         results = await asyncio.gather(*(_one(req) for req in requests))
 
     # Reuse the engine's combine helpers: drop empty frames and concat, and fold
-    # the per-location responses into one (lowest-remaining rate-limit headers +
-    # cumulative elapsed), keeping the first request's URL as the query identity.
+    # the per-location responses into one (headers from the response with the
+    # lowest reported remaining quota plus summed response durations), keeping
+    # the first request's URL as the query identity.
     frames = [frame for frame, _ in results]
     responses = [resp for _, resp in results]
     return _combine_chunk_frames(frames), _combine_chunk_responses(

tests/waterdata_utils_test.py

@@ -677,7 +677,7 @@ def test_get_resp_data_always_materializes_id_column():
 
 
 def test_get_resp_data_flattens_nested_properties():
-    """Nested GeoJSON properties keep the historical underscore columns."""
+    """Nested GeoJSON properties keep underscore-separated column names."""
     resp = mock.MagicMock()
     resp.json.return_value = {
         "features": [