classification
Title: Improve docstring for str.index
Type: Stage: resolved
Components: Documentation Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lisroach Nosy List: CuriousLearner, Mariatta, benhoyt, lisroach, ncoghlan, rhettinger, serhiy.storchaka, xiang.zhang
Priority: normal Keywords:

Created on 2017-02-13 20:57 by rhettinger, last changed 2017-04-10 21:38 by Mariatta. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 147 closed CuriousLearner, 2017-02-18 06:41
PR 204 open mark.dickinson, 2017-02-20 20:59
PR 256 merged lisroach, 2017-02-23 16:58
PR 1028 merged Mariatta, 2017-04-07 10:26
Messages (17)
msg287724 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-02-13 20:57
The docstring for str.index() needs to be synced with the main documentation rather than presuming knowledge of str.find().

Currently it reads:

>>> help(str.index)
Help on method_descriptor:

index(...)
    S.index(sub[, start[, end]]) -> int
    
    Like S.find() but raise ValueError when the substring is not found.

And it should read more like str.find:

help(str.find)
Help on method_descriptor:

find(...)
    S.find(sub[, start[, end]]) -> int
    
    Return the lowest index in S where substring sub is found,
    such that sub is contained within S[start:end].  Optional
    arguments start and end are interpreted as in slice notation.
    
    Return -1 on failure.

Lisa, it would be great if you could draft-up a patch.
msg288025 - (view) Author: Ben Hoyt (benhoyt) * Date: 2017-02-17 17:18
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."
msg288027 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-17 17:47
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).
msg288234 - (view) Author: Lisa Roach (lisroach) * (Python committer) Date: 2017-02-20 20:12
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: 

https://github.com/python/cpython/compare/master...lisroach:master

If it looks good I will make a pull request. Thanks!
msg288235 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-20 20:24
No need to add Argument Clinic code. Just make docstrings in the style of of Argument Clinic descriptions.
msg288353 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-02-22 11:22
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.
msg288354 - (view) Author: Sanyam Khurana (CuriousLearner) * (Python triager) Date: 2017-02-22 11:29
Mariatta, I apologize for this. I completely missed to see that this was already assigned to someone else.

I've withdrawn my PR.

Thanks.
msg288402 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-02-23 04:42
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.
msg288404 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-23 05:03
Lisa, could you make a PR? It could let more developers see your patch.
msg288405 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-02-23 05:20
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.
msg288408 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-23 05:42
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.
msg288465 - (view) Author: Lisa Roach (lisroach) * (Python committer) Date: 2017-02-23 17:02
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.
msg288470 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-02-23 19:21
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?
msg288494 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-23 23:30
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.
msg291154 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-05 05:36
New changeset 43ba8861e0ad044efafa46a7cc04e12ac5df640e by Raymond Hettinger (Lisa Roach) in branch 'master':
bpo-29549: Fixes docstring for str.index (#256)
https://github.com/python/cpython/commit/43ba8861e0ad044efafa46a7cc04e12ac5df640e
msg291159 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-05 06:32
Thanks Lisa.
msg291397 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-04-09 22:17
New changeset 577fc04a7157f6e904cffd6a0e1ad83d3460acd6 by Mariatta in branch '3.6':
[3.6] bpo-29549: Fixes docstring for str.index (GH-256) (GH-1028)
https://github.com/python/cpython/commit/577fc04a7157f6e904cffd6a0e1ad83d3460acd6
History
Date User Action Args
2017-04-10 21:38:58Mariattasetversions: - Python 2.7
2017-04-09 22:17:08Mariattasetmessages: + msg291397
2017-04-07 10:26:11Mariattasetpull_requests: + pull_request1185
2017-04-05 06:32:02rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg291159

stage: patch review -> resolved
2017-04-05 05:36:24rhettingersetmessages: + msg291154
2017-02-23 23:30:03serhiy.storchakasetmessages: + msg288494
2017-02-23 19:21:09Mariattasetmessages: + msg288470
2017-02-23 17:02:00lisroachsetmessages: + msg288465
2017-02-23 16:58:48lisroachsetpull_requests: + pull_request225
2017-02-23 05:42:36serhiy.storchakasetmessages: + msg288408
2017-02-23 05:20:12rhettingersetmessages: + msg288405
2017-02-23 05:03:23xiang.zhangsetnosy: + xiang.zhang

messages: + msg288404
stage: needs patch -> patch review
2017-02-23 04:42:21rhettingersetmessages: + msg288402
2017-02-22 11:29:41CuriousLearnersetnosy: + CuriousLearner
messages: + msg288354
2017-02-22 11:22:43Mariattasetnosy: + Mariatta
messages: + msg288353
2017-02-20 20:59:58mark.dickinsonsetpull_requests: + pull_request172
2017-02-20 20:24:18serhiy.storchakasetmessages: + msg288235
2017-02-20 20:12:37lisroachsetmessages: + msg288234
2017-02-18 11:45:08ncoghlansetnosy: + ncoghlan
2017-02-18 06:41:35CuriousLearnersetpull_requests: + pull_request107
2017-02-17 17:47:54serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg288027
2017-02-17 17:18:24benhoytsetnosy: + benhoyt
messages: + msg288025
2017-02-13 20:57:22rhettingercreate