bpo-36144: Add PEP 584 operators to collections.ChainMap by curtisbucher · Pull Request #18832 · python/cpython (original) (raw)

Before I finish reviewing this, I have to bring up a tricky issue. There's a subtle behavior (I can't even find it in the reference manual :-( ) for binary operators.

Consider a class B that implements for example __add__ and __radd__ (the same applies to all binary operators). Now consider a subclass C that overrides __add__ and __radd__. Let b = B() and c = C(), and evaluate b + c. Surprise: this calls C.__radd__ rather than B.__add__ (first)!

The reason for this exception is that if we didn't have this special behavior (i.e., if this would follow the normal behavior of trying B.__add__ first), it would be difficult for the subclass C to override the behavior of + completely. A reasonable implementation of B.__add__ would look like this:

class B: def add(self, other): if not isinstance(other, B): return NotImplemented # return an instance of B representing self+other

This would totally succeed for b + c, and return an instance of B. (And the reason for that is also subtle -- since Python allows subclass constructors to have a signature that differs from the superclass constructor, there's no safe way for B to create an instance of self.__class__. We went over this extensively earlier in the discussion about dict | dict. :-)

Anyway, because we have this exception, I have constructed an example that behaves unintuitively (I think). Consider:

class C(ChainMap): def ror(self, other): return super().ror(other)

c1 = ChainMap() c2 = C()

print(c1 | c2) # C(ChainMap({})) # !? print(c2 | c1) # C({})

However, if we don't override the __ror__ method, the first result printed changes to

print(c1 | c2) # ChainMap({})

The output without overridden __ror__ is easily understood: both calls invoke ChainMap.__or__, and it returns an instance of the class of self. But when we override __ror__ in C (i.e. the way shown in the example) we find that C.__ror__ is called, which then calls ChainMap.__ror__. And that has an (IMO) unexpected behavior, resulting in a subclass of ChainMap whose maps[0] is itself a ChainMap.

I'm not sure, but I think the only way out may be to define __ror__ differently, in a way that's more symmetric with __or__.

Thoughts?