Skip to content
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

list and tuple index methods should accept None parameters #74121

Closed
gwk mannequin opened this issue Mar 28, 2017 · 11 comments
Closed

list and tuple index methods should accept None parameters #74121

gwk mannequin opened this issue Mar 28, 2017 · 11 comments
Assignees
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@gwk
Copy link
Mannequin

gwk mannequin commented Mar 28, 2017

BPO 29935
Nosy @rhettinger, @serhiy-storchaka, @gwk, @JelleZijlstra
PRs
  • bpo-29935: Fix error messages in the index() method of tuple, list and deque #887
  • bpo-29935: Fixed error messages in the index() method of tuple, list … #907
  • bpo-29935: Fixed error messages in the index() method of tuple, list … #909
  • bpo-29935: Fixed error messages in the index() method of tuple and li… #910
  • 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/serhiy-storchaka'
    closed_at = <Date 2017-03-30.17:34:56.393>
    created_at = <Date 2017-03-28.16:16:21.949>
    labels = ['interpreter-core', 'type-bug', '3.7']
    title = 'list and tuple index methods should accept None parameters'
    updated_at = <Date 2017-03-30.17:34:56.392>
    user = 'https://github.com/gwk'

    bugs.python.org fields:

    activity = <Date 2017-03-30.17:34:56.392>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2017-03-30.17:34:56.393>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2017-03-28.16:16:21.949>
    creator = 'gwk'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29935
    keywords = []
    message_count = 11.0
    messages = ['290737', '290783', '290786', '290788', '290822', '290838', '290851', '290853', '290860', '290864', '290865']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'serhiy.storchaka', 'gwk', 'JelleZijlstra']
    pr_nums = ['887', '907', '909', '910']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29935'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

    @gwk
    Copy link
    Mannequin Author

    gwk mannequin commented Mar 28, 2017

    As of python3.6, passing None to the start/end parameters of list.index and tuple.index raises the following exception:
    "slice indices must be integers or None or have an index method"

    This suggests that the intent is to support None as a valid input. This would be quite useful for the end parameter, where the sensible default is len(self) rather than a constant. Note also that str, bytes, and bytearray all support None.

    I suggest that CPython be patched to support None for start/end. Otherwise, at the very least the exception message should be changed.

    Accepting None will make the optional start/end parameters for this method more consistent across the types, which is especially helpful when using type annotations / checking.

    @DimitrisJim DimitrisJim mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.7 (EOL) end of life labels Mar 28, 2017
    @gwk gwk mannequin added the type-feature A feature request or enhancement label Mar 28, 2017
    @serhiy-storchaka serhiy-storchaka self-assigned this Mar 28, 2017
    @rhettinger
    Copy link
    Contributor

    The error message should be fixed and the API should be left alone. A *None* value doesn't do a good jog of communicating intent.

    @serhiy-storchaka
    Copy link
    Member

    I concur with Raymond. Proposed patch fixes error messages and doesn't change the public API.

    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Mar 29, 2017
    @gwk
    Copy link
    Mannequin Author

    gwk mannequin commented Mar 29, 2017

    I think it is a mistake not to support None values. Please consider:

    The existing message clearly suggests that the intent is to support the same set of values as the start/stop parameters of the slice type. str, bytes, and bytearray all support None for index, as well as count, find, rfind, and rindex.

    Not supporting None makes the type signature of list.index and tuple.index subtly different from those of str, bytes, and bytearray, when they could otherwise be interchangeable. It makes the type signature of index inconsistent across the types.

    Furthermore, it makes converting slice start/stop to these arguments a pain. Compare the following:

        seq.index(start=slice.start, end=slice.stop) # accepting None
    seq.index(start=(0 if slice.start is None else slice.start), end=(len(seq) if slice.stop is None else slice.stop)) # current behavior.
    

    I do not buy the argument that the latter communicates intent better, and it is adds bug-prone baggage to every call site.

    Similarly, being able to pass None for end/stop parameters is very convenient in the case of optional passthrough arguments, saving the programmer from this:
    def f(seq, end=None):
    i = seq.index(start=0, end=(len(seq) if end is None else end)) # current behavior.
    ...

    Note that for the programmer writing f, None (or some other distinct sentinel value) is a requirement for correctness, because end=len(seq) is incorrect.

    I would understand opposition to this on the basis of not changing existing behavior in any way, but "communicating intent" seems like a thin argument in comparison to the above.

    @JelleZijlstra
    Copy link
    Member

    I agree with George that supporting None here is the better option.

    This problem also applies to collections.deque. tuple, list, and deque all have very similar index implementations, and it would be nice to merge their argument parsing boilerplate.

    @rhettinger
    Copy link
    Contributor

    Sorry, modifying these mature APIs is a really bad idea. They are worked well as-is of over a decade. Seeing people use None for these arguments makes the code less readable.

    Python has a number of APIs where the number of arguments controls the behavior. For example, type() with one argument does something very different that type() with three arguments. Likewise, iter() behaves differently with two arguments than three. The pattern used by index() shows-up in a number of places where we have optional start and end arguments. These APIs are a long standing Python norm, both for the core and for third-party modules. Type checking will have to adapt to norms rather than having all of Python change for the convenience of typing.

    Serhiy, please go ahead with the doc fix.

    If the OP really wants to insist on changing old and stable APIs through out the language in a way that I consider harmful to readability, we can battle it out in a PEP and on mailing lists.

    @serhiy-storchaka
    Copy link
    Member

    Thank you Raymond for your review.

    In any case we should first fix error messages in maintain releases. Changing signatures of methods of basic type should be discussed on the mailing lists. I don't think this needs a PEP, unless this change will be extended to many other methods and types.

    @serhiy-storchaka
    Copy link
    Member

    New changeset d4edfc9 by Serhiy Storchaka in branch 'master':
    bpo-29935: Fixed error messages in the index() method of tuple, list and deque (#887)
    d4edfc9

    @serhiy-storchaka
    Copy link
    Member

    New changeset bf4bb2e by Serhiy Storchaka in branch '3.6':
    bpo-29935: Fixed error messages in the index() method of tuple, list and deque (#887) (#907)
    bf4bb2e

    @serhiy-storchaka
    Copy link
    Member

    New changeset 8b8bde4 by Serhiy Storchaka in branch '3.5':
    bpo-29935: Fixed error messages in the index() method of tuple, list and deque (#887) (#907) (#909)
    8b8bde4

    @serhiy-storchaka
    Copy link
    Member

    New changeset 079f21f by Serhiy Storchaka in branch '2.7':
    bpo-29935: Fixed error messages in the index() method of tuple and list (#887) (#907) (#910)
    079f21f

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants