You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Good call. Additionally, it's weird to me that the docstring for str.find() says "Return -1 on failure." because the string not being found is not a "failure". Like the docs (https://docs.python.org/3.5/library/stdtypes.html#str.find) say, the str.find() docstring should say "Return -1 if sub is not found."
str.index() still is not converted to Argument Clinic (this is not easy). The docstring needs to be rewritten when convert it to Argument Clinic in any case. It would be nice to make it Argument Clinic compatible right now. The first line line should be short description, following paragraph(s) can describe more details, the description shouldn't refer to the metavariable S.
Docstrings for str, bytes and bytearray methods should be synchronised. Look also at docstrings for the index() method in other sequence types (list, tuple, deque, array, Sequence).
I tried to have a go at making the str.index Argument Clinic compatible, Serhiy can you take a look at my commits and let me know if this is the correct way of doing it:
I see that you have PRs against this issue.
The issue is currently assigned to Lisa.
I respectfully request that you choose a different issue to work on and to withdraw your pull request.
Please choose an issue that is not currently assigned to anyone. If an issue is assigned, please ask first before working on it.
Serhiy, I'm not sure what you're referring to by "Just make docstrings in the style of of Argument Clinic descriptions". For the time being, I would like the docstring to match that of str.find(). The point of the patch is to decouple the two.
AC issues and migrations can wait. At this point, AC adoption has been more disruptive that helpful in a number of cases.
Please everyone. Ease up. This is a very simple patch. Copy what is done in str.find to str.index. It doesn't need infinite discussion and debate. I assigned this to Lisa to give her practice. It wasn't intended to have five developers jump in and shove it back and forth. Let's let simple things be simple and not waste the time of new contributors.
I mean that this work is useless since it will be lost when convert str.index to Argument Clinic. Well, even when don't use Argument Clinic, but make str.index() be supported by inspect.signature(), the docstring should be significantly rewritten.
I think there are many other easy documentation issues for practicing.
And all proposed PRs change only the docstring of str.index. They don't touch docstrings of str.rindex, bytes.index, bytes.rindex, bytearray.index and bytearray.rindex. I can't approve such changes.
I'll just go ahead and make my PR, let me know what else needs to be done.
Serhiy, if you could point me in the direction of how to write the docstring so that it is in the Argument Clinic style I would be happy to take a look.
Activity
rhettinger commentedon Feb 13, 2017
The docstring for str.index() needs to be synced with the main documentation rather than presuming knowledge of str.find().
Currently it reads:
And it should read more like str.find:
Lisa, it would be great if you could draft-up a patch.
benhoyt commentedon Feb 17, 2017
Good call. Additionally, it's weird to me that the docstring for str.find() says "Return -1 on failure." because the string not being found is not a "failure". Like the docs (https://docs.python.org/3.5/library/stdtypes.html#str.find) say, the str.find() docstring should say "Return -1 if sub is not found."
serhiy-storchaka commentedon Feb 17, 2017
str.index() still is not converted to Argument Clinic (this is not easy). The docstring needs to be rewritten when convert it to Argument Clinic in any case. It would be nice to make it Argument Clinic compatible right now. The first line line should be short description, following paragraph(s) can describe more details, the description shouldn't refer to the metavariable S.
Docstrings for str, bytes and bytearray methods should be synchronised. Look also at docstrings for the index() method in other sequence types (list, tuple, deque, array, Sequence).
lisroach commentedon Feb 20, 2017
I tried to have a go at making the str.index Argument Clinic compatible, Serhiy can you take a look at my commits and let me know if this is the correct way of doing it:
master...lisroach:master
If it looks good I will make a pull request. Thanks!
serhiy-storchaka commentedon Feb 20, 2017
No need to add Argument Clinic code. Just make docstrings in the style of of Argument Clinic descriptions.
Mariatta commentedon Feb 22, 2017
CuriousLearner,
I see that you have PRs against this issue.
The issue is currently assigned to Lisa.
I respectfully request that you choose a different issue to work on and to withdraw your pull request.
Please choose an issue that is not currently assigned to anyone. If an issue is assigned, please ask first before working on it.
Thank you.
CuriousLearner commentedon Feb 22, 2017
Mariatta, I apologize for this. I completely missed to see that this was already assigned to someone else.
I've withdrawn my PR.
Thanks.
rhettinger commentedon Feb 23, 2017
Serhiy, I'm not sure what you're referring to by "Just make docstrings in the style of of Argument Clinic descriptions". For the time being, I would like the docstring to match that of str.find(). The point of the patch is to decouple the two.
AC issues and migrations can wait. At this point, AC adoption has been more disruptive that helpful in a number of cases.
zhangyangyu commentedon Feb 23, 2017
Lisa, could you make a PR? It could let more developers see your patch.
rhettinger commentedon Feb 23, 2017
Please everyone. Ease up. This is a very simple patch. Copy what is done in str.find to str.index. It doesn't need infinite discussion and debate. I assigned this to Lisa to give her practice. It wasn't intended to have five developers jump in and shove it back and forth. Let's let simple things be simple and not waste the time of new contributors.
serhiy-storchaka commentedon Feb 23, 2017
I mean that this work is useless since it will be lost when convert str.index to Argument Clinic. Well, even when don't use Argument Clinic, but make str.index() be supported by inspect.signature(), the docstring should be significantly rewritten.
I think there are many other easy documentation issues for practicing.
And all proposed PRs change only the docstring of str.index. They don't touch docstrings of str.rindex, bytes.index, bytes.rindex, bytearray.index and bytearray.rindex. I can't approve such changes.
lisroach commentedon Feb 23, 2017
I'll just go ahead and make my PR, let me know what else needs to be done.
Serhiy, if you could point me in the direction of how to write the docstring so that it is in the Argument Clinic style I would be happy to take a look.
Mariatta commentedon Feb 23, 2017
Thanks for the PR, Lisa.
Serhiy, Raymond, could you both review this? Thanks.
I can do the merge and backport once it gets both of your approval :)
Also,should this be applied to 3.5 too?
serhiy-storchaka commentedon Feb 23, 2017
Write docstrings in following style:
"index($self, sub, start=0, end=sys.maxsize, /)\n"
"--\n"
"\n"
"<The summary line>\n"
"\n"
"<Rest of multiline\n"
"description>"
Extract the summary line. Don't use the metavariable "S" since it no longer defined in the signature.
rhettinger commentedon Apr 5, 2017
New changeset 43ba886 by Raymond Hettinger (Lisa Roach) in branch 'master':
bpo-29549: Fixes docstring for str.index (#256)
43ba886
rhettinger commentedon Apr 5, 2017
Thanks Lisa.
Mariatta commentedon Apr 9, 2017
New changeset 577fc04 by Mariatta in branch '3.6':
[3.6] bpo-29549: Fixes docstring for str.index (GH-256) (GH-1028)
577fc04