Fix bucket dataset card handling and push metadata accounting#8354
Open
pjh4993 wants to merge 3 commits into
Open
Fix bucket dataset card handling and push metadata accounting#8354pjh4993 wants to merge 3 commits into
pjh4993 wants to merge 3 commits into
Conversation
Four fixes along the storage-bucket read/write code paths: 1. load.py (HubBucketDatasetModuleFactory.get_module): the parsed README was passed as a DatasetCard instead of DatasetCard.data, so loading any bucket dataset with a README.md raised "AttributeError: 'DatasetCard' object has no attribute 'get'". Use .data like every sibling factory. 2. load.py (same factory): the standalone-YAML `except FileNotFoundError` branch clobbered dataset_card_data instead of setting standalone_yaml_data, discarding the parsed card. Assign standalone_yaml_data = None. 3. iterable_dataset.py (_push_parquet_shards_to_hub): the per-worker tuple yields dataset_nbytes in the 4th position, but the collector unpacked it as uploaded_size and never accumulated dataset_nbytes, so the resulting SplitInfo.num_bytes was always 0. Accumulate dataset_nbytes. 4. arrow_dataset.py (_get_updated_dataset_card): the legacy-infos branch returned json.dumps(dataset_infos) — the wrong variable, which is also unbound when no README exists — instead of the assembled legacy infos dict. The function is typed Optional[dict] and both call sites json.dumps() the result, so returning the dict fixes the double-encoding (dataset_infos.json was written as a JSON string, not an object) and the crash. Adds a hermetic regression test for fix (4) over an in-memory filesystem; the bucket read/write paths themselves require live Hub credentials. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the single dataset_infos.json test with tests/test_buckets.py, one hermetic test per fix, each verified to fail on the pre-fix code: - test_bucket_module_uses_dataset_card_data_not_card (fix 1, load.py) - test_bucket_module_preserves_card_when_standalone_yaml_missing (fix 2, load.py) - test_push_parquet_shards_reports_dataset_nbytes (fix 3, iterable_dataset.py) - test_get_updated_dataset_card_returns_legacy_infos_as_dict (fix 4, arrow_dataset.py) The loader / push paths are driven in isolation over in-memory filesystems and a stubbed shard worker, so no live Hub access is needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repo test files use no module or function docstrings; convert the essential rationale to brief inline comments to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
Four fixes on the
buckets/...read/write paths, hit bypush_to_hub→load_dataset. The loader one crashes on any dataset with a README; the writer ones corrupt uploaded metadata.Key Changes
src/datasets/load.py): passed the card object instead of its.data, crashing on load. Now uses.data.src/datasets/load.py): the missing-file case wiped the parsed card. Now leaves it intact.src/datasets/iterable_dataset.py): worker byte count was dropped, soSplitInfo.num_byteswas always 0. Now accumulated.dataset_infos.json(src/datasets/arrow_dataset.py): serialized the wrong variable, always crashing (UnboundLocalErrorwith no README,TypeErrorwith one). Now returns the assembled dict for the caller to encode.Tests
One hermetic test per fix in
tests/test_buckets.py(in-memory filesystems / a stubbed shard worker, no Hub), each verified to fail on the pre-fix code — same order as above:test_bucket_module_uses_dataset_card_data_not_card— guardssrc/datasets/load.pytest_bucket_module_preserves_card_when_standalone_yaml_missing— guardssrc/datasets/load.pytest_push_parquet_shards_reports_dataset_nbytes— guardssrc/datasets/iterable_dataset.pytest_get_updated_dataset_card_returns_legacy_infos_as_dict— guardssrc/datasets/arrow_dataset.pypytest tests/test_buckets.py→4 passed;ruff check/ruff format --checkclean.🤖 Generated with Claude Code