Skip to content

bpo-30662: fixed OrderedDict.__init__ docstring re PEP 468#2179

Merged
rhettinger merged 5 commits into
python:masterfrom
jonathaneunice:fix-issue-30662
Sep 5, 2017
Merged

bpo-30662: fixed OrderedDict.__init__ docstring re PEP 468#2179
rhettinger merged 5 commits into
python:masterfrom
jonathaneunice:fix-issue-30662

Conversation

@jonathaneunice

@jonathaneunice jonathaneunice commented Jun 14, 2017

Copy link
Copy Markdown
Contributor

As of Python 3.6, PEP 468 indicates that kwarg order is no longer arbitrary, but stable wrt to the order of args stated in source code. OrderedDict's docstring should be clear about that, not describe the situation that obtained in Python 3.5 and prior.

@mention-bot

Copy link
Copy Markdown

@jonathaneunice, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rhettinger, @serhiy-storchaka and @tiran to be potential reviewers.

@jonathaneunice

Copy link
Copy Markdown
Contributor Author

Would be happy with it simply saying "Initialize an ordered dictionary. The signature is the same as regular dictionaries." I.e., just truncating it so not actively wrong wrt PEP 468. But it's a big change in semantic guarantees, so a bit more explantation might be warranted.

Pending further review, have tightened comment wording and made sure the C implementation has an equivalent comment. (Thanks to Serhiy Storchaka suggestion on bugs.python.org)

@jonathaneunice

Copy link
Copy Markdown
Contributor Author

Have just reviewed the collections docs (Doc/library/collections.rst). They seem quite up-to-date re PEP 468. No changes seem needed there.

Comment thread Lib/collections/__init__.py Outdated
regular dictionaries, but keyword arguments are not recommended because
their insertion order is arbitrary.

regular dictionaries. As of Python 3.6, keyword argument insertion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There should be an extra whitespace aftere the period.

Comment thread Lib/collections/__init__.py Outdated
their insertion order is arbitrary.

regular dictionaries. As of Python 3.6, keyword argument insertion
order is stable. (PEP 468)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMHO preserved is more meaningful than stable.

Comment thread Objects/odictobject.c Outdated
"Initialize an ordered dictionary. The signature is the same as\n\
regular dictionaries, but keyword arguments are not recommended because\n\
their insertion order is arbitrary.\n\
regular dictionaries. As of Python 3.6, keyword argument insertion\n\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The same as in the previous comment: extra whitespace after period, preserved instead of stable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@marco-buttu Thank you. Wording and post-period spacing updated.

Comment thread Lib/collections/__init__.py Outdated
regular dictionaries, but keyword arguments are not recommended because
their insertion order is arbitrary.

regular dictionaries. As of Python 3.6, keyword argument insertion

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't mention Python 3.6 or PEP 468 as that's the past and just how things are in Python 3.7 which this docstring is for. Same goes for the other docstring change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should I mention that keyword order is preserved? Or just drop that altogether and fall back to the compatibility claim alone?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about something like "regular dictionaries. Keyword argument order is preserved."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ericsnowcurrently Agreed. That's exactly the current proposal.

@jonathaneunice jonathaneunice left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Short, crisp notation of order preservation seems workable consensus.

Comment thread Lib/collections/__init__.py Outdated
regular dictionaries, but keyword arguments are not recommended because
their insertion order is arbitrary.

regular dictionaries. As of Python 3.6, keyword argument insertion

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ericsnowcurrently Agreed. That's exactly the current proposal.

@rhettinger rhettinger merged commit faa57cb into python:master Sep 5, 2017
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Sep 5, 2017
…honGH-2179)

* fixed OrderedDict.__init__ docstring re PEP 468

* tightened comment and mirrored to C impl

* added space after period per marco-buttu

* preserved substituted for stable

* drop references to Python 3.6 and PEP 468
(cherry picked from commit faa57cb)
@bedevere-bot

Copy link
Copy Markdown

GH-3370 is a backport of this pull request to the 3.6 branch.

Mariatta added a commit that referenced this pull request Sep 6, 2017
…2179) (GH-3370)

* fixed OrderedDict.__init__ docstring re PEP 468

* tightened comment and mirrored to C impl

* added space after period per marco-buttu

* preserved substituted for stable

* drop references to Python 3.6 and PEP 468
(cherry picked from commit faa57cb)
GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017
* fixed OrderedDict.__init__ docstring re PEP 468

* tightened comment and mirrored to C impl

* added space after period per marco-buttu

* preserved substituted for stable

* drop references to Python 3.6 and PEP 468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants