Skip to content

fix: reject nonpositive batch sizes before consuming items - #62

Closed
tongwentao wants to merge 1 commit into
wolph:developfrom
tongwentao:fix/positive-batch-size
Closed

tongwentao wants to merge 1 commit into
wolph:developfrom
tongwentao:fix/positive-batch-size

Conversation

@tongwentao

@tongwentao tongwentao commented Oct 22, 2026 •

Copy link
Copy Markdown

batcher(range(3), 0) and batcher(range(3), -1) currently return [[0, 1, 2]]: the batch length can never equal the requested size. abatcher has the same problem with negative sizes, and accepts zero when an interval is supplied. With an unbounded source and no interval this can keep buffering without yielding.

Validate the batch size before consuming or scheduling any source items, raising ValueError for zero and negative values. abatcher(batch_size=None, interval=...) keeps its interval-only behavior. The zero-size/no-interval async case now raises ValueError instead of the existing assertion error. Both public docstrings and the changelog describe the requirement.

Behavior and review notes

Because these APIs return generators, the validation takes effect when iteration begins, before the first source item is consumed. It does not make generator construction eager. Positive-size batching retains its existing yield behavior, and an async interval-only call remains valid with batch_size=None; the intentional compatibility change is the exception type for invalid sizes.

Validation on Python 3.13.5:

  • All 12 new regression cases fail before the fix; they cover empty/nonempty sources and async batching with/without an interval, and check that invalid sizes consume no items.
  • Full test/doctest suite: 386 passed, 100% statement and branch coverage.
  • Ruff lint/format, Mypy, basedpyright, Pyrefly and Codespell passed.
  • Sphinx HTML build with warnings treated as errors passed.

Other interpreter versions are left to CI. This change was prepared and validated with Codex.

@wolph

wolph commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thank you for the pull request. The tests that check nothing is consumed are a nice touch.

The catch is that sizes below 1 have been accepted since batcher was added in 3.3.0, and they're easy to reach by accident: batcher(rows, len(rows) // workers) gets a size of 0 whenever there are fewer rows than workers, and batcher([], 0) returns [] on 4.1.4. With this change those jobs start raising ValueError, and since many people install python-utils without an upper bound, they would get that from a minor release. abatcher(source, batch_size=0, interval=5) is a similar case: it batches by interval only today, and the interval already keeps the buffer bounded.

I wouldn't oppose rejecting these sizes, but I'd rather do it in two steps: a DeprecationWarning for sizes below 1 in 4.x that keeps the current result, and the ValueError in 5.0. For abatcher with an interval, a size below 1 could simply keep meaning "no size limit". Would you be up for changing the PR that way?

@wolph wolph closed this Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants