gh-152030: Clarify sequence.index() signature#152092
Conversation
Updated the method signature for sequence.index to use the positional-only marker (/), replacing the old bracket syntax. Fixes python#152030
Documentation build overview
6 files changed ·
|
skirpichev
left a comment
There was a problem hiding this comment.
Looks good, as a fix for sphinx docs.
This will not fix the issue, however. Method signatures should be fixed too (at least for str/bytes.index(), maybe more).
|
@skirpichev Thanks for the review on the docs. I went ahead and dug into the C code to fix the underlying signatures for these methods as well. Here is what I updated: The C-Code: I updated the Argument Clinic [clinic input] blocks for the master find methods in Objects/unicodeobject.c, Objects/bytesobject.c, and Objects/bytearrayobject.c. I removed the hardcoded @text_signature overrides and properly mapped the c_default values for start and end. I compiled and ran the test suite locally. test_inspect now passes cleanly with 0 failures, and the bracket-free signatures generate perfectly across all types! |
skirpichev
left a comment
There was a problem hiding this comment.
Sorry, probably you misinterpreted me. My suggestion was to make code changes in a separate pr.
But now, as signatures for count/find/etc were changed - please update sphinx docs too.
|
|
||
| sub: object | ||
| start: slice_index(accept={int, NoneType}, c_default='0') = None | ||
| start: slice_index(accept={int, NoneType}, c_default='0') = 0 |
There was a problem hiding this comment.
I think you can drop accept, it's a default value. Same for c_default.
|
@subscapularis, please do not click the "Update branch" button without a good reason because it notifies everyone watching the PR that there are new changes, when there are not, and it uses up limited CI resources. |
|
Ah my apologies for the 'Update branch' spam, didn't realize it pinged all the watchers Since my initial docs fix, I ended up going down the rabbit hole to fix the underlying C-signatures as well. Since we are keeping it all in this PR, here is a complete summary of everything that has been updated:
Removed the hardcoded @text_signature overrides Dropped the redundant accept and c_default parameters from the start arguments Also as far as i know methods like count, index, rfind, and rindex clone from find so, this single fix cascaded cleanly to all of them across all three types
Formatted them using the stacked multiple-signature layout with start=0 and stop=sys.maxsize as you suggested I also just ran a global clinic.py --make sync to ensure all the hidden .c.h checksums are perfectly up to date for the CI bots Thank you for being so patient and letting me keep all these fixes in one place, let me know if everything looks good to go |
Updated the method signature for sequence.index to use the positional-only marker (/), replacing the old bracket syntax.
index#152030