Skip to content

Commit 9c33c83

Browse files
thodson-usgsclaude
andcommitted
docs(ogc): apply /simplify quality-review nits (comments only)
Two documentation/comment fixes from the 4-angle cleanup review; no logic change: - utils._require_positive_int: soften the "count-like knobs ... can't drift" claim. ChunkPlan.max_chunks deliberately bypasses the helper (internal, already-int, keeps its own "1 disables fan-out" message), so scope the claim to the two user-facing boundary knobs (max_rows, parallel_chunks). - ChunkPlan._refine: note that its ranking key is atom count (len), not URL bytes like _plan, because the fan-out pass balances work rather than fitting a byte budget. Considered-but-skipped from the review: extracting a shared _largest_splittable helper for the _plan/_refine scans (line-neutral, adds callable indirection — the depth review judged it more indirection than it removes); removing _refine's max_chunks<=1 early-return (redundant with the while-guard but a clearer explicit "off" marker); routing ChunkPlan's <1 guard through _require_positive_int (different contract, better bespoke message); two _refine micro-optimizations (not worth it at max_chunks<=32). Signed-off-by: thodson-usgs <thodson@usgs.gov> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0cb5bfa commit 9c33c83

2 files changed

Lines changed: 10 additions & 5 deletions

File tree

dataretrieval/ogc/planning.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -548,9 +548,11 @@ def _refine(self, max_chunks: int) -> None:
548548
# ``k`` into ``k+1``, so it adds ``total // k`` sub-requests (the
549549
# product of the other axes) regardless of which chunk — hence the
550550
# budget test is per axis, not per chunk. Skipping an over-budget
551-
# axis makes ``max_chunks`` a true ceiling. A chunk of size 1 can't
552-
# be split further. ``max`` with a stable input order breaks ties by
553-
# axis order, then lowest index within an axis.
551+
# axis makes ``max_chunks`` a true ceiling. The ranking key is atom
552+
# count (``len``), not URL bytes like ``_plan`` — this pass balances
553+
# work across sub-requests rather than fitting a byte budget. A
554+
# chunk of size 1 can't be split further. Stable input order breaks
555+
# ties by axis order, then lowest index within an axis.
554556
candidate: tuple[_Axis, int] | None = None
555557
candidate_size = -1
556558
for axis in self.axes:

dataretrieval/utils.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ def _require_positive_int(
7676
not only ``int``) but rejects ``bool`` — an ``Integral`` subtype that is
7777
nonsensical as a count. A non-integer (float, str, ``None``) or a value
7878
``< 1`` raises before any I/O, rather than crashing later (e.g. deep in
79-
``pd.DataFrame.head``). Shared by the count-like knobs (``max_rows`` and
80-
``parallel_chunks(n)``) so they validate by identical rules and can't drift.
79+
``pd.DataFrame.head``). Shared by the user-facing count knobs ``max_rows``
80+
and ``parallel_chunks(n)`` so their boundary validation can't drift.
81+
(``ChunkPlan.max_chunks`` is an internal, already-``int`` precondition with
82+
its own domain-specific message, so it keeps a lighter ``< 1`` guard rather
83+
than routing through here.)
8184
8285
Parameters
8386
----------

0 commit comments

Comments
 (0)