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

Reconsider comparison chaining for containment tests #76236

Open
ncoghlan opened this issue Nov 17, 2017 · 12 comments
Open

Reconsider comparison chaining for containment tests #76236

ncoghlan opened this issue Nov 17, 2017 · 12 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@ncoghlan
Copy link
Contributor

BPO 32055
Nosy @gvanrossum, @mdickinson, @ncoghlan, @taleinat, @serhiy-storchaka
PRs
  • bpo-32055: Raise SyntaxWarning for chained in and not in. #4501
  • 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 = None
    closed_at = None
    created_at = <Date 2017-11-17.07:43:17.428>
    labels = ['type-bug', '3.7']
    title = 'Reconsider comparison chaining for containment tests'
    updated_at = <Date 2018-09-12.12:10:47.493>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2018-09-12.12:10:47.493>
    actor = 'mark.dickinson'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2017-11-17.07:43:17.428>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32055
    keywords = ['patch']
    message_count = 12.0
    messages = ['306411', '306413', '306415', '306418', '306419', '306734', '306760', '306848', '316318', '324687', '324789', '325076']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'mark.dickinson', 'ncoghlan', 'taleinat', 'serhiy.storchaka']
    pr_nums = ['4501']
    priority = 'low'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32055'
    versions = ['Python 3.7']

    @ncoghlan
    Copy link
    Contributor Author

    (I thought there was an open low priority issue for this, but I can't find it, so filing a new one)

    Currently, "in" and "not in" are classified as comparison operations in the language grammar, even though they're not actually documented as such (see https://bugs.python.org/issue28617 in relation to the latter point).

    Issue reports like https://bugs.python.org/issue30965, user questions like https://stackoverflow.com/questions/45180899/unexpected-result-from-in-operator-python/45180967, and behaviour puzzles like https://github.com/cosmologicon/pywat#operator-precedence suggest that the existing behaviour isn't particular intuitive to users.

    At the language design level (as far as I am aware), the benefit of treating "in" and "not in" as comparison operators is to ensure they share a precedence level with the other comparisons.

    While this is mostly a pretty harmless quirk, I think it's weird enough and useless enough for us to at least consider refining the Grammar such that "in" and "not in" live at the same level as other comparison operators, but *don't* participate in comparison chaining (i.e. "a == b in c" and "a in c == b" would both be syntax errors that required parentheses to disambiguate the desired associativity).

    @ncoghlan ncoghlan added 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Nov 17, 2017
    @ncoghlan
    Copy link
    Contributor Author

    https://bugs.python.org/issue14247 is another older issue related to this point (again related to a user finding the current behaviour genuinely unintuitive)

    @serhiy-storchaka
    Copy link
    Member

    a in b < c makes sense if b and c are sets.

    @ncoghlan
    Copy link
    Contributor Author

    Aye, there are definitely cases where the answer isn't nonsense.

    Even for sets though, "a in b < c" isn't an intuitive spelling of "a in b and b < c" the way that "a < b < c" is an intuitive spelling of an ordering relation.

    Hence filing this as a low priority issue - it's a weird quirk, and potentially worth changing to avoid a particular "Wat?" moment when folks first see it, but it isn't a bug magnet the way some other historical constructs have been.

    @ncoghlan
    Copy link
    Contributor Author

    Just a note on why I find "a in b < c" unintuitive, even for sets: my brain initially wanted to read it as equivalent to "a in b & c".

    @serhiy-storchaka
    Copy link
    Member

    PR 4501 makes the parser emitting a SyntaxWarning for chained in and not in comparison operators. In future it can be turned into a SyntaxError.

    But this should be first discussed on Python-Dev.

    @ncoghlan
    Copy link
    Contributor Author

    If we do decide to do this, I'm inclined to eventually make the change at the Grammar level rather than the AST level.

    Current:

    comparison: expr (comp_op expr)*
    comp_op: '<'|'>'|'=='|'>='|'<='|'<>'|'!='|'in'|'not' 'in'|'is'|'is' 'not'
    

    Future:

    comparison: expr (comp_op expr)* | (containment_check)
    comp_op: '<'|'>'|'=='|'>='|'<='|'<>'|'!='|'is'|'is' 'not'
    containment_check: ('in'|'not' 'in') expr
    

    However, we'd still need an intermediate step like your PR in order to emit SyntaxWarning while still retaining the current semantics.

    @serhiy-storchaka
    Copy link
    Member

    I'm reading the list of most rated in a week questions on StackOverflow (https://python-weekly.blogspot.com/), and it seems to me that the question about chained "in" and "not in" is raised every few months. This may be the one of the most confusing quirks.

    @serhiy-storchaka
    Copy link
    Member

    On other hand, beginners are confused even by chained "==". There are regular reports about this on the tracker and questions on Python-related resources.

    I have found the chained "in" is useful in the following case

    len(string) >= 2 and string[0] == string[-1] in ("'", '"')
    

    for testing if the string is quoted with a single or double quotes.

    @taleinat
    Copy link
    Contributor

    taleinat commented Sep 6, 2018

    Has this been discussed on python-dev? If so, what was the result?

    @serhiy-storchaka
    Copy link
    Member

    This had not been discussed.

    @gvanrossum
    Copy link
    Member

    I agree that it would be less confusing if in/not in did not allow chaining, the occasional (surely very rare) useful example notwithstanding.

    Then again if we're going to forbid (or even discourage) unusual combinations we might also want to frown at a < b > c -- surely in mathematical circles the chaining always goes in one direction only, e.g. a < b <= c or a >= b ==c > d.

    Finally as long as we're refining the terminology, maybe we could strive to distinguish "comparison" (== and !=) from "ordering" (<, <=, >, >=)?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 23, 2023
    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

    5 participants