ForwardRef: remove __hash__ and __eq__

[JelleZijlstra]

Feature or enhancement

Currently, annotationlib.ForwardRef has a __hash__ and __eq__ method, which look at a few attributes:

cpython/Lib/typing.py

Line 1096 in 8b4a0d6

def __eq__(self, other):

(3.13). These look unsound already; it's possible for two ForwardRefs to compare equal but hash differently (if they have the same evaluated value but not the same __forward_module__). This gets worse on 3.14, where ForwardRef has gained a few more fields (

cpython/Lib/annotationlib.py

Line 232 in a472244

def __eq__(self, other):

).

I think it's better to remove the __eq__ and __hash__ methods from ForwardRef objects, and make it so two ForwardRefs are equal only if they're identical. I don't see a good use case for comparing two ForwardRefs for equality; if you want to know that they refer to the same thing, you should evaluate them and compare the type object that comes out.

This came up in agronholm/typeguard#492 where the current implementation of equality caused some grief. cc people involved with some runtime type checking tools: @agronholm @Viicos @leycec.

Linked PRs

• gh-129463, gh-128593: Simplify ForwardRef #129465
• gh-129463: Remove two attributes from ForwardRef equality #132283

[zzzeek]

so this broke things for us in SQLAlchemy, however I'm still trying to see if this is only local to our test suite.

The previous behavior of ForwardRef made perfect sense:

import typing


def make_fw_ref(anno: str) -> typing.ForwardRef:
    return typing.Union[anno]

assert make_fw_ref("str") == typing.ForwardRef("str")

why should that assertion now fail? it looks to be strictly the __code__ attribute check in __eq__() that makes that particular comparison fail.

[JelleZijlstra]

The change definitely makes it more annoying to do equality comparisons with ForwardRef objects. In CPython we added the test.support.EqualToForwardRef helper for that. If you run into this in user code, it's worth looking into why you are using equality comparisons with ForwardRef. In your test case, for example, why are you creating a ForwardRef object in two different ways?

On reflection though I do think it makes sense to remove .__code__ from the equality check; it's used only as an internal cache.

[zzzeek]

I'm pretty sure we're hitting this only because we are looking in tests at the output of functions we use that modify types and comparing them to what's expected.

we rely a lot on a function de_optionalize_union_types that seeks to remove None from union objects, since we are trying to match these type objects inside of a lookup dictionary that links typing objects to SQL database types. None is something we use to indicate "NULL" on a SQL column but we don't use it for the database type itself.

the failures we have are in our test suite for de_optionalize_union_types. So yes I can make a comparison routine as well, however this gets a tedious when i need to compare something like typing.List[typing.ForwardRef("TA_list")] since I have to unpack the whole structure

[zzzeek]

for those tests what I can do is make my own mock ForwardRef that has its own __eq__, then I have to reverse the order of comparisons

[JelleZijlstra]

#132283 removes the __code__ and __ast_node__ attributes from comparison. That makes your test case above pass; it may not fix the issue for all ForwardRefs you might run into though.

[zzzeek]

OK , thanks. at the same time I vendored EqualToForwardRef and am running that in CI now