Issue1873148

classification
Title: list.__iadd__ not switching to __radd__ on NotImplemented
Type: Severity: normal
Components: Core Versions:
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pjenvey Nosy List: jek, pedronis, pjenvey
Priority: normal Keywords:

Created on 2008-01-16.19:32:01 by jek, last changed 2008-06-20.06:17:13 by pjenvey.

Files
File name Uploaded Description Edit Remove
iadd.py jek, 2008-01-16.19:32:02 testcase
Messages
msg2060 (view) Author: jason kirtland (jek) Date: 2008-01-16.19:32:01
A generic 's += o' doesn't seem to be processing NotImplemented returned by s.__iadd__(o).  On CPython, evaluation seems to fall back to o.__radd__(s) in this case.  In Jython, s == NotImplemented after that +=.

The __iadd__ on Jython's list does seem to recognize NotImplemented but doesn't fall back to __radd__.
msg2061 (view) Author: Philip Jenvey (pjenvey) Date: 2008-01-16.22:36:05
__add__ and other binary ops are translated from the compiler into a call of their wrapper method (for __add__ that wrapper method is PyObject._add). These wrapper methods call into their dunder methods (like __add__) via PyObject's _binop_rule. binop_rule manages cases like this: i.e. trying alternative means, such as __radd__, if __add__ returns NotImplemented

Unfortunately the inplace operations are being translated directly into a PyObject.__iadd__ method call (there is no __iadd). That's why jek's example just returns the NotImplemented directly -- it should have at least raised a TypeError.

It looks like we need a similar wrapper for inplace ops, i.e. _iadd, that probably needs to utilize _binop_rule. If we do utilize _binop_rule for all inplace ops, then we also need to be expose the inplace methods as binary (type = MethodType.BINARY, or expose_binary in the old expose templates)
msg3293 (view) Author: Philip Jenvey (pjenvey) Date: 2008-06-20.06:17:05
this is fixed on the asm branch with:

r4684, r4693, r4694

Note that the iadd.py test still doesn't work exactly as CPython does, 
it's now:

123
type(l) is NotList False
type(l_too) is list True

Which matches pypy. The discrepancy is due to CPython's odd behavior 
with builtins

Basically in this example, list __iadd__ works on iterable objects:

Python 2.5.2 (r252:60911, Apr 22 2008, 12:00:45) 
[GCC 4.0.1 (Apple Computer, Inc. build 5367)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from iadd import NotList
>>> l = []
>>> l.__iadd__(NotList([1,2,3]))
[1, 2, 3]

Since NotList is iterable, why is NotList.__radd__ called instead of 
list.__iadd__ using NotList.__iter__? Because CPython makes subclass 
slots (like __add/radd__) take precedent over builtin slots in cases 
like this. A better example is:

Python 2.5.2 (r252:60911, Apr 22 2008, 12:00:45) 
[GCC 4.0.1 (Apple Computer, Inc. build 5367)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class Base(object):
...     def __imul__(self, other):
...             return 'Base imul'
... 
>>> class Foo(Base):
...     def __mul__(self, other):
...             return 'Foo mul'
... 
>>> f = Foo()
>>> f *= 1
>>> f # super __imul__ was consulted before Foo __mul__
'Base imul'
>>> class Bar(list):
...     def __mul__(self, other):
...             return 'Bar mul'
... 
>>> b = Bar()
>>> b *= 1
>>> b # super __imul__ *NOT* consulted before Bar __mul__
'Bar mul'
>>> list.__imul__ # there's really a super __imul__ slot
<slot wrapper '__imul__' of 'list' objects>

Whereas Jython/Pypy would have called list.__imul__ and returned an 
empty list

CPython's behavior seems inconsistent, and I don't see it documented 
anywhere (reading http://docs.python.org/ref/coercion-rules.html gives 
you the impression Jython/Pypy are correct).

We and the Pypy folk probably need to follow up on this with CPython 
folk. (ccing pedronis)
History
Date User Action Args
2008-06-20 06:17:13pjenveysetstatus: open -> closed
nosy: + pedronis
resolution: fixed
messages: + msg3293
2008-06-12 18:52:45pjenveysetassignee: pjenvey
2008-01-16 19:32:01jekcreate