-
-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
gh-116938: Clarify documentation of dict
and dict.update
regarding the positional argument they accept
#125213
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4505,14 +4505,14 @@ can be used interchangeably to index the same dictionary entry. | |
``dict([('foo', 100), ('bar', 200)])``, ``dict(foo=100, bar=200)`` | ||
|
||
If no positional argument is given, an empty dictionary is created. | ||
If a positional argument is given and it is a mapping object, a dictionary | ||
is created with the same key-value pairs as the mapping object. Otherwise, | ||
the positional argument must be an :term:`iterable` object. Each item in | ||
the iterable must itself be an iterable with exactly two objects. The | ||
first object of each item becomes a key in the new dictionary, and the | ||
second object the corresponding value. If a key occurs more than once, the | ||
last value for that key becomes the corresponding value in the new | ||
dictionary. | ||
If a positional argument is given and it defines a ``keys()`` method, a | ||
dictionary is created by calling :meth:`~object.__getitem__` on the argument with | ||
each returned key from the method. Otherwise, the positional argument must be an | ||
:term:`iterable` object. Each item in the iterable must itself be an iterable | ||
with exactly two elements. The first element of each item becomes a key in the | ||
new dictionary, and the second element the corresponding value. If a key occurs | ||
more than once, the last value for that key becomes the corresponding value in | ||
the new dictionary. | ||
|
||
If keyword arguments are given, the keyword arguments and their values are | ||
added to the dictionary created from the positional argument. If a key | ||
|
@@ -4669,10 +4669,11 @@ can be used interchangeably to index the same dictionary entry. | |
Update the dictionary with the key/value pairs from *other*, overwriting | ||
existing keys. Return ``None``. | ||
|
||
:meth:`update` accepts either another dictionary object or an iterable of | ||
key/value pairs (as tuples or other iterables of length two). If keyword | ||
arguments are specified, the dictionary is then updated with those | ||
key/value pairs: ``d.update(red=1, blue=2)``. | ||
:meth:`update` accepts either another object with a ``keys()`` method (in | ||
which case :meth:`~object.__getitem__` is called with every key returned from | ||
the method). or an iterable of key/value pairs (as tuples or other iterables | ||
of length two). If keyword arguments are specified, the dictionary is then | ||
updated with those key/value pairs: ``d.update(red=1, blue=2)``. | ||
Comment on lines
+4672
to
+4676
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a premature period on line 4674 after closing brace. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! Want to file a PR to fix it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
.. method:: values() | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -962,7 +962,7 @@ def clear(self): | |||||||||||||||||||||||
|
||||||||||||||||||||||||
def update(self, other=(), /, **kwds): | ||||||||||||||||||||||||
''' D.update([E, ]**F) -> None. Update D from mapping/iterable E and F. | ||||||||||||||||||||||||
If E present and has a .keys() method, does: for k in E: D[k] = E[k] | ||||||||||||||||||||||||
If E present and has a .keys() method, does: for k in E.keys(): D[k] = E[k] | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per #116938 (comment) |
||||||||||||||||||||||||
If E present and lacks .keys() method, does: for (k, v) in E: D[k] = v | ||||||||||||||||||||||||
In either case, this is followed by: for k, v in F.items(): D[k] = v | ||||||||||||||||||||||||
Comment on lines
+965
to
967
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I would like it to be more specific:
Suggested change
In the web docs the requirements are strict:
Usage of words class Object:
keys = []
def __iter__(self, /):
return iter(self.keys) This class clearly lacks .keys() method because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine IMO. It's much less important for docstrings to be completely precise than it is for the ReST documentation. The most important thing for the docstrings is for them to be concise and easily readable in a terminal or editor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, my bad. In that case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but the docstring for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this PR the first sentence was updated from Lines 4521 to 4525 in 2982bdb
Consider: class Object:
def keys(self):
return [1, 2]
def __iter__(self):
return [3, 4]
def __getitem__(self, item):
match item:
case 1: return 'one'
case 2: return 'two'
case 3: return 'three'
case 4: return 'four'
return item
print(dict(Object()))
d = {}
d.update(Object())
print(d) Both times the result is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I see what you mean now. Yes, I suppose that could also be clarified. |
||||||||||||||||||||||||
''' | ||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of using double spaces after periods? I see this is also present in the original .rst. Is there a special meaning for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There used to be a convention to put two spaces after a period. It's been slowly dying out for a long time now, but you still see it a lot in older pieces of writing. (This article in the Python docs is very old.) The most important thing is to stay consistent in whatever article you're editing at any one point in time; consistency between articles on this point is not a goal for CPython's docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. There was two spaces after period on line 4673 (4675 right now), but after this PR it is gone, should it be added back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wouldn't worry about it too much :-)