classification
Title: set_display evaluation order doesn't match documented behaviour
Type: behavior Stage: commit review
Components: Interpreter Core Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Hamish Campbell, NeilGirdhar, abarry, docs@python, python-dev, r.david.murray, rhettinger, serhiy.storchaka
Priority: high Keywords: patch

Created on 2016-01-06 02:42 by Hamish Campbell, last changed 2016-09-08 22:32 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
build_set.diff rhettinger, 2016-01-06 05:43 Rough first draft patch needs test and more thought review
build_set2.diff rhettinger, 2016-01-06 16:19 Add tests review
Messages (17)
msg257579 - (view) Author: Hamish Campbell (Hamish Campbell) Date: 2016-01-06 02:42
It looks like the behaviour of set displays do not match behaviour in some cases. The documentation states:

"A set display yields a new mutable set object, the contents being specified by either a sequence of expressions or a comprehension. When a comma-separated list of expressions is supplied, its elements are evaluated from left to right and added to the set object. When a comprehension is supplied, the set is constructed from the elements resulting from the comprehension."

Note the following:

   >>> foo = { True, 1 }
   >>> print(foo)
   {1}

However, if we add elements 'left to right':

   >>> foo = set()
   >>> foo.add(True)
   >>> foo.add(1)
   >>> print(foo)
   {True}

Note that similar documentation for dict displays produces the expected result.

"If a comma-separated sequence of key/datum pairs is given, they are evaluated from left to right to define the entries of the dictionary: each key object is used as a key into the dictionary to store the corresponding datum. This means that you can specify the same key multiple times in the key/datum list, and the final dictionary’s value for that key will be the last one given."

   >>> foo = {}
   >>> foo[True] = 'bar'
   >>> foo[1] = 'baz'
   >>> print(foo)
   {True: 'baz'}

Which matches the dict display construction:

   >>> foo = { True: 'bar', 1: 'baz'}
   >>> print(foo)
   {True: 'baz'}

Note that I've tagged this as a documentation bug, but it seems like the documentation might be the preferred implementation.
msg257580 - (view) Author: Hamish Campbell (Hamish Campbell) Date: 2016-01-06 02:45
Apologies, that first line should read "It looks like the documentation of set displays do not match behaviour in some cases".
msg257582 - (view) Author: Hamish Campbell (Hamish Campbell) Date: 2016-01-06 02:54
Note also the differences here:

   >>> print(set([True, 1]))
   {True}
   >>> print({True, 1})
   {1}
   >>> print({x for x in [True, 1]})
   {True}
msg257584 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-01-06 03:40
Set displays appear to be the culprit here:

>>> class A:
...   count = 0
...   def __init__(self):
...     self.cnt = self.count
...     type(self).count += 1
...   def __eq__(self, other):
...     return type(self) is type(other)
...   def __hash__(self):
...     return id(type(self))
...
>>> e={A(), A(), A(), A(), A()}
>>> e
{<__main__.A object at 0x002BB2B0>}
>>> list(e)[0].cnt
4
>>> list(e)[0].count
5
>>> A.count = 0
>>> q=set([A(), A(), A(), A(), A()])
>>> q
{<__main__.A object at 0x002BB310>}
>>> list(q)[0].cnt
0
>>> list(q)[0].count
5

I'm guessing this is an optimization and/or set displays just don't keep ordering at definition time.

Do you have a use case where `x == y`/`hash(x) == hash(y)` does not mean that `x` and `y` should be interchangeable? True and 1 are 100% interchangeable, minus their str() output, and my example is very unlikely to ever appear in actual code.

Probably just better to fix the docs to specify that sets literals don't check ordering.
msg257585 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-01-06 04:35
The culprit is the BUILD_SET opcode in Python/ceval.c which unnecessarily loops backwards (it looks like it was copied from the BUILD_TUPLE opcode).
msg257590 - (view) Author: Hamish Campbell (Hamish Campbell) Date: 2016-01-06 08:46
> Do you have a use case where `x == y`/`hash(x) == hash(y)` does not mean that `x` and `y` should be interchangeable? True and 1 are 100% interchangeable, minus their str() output, and my example is very unlikely to ever appear in actual code.

No I don't have a use case :)

> The culprit is the BUILD_SET opcode in Python/ceval.c which unnecessarily loops backwards (it looks like it was copied from the BUILD_TUPLE opcode).

Incidentally, pypy seems to behave the same as reported here.
msg257691 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2016-01-07 14:09
If BUIlD_SET is going to be changed, it would probably be could to keep BUILD_SET_UNPACK consistent.
msg264307 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-26 15:26
LGTM.
msg264308 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2016-04-26 15:30
Please don't forget to fix BUILD_SET_UNPACK to match.
msg264309 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2016-04-26 15:32
Also, please add the following test: "{*{True, 1}}"

Should be True.
msg264312 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-26 15:54
> Please don't forget to fix BUILD_SET_UNPACK to match.

Isn't it already correct?

> Also, please add the following test: "{*{True, 1}}"

Did you meant "{*{True}, *{1}}"?
msg264314 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2016-04-26 15:58
Ah, sorry, I somehow went cross-eyed.  Not sure offhand which test would test the BUILD_TUPLE_UNPACK, but I think you're right Serhiy.  Could just add both?
msg270111 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-07-10 17:55
Ping.  (Raymond, if you are OK with someone else committing this, you could un-assign it.)
msg270132 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-07-10 20:39
I've got it.
msg275178 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-08 21:46
New changeset 0967531fc762 by Raymond Hettinger in branch '3.5':
Issue #26020:  Fix evaluation order for set literals
https://hg.python.org/cpython/rev/0967531fc762
msg275186 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-08 22:25
New changeset a8d504600c18 by Raymond Hettinger in branch '2.7':
Issue #26020: Fix evaluation order for set literals
https://hg.python.org/cpython/rev/a8d504600c18
msg275187 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-08 22:27
New changeset aa2bf603a9bd by Raymond Hettinger in branch '2.7':
Issue #26020: Add news entry
https://hg.python.org/cpython/rev/aa2bf603a9bd
History
Date User Action Args
2016-09-08 22:32:50rhettingersetstatus: open -> closed
resolution: fixed
2016-09-08 22:27:40python-devsetmessages: + msg275187
2016-09-08 22:25:36python-devsetmessages: + msg275186
2016-09-08 21:46:18python-devsetnosy: + python-dev
messages: + msg275178
2016-07-10 20:39:17rhettingersetmessages: + msg270132
2016-07-10 17:55:29r.david.murraysetnosy: + r.david.murray
messages: + msg270111
2016-04-26 15:58:48NeilGirdharsetmessages: + msg264314
2016-04-26 15:54:52serhiy.storchakasetmessages: + msg264312
2016-04-26 15:32:21NeilGirdharsetmessages: + msg264309
2016-04-26 15:30:26NeilGirdharsetmessages: + msg264308
2016-04-26 15:26:58serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg264307

components: + Interpreter Core, - Documentation
type: behavior
stage: patch review -> commit review
2016-04-26 08:24:10rhettingersetpriority: normal -> high
2016-01-07 14:09:53NeilGirdharsetnosy: + NeilGirdhar
messages: + msg257691
2016-01-06 16:21:05rhettingersetversions: + Python 2.7, Python 3.5
2016-01-06 16:20:06rhettingersetstage: patch review
2016-01-06 16:19:53rhettingersetfiles: + build_set2.diff
2016-01-06 08:46:13Hamish Campbellsetmessages: + msg257590
2016-01-06 05:43:39rhettingersetfiles: + build_set.diff
keywords: + patch
2016-01-06 04:35:22rhettingersetassignee: docs@python -> rhettinger

messages: + msg257585
nosy: + rhettinger
2016-01-06 03:40:53abarrysetnosy: + abarry

messages: + msg257584
versions: - Python 2.7, Python 3.2, Python 3.3, Python 3.4, Python 3.5
2016-01-06 02:54:07Hamish Campbellsetmessages: + msg257582
2016-01-06 02:45:04Hamish Campbellsetmessages: + msg257580
2016-01-06 02:42:21Hamish Campbellcreate