-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
gh-137609: Update signatures of builtins in the documentation #137610
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
base: main
Are you sure you want to change the base?
gh-137609: Update signatures of builtins in the documentation #137610
Conversation
Show signatures that match the actual signatures or future multisignatures for all functions, classes and methods in the "builtins" module.
…uiltins This is to pair with pythonGH-137610.
…uiltins This is to pair with pythonGH-137610.
See also #137611. |
.. class:: range(stop, /) | ||
range(start, stop, step=1, /) |
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.
See #125945 for range
.. function:: min(iterable, /, *, key=None) | ||
min(iterable, /, *, default, key=None) | ||
min(arg1, arg2, /, *args, key=None) |
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.
See #131868 for min/max.
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 approve of the issue, the addition of /
s, the renamings, and most of the details. The main question for me is relative positioning of /
and *args
.
Fewer versus more lines is partly style preference and partly technical accuracy, and the signature needed to have an inspect.signature and to write a python version of the same or similar function, versus ease of understanding how to call the function. Are "future multisignatures" a real possibility?
Doc/library/functions.rst
Outdated
.. class:: bytearray() | ||
bytearray(source) | ||
bytearray(source, encoding) | ||
bytearray(source, encoding, errors) |
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 believe only two lines are needed.
.. class:: bytearray() | |
bytearray(source) | |
bytearray(source, encoding) | |
bytearray(source, encoding, errors) | |
.. class:: bytearray(source=b'') | |
bytearray(source, encoding, errors='strict') |
See also #137100, which is also about the text that follows.
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.
Some descriptions have separate signature for no argument, others merge it with a signature with one argument. See for example dict
which could be written as dict(mapping_or_iterable=(), **kwargs)
, but is written as three semantically different signatures. I tried to be more consistent and chose the former variant. But I have no such strong preference.
Doc/library/functions.rst
Outdated
.. class:: bytes() | ||
bytes(source) |
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.
Same comments as for bytearray.
max(arg1, arg2, *args, key=None) | ||
.. function:: max(iterable, /, *, key=None) | ||
max(iterable, /, *, default, key=None) | ||
max(arg1, arg2, /, *args, key=None) |
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.
To me, the /,
belongs after *args
as the latter only collects the remaining positional args.
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.
This is not valid syntax.
.. function:: max(iterable, /, *, key=None) | ||
max(iterable, /, *, default, key=None) |
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.
Why the inconsistency of type mode, normal versus italic? Applies to next line also.
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 guess this is a flaw of the GitHub highligher.
Doc/library/functions.rst
Outdated
str(object, encoding) | ||
str(object, encoding, errors) | ||
str(object, *, errors) |
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.
This is tough. There is a default encoding if errors
is given, not if not. str(b'abc')=="b'abc'"
(the 6 char repr()) while str(b'abc',errors='strict')=='abc'
(3 chars).
When you're done making the requested changes, leave the comment: |
@@ -54,15 +54,15 @@ are always available. They are listed here in alphabetical order. | |||
.. |func-bytearray| replace:: ``bytearray()`` | |||
.. |func-bytes| replace:: ``bytes()`` | |||
|
|||
.. function:: abs(x) | |||
.. function:: abs(number, /) | |||
|
|||
Return the absolute value of a number. The argument may be an |
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.
Return the absolute value of a number. The argument may be an | |
Return the absolute value of *number*. *number* may be an |
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 think that there is no need to refer the argument by name if it is the only argument. I think it's easier to understand if you don't get distracted by the name. And if you were to read it out loud, which variant sounds better?
|
||
Return a reverse :term:`iterator`. *seq* must be an object which has | ||
Return a reverse :term:`iterator`. The argument must be an object which has |
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.
Why not update the previous?
Return a reverse :term:`iterator`. The argument must be an object which has | |
Return a reverse :term:`iterator`. *sequence* must be an object which has |
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.
Actually, sequence
is not so good name. The argument can be a dict.
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 have made the requested changes; please review again.
@@ -54,15 +54,15 @@ are always available. They are listed here in alphabetical order. | |||
.. |func-bytearray| replace:: ``bytearray()`` | |||
.. |func-bytes| replace:: ``bytes()`` | |||
|
|||
.. function:: abs(x) | |||
.. function:: abs(number, /) | |||
|
|||
Return the absolute value of a number. The argument may be an |
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 think that there is no need to refer the argument by name if it is the only argument. I think it's easier to understand if you don't get distracted by the name. And if you were to read it out loud, which variant sounds better?
Doc/library/functions.rst
Outdated
.. class:: bytearray() | ||
bytearray(source) | ||
bytearray(source, encoding) | ||
bytearray(source, encoding, errors) |
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.
Some descriptions have separate signature for no argument, others merge it with a signature with one argument. See for example dict
which could be written as dict(mapping_or_iterable=(), **kwargs)
, but is written as three semantically different signatures. I tried to be more consistent and chose the former variant. But I have no such strong preference.
max(arg1, arg2, *args, key=None) | ||
.. function:: max(iterable, /, *, key=None) | ||
max(iterable, /, *, default, key=None) | ||
max(arg1, arg2, /, *args, key=None) |
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.
This is not valid syntax.
.. function:: max(iterable, /, *, key=None) | ||
max(iterable, /, *, default, key=None) |
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 guess this is a flaw of the GitHub highligher.
|
||
Return a reverse :term:`iterator`. *seq* must be an object which has | ||
Return a reverse :term:`iterator`. The argument must be an object which has |
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.
Actually, sequence
is not so good name. The argument can be a dict.
Show signatures that match the actual signatures or future multisignatures for all functions, classes and methods in the "builtins" module.
📚 Documentation preview 📚: https://cpython-previews--137610.org.readthedocs.build/