refactor(ogc): adopt combining module directly; drop dead re-export shims

Follow-up cleanup on the module-boundary refactor so the split lands in
the import graph, not just the file layout.

- Import the recombination helpers straight from ogc.combining at every
  call site (chunking, engine, wateruse, tests) and delete planning.py's
  backwards-compat re-export block + __all__. The new module previously
  had zero direct importers; everything routed through planning.

- Drop the dead TYPE_CHECKING re-export of _Fetch/_Finalize/
  _passthrough_result from interruptions.py (no importers) and repoint the
  lone Sphinx xref in shaping.py to ogc.chunking._Finalize, their real home.

- Remove ChunkInterrupted.__setstate__: __getstate__ already writes the
  _pickled_* snapshot keys and the base DataRetrievalError.__setstate__
  restores them via __dict__.update, so the override was redundant (the
  property getattr fallbacks cover the hand-constructed case).

- Correct _classify_transient's docstring and _retryable's comment, which
  claimed a delegation that does not — and deliberately should not — exist
  (the two use intentionally different type sets).

No behavior change; mypy --strict and the affected test suites pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

dataretrieval/ogc/chunking.py

@@ -86,14 +86,14 @@
 from dataretrieval.utils import HTTPX_DEFAULTS, Ambient, _require_positive_int
 
 from . import progress as _progress
-from .interruptions import (
-    ChunkInterrupted,
-)
-from .planning import (
-    ChunkPlan,
+from .combining import (
     _combine_chunk_frames,
     _combine_chunk_responses,
 )
+from .interruptions import (
+    ChunkInterrupted,
+)
+from .planning import ChunkPlan
 from .retry import (
     _NO_RETRY,
     RetryPolicy,

dataretrieval/ogc/engine.py

@@ -47,9 +47,9 @@
 from dataretrieval.ogc import chunking
 from dataretrieval.ogc import progress as _progress
 from dataretrieval.ogc.chunking import get_active_client
+from dataretrieval.ogc.combining import _QUOTA_HEADER, _merge_response, _safe_elapsed
 from dataretrieval.ogc.dates import _DATE_RANGE_PARAMS, _format_api_dates
 from dataretrieval.ogc.errors import _paginated_failure_message, _raise_for_non_200
-from dataretrieval.ogc.planning import _QUOTA_HEADER, _merge_response, _safe_elapsed
 from dataretrieval.ogc.shaping import GEOPANDAS, _finalize_ogc, _get_resp_data
 from dataretrieval.utils import (
     HTTPX_DEFAULTS,

dataretrieval/ogc/interruptions.py

@@ -22,16 +22,6 @@
     from dataretrieval.ogc.chunking import ChunkedCall
 
 
-# These type aliases define the ChunkedCall contract and live in chunking.py;
-# re-exported here for backwards compatibility.
-if TYPE_CHECKING:
-    from dataretrieval.ogc.chunking import _Fetch as _Fetch  # noqa: F401
-    from dataretrieval.ogc.chunking import _Finalize as _Finalize  # noqa: F401
-    from dataretrieval.ogc.chunking import (  # noqa: F401
-        _passthrough_result as _passthrough_result,
-    )
-
-
 class ChunkInterrupted(DataRetrievalError):
     """
     Base class for mid-stream chunk failures whose completed work is
@@ -134,8 +124,8 @@ def partial_frame(self) -> pd.DataFrame:
         """Live view of work completed so far — delegates to ``call.partial_frame``.
 
         Returns an empty ``DataFrame`` when ``call`` is ``None`` (degraded /
-        unpickled state), unless a pickled snapshot was restored via
-        ``__setstate__``.
+        unpickled state), unless a pickled snapshot was restored during
+        unpickling.
         """
         if self.call is not None:
             return self.call.partial_frame
@@ -164,24 +154,16 @@ def __getstate__(self) -> dict[str, Any]:
         # resume was never possible anyway).
         #
         # Since ``partial_frame`` and ``partial_response`` are now live
-        # properties (delegating to ``self.call``), we must snapshot them
-        # here so the pickled state includes the data that was available
-        # at serialization time.
+        # properties (delegating to ``self.call``), snapshot them here into
+        # private ``_pickled_*`` keys. The base ``__setstate__`` restores
+        # those keys via ``self.__dict__.update``, and the properties fall
+        # back to them (through ``getattr``) once ``call`` is ``None`` — so
+        # no ``__setstate__`` override is needed.
         state = {**super().__getstate__(), "call": None}
         state["_pickled_partial_frame"] = self.partial_frame.copy()
         state["_pickled_partial_response"] = self.partial_response
         return state
 
-    def __setstate__(self, state: dict[str, Any] | None) -> None:
-        state = state or {}
-        # Restore the pickled partial snapshots into private backing attrs
-        # that the properties will fall back to when call is None.
-        self._pickled_partial_frame = state.pop(
-            "_pickled_partial_frame", pd.DataFrame()
-        )
-        self._pickled_partial_response = state.pop("_pickled_partial_response", None)
-        super().__setstate__(state)
-
 
 class QuotaExhausted(ChunkInterrupted):
     """

dataretrieval/ogc/planning.py

@@ -8,10 +8,10 @@
 :mod:`dataretrieval.ogc.retry` (retry policy), which import the plan and
 drive it.
 
-Result recombination utilities (:func:`_combine_chunk_frames`,
-:func:`_combine_chunk_responses`, etc.) live in the sibling
-:mod:`dataretrieval.ogc.combining` module; they are re-exported here for
-backwards compatibility.
+Result recombination — reassembling the per-chunk frames and responses
+back into one result (:func:`_combine_chunk_frames`,
+:func:`_combine_chunk_responses`, etc.) — lives in the sibling
+:mod:`dataretrieval.ogc.combining` module, which callers import directly.
 """
 
 from __future__ import annotations
@@ -594,30 +594,3 @@ def iter_sub_args(self) -> Iterator[dict[str, Any]]:
             for axis, chunk in zip(self.axes, combo, strict=False):
                 sub_args[axis.arg_key] = axis.render(chunk)
             yield sub_args
-
-
-# ---------------------------------------------------------------------------
-# Backwards-compatibility re-exports: these utilities moved to
-# ``dataretrieval.ogc.combining`` but existing importers (chunking.py,
-# engine.py, wateruse.py, tests) may reference them via ``planning``.
-# ---------------------------------------------------------------------------
-from dataretrieval.ogc.combining import (  # noqa: E402, F401
-    _QUOTA_HEADER,
-    _combine_chunk_frames,
-    _combine_chunk_responses,
-    _lowest_remaining,
-    _merge_response,
-    _safe_elapsed,
-    _set_response_url,
-)
-
-__all__ = [
-    "ChunkPlan",
-    "_QUOTA_HEADER",
-    "_combine_chunk_frames",
-    "_combine_chunk_responses",
-    "_lowest_remaining",
-    "_merge_response",
-    "_safe_elapsed",
-    "_set_response_url",
-]

dataretrieval/ogc/retry.py

@@ -197,10 +197,11 @@ def _classify_transient(
     Classify a SINGLE exception as a known transient (resumable) failure.
 
     Does NOT walk the ``__cause__`` chain — inspects only the exception
-    passed in. This is the shared taxonomy that both
-    :func:`_classify_chunk_error` (chain-walking) and :func:`_retryable`
-    (top-level only) delegate to, so a new error type need only be added
-    in one place.
+    passed in. Factored out of :func:`_classify_chunk_error` (which walks
+    the ``__cause__`` chain and calls this on each link) so the transient
+    taxonomy lives in one place. :func:`_retryable` deliberately does *not*
+    delegate here — it uses a narrower set (``httpx.InvalidURL`` and a bare
+    ``httpx.HTTPError`` are resumable but not worth an automatic retry).
 
     Parameters
     ----------
@@ -292,9 +293,12 @@ def _retryable(exc: BaseException) -> tuple[bool, float | None]:
         (seconds) when the transient carried one, else ``None``.
     """
     # Only initial-page raw transients (TransientError, TransportError) are
-    # retryable — not wrapped mid-pagination DataRetrievalError. We use
-    # _classify_transient for the typed status errors and handle bare
-    # TransportError (which isn't a TransientError) separately.
+    # retryable — not a wrapped mid-pagination DataRetrievalError. This is a
+    # deliberately narrower taxonomy than _classify_transient's: RateLimited
+    # and ServiceUnavailable are caught via their TransientError base, but
+    # httpx.InvalidURL and bare httpx.HTTPError are intentionally excluded
+    # (a too-long cursor or non-transport HTTP error won't fix on retry), so
+    # this stays a separate check rather than delegating to _classify_transient.
     if isinstance(exc, TransientError):
         return True, exc.retry_after
     if isinstance(exc, httpx.TransportError):

dataretrieval/ogc/shaping.py

@@ -373,7 +373,7 @@ def _finalize_ogc(
     as :class:`~dataretrieval.utils.BaseMetadata`.
 
     Injected into the chunker as its ``finalize`` hook (see
-    :data:`~dataretrieval.ogc.interruptions._Finalize`) so the
+    :data:`~dataretrieval.ogc.chunking._Finalize`) so the
     un-interrupted return *and* a resumed ``ChunkInterrupted.call.resume()``
     produce the same post-processed ``(DataFrame, BaseMetadata)`` shape, not
     the chunker's raw frame and bare ``httpx.Response``.

dataretrieval/wateruse.py

@@ -53,8 +53,8 @@
 
 from dataretrieval.codes.states import to_state
 from dataretrieval.exceptions import DataRetrievalError
+from dataretrieval.ogc.combining import _combine_chunk_frames, _combine_chunk_responses
 from dataretrieval.ogc.engine import _paginate, _run_sync
-from dataretrieval.ogc.planning import _combine_chunk_frames, _combine_chunk_responses
 from dataretrieval.utils import (
     HTTPX_DEFAULTS,
     BaseMetadata,

tests/waterdata_chunking_test.py

@@ -47,6 +47,11 @@
     multi_value_chunked,
     parallel_chunks,
 )
+from dataretrieval.ogc.combining import (
+    _QUOTA_HEADER,
+    _combine_chunk_frames,
+    _combine_chunk_responses,
+)
 from dataretrieval.ogc.interruptions import (
     ChunkInterrupted,
     QuotaExhausted,
@@ -56,10 +61,7 @@
     _LIST_SEP,
     _NEVER_CHUNK,
     _OR_SEP,
-    _QUOTA_HEADER,
     ChunkPlan,
-    _combine_chunk_frames,
-    _combine_chunk_responses,
     _extract_axes,
     _request_bytes,
     _safe_request_bytes,

tests/wateruse_test.py

@@ -306,7 +306,7 @@ def test_fan_out_surfaces_final_rate_limit_header(httpx_mock):
     assert md.header["x-ratelimit-remaining"] == "850"
 
 
-# (response aggregation now reuses ogc.planning._combine_chunk_responses; the
+# (response aggregation now reuses ogc.combining._combine_chunk_responses; the
 # integration test above pins the rate-limit-header behavior end-to-end.)