Repository navigation
Conversation
require_decoding dropped ignore_decode_attribute when recursing into dicts, lists and List/LargeList, so nested decode=False features were not detected as decodable (e.g. push_to_hub embed_external_files). Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
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.
What
require_decoding(feature, ignore_decode_attribute=...)dropsignore_decode_attributewhen it recurses: thedict,list/tuple,LargeListandListbranches callrequire_decoding(f)without forwarding the flag. Only a top-level feature honours it.ignore_decode_attribute=Trueis passed byDataset.push_to_hub,Dataset._estimate_nbytes(arrow_dataset.py) andIterableDataset.push_to_hub(iterable_dataset.py) to decide which columns' external files get embedded into the shards. For a nesteddecode=Falsefeature — e.g.{"img": Image(decode=False)}orList(Image(decode=False))— the flag was lost, sorequire_decoding(..., ignore_decode_attribute=True)returnedFalse. Those columns would be pushed as file paths instead of embedded bytes, and their size under-estimated for sharding.(Same forwarding bug as the already-merged #8349, which fixed
require_storage_embed.)Fix
Forward
ignore_decode_attributein the four recursive calls.Tests
Adds
test_require_decoding_ignore_decode_attribute_nestedtotests/features/test_features.py, parametrized over a bareAudio(decode=False), a dict, a list,LargeListandList; each assertsrequire_decoding(f, ignore_decode_attribute=True)isTrueandrequire_decoding(f)isFalse. Verified fail-on-revert: on upstream the four nested cases fail; with the fix all 7 pass.