Skip to content

fix(batching): reject empty boundaries in length_buckets - #14

Open
oliviabrn wants to merge 1 commit into
fufuchiu:mainfrom
oliviabrn:fix-batching-empty-boundaries
Open

oliviabrn wants to merge 1 commit into
fufuchiu:mainfrom
oliviabrn:fix-batching-empty-boundaries

Conversation

@oliviabrn

Copy link
Copy Markdown

When receives an empty tuple or list, it silently places all sequences into bucket 0 (the overflow bucket). This is almost certainly a mistake by the caller - why call a bucketing function with no buckets?

The existing validation () passes for empty lists since . This adds an explicit check before the sorted check, raising a clear "boundaries cannot be empty" error.

Added regression tests in covering both tuple and list empty inputs.

Passing an empty boundaries tuple silently puts all sequences into
bucket 0, which is surprising and almost certainly a caller mistake.
Explicitly reject this case with a clear error message.
@oliviabrn
oliviabrn requested a review from fufuchiu as a code owner September 10, 2026 04:15
@fufuchiu

Copy link
Copy Markdown
Owner

I checked a4924e1: the package builds, all 312 tests pass, and lint/format checks pass.

There is an API behavior decision to settle before merging. Currently, length_buckets([10, 20], boundaries=()) returns {0: [0, 1]}, so empty boundaries can represent a single overflow bucket. This PR changes that call to raise ValueError.

Could you describe a caller case that requires rejecting this configuration and document the intended behavior, including when lengths is also empty? I am keeping the PR open while that compatibility question is clarified.

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