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

Replace OrderedDict with regular dict in namedtuple's _asdict() method. #80045

Closed
rhettinger opened this issue Jan 31, 2019 · 7 comments
Closed
Assignees
Labels
3.8 only security fixes stdlib Python modules in the Lib dir

Comments

@rhettinger
Copy link
Contributor

BPO 35864
Nosy @rhettinger, @ericsnowcurrently
PRs
  • bpo-35864: Replace OrderedDict with regular dict in namedtuple() #11708
  • bpo-35864: Replace OrderedDict with regular dict in namedtuple() #11708
  • bpo-35864: Replace OrderedDict with regular dict in namedtuple() #11708
  • bpo-35864: fix namedtuple._asdict() docstring #11720
  • bpo-35864: fix namedtuple._asdict() docstring #11720
  • bpo-35864: fix namedtuple._asdict() docstring #11720
  • bpo-35864: fix namedtuple._asdict() docstring #11720
  • bpo-35864: Move dangling bullet points into named subsections #13046
  • 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 2019-01-31.17:43:18.498>
    created_at = <Date 2019-01-31.02:20:11.952>
    labels = ['3.8', 'library']
    title = "Replace OrderedDict with regular dict in namedtuple's _asdict() method."
    updated_at = <Date 2019-05-02.00:28:22.429>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2019-05-02.00:28:22.429>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-01-31.17:43:18.498>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-01-31.02:20:11.952>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35864
    keywords = ['patch', 'patch', 'patch']
    message_count = 7.0
    messages = ['334602', '334617', '334694', '334695', '334696', '334697', '334720']
    nosy_count = 2.0
    nosy_names = ['rhettinger', 'eric.snow']
    pr_nums = ['11708', '11708', '11708', '11720', '11720', '11720', '11720', '13046']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue35864'
    versions = ['Python 3.8']

    @rhettinger
    Copy link
    Contributor Author

    Now that regular dicts are ordered and compact, it makes more sense for the _asdict() method to create a regular dict (as it did in its early days) rather than an OrderedDict. The regular dict is much smaller, much faster, and has a much cleaner looking repr.

    Historically we would go through a deprecation period for a possibly breaking change; however, it was considered more benefit to users and less disruptive to make the update directly. See the thread starting at: https://mail.python.org/pipermail/python-dev/2019-January/156150.html

    @rhettinger rhettinger added the 3.8 only security fixes label Jan 31, 2019
    @rhettinger rhettinger self-assigned this Jan 31, 2019
    @rhettinger rhettinger added the stdlib Python modules in the Lib dir label Jan 31, 2019
    @rhettinger
    Copy link
    Contributor Author

    New changeset 0bb4bdf by Raymond Hettinger in branch 'master':
    bpo-35864: Replace OrderedDict with regular dict in namedtuple() (bpo-11708)
    0bb4bdf

    @ericsnowcurrently
    Copy link
    Member

    Thanks for doing this, Raymond!

    FYI, I found a couple of places where it still refers to OrderedDict:

    1. in the doc entry in collections.rst [1]
    2. in the docstring [2]

    [1] 0bb4bdf#diff-a2f0632ea84b755c7ef5b9bd291c7fdfR890
    [2] 0bb4bdf#diff-8a750c700ae5ac1d0a14922de83e99ccR432

    @ericsnowcurrently
    Copy link
    Member

    Also, there's a potentially misleading detail that you might consider correcting in the text. However, making it correct might make it slightly less clear, so it's a bit of a judgement call on how important it is to be explicit here. (FWIW, I think it's worth it to some extent.)

    The language reference doesn't guarantee that dictionaries are order-preserving (yet). [1] Therefore there can be (are?) Python implementations where dict isn't ordered. Users of those implementations may be confused by the docs (and docstring) saying the method returns a dict.

    This could be addressed by changing those places to say something like it returns an insertion-ordered mapping. The docs entry would also have an "CPython implementation detail" part saying it's actually a dict in CPython.

    Since "insertion-ordered mapping" isn't nearly as clear as "dict" (even if more correct), it may make more sense to simply say "dict". However, in that case I'd recommend at the very least to add a "CPython implementation detail" part to the docs entry which says in CPython it returns a dict, but in other implementations it may be some other order-preserving mapping.

    Honestly, after having written that the latter option seems more sensible. :)

    [1] https://docs.python.org/3.8/reference/datamodel.html#index-29

    @ericsnowcurrently
    Copy link
    Member

    If you prefer, I'd be glad to open separate issues for either thing.

    @ericsnowcurrently
    Copy link
    Member

    FWIW, both PEP-468 (kwargs order) and PEP-520 (class definition order) specify order-preserving mapping rather than dict. The main difference is that they are about language features rather than something out of the stdlib. :)

    @rhettinger
    Copy link
    Contributor Author

    New changeset 85d83ec by Raymond Hettinger (Amador Pahim) in branch 'master':
    bpo-35864: fix namedtuple._asdict() docstring (GH-11720)
    85d83ec

    @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
    Labels
    3.8 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants