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.

Sign up for GitHub

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

Jump to bottom

ambiguous documentation for dict.popitem #78304

Closed
mr-nfamous mannequin opened this issue Jul 16, 2018 · 8 comments
Closed

ambiguous documentation for dict.popitem #78304

mr-nfamous mannequin opened this issue Jul 16, 2018 · 8 comments
Assignees
@rhettinger
Labels
3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir

Comments

@mr-nfamous
Copy link
Mannequin

mr-nfamous mannequin commented Jul 16, 2018

BPO 34123
Nosy @arigo, @rhettinger, @methane, @mr-nfamous
PRs
  • bpo-34123: Fix missed documentation update for dict.popitem(). #8292
  • [3.7] bpo-34123: Fix missed documentation update for dict.popitem(). (GH-8292) #8307
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
closed_at = <Date 2018-07-17.02:08:43.847>
created_at = <Date 2018-07-16.06:00:27.043>
labels = ['3.7', '3.8', 'docs']
title = 'ambiguous documentation for dict.popitem'
updated_at = <Date 2018-07-17.02:08:43.846>
user = 'https://github.com/mr-nfamous'

    bugs.python.org fields:

    activity = <Date 2018-07-17.02:08:43.846>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2018-07-17.02:08:43.847>
closer = 'rhettinger'
components = ['Documentation']
creation = <Date 2018-07-16.06:00:27.043>
creator = 'bup'
dependencies = []
files = []
hgrepos = []
issue_num = 34123
keywords = ['patch']
message_count = 8.0
messages = ['321708', '321709', '321712', '321714', '321716', '321741', '321782', '321787']
nosy_count = 5.0
nosy_names = ['arigo', 'rhettinger', 'methane', 'docs@python', 'bup']
pr_nums = ['8292', '8307']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue34123'
versions = ['Python 3.7', 'Python 3.8']
    @mr-nfamous
    Copy link
    Mannequin Author

    mr-nfamous mannequin commented Jul 16, 2018

    https://docs.python.org/3/library/stdtypes.html#dict.popitem

    dict.popitem no longer returns an "arbitrary" (key, value) pair as the documentation suggests. Rather, it always returns the pair whose key was most recently inserted (ie., the last entry in dk_entries).

    Perhaps the docs could reflect that this method is now always LIFO rather arbitrary now that insertion order is guaranteed?

    @mr-nfamous mr-nfamous mannequin assigned docspython Jul 16, 2018
    @mr-nfamous mr-nfamous mannequin added 3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir labels Jul 16, 2018
    @methane
    Copy link
    Member

    methane commented Jul 16, 2018

    I think it is implementation detail yet.
    Only iteration order is guaranteed.

    @rhettinger
    Copy link
    Contributor

    My opinion is that it is in fact guaranteed. It makes no sense for all other aspects of the dict behavior to be guaranteed and not this one. This is what PyPy and CPython already do and there is no scenario where an implementation would be able to append a new pair at the end but unable to reverse the operation.

    IMO, leaving this in an ambiguous state would created problems unnecessarily and it would preclude reasonable uses of the the ordering feature.

    @rhettinger rhettinger assigned rhettinger and unassigned docspython Jul 16, 2018
    @methane
    Copy link
    Member

    methane commented Jul 16, 2018

    @armin Rigo
    How do you think about this?
    Is there no possible optimizations by breaking LIFO dict.popitem()?

    @rhettinger
    Copy link
    Contributor

    Is there no possible optimizations by breaking LIFO dict.popitem()?

    Even if there were a possible optimization, we wouldn't care. The API is too desirable to forgo in the name of micro-optimization. We don't design our APIs that way -- trading the relevant and actionable for ethereal and unknown.

    Also, it is common sense that addition of a key/value pair at the end is a readily undoable operation (no more expensive to remove than it was to add). There will be a dummy entry in the hash array just like there is now.

    @arigo
    Copy link
    Mannequin

    arigo mannequin commented Jul 16, 2018

    Agreed with Raymond.

    @rhettinger
    Copy link
    Contributor

    New changeset 01b7d58 by Raymond Hettinger in branch 'master':
    bpo-34123: Fix missed documentation update for dict.popitem(). (GH-8292)
    01b7d58

    @rhettinger
    Copy link
    Contributor

    New changeset bfa8a35 by Raymond Hettinger (Miss Islington (bot)) in branch '3.7':
    bpo-34123: Fix missed documentation update for dict.popitem(). (GH-8292) (GH#8307)
    bfa8a35

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @rhettinger rhettinger

    Labels
    3.7 (EOL) end of life 3.8 only security fixes docs Documentation in the Doc dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @methane @rhettinger