Skip to content

Raise a clear error for out-of-bounds IterableDataset.shard arguments - #8690

Open
simpleqt wants to merge 1 commit into
huggingface:mainfrom
simpleqt:sq/shard-bounds-check
Open

simpleqt wants to merge 1 commit into
huggingface:mainfrom
simpleqt:sq/shard-bounds-check

Conversation

@simpleqt

Copy link
Copy Markdown

Problem

IterableDataset.shard documents that "n should be less or equal to the number of shards in the dataset dataset.num_shards", but violating that (or passing index >= num_shards) crashes with an opaque IndexError from deep inside _merge_gen_kwargs:

>>> from datasets import IterableDataset
>>> ids = IterableDataset.from_generator(lambda: ({"x": i} for i in range(10)))
>>> ids.shard(num_shards=3, index=1)
IndexError: list index out of range
  File "datasets/utils/sharding.py", line 72, in _merge_gen_kwargs

Worse, for contiguous sharding shard(num_shards=3, index=0) on a single-shard dataset silently returned the whole dataset instead of one shard (reproduced on the latest main and on the released 4.4.1).

Fix

Validate both arguments in IterableDataset.shard and raise a ValueError that names the expected bounds:

ValueError: num_shards (3) must be less than or equal to the number of shards in the dataset (1); the requested shard would otherwise be empty.
ValueError: index (5) must be in the range [0, num_shards) = [0, 1)

Internal callers are unaffected: map already lowers num_proc to num_shards before calling shard, and every path that used to survive produced either the whole dataset or an empty shard.

Tests

  • Added test_shard_with_too_many_shards_raises_clear_error and test_shard_with_out_of_range_index_raises_clear_error; on unfixed main they fail with the IndexError above (verified by temporarily reverting the fix).
  • pytest tests/test_iterable_dataset.py -k "shard or map": 155 passed.
  • ruff check + ruff format --check clean.

IterableDataset.shard documents that num_shards must not exceed
dataset.num_shards, but violating it (or passing index >= num_shards)
crashed with an opaque IndexError from the list comprehension in
_merge_gen_kwargs - and for contiguous sharding, shard(n, 0) with n >
num_shards silently returned the whole dataset instead of one shard.

Validate both arguments in shard() and raise a ValueError that names the
expected bounds.

Signed-off-by: simpleqt <89645338+simpleqt@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 26, 2026 17:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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