fix(python): honor custom Equals and GetHashCode in structural values#4850
Merged
Conversation
An F# class can override Equals and GetHashCode without opting into Python's __eq__ and __hash__ protocols. The Python runtime preferred the Python protocols, so such a class fell through to object.__hash__ (identity), and that identity hash leaked into unions, records and arrays. Union equality was also field-wise Python ==. Align the Python runtime with the TypeScript one: - safe_hash delegates to identity_hash, so F# GetHashCode wins over Python __hash__ - structural_hash dispatches like TS structuralHash: None/bool/str, then GetHashCode, then numbers, then element-wise array_hash for Array/tuple/list, then Python hash, then identity. It no longer raises TypeError on a Python list - Union.Equals compares fields with equal_arrays, and Union.GetHashCode combines number_hash(tag) with structural_hash per field instead of hashing a Python tuple - @tagged_union builds the dataclass with eq=False so case classes inherit F# equality - record_get_hashcode combines per-field structural_hash instead of hashing a Python tuple EquatableBase declares __eq__, so Python implicitly sets its __hash__ to None, and that None shadows HashableBase.__hash__ in the MRO. Define __hash__ on Union and Record so both stay hashable from Python; records without a generated __hash__ were unhashable before. Fixes #4834 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Python Type Checking Results (Pyright)
Excluded files with errors (4 files)These files have known type errors and are excluded from CI. Remove from
|
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 #4834
Problem
An F# class can override
EqualsandGetHashCodewithout opting into Python's__eq__and__hash__protocols. The Python runtime checked the Python protocols first, so such a class reached the inheritedobject.__hash__(identity hash), and that identity hash leaked into unions, records and arrays. Union equality was also field-wise Python==.Running the reproduction from the issue on
main:All eight now print
true, matching .NET and the JS/TS targets.Changes
This is solution 1 from the issue — correct the existing helpers, so already-generated Python code is fixed by upgrading
fable-library. No compiler changes. The Python runtime now mirrors the TypeScript one.fable_library/util.pysafe_hashdelegates toidentity_hash, so F#GetHashCodewins over Python__hash__(it was the other way round).structural_hashwasint32(hash(x)). It now dispatches like TSstructuralHash:None/bool/str, thenGetHashCode, then numbers, then element-wisearray_hashforArray/tuple/list, then Python hash, then identity.array_hashis fixed by the same change. As a side effect,structural_hashon a Pythonlistno longer raisesTypeError.fable_library/union.pyUnion.Equalscompares fields withequal_arrays(F#equals) instead of Python==.Union.GetHashCodecombinesnumber_hash(tag)withstructural_hashper field instead of hashing a Python tuple.@tagged_unionbuilds the dataclass witheq=False, so case classes inherit F# equality rather than dataclass field-wise==.fable_library/record.pyrecord_get_hashcodecombines per-fieldstructural_hashinstead of hashing a Python tuple.Incidental fix
EquatableBasedeclares__eq__, so Python implicitly setsEquatableBase.__hash__ = None, and thatNoneshadowsHashableBase.__hash__in the MRO. #4805 worked around this inside@tagged_union; witheq=Falsethat workaround no longer applies, so__hash__is now defined onUnionandRecorddirectly. This also fixes records: 9 of the 106 generated record classes in the Python test output have no emitted__hash__and were unhashable from Python.Tests
Added
Custom Equals and GetHashCode are used by structural valuestotests/Python/TestComparison.fsandtests/Js/Main/ComparisonTests.fs. JS/TS already behaved correctly, so that one is a regression guard.ruff format --checkclean.Performance
Measured on the path Fable actually generates for union equality (
util.equals), 200k iterations:equals(a, b)safe_hash(a)a == bThe compiled path is slightly faster. Python-level
==on a union case is slower because dataclass tuple comparison is replaced by F# equality — that is the point of the fix.🤖 Generated with Claude Code