Write pushed features to card when re-pushing a split - #8609
Open
belambert wants to merge 1 commit into
Open
Conversation
_get_updated_dataset_card updated the splits and sizes of the existing dataset_info but never the features, so re-pushing a dataset's only split with different columns left the card declaring a schema the new shards don't have, and the dataset failed to cast on load. Reaching the assignment with other splits on the repo means the features already matched, so this only takes effect when the push replaces them all.
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.
Fixes #8608.
_get_updated_dataset_cardupdates the splits and sizes of the existingdataset_infowhena config is re-pushed, but never assigns the pushed
featuresto it. Re-pushing a dataset'sonly split with different columns therefore leaves the card declaring the previous schema
while the shards hold the new one, and the dataset fails to cast on load:
The existing mismatch guard doesn't catch this because it only runs when other splits are
present (
any(s != split for s in repo_info.splits)), which is never true for a single-splitdataset being replaced. The card ends up half-updated -
num_examplesfrom the new push,featuresfrom the old one - so the push looks like it worked.The change
repo_info.splits[split] = split_info + repo_info.features = features info_to_dump = repo_infoReaching that line with other splits on the repo means the features already matched, or the
guard above would have raised, so the assignment is a no-op in that case. The only behaviour
change is the case the guard deliberately skips: a push that replaces every split the card
describes, where the pushed features are the authoritative ones.
Both
Dataset.push_to_hubandDatasetDict.push_to_hubgo through this function, so onechange covers both.
Tests
Two unit tests alongside the existing
_get_updated_dataset_cardtests intests/test_buckets.py, using the same in-memory filesystem pattern:test_get_updated_dataset_card_updates_features_when_replacing_sole_split- the newfeatures reach the card, and the split count still updates. Fails on
main, passes here.test_get_updated_dataset_card_rejects_features_mismatch_against_other_splits- theexisting
ValueErrorstill raises when a second split would be left disagreeing.pytest tests/test_buckets.py tests/test_info.py tests/test_metadata_util.pypasses (33tests), and
ruff check/ruff format --checkare clean on both files. I have not runtests/test_upstream_hub.py, which needs the CI Hub token.Open question
I went with silently writing the new features, since that matches what the shards actually
contain. If you'd rather the user were told that the declared schema changed under an
existing config, a warning at that point would be easy to add - happy to adjust.