Skip to content

Restore _legacy_no_dict_keys_sorting so _check_legacy_cache2 can find 2.15.0 caches. - #8605

Open
cpruijsen wants to merge 1 commit into
huggingface:mainfrom
cpruijsen:fix/issue-8410
Open

Restore _legacy_no_dict_keys_sorting so _check_legacy_cache2 can find 2.15.0 caches.#8605
cpruijsen wants to merge 1 commit into
huggingface:mainfrom
cpruijsen:fix/issue-8410

Conversation

@cpruijsen

Copy link
Copy Markdown

Summary

Restore the early return in Pickler._batch_setitems that honours _legacy_no_dict_keys_sorting, keeping the *args / **kwargs passthrough from #7817. #7817 updated Pickler._batch_setitems for Python 3.14's extra argument and, in the same hunk, dropped that return. _check_legacy_cache2 still patches _legacy_no_dict_keys_sorting to recompute the config_id datasets 2.15.0 would have written, so since 4.4.0 the lookup hashes dicts with sorted keys and misses old cache directories when data_files keys are not already sorted ({"train": ..., "test": ...}).

With _legacy_no_dict_keys_sorting set, Hasher.hash({"train": ["train.csv"], "test": ["test.csv"]}) is again 711511d8f1d9bc25, the value measured from 2.15.0 (issue author, Python 3.11) and reproduced here on 3.12. Default hashing is unchanged: the same two dicts still hash equal without the flag.

Chose option 1 on #8410: restore the reader rather than delete the 2.14/2.15 compat path (option 2). That is what #6514 implemented and is two lines to revert. Can drop _check_legacy_cache2 / the flag instead if you would rather retire that layout.

Fixes #8410

Test plan

  • tests/test_fingerprint.py::HashingTest::test_hash_dict_legacy_no_dict_keys_sorting fails without the early return (cfd3b51f0f8e9fd8 == cfd3b51f0f8e9fd8) and passes with it
  • Default (unpatched) hashing of the same two dicts remains equal
  • Neighbouring HashingTest cases that do not need optional extras: 9 passed, 6 deselected
  • Optional: load a dataset whose 2.15.0 cache dir used dict-valued data_files with unsorted keys and confirm builder.cache_dir points at that directory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_legacy_no_dict_keys_sorting has no reader since #7817, so the 2.15.0 legacy cache lookup computes the wrong config_id

1 participant