Skip to content

Improve docstring for str.index #73735

Closed
Closed

Description

@rhettinger
BPO 29549
Nosy @rhettinger, @ncoghlan, @benhoyt, @serhiy-storchaka, @zhangyangyu, @lisroach, @Mariatta, @CuriousLearner
PRs
  • bpo-29549: Fixes docstring for str.index #147
  • bpo-29602: fix signed zero handling in complex constructor #204
  • bpo-29549: Fixes docstring for str.index #256
  • [3.6] bpo-29549: Fixes docstring for str.index (GH-256) #1028
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/lisroach'
    closed_at = <Date 2017-04-05.06:32:02.545>
    created_at = <Date 2017-02-13.20:57:22.161>
    labels = ['3.7', 'docs']
    title = 'Improve docstring for str.index'
    updated_at = <Date 2017-04-10.21:38:58.744>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2017-04-10.21:38:58.744>
    actor = 'Mariatta'
    assignee = 'lisroach'
    closed = True
    closed_date = <Date 2017-04-05.06:32:02.545>
    closer = 'rhettinger'
    components = ['Documentation']
    creation = <Date 2017-02-13.20:57:22.161>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29549
    keywords = []
    message_count = 17.0
    messages = ['287724', '288025', '288027', '288234', '288235', '288353', '288354', '288402', '288404', '288405', '288408', '288465', '288470', '288494', '291154', '291159', '291397']
    nosy_count = 8.0
    nosy_names = ['rhettinger', 'ncoghlan', 'benhoyt', 'serhiy.storchaka', 'xiang.zhang', 'lisroach', 'Mariatta', 'CuriousLearner']
    pr_nums = ['147', '204', '256', '1028']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue29549'
    versions = ['Python 3.6', 'Python 3.7']

    Activity

    1. rhettinger commented on Feb 13, 2017

      @rhettinger
      ContributorAuthor

      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.

    2. benhoyt commented on Feb 17, 2017

      benhoytmannequin
      Mannequin

      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."

    3. serhiy-storchaka commented on Feb 17, 2017

      @serhiy-storchaka
      Member

      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).

    4. lisroach commented on Feb 20, 2017

      @lisroach
      Contributor

      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!

    5. serhiy-storchaka commented on Feb 20, 2017

      @serhiy-storchaka
      Member

      No need to add Argument Clinic code. Just make docstrings in the style of of Argument Clinic descriptions.

    6. Mariatta commented on Feb 22, 2017

      @Mariatta
      Member

      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.

    7. CuriousLearner commented on Feb 22, 2017

      @CuriousLearner
      Member

      Mariatta, I apologize for this. I completely missed to see that this was already assigned to someone else.

      I've withdrawn my PR.

      Thanks.

    8. rhettinger commented on Feb 23, 2017

      @rhettinger
      ContributorAuthor

      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.

    9. zhangyangyu commented on Feb 23, 2017

      @zhangyangyu
      Member

      Lisa, could you make a PR? It could let more developers see your patch.

    10. rhettinger commented on Feb 23, 2017

      @rhettinger
      ContributorAuthor

      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.

    11. serhiy-storchaka commented on Feb 23, 2017

      @serhiy-storchaka
      Member

      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.

    12. lisroach commented on Feb 23, 2017

      @lisroach
      Contributor

      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.

    13. Mariatta commented on Feb 23, 2017

      @Mariatta
      Member

      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?

    14. serhiy-storchaka commented on Feb 23, 2017

      @serhiy-storchaka
      Member

      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.

    15. rhettinger commented on Apr 5, 2017

      @rhettinger
      ContributorAuthor

      New changeset 43ba886 by Raymond Hettinger (Lisa Roach) in branch 'master':
      bpo-29549: Fixes docstring for str.index (#256)
      43ba886

    16. rhettinger commented on Apr 5, 2017

      @rhettinger
      ContributorAuthor

      Thanks Lisa.

    17. Mariatta commented on Apr 9, 2017

      @Mariatta
      Member

      New changeset 577fc04 by Mariatta in branch '3.6':
      [3.6] bpo-29549: Fixes docstring for str.index (GH-256) (GH-1028)
      577fc04

    18. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.7 (EOL)end of lifedocsDocumentation in the Doc dir

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions