Does this trigger the edge case you fix in#9074?

#9074 is actually totally unrelated (that came up when I was trying to build a weak cache of FT2Font -> FT_Face for mpl_cairo some time ago).
This only weakrefs renderers, which are Python-level objects (not extension types) so they're always weakrefable (for example, mpl_cairo's renderer class inherits from both the extension class and RendererBase).

Question... this all hinges on the definition of the equality of weakrefs and to not hold hard references to large python objects. The property tuple that contains the weakref here is meant to get hashed, which means that the weakrefs aren't directly compared, right? I don't know what the rules are with weakref hashes.

Weak references are hashable if the object is hashable. They will maintain their hash value even after the object was deleted. If hash() is called the first time only after the object was deleted, the call will raise TypeError.

Ah, so the hashes of two weakrefs that point to the same object will be the same?

Yes.

Doing a bit more digging, I think we can simplify this even more and just usehash(renderer).

I think there is a (probably impossible) race condition here is that if you try to take the hash for the first time after the object has gone away.

In [108]:classfoo():passIn [109]: (weakref.ref(foo()), )in {}---------------------------------------------------------------------------TypeErrorTraceback (mostrecentcalllast)<ipython-input-109-32cf9cf15e2e>in<module>()---->1 (weakref.ref(foo()), )in {}TypeError:weakobjecthasgoneaway

The docs from__hash__ say

User-defined classes haveeq() andhash() methods by default; with them, all objects compare unequal (except with themselves) and x.hash() returns an appropriate value such that x == y implies both that x is y and hash(x) == hash(y).

Isn't that what the original code did (before the uid change)? The uid approach tried to fix an edge case where id() of two renderers created at different times could collide, right?

no, the original code was usingid(obj)

If ids can collide, hashs can collide too.

In [1]: class C:   ...:     def __init__(self, a): self.a = a   ...: In [2]: id(C(1)) == id(C(2))  # relying on the fact that the first C gets gc'd immediatelyOut[2]: TrueIn [3]: hash(C(1)) == hash(C(2))Out[3]: True

The only use of get_prop_tup in the codebase immediately uses the tuple for a dict lookup (in _get_layout), while the renderer is definitely still alive, so there won't be a race condition.
I'm not convinced we should bother with other use cases.

Hmm, I wonder if that is an edge case that should go up to cpython?

I think this should go in as it eliminates a epi-cycle that we added this cycle.

id(C()) == id(C()) makes sense because id just returns the pointer address and the object is being reused (there's probably a free list somewhere).
There is no guarantee whatsoever that different objects have different hashes (and that's even more true when the two objects are not even alive at the same time). The only guarantee is that two different objects have different hashes.
So I don't see anything wrong with cpython here.

In [122]:classfoo():...In [123]:foo()==foo()Out[123]:FalseIn [125]:id(foo())==id(foo())Out[125]:TrueIn [126]:hash(foo())==hash(foo())Out[126]:TrueIn [127]:foo()==foo()Out[127]:FalseIn [128]:foo()isfoo()Out[128]:False

violates the docs onhash. I suspect the docs should just needs a caveat about objects must exist for all the conditions to hold.

Ah, I see. Inhttps://docs.python.org/3/reference/datamodel.html#object.__hash__

x.hash() returns an appropriate value such that x == y implies both that x is y and hash(x) == hash(y).

should be

x.hash() returns an appropriate value such that x == y and x is y both imply that hash(x) == hash(y).

right?

I think the sentence is ok as it is, it may need some clarity that it is 'if' not 'iff' though.