Apply the indices mapping in Dataset.unique - #8383
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
…th as the table Dataset.unique only took the indices mapping into account when it had a different number of rows than the underlying table. An indices mapping can also repeat some rows and drop others while keeping the same length, e.g. after ds.select([0, 0, 1]) or a bootstrap resampling, and in that case unique was reading the raw table and reported values that are not in the dataset. class_encode_column builds its ClassLabel names from unique, so it inherited the same problem and created classes for rows that were filtered out. Apply the indices mapping whenever there is one. Only the requested column is materialized, so this doesn't fall back to flattening the whole dataset.
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.
Dataset.uniqueonly takes the indices mapping into account when it has a different number of rows than the underlying table:An indices mapping can also repeat some rows and drop others while keeping the same length. In that case
uniquereads the raw table and returns values that are not in the dataset:class_encode_columnbuilds itsClassLabelnames fromunique, so it inherits the bug and creates a class for rows that were dropped:This is reachable from ordinary use —
selectwith repeats (bootstrap resampling), and any indices mapping whose length happens to match the table.This applies the indices mapping whenever there is one. Only the requested column is materialized, via the existing
query_table(..., key=column, indices=self._indices)path, so it deliberately avoids falling back toflatten_indices()— that would makeshuffle().unique()rewrite the whole dataset.Added
test_unique_with_indices_mapping_of_same_lengthandtest_class_encode_column_with_indices_mapping_of_same_length; both fail onmainwithassert ['a', 'b', 'c'] == ['a', 'b']and pass with the change.tests/test_arrow_dataset.py,tests/test_dataset_dict.pyandtests/test_search.pypass (470 passed, 38 skipped).