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

Remove reference to undefined dictionary ordering in Tutorial #73639

Closed
DimitrisJim mannequin opened this issue Feb 5, 2017 · 20 comments
Closed

Remove reference to undefined dictionary ordering in Tutorial #73639

DimitrisJim mannequin opened this issue Feb 5, 2017 · 20 comments
Assignees
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir

Comments

@DimitrisJim
Copy link
Mannequin

DimitrisJim mannequin commented Feb 5, 2017

BPO 29453
Nosy @rhettinger, @bitdancer, @methane, @serhiy-storchaka, @zhangyangyu, @DimitrisJim, @Mariatta
PRs
  • bpo-29453: Remove reference to undefined dictionary ordering in Tutorial #140
  • [3.6] bpo-29453: Remove reference to undefined dictionary ordering in Tutor… #208
  • Files
  • controlflowdiff.patch
  • controlflowdiff2.patch
  • 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/Mariatta'
    closed_at = <Date 2017-02-21.18:32:35.967>
    created_at = <Date 2017-02-05.15:17:27.092>
    labels = ['3.7', 'docs']
    title = 'Remove reference to undefined dictionary ordering in Tutorial'
    updated_at = <Date 2017-02-21.18:32:35.966>
    user = 'https://github.com/DimitrisJim'

    bugs.python.org fields:

    activity = <Date 2017-02-21.18:32:35.966>
    actor = 'Mariatta'
    assignee = 'Mariatta'
    closed = True
    closed_date = <Date 2017-02-21.18:32:35.967>
    closer = 'Mariatta'
    components = ['Documentation']
    creation = <Date 2017-02-05.15:17:27.092>
    creator = 'Jim Fasarakis-Hilliard'
    dependencies = []
    files = ['46524', '46525']
    hgrepos = []
    issue_num = 29453
    keywords = ['patch']
    message_count = 20.0
    messages = ['287048', '287050', '287051', '287053', '287054', '287056', '287057', '287058', '287059', '287060', '287061', '287092', '287138', '287219', '288047', '288081', '288098', '288272', '288313', '288314']
    nosy_count = 8.0
    nosy_names = ['rhettinger', 'r.david.murray', 'methane', 'docs@python', 'serhiy.storchaka', 'xiang.zhang', 'Jim Fasarakis-Hilliard', 'Mariatta']
    pr_nums = ['140', '208']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue29453'
    versions = ['Python 3.6', 'Python 3.7']

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 5, 2017

    Removes keys = sorted(keywords.keys()) from function example and removes the text that describes why this was necessary. As per PEP-468, this note is obsolete for 3.6+

    Also changes the ordering of the function call to match the previous output.

    @DimitrisJim DimitrisJim mannequin assigned docspython Feb 5, 2017
    @DimitrisJim DimitrisJim mannequin added 3.7 (EOL) end of life docs Documentation in the Doc dir labels Feb 5, 2017
    @methane
    Copy link
    Member

    methane commented Feb 5, 2017

    LGTM

    @bitdancer
    Copy link
    Member

    It is not (yet) a language requirement that ordinary dictionaries be ordered. This patch may become appropriate in 3.7, but that has not yet been determined. It is not appropriate for 3.6. In 3.6, the order of keys in an ordinary dictionary is still undefined, even though it is in practice consistent in CPython.

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 5, 2017

    Isn't it a language requirement that **kwargs be ordered in 3.6, David?

    PEP-468 states that **kwargs is to be an ordered mapping and, if I'm not mistaken, that was done in order to not depend on the fact that dicts became ordered. I might have understood something wrong, though :-)

    @zhangyangyu
    Copy link
    Member

    David, actually I have the same thoughts as Jim. Ordered ordinary dicts is not a feature but ordered **kwargs is in 3.6. They seems not the same thing.

    @serhiy-storchaka
    Copy link
    Member

    I would not change the order of keyword arguments, but rather change the output.

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 5, 2017

    It was a random decision on my part, Serhiy, since I didn't see any difference. Why would you go the other way around?

    @serhiy-storchaka
    Copy link
    Member

    Because it shows preserving the order of keyword arguments (rather than sorting by keyword name).

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 5, 2017

    Indeed, good point. Changed it to the suggested way.

    @serhiy-storchaka
    Copy link
    Member

    LGTM. Thanks Jim.

    But maybe it is worth to mention that the output corresponds to the order of passed keyword arguments.

    @bitdancer
    Copy link
    Member

    You are correct, I didn't read the full context of the diff. My apologies.

    @rhettinger
    Copy link
    Contributor

    Patch 2 looks fine to me.

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 6, 2017

    But maybe it is worth to mention that the output corresponds to the order of passed keyword arguments

    Should I add this note? It looks fine to me as is but I'm not the experienced one here :-)

    @serhiy-storchaka
    Copy link
    Member

    I don't know. David, Raymond, what are your thoughts?

    @rhettinger
    Copy link
    Contributor

    > But maybe it is worth to mention that the output
    > corresponds to the order of passed keyword arguments

    Should I add this note?

    Yes, please. This is a section on keyword arguments, making it the preferred place to mention the new guaranteed output order.

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Feb 18, 2017

    Added the following short sentence to the PR, which I believe makes the point clear:

    Note that the order in which the keyword arguments are printed is guaranteed to match the order in which they were provided in the function call.

    @rhettinger
    Copy link
    Contributor

    Thanks for the revision. Assigning to Mariatta so that she can apply the PR.

    @Mariatta
    Copy link
    Member

    New changeset 32e8f9b by Mariatta in branch 'master':
    bpo-29453: Remove reference to undefined dictionary ordering in Tutorial (GH-140)
    32e8f9b

    @Mariatta
    Copy link
    Member

    New changeset 9b49133 by GitHub in branch '3.6':
    bpo-29453: Remove reference to undefined dictionary ordering in Tutorial (GH-140) (#208)
    9b49133

    @Mariatta
    Copy link
    Member

    Thanks everyone. PR has been merged and backported to 3.6 :)
    Closing this issue.

    @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.7 (EOL) end of life docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants