Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-36059: Update OrderedDict() docs to reflect that regular dicts are now ordered #11966

Merged
merged 3 commits into from Feb 21, 2019

Conversation

rhettinger
Copy link
Contributor

@rhettinger rhettinger commented Feb 21, 2019

  • Describes the diminished significance of OrderedDict

  • Enumerate the differences with a regular dict (people need to know this in detail before migrating existing code from OrderedDict to regular dicts).

  • Remove examples that are no longer meaningful.

  • Update other examples and a new one

https://bugs.python.org/issue36059

return self.__class__, (OrderedDict(self),)
def __getitem__(self, key):
value = super().__getitem__(key)
self.move_to_end(key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retrun value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Thanks for noticing.

super().__setitem__(key, value)
if len(self) > self.maxsize:
oldest = next(iter(self))
del self[oldest]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super().popitem(last=False)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bummer but that actually doesn't work in this example. Unfortunately, popitem() calls the overridden _getitem_() method which moves the value.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update also the short description at the top: "dict subclass that remembers the order entries were added".

* A regular :class:`dict` was designed to be very good at mapping
operations. Tracking insertion order was secondary. In contrast,
:class:`OrderedDict` was designed to be good at reordering operations.
Efficient mapping operations were secondary.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is misleading. Mapping in OrderedDict is so effective as in dict. Only mutating operations (and maybe iteration) can be slower.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I'll add a clarification.

the equality operation for :class:`dict` only requires equal contents
regardless of order.

* The :class:`OrderedDict` class has a :meth:`popitem` method that accepts an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dict class also has a popitem method. Maybe reword this entry as "The popitem method of the OrderedDict class accepts ..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

@serhiy-storchaka
Copy link
Member

I would make descriptions of differences between OrderedDict and dict more brief. More details can be added below, in the main text or in FAQ. For example, keep just "Equality tests between :class:OrderedDict objects are order-sensitive."

@rhettinger
Copy link
Contributor Author

I prefer having the bullet points up front. Once dicts became ordered, the top documentation question about OrderedDict has become, "how is its API and performance" different from a regular dict that remembers insertion order. In particular, there are now two central questions to be answered 1) what are the implications from substituting a regular dict for an OrderedDict in existing code that uses an OrderedDict (this question has come up for core-devs several times), and 2) what capabilities does the OrderedDict have that justifies that it still exists.

I would like to answer these up front and completely.

The brief descriptions of what the methods do are in the regular class docs that follow.

@rhettinger rhettinger merged commit 49fd6dd into python:master Feb 21, 2019
@miss-islington
Copy link
Contributor

Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2019
…e now ordered (pythonGH-11966)

(cherry picked from commit 49fd6dd)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
@rhettinger rhettinger deleted the ordered_dict_docs branch February 21, 2019 08:12
rhettinger added a commit that referenced this pull request Feb 21, 2019
…e now ordered (GH-11966) (GH-#11972)

(cherry picked from commit 49fd6dd)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants