bpo-32337: Update tutorial and documentation related with dict order. #4973

shangdahao · 2017-12-22T10:14:55Z

https://bugs.python.org/issue32337

socketpair · 2017-12-23T14:40:15Z

I'm voting AGAINST this. order in dict has never been guaranteed. This may be changed in future. Use https://docs.python.org/3.7/library/collections.html#collections.OrderedDict if you want.

srinivasreddy · 2017-12-23T19:55:16Z

@socketpair order is guaranteed in 3.7. And @rhettinger is proposing to deprecate OrderedDict altogether.

socketpair · 2017-12-23T19:59:07Z

Doc/tutorial/datastructures.rst

@@ -497,7 +497,8 @@ You can't use lists as keys, since lists can be modified in place using index
 assignments, slice assignments, or methods like :meth:`append` and
 :meth:`extend`.

-It is best to think of a dictionary as an unordered set of *key: value* pairs,
+Dictionaries are guaranteed to retain insertion order.
+It is best to think of a dictionary as an ordered set of *key: value* pairs,


dictionary as an ordered set can be misindestood as ordered by key, intead of ordered by insertion order.
It best to write here, that dict will be enumerated (.keys(), items(), .values()) in order of insertion. And also specify "not ordered by key".

C++ has orderdmap and unorderedmap. Orderedmap orders by key.

Also, in some natural languages, "ordered" and "sorted" are the same :(

Agree, thanks.

I don’t know if other languages are relevant here. If Python docs make a clear explanation of sorted and ordered then things should not be confusing.

socketpair · 2017-12-23T20:03:17Z

Doc/tutorial/datastructures.rst

@@ -510,7 +511,7 @@ value associated with that key is forgotten.  It is an error to extract a value
 using a non-existent key.

 Performing ``list(d.keys())`` on a dictionary returns a list of all the keys
-used in the dictionary, in arbitrary order (if you want it sorted, just use
+used in the dictionary, in insertion order (if you want it sorted, just use


maybe add example how to "permanently" sort a dict ? like:

xxx = dict(sorted(xxx.items(), key=operator.itemgetter(0)))

It’s make sense, thanks.

I don't think so. This is tutorial. We should focus about very important part.

Previously, "unordered" was important for new users to avoid pitfall.
On the other hand, "preserving insertion order" is not so important.

socketpair · 2017-12-24T13:31:37Z

Lib/test/test_dict.py

@@ -1176,6 +1176,48 @@ def iter_and_mutate():

        self.assertRaises(RuntimeError, iter_and_mutate)



do we need to test order for dict comprehensions ?

Yes, thanks!

methane · 2017-12-26T09:33:23Z

Doc/tutorial/datastructures.rst

@@ -497,7 +497,9 @@ You can't use lists as keys, since lists can be modified in place using index
 assignments, slice assignments, or methods like :meth:`append` and
 :meth:`extend`.

-It is best to think of a dictionary as an unordered set of *key: value* pairs,
+Dictionaries are guaranteed to retain insertion order, not ordered by key.


Just remove unordered, and don't mention about ordering here.
While dict preserves insertion order, it's not so important in "What is dict?".
For example, dict.__eq__ ignores order.

shangdahao · 2017-12-26T11:28:49Z

Done. Thanks methane for your explanation.

methane · 2017-12-26T11:47:35Z

I feel NEWS entry is too short.
And did you saw this comment ?

shangdahao · 2017-12-26T12:17:41Z

Oh sorry, I did't saw it.

That solution is great, and I am going to remove the tests in this PR.

And I am wondering is there necessary to keep the "test_copy_ordering" and 'test_comprehension_ordering' here? Since there have no tests for them.

methane · 2018-01-10T11:17:22Z

I'm OK for document body, but I think someone can write more good commit message and NEWS entry. (Sorry, I'm not good English writer)

This PR updates only tutorial. But I think other commits will be made before 3.7 final can
share same NEWS entry. (Lot's of redundant NEWS entry make chengelog less usable)

zhangyangyu · 2018-01-26T07:46:08Z

Since we are editing this part, I have a question why do we use list(dict.keys()) and sorted(dict.keys())? Isn't list(dict) or sorted(dict) enough? I have seen many codes using this redundant keys call, even in Python2 testing membership.

merwok · 2018-01-26T17:31:03Z

Misc/ACKS

@@ -1788,3 +1788,4 @@ Jelle Zijlstra
 Gennadiy Zlobin
 Doug Zongker
 Peter Åstrand
+Shang Hui


Could you move this line with other S or H names? (It’s loosely sorted by last name, for names that have a concept of last name)

OK, I will do it.

shangdahao · 2018-01-28T09:01:51Z

Thanks zhangyangyu and merwok, I have made a update as your suggestions.

zhangyangyu · 2018-01-28T12:35:58Z

It's just my question. I am not sure which is actually wanted. But at least if you change it, the footprint link is not needed either.

merwok · 2018-01-28T17:42:49Z

I think that people may use d.keys() for membership or iteration because they may not be fully comfortable with the fact that iteration/membership on dicts uses keys, not items.

I don’t know if the keys method pre-dates the iteration protocol, but given that keys was never deprecated, to me it’s an indication that it’s fine if people use it, even if it’s technically redundant.

Here in the tutorial, I would go for not changing it.

methane · 2018-01-28T20:25:42Z

I think that people may use d.keys() for membership or iteration because they may not be fully comfortable with the fact that iteration/membership on dicts uses keys, not items.

But tutorial should teach preferred way, not redundant way?

…dict is now guaranteed.

JulienPalard

LGTM

merwok · 2018-02-13T17:01:33Z

Doc/library/collections.rst

@@ -224,7 +224,7 @@ For example::
 .. class:: Counter([iterable-or-mapping])

    A :class:`Counter` is a :class:`dict` subclass for counting hashable objects.
-    It is an unordered collection where elements are stored as dictionary keys
+    It is a collection where elements are stored as dictionary keys


I found more occurrences of «arbitrary order» and usage of sorted to make examples deterministic in the rest of the file. I think careful thought should be given to the guarantees given in this file: it may be easy to say that elements returns in insertion order, but most_common may need to stay under-defined.

If you could get the attention of @rhettinger on the bug tracker to see what’s the best thing to do here, or possibly to not touch this file in this PR, that would be best.

Thanks for you great review, but I am not very understand your meaning about:

"but most_common may need to stary under-defined.".

Could you explain it a little more?

Fixed the typo!

The basic idea is that if we guarantee that all these methods return things in insertion order now, we may preclude future implementation changes or optimizations. Each method needs to be checked to see if it’s useful to change the doc that says «unordered» (if it’s accurate now) or not (if we don’t want to guarantee it).

@merwok How about creating another pull request about it?
This pull request is too long discussed.

I think that if a file is updated, it should be updated exhaustively, not add more ambiguity than was before. But I don’t want to block the updates to the tutorial, so feel free to merge when you are satisfied.

bedevere-bot · 2018-02-13T17:01:41Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

shangdahao · 2018-02-14T05:51:41Z

I have made the requested changes; please review again

bedevere-bot · 2018-02-14T05:51:43Z

Thanks for making the requested changes!

@zhangyangyu, @JulienPalard, @merwok: please review the changes made to this pull request.

socketpair · 2018-02-17T21:38:00Z

Document somewhere that two dicts with same contents, but different order of key-value pairs are equal.

i.e. assert {1:2, 3:4} == {3:4, 1:2}.

It is not obvious, and I needed to check that before use.

merwok · 2018-02-17T22:22:38Z

@socketpair Dict equality is already documented to not rely on order.

socketpair · 2018-03-25T17:07:56Z

https://docs.python.org/3/library/json.html#json.load

methane · 2018-03-25T17:38:13Z

@socketpair See #5001

methane · 2018-04-04T04:53:54Z

Let's merge this as-is. We discussed long enough.
We can improve document later.

miss-islington · 2018-04-04T04:55:08Z

Thanks @shangdahao for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

(cherry picked from commit dfbbbf1) Co-authored-by: hui shang <[email protected]>

bedevere-bot · 2018-04-04T04:56:18Z

GH-6368 is a backport of this pull request to the 3.7 branch.

(cherry picked from commit dfbbbf1) Co-authored-by: hui shang <[email protected]>

the-knights-who-say-ni added the CLA signed label Dec 22, 2017

bedevere-bot added the awaiting review label Dec 22, 2017

shangdahao force-pushed the bpo-32337 branch from e9c06d0 to 3936c37 Compare December 22, 2017 11:16

srinivasreddy approved these changes Dec 23, 2017

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Dec 23, 2017

socketpair reviewed Dec 23, 2017

View reviewed changes

socketpair reviewed Dec 24, 2017

View reviewed changes

shangdahao force-pushed the bpo-32337 branch from a69a5e7 to 73b6e50 Compare December 24, 2017 14:53

methane reviewed Dec 26, 2017

View reviewed changes

shangdahao changed the title ~~bpo-32337: Add tests and update doc for dict order.~~ bpo-32337: Update doc for dict order. Dec 28, 2017

methane changed the title ~~bpo-32337: Update doc for dict order.~~ bpo-32337: Update tutorial of dict Jan 10, 2018

methane changed the title ~~bpo-32337: Update tutorial of dict~~ bpo-32337: Update dict tutorial for preserving insertion order behavior Jan 10, 2018

serhiy-storchaka requested a review from rhettinger January 12, 2018 09:09

methane added the docs Documentation in the Doc dir label Jan 26, 2018

merwok reviewed Jan 26, 2018

View reviewed changes

bpo-32337: Update tutorial for dict order, since Python 3.7 order of …

90384e6

…dict is now guaranteed.

shangdahao force-pushed the bpo-32337 branch from 9dab891 to 90384e6 Compare January 29, 2018 14:30

JulienPalard approved these changes Feb 10, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Feb 10, 2018

zhangyangyu added the needs backport to 3.7 label Feb 13, 2018

zhangyangyu approved these changes Feb 13, 2018

View reviewed changes

merwok requested changes Feb 13, 2018

View reviewed changes

bedevere-bot removed the awaiting merge label Feb 13, 2018

bedevere-bot added the awaiting changes label Feb 13, 2018

Removed unclear changes.

0fa8c6b

bedevere-bot added awaiting change review and removed awaiting changes labels Feb 14, 2018

Tutorial is a part of document

9f42748

methane approved these changes Mar 13, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 13, 2018

methane merged commit dfbbbf1 into python:master Apr 4, 2018

bedevere-bot removed the awaiting merge label Apr 4, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 4, 2018

bpo-32337: Update documentats about dict order (pythonGH-4973)

b757f64

(cherry picked from commit dfbbbf1) Co-authored-by: hui shang <[email protected]>

bedevere-bot removed the needs backport to 3.7 label Apr 4, 2018

miss-islington added a commit that referenced this pull request Apr 4, 2018

bpo-32337: Update documentats about dict order (GH-4973)

9216dff

(cherry picked from commit dfbbbf1) Co-authored-by: hui shang <[email protected]>

		@@ -1176,6 +1176,48 @@ def iter_and_mutate():

		self.assertRaises(RuntimeError, iter_and_mutate)

Uh oh!

bpo-32337: Update tutorial and documentation related with dict order. #4973

bpo-32337: Update tutorial and documentation related with dict order. #4973

Uh oh!

Conversation

shangdahao commented Dec 22, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socketpair commented Dec 23, 2017

Uh oh!

srinivasreddy commented Dec 23, 2017

Uh oh!

socketpair Dec 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shangdahao commented Dec 26, 2017

Uh oh!

methane commented Dec 26, 2017

Uh oh!

shangdahao commented Dec 26, 2017

Uh oh!

methane commented Jan 10, 2018

Uh oh!

zhangyangyu commented Jan 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shangdahao commented Jan 28, 2018

Uh oh!

zhangyangyu commented Jan 28, 2018

Uh oh!

merwok commented Jan 28, 2018

Uh oh!

methane commented Jan 28, 2018

Uh oh!

JulienPalard left a comment

Choose a reason for hiding this comment

Uh oh!

merwok Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Feb 13, 2018

Uh oh!

shangdahao commented Feb 14, 2018

Uh oh!

bedevere-bot commented Feb 14, 2018

Uh oh!

socketpair commented Feb 17, 2018

Uh oh!

merwok commented Feb 17, 2018

Uh oh!

socketpair commented Mar 25, 2018

shangdahao commented Dec 22, 2017 •

edited by bedevere-bot

Loading

socketpair Dec 23, 2017 •

edited

Loading

merwok Feb 13, 2018 •

edited

Loading