Skip to content

Include inherited slots when hashing slotted objects - #642

Open
Kenoia wants to merge 1 commit into
qlustered:devfrom
Kenoia:fix/inherited-slot-hashing
Open

Kenoia wants to merge 1 commit into
qlustered:devfrom
Kenoia:fix/inherited-slot-hashing

Conversation

@Kenoia

@Kenoia Kenoia commented Oct 7, 2026 •

Copy link
Copy Markdown

DeepHash iterates obj.__slots__, which can omit parent-defined slots and split a string declaration into characters. Different slot values can then produce equal hashes, causing ignore_order=True to miss changes.

  • Collect declared slots across the MRO and treat a string declaration as one slot
  • Keep the existing uninitialized-slot and __getattr__ behavior
  • Add 11 regression/control cases for inheritance, declaration forms, fallback behavior and ignored type groups

Related: #506 and #577

Validation:

  • Full pytest suite, including slow tests: 1320 passed, 8 skipped (Python 3.13)
  • All 11 added regression and control cases pass
  • Pyright and the CI flake8 error gate passed

@seperman seperman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review: #642 Include inherited slots when hashing slotted objects

Summary: This PR changes DeepHash._get_slots_dict to walk type(obj).__mro__ and read each class's own __slots__, treating a string declaration as one name. Before, it iterated obj.__slots__, which only shows the most-derived declaration and splits a string into characters. On dev, instances of fully-slotted subclasses (e.g. class Derived(Base): __slots__ = ()) and of classes with __slots__ = 'value' are hashed without their slot values. Instances with different values therefore get the same hash, and ignore_order=True misses the change. Reading the code, this PR fixes that.

Overlap with other PRs

  • #609 (merged to dev) fixed DeepDiff._dict_from_slots in deepdiff/diff.py: parent-declared mangled slots were lost on subclass instances (#506). This PR changes a different function, DeepHash._get_slots_dict in deepdiff/deephash.py. It complements #609, does not duplicate it, and does not conflict with it. After both, DeepHash and DeepDiff include the same non-mangled slots. Mangled names still differ (see below).
  • This builds on #577 (merged), which added _get_slots_dict and its __getattr__ fallback.
  • No other open PR touches this code. The PR merges cleanly with #639 and #641, which also change deephash.py (checked with git merge-tree).

Security

No security concerns found in static review. The change only reads class __dict__ entries and instance attributes. It adds no new I/O, imports or dynamic evaluation, and the diff has no hidden Unicode.

Correctness

  • deepdiff/deephash.py:303-306: cls.__dict__.get('__slots__', ()) returns only the slots each class declares itself, which is the right approach. If a subclass declares a slot name again, the entry is overwritten with the same value. _prep_dict sorts its entries, so MRO order does not affect the hash.

  • Private (mangled) slots are still looked up by the name in the declaration (deepdiff/deephash.py:309), e.g. __id rather than _Base__id, so the lookup never finds them. By default ignore_private_variables drops these keys anyway. Two cases are still affected:

    • With ignore_private_variables=False, DeepHash leaves these slots out, while DeepDiff._dict_from_slots includes them (since #609).
    • If the class defines __getattr__, the fallback at line 315 calls __getattr__('__id'). If that raises, the whole object becomes unprocessed. This used to happen only for slots declared on the leaf class; it now also happens for inherited ones.

    The loop already knows the declaring cls, so it could unmangle the name here at little cost. Python removes leading underscores from the class name when it mangles: inside class _Foo, __x becomes _Foo__x.

  • Behavior change: if an inherited slot is not initialized and __getattr__ raises for it, the object is now unprocessed (asserted at tests/test_hash.py:356). This matches the existing leaf-slot behavior from #577. Note that with ignore_order, DeepDiff logs such items and leaves them out of the comparison.

  • Question: _prep_obj uses hasattr(obj, "__slots__") (line 452) to decide whether to try this strategy. That check is also true when __slots__ comes from __getattr__ delegation, as with transparent proxies. Before, the delegated slot names were used and their values read through __getattr__. Walking type(obj).__mro__ won't find them, so such objects would now hash without the delegated values. Do you want to keep that case working, e.g. by falling back to obj.__slots__ when the walk finds no declared slots?

  • Out of scope, for reference: if any base class lacks __slots__, instances have a __dict__ and the o.__dict__ strategy is used, so slot values are still not hashed. DeepDiff covers that case through detailed__dict__.

  • Hash stability: hashes of slotted-subclass instances and of string-__slots__ instances will change. That is the intended fix, and the CHANGELOG entry covers it.

Tests

  • Reading the code, the new cases at tests/test_hash.py:304, :338 (string form), :356 and :380 should fail on dev. There, obj.__slots__ shows only the leaf declaration or the characters of a string, so neither hashes[old] != hashes[new] nor the ignore_type_in_groups equality holds. The dict-form __slots__ case and the equal comparisons are controls.
  • No test covers a private (mangled) slot declared on a parent class. One would be worth adding if unmangling goes in.

Suggestions

  • Consider resolving mangled slot names with the declaring class, so DeepHash behaves like DeepDiff._dict_from_slots.
  • Optional: add a proxy-style regression test if the question above matters.

AI-assisted static review; the code was not executed.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.08%. Comparing base (59acdd9) to head (de1561f).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #642   +/-   ##
=======================================
  Coverage   95.07%   95.08%           
=======================================
  Files          18       18           
  Lines        4918     4922    +4     
=======================================
+ Hits         4676     4680    +4     
  Misses        242      242           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants