-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-125897: Fix range function doc referring to step as a kwarg #125945
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?
Conversation
Addresses the python#125897 documentation discrepancy that the range function accepts step as a kwarg
99bbb0c
to
26e2f2c
Compare
@kbaikov, please avoid force-pushes: https://devguide.python.org/getting-started/pull-request-lifecycle |
@@ -1713,8 +1713,8 @@ are always available. They are listed here in alphabetical order. | |||
|
|||
|
|||
.. _func-range: | |||
.. class:: range(stop) | |||
range(start, stop, step=1) | |||
.. class:: range(stop, /) |
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 an issue with this syntax because, in this signature, stop
is actually start
and is only treated as stop
because 1 arg is passed. While I understand that, in the second signature, it is intended to convey that step
is a positional arg with a default value, you cannot treat the first signature the same. At least I think not. I'm happy to be proven wrong and learn something new. How would you write start=0
in the first signature?
If the step argument is omitted, it defaults to 1. If the start argument is omitted, it defaults to 0.
IMO, this is completely irrelevant. Yes, that is how range
functions internally, but, for the average reader (myself included), I think it would be less confusing to just have 3 signatures if the intent is to avoid the [, step]
syntax.
range(stop)
range(start, stop)
range(start, stop, step)
It may also be pertinent to rephrase the description as well.
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 an issue with this syntax because, in this signature, stop is actually start and is only treated as stop because 1 arg is passed.
No. Stop is stop and start is start. The point is that this function has multiple signatures. Each signature is a valid pure-python function signature. Closest pure-python version, I think, is the typing.overload.
How would you write start=0 in the first signature?
There is no start argument in the first signature.
would be less confusing to just have 3 signatures if the intent is to avoid the [, step] syntax.
range(stop)
[...]
The problem is that this signature means you can do range(stop=123)
, which is wrong. The /
syntax might be boring, but this is a part of the language, long time ago.
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.
No. Stop is stop and start is start. The point is that this function has multiple signatures. Each signature is a valid pure-python function signature. Closest pure-python version, I think, is the typing.overload.
There is no start argument in the first signature.
Sure, but I am describing how it works internally. To me, it seems inconsistent that the description explains the behavior under-the-hood but then makes a "special case" for the second signature. Why not just have 3 and explain that in the first signature you start from 0? Why confuse the reader with "If the start argument is omitted..." as if it has a single signature?
The problem is that this signature means you can do range(stop=123), which is wrong. The / syntax might be boring, but this is a part of the language, long time ago.
Well, you can, but you'll get a TypeError
. Nothing is stopping you from doing it with /
, either. Are we expecting folks to not understand that stop
is a positional arg without the /
?
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 confuse the reader with "If the start argument is omitted..." as if it has a single signature?
Could you propose alternative version for the documentation of range()?
Well, you can, but you'll get a TypeError.
That means you can't.
Nothing is stopping you from doing it with /, either.
Language syntax. It tells me, that function doesn't support 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.
IMO, the signature(s) should be consistent with the behavior of the function and with each other.
Internally, range
is overloaded:
- case 1: stop = args[0], start = 0, step = 1
range(stop, /)
- case 2: start = args[0], stop = args[1], step = 1
range(start, stop, /)
- case 3: start = args[0], stop = args[1], step = args[2]
range(start, stop, step, /)
I have never read a signature like range(stop)
and thought to myself to call it like range(stop=123)
. I have always understood, at-a-glance, that this is a positional arg and should be called like range(123)
(even though it can be called like stop=123
, just not for range
).
The same is true of range(start, stop, step=1)
. That reads, to me, at-a-glance, that step
is a keyword arg, not a positional arg with a default value.
In fact, range
agrees with me!
>>> range(0, 1, step=1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: range() takes no keyword arguments
The only time I would read step
as a positional arg with a default value is the signature range(start, stop, step=1, /)
; however, you cannot represent the other signature with default values like range(start=0, stop, step=1, /)
. This is inconsistent.
If the goal is to write signatures in such a way that language like "If the step argument is omitted, it defaults to 1. If the start argument is omitted, it defaults to 0." is not necessary then I don't think range(stop, /)
accomplishes that. Why does one get to be "fully described" but not the other? Unfortunately, I don't think this is possible for range
as-is.
P.S.
You linked to 4.9. More on Defining Functions in your comment and after reading it I'm left wondering: What the hell is the difference between a positional arg with a default value and a keyword arg?
>>> def foo(a=3, b=4):
... print(a, b)
...
>>> foo(b=2, a=1)
1 2
>>> foo(1, 2)
1 2
>>> foo()
3 4
>>> args = [5,6]
>>> foo(*args)
5 6
>>> kwargs = {"a": 7, "b": 8}
>>> foo(**kwargs)
7 8
According to this, the answer is: "positional argument: an argument that is not a keyword argument."
😅
I guess we should all be writing code and documentation using /
and *
.
Anyway, I'm sure you are all much smarter than me and will choose the best option.
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.
Thanks @JelleZijlstra. @travis-pds Hmm... I tend to take the approach of user first. Undesirable is too strong a word. It was not ideal when 3.8 was released. Time has passed and @JelleZijlstra has captured well the current thinking. I tend to read most of the decisions as in most cases... yet, it may make sense in some cases to do something else if it benefits readability and comprehension.
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.
It was decided in python/devguide#1344 to include the slashes, superseding those old PRs.
Hence I am asking why only range
should get the slashes again and not all other functions that lost them in #99476? Like e.g. bool(x=False, /)
?
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 it still seems inconsistent to only consider range here
@chris-eibl, this pr address concrete issue with range, not about mechanical reversion of #99476.
Like e.g. bool(x=False, /)?
Please open issue.
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.
Just for bool
? This does not seem consistent to me, but the mechanical reversion does not seem to be liked either.
Therefore, I leave it to you core dev(s) to decide which and how many issues to create, since there are more canditates than bool
.
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.
A friendly ping. Is there any decision on this issue/PR? How do i proceed? |
CC @nedbat Perhaps, https://devguide.python.org/documentation/style-guide/#function-signatures requires a clarification. Is this for new code? How we should handle current sphinx docs, that use funny square bracket syntax? |
Addresses the #125897 documentation discrepancy that the range function accepts step as a kwarg
📚 Documentation preview 📚: https://cpython-previews--125945.org.readthedocs.build/
range
params discrepancy across versions #125897