Skip to content

Restore the _legacy_no_dict_keys_sorting branch in Pickler._batch_setitems - #8411

Open
ebarkhordar wants to merge 1 commit into
huggingface:mainfrom
ebarkhordar:fix/8410-legacy-no-dict-keys-sorting
Open

Restore the _legacy_no_dict_keys_sorting branch in Pickler._batch_setitems#8411
ebarkhordar wants to merge 1 commit into
huggingface:mainfrom
ebarkhordar:fix/8410-legacy-no-dict-keys-sorting

Conversation

@ebarkhordar

Copy link
Copy Markdown
Contributor

Root cause

Pickler._legacy_no_dict_keys_sorting lost its only reader in #7817. That PR rewrote _batch_setitems to accept the argument Python 3.14 passes, and in the same hunk dropped the early return:

-    def _batch_setitems(self, items):
-        if self._legacy_no_dict_keys_sorting:
-            return super()._batch_setitems(items)
+    def _batch_setitems(self, items, *args, **kwargs):

_check_legacy_cache2 still patches the flag on to rebuild the 2.15.0 config_id, so that lookup now hashes with sorted dict keys and no longer matches the directory 2.15.0 wrote. The flag appears twice in src/, as a definition and as that patch.object write, and nothing reads it.

The fix

Restore the branch, keeping the *args/**kwargs passthrough #7817 added so the 3.14 fix is preserved on both paths.

Verification

  • The two new legacy tests fail on main (assert [('a', 2), ('b', 1)] == [('b', 1), ('a', 2)]) and pass with this change; the third pins the default order-insensitive behaviour and passes either way.
  • pytest tests/test_fingerprint.py: 24 passed / 9 skipped before, 26 passed / 9 skipped after, in a clean container on Python 3.11.15 with dill 0.4.1.
  • With the fix, Hasher.hash({"train": ["train.csv"], "test": ["test.csv"]}) under the patched flag returns 711511d8f1d9bc25, which is the value datasets==2.15.0 actually produces for that object. On main it returns the sorted-key hash cfd3b51f0f8e9fd8. 4.4.0 is the first release where that regressed, matching git tag --contains f7c8e46ec.
  • Not checked: I did not build a 2.15.0 cache directory and load it under HEAD. The hash mismatch is measured; the effect on the directory lookup is read from _check_legacy_cache2.

If you would rather retire the 2.14/2.15 compat path than restore it, that is the alternative I raised in the issue and I am happy to send that instead.

Fixes #8410

…items

huggingface#7817 removed the early return that read the flag while fixing the
Python 3.14 signature, leaving builder.py's 2.15.0 cache lookup
computing a config_id that no longer matches what 2.15.0 wrote.
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