Fix Dataset.take raising IndexError when taking more elements than available#8312
Open
vidigoat wants to merge 1 commit into
Open
Fix Dataset.take raising IndexError when taking more elements than available#8312vidigoat wants to merge 1 commit into
vidigoat wants to merge 1 commit into
Conversation
…ailable Dataset.take(n) was implemented as self.select(range(n)), so when n exceeds the dataset length it raised IndexError instead of returning the whole dataset. This is inconsistent with IterableDataset.take, which clamps and returns all available elements (the two were meant to be aligned, see huggingface#6813); Dataset.skip already clamps correctly. Clamp n to len(self) so take returns the whole dataset when asked for more.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Dataset.take(n)is implemented asself.select(range(n)), so whennis larger than the dataset length it raisesIndexError: Index N out of range for dataset of size Minstead of returning the whole dataset. This is inconsistent withIterableDataset.take, which clamps and returns all available elements — the two were meant to be aligned (see #6813).Dataset.skipalready clamps correctly (range(n, len(self))); onlytakewas affected.This clamps
ntolen(self)sotakereturns the whole dataset when asked for more, matchingIterableDataset.takeand the "firstnelements" semantics documented in the method.Steps to reproduce (before this change)
IterableDataset.take(5)on the same data returns all 3 rows.Test plan
test_taketoBaseDatasetTest(runs both in-memory and on-disk) covering fewer / exact / more-than-available / zero.n > lencase fails on currentmain(IndexError) and passes with this change; existing behavior forn <= lenis unchanged.AI disclosure
This fix was prepared with the assistance of an AI coding assistant; the bug was reproduced and the behavior verified before submitting.