msg189201 - (view) |
Author: Andy Chugunov (andy.chugunov) |
Date: 2013-05-14 08:19 |
At the same time append() succeeds silently, while simple '+' fails.
Here's an example:
>>> a = ([1],)
>>> a[0].append(2)
>>> a
([1, 2],)
>>> a[0] += [3]
Traceback (most recent call last):
File "<pyshell#47>", line 1, in <module>
a[0] += [3]
TypeError: 'tuple' object does not support item assignment
>>> a
([1, 2, 3],)
>>> a[0] = a[0] + [4]
Traceback (most recent call last):
File "<pyshell#49>", line 1, in <module>
a[0] = a[0] + [4]
TypeError: 'tuple' object does not support item assignment
>>> a
([1, 2, 3],)
>>>
Looks like a self-contradictory behavior. Unfortunately, I'm not yet advanced enough to figure out where the problem might be and submit a fix.
Tested with v3.3.1 on Windows 7 (64-bit), and v3.2.3 and v2.7.3 on Debian 7 (also 64-bit).
|
msg189202 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-14 08:52 |
This is a side effect to the way the in place operators work.
Basically "a[0] += [3]" is evaluated as:
a[0] = a[0].__iadd__([3])
The call to __iadd__ succeeds, which is why the list is updated, but you get an exception when the interpreter tries to update item 0 of the tuple.
|
msg189203 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-14 09:02 |
I'm closing this issue as rejected because the current behavior is intentional, although it is confusing (IMO).
|
msg189458 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-05-17 14:42 |
This gets reported periodically. I wonder if it is time for a FAQ entry. There doesn't seem to be one yet.
|
msg189460 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-17 15:07 |
You've got a point there. What about this patch (but then with proper english grammer)?
BTW. Actually fixing this wart would be possible, but at a significant cost: you'd have to change the implementation of LHS += RHS from:
tmp = LHS
tmp = tmp.__iadd__(RHS)
LHS = tmp
to:
tmp = LHS
LHS = tmp
tmp = tmp.__iadd__(RHS)
LHS = tmp
The odd assignment on the second line would detect that the LHS is immutable in 99+% of use cases before updating the RHS.
My gut feeling is that an implementation of this would have too high a cost (both in runtime performance and in a more complicated compiler), although it would be interesting to actually see a patch.
|
msg189471 - (view) |
Author: Andy Chugunov (andy.chugunov) |
Date: 2013-05-17 17:07 |
Thank you for the clarification! The exception is appropriate as tuples have to stay immutable. Got it.
Could you please also explain a bit about the append() call? Should it in theory raise an exception as well or is such clean behavior intended?
|
msg189498 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-05-18 07:23 |
Another 'fix' would be to allow assignment to a tuple item in the case that the item being assigned is the same (in the sense of 'is') as the item already in the tuple. Then the '+=' operation would succeed, but the tuple would remain immutable.
|
msg189499 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-05-18 07:25 |
@andy.chugunov: tuples are immutable in the sense that you can't put a new object into a tuple or remove objects from a tuple. That doesn't mean that tuples can't contain mutable objects, or prevent you from mutating the objects inside a tuple.
So the append method call is fine: it's not modifying the tuple itself (the tuple still has references to exactly the same objects both before and after the append call); it's merely mutating one the objects inside the tuple.
|
msg189622 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-05-19 20:03 |
Mark, I rather like your suggestion. Do you think Guido will go for it? :)
In the meantime, I've attached a rewritten version of Ronald's FAQ entry. I think the FAQ is why it fails when the extend succeeds, not why assigning to a tuple raises an error, so I revised the question and answer accordingly.
|
msg189640 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-05-20 08:18 |
David: not sure. I don't think I'd go for it---I think the cure is worse than the disease.
> I think the FAQ is why it fails when the extend succeeds
Agreed.
|
msg189648 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-20 10:41 |
David: your update to the FAQ looks ok to me.
I don't like updating the __setitem__ of tuple to allow setting an identical value (that is 'a[0] = a[0]'), it is a bit too magic to my taste and hides the real problem.
I'd slightly prefer my idea: update the code generated for += to try performing the assignment before calling __iadd__. I hadn't looked at the AST compiler before, but at first glance implementing my idea seems easier than I had expected and I'm working on an experimental patch.
|
msg189649 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-05-20 11:27 |
> it is a bit too magic to my taste and hides the real problem.
Agreed. I do wonder whether there's a solution that allows the operation to succeed, though.
|
msg189650 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-20 11:39 |
issue-17973-experimental.txt is a very crude first attempt at catching augment assignment to an immutable LHS. On first glance the code works, but it causes problems in the test suite and hence isn't correct yet. The generated code also isn't optimal, the LHS is evaluated too many times.
With the patch ``([],)[0] += ['a']`` raises an exception, and doesn't update the list.
I also haven't done benchmarking of the code, I'd expect a slowdown in micro benchmarks for augmented assignment when the LHS isn't a simple name because there is more byte code (``a[0] += b`` disassembly expands from 8 to 14 lines).
|
msg189651 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-20 11:42 |
Allowing the operation to succeed wouldn't be right, you are assigning to an immutable location after all.
|
msg189652 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-05-20 11:44 |
Unfortunately, I don't think the checking first approach can work either: in the case where the object *does* accept assignments, it will now be assigned to twice. If there are side-effects from those assignments, then that will change behaviour.
An example is the Enthought traits library, where assignment to a list item causes a notification to be fired; with this change, listeners would now receive two events instead of one.
I suspect there just isn't a good solution here.
|
msg189653 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2013-05-20 12:38 |
what if we try the assignment, and catch TypeError only if __iadd__ returned self?
|
msg189666 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-05-20 14:35 |
New changeset b363473cfe9c by R David Murray in branch '3.3':
#17973: Add FAQ entry for ([],)[0] += [1] both extending and raising.
http://hg.python.org/cpython/rev/b363473cfe9c
New changeset 6d2f0edb0758 by R David Murray in branch 'default':
Merge #17973: Add FAQ entry for ([],)[0] += [1] both extending and raising.
http://hg.python.org/cpython/rev/6d2f0edb0758
New changeset 8edfe539d4c6 by R David Murray in branch '2.7':
#17973: Add FAQ entry for ([],)[0] += [1] both extending and raising.
http://hg.python.org/cpython/rev/8edfe539d4c6
|
msg189667 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-05-20 14:38 |
You still have the side effect problem, as far as I can see.
I'm going to close this doc issue, if you guys do come up with some clever fix, you can reopen it again :)
|
msg189725 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2013-05-21 06:21 |
I agree that there's probably no good solution here.
Catching TypeError would require emitting a lot more byte code, and would change the semantics of augmented assignment, in particular it wouldn't really be an assignment statement anymore (and hides some flow control).
|
msg189764 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-05-21 15:47 |
New changeset 6ab88d6527f1 by R David Murray in branch '3.3':
#17973: fix technical inaccuracy in faq entry (it now passes doctest).
http://hg.python.org/cpython/rev/6ab88d6527f1
New changeset 26588b6a39d9 by R David Murray in branch 'default':
merge #17973: fix technical inaccuracy in faq entry (it now passes doctest).
http://hg.python.org/cpython/rev/26588b6a39d9
New changeset 7fce9186accb by R David Murray in branch '2.7':
#17973: fix technical inaccuracy in faq entry (it now passes doctest).
http://hg.python.org/cpython/rev/7fce9186accb
|
msg190440 - (view) |
Author: Andy Chugunov (andy.chugunov) |
Date: 2013-06-01 07:14 |
Thank you guys for all the efforts you put in solving and answering this.
Just so that we're clear.
It is perfectly legitimate to extend lists withing tuples. It just doesn't seem possible to make augmented assignment and simple assignment handle this particular operation correctly without unduly sacrificing their general efficiency. Use append() on the lists instead.
Is that correct?
|
msg190446 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-06-01 14:02 |
I believe that summary is correct.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:45 | admin | set | github: 62173 |
2013-06-01 14:02:21 | r.david.murray | set | messages:
+ msg190446 |
2013-06-01 07:14:28 | andy.chugunov | set | messages:
+ msg190440 |
2013-05-21 15:48:00 | python-dev | set | messages:
+ msg189764 |
2013-05-21 06:21:09 | ronaldoussoren | set | messages:
+ msg189725 |
2013-05-20 14:38:25 | r.david.murray | set | status: open -> closed resolution: fixed messages:
+ msg189667
stage: patch review -> resolved |
2013-05-20 14:35:16 | python-dev | set | nosy:
+ python-dev messages:
+ msg189666
|
2013-05-20 12:38:06 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages:
+ msg189653
|
2013-05-20 12:35:44 | ezio.melotti | set | nosy:
+ ezio.melotti
|
2013-05-20 11:44:46 | mark.dickinson | set | messages:
+ msg189652 |
2013-05-20 11:42:43 | ronaldoussoren | set | messages:
+ msg189651 |
2013-05-20 11:39:46 | ronaldoussoren | set | files:
+ issue-17973-experimental.txt
messages:
+ msg189650 |
2013-05-20 11:27:11 | mark.dickinson | set | messages:
+ msg189649 |
2013-05-20 10:41:01 | ronaldoussoren | set | messages:
+ msg189648 |
2013-05-20 08:18:30 | mark.dickinson | set | messages:
+ msg189640 |
2013-05-19 20:03:09 | r.david.murray | set | status: closed -> open files:
+ augmented_tuple_assignment_faq.patch
components:
+ Documentation title: '+=' on a list inside tuple both succeeds and raises an exception -> FAQ entry for: '+=' on a list inside tuple both succeeds and raises an exception resolution: rejected -> (no value) versions:
+ Python 3.4 messages:
+ msg189622 stage: patch review |
2013-05-18 07:25:33 | mark.dickinson | set | messages:
+ msg189499 |
2013-05-18 07:23:10 | mark.dickinson | set | nosy:
+ mark.dickinson messages:
+ msg189498
|
2013-05-17 17:07:11 | andy.chugunov | set | messages:
+ msg189471 |
2013-05-17 15:07:14 | ronaldoussoren | set | files:
+ faq-update.patch keywords:
+ patch messages:
+ msg189460
|
2013-05-17 14:42:34 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg189458
|
2013-05-14 09:02:09 | ronaldoussoren | set | status: open -> closed resolution: rejected messages:
+ msg189203
|
2013-05-14 08:59:07 | flox | set | nosy:
+ flox
|
2013-05-14 08:53:13 | ronaldoussoren | set | assignee: ronaldoussoren |
2013-05-14 08:52:10 | ronaldoussoren | set | nosy:
+ ronaldoussoren messages:
+ msg189202
|
2013-05-14 08:19:37 | andy.chugunov | create | |