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

Docs of match statement incorrectly claim that int(0|1) doesn't match False #96359

Closed
L3viathan opened this issue Aug 28, 2022 · 9 comments
Closed
Labels
docs Documentation in the Doc dir

Comments

@L3viathan
Copy link
Contributor

Documentation

In https://docs.python.org/3/reference/compound_stmts.html#class-patterns, the following sentence:

These classes accept a single positional argument, and the pattern there is matched against the whole object rather than an attribute. For example int(0|1) matches the value 0, but not the values 0.0 or False.

should be changed to

These classes accept a single positional argument, and the pattern there is matched against the whole object rather than an attribute. For example int(0|1) matches the value 0, but not the value 0.0.

Since bool is a subclass of int, case int(0|1) actually does match False:

>>> match False:
...     case int(0|1):
...         print("matches")
...     case _:
...         print("doesn't match")
matches
@L3viathan L3viathan added the docs Documentation in the Doc dir label Aug 28, 2022
@m-rutter
Copy link

m-rutter commented Aug 28, 2022

I originally raised this on the python discord server here:

https://discord.com/channels/267624335836053506/709904092280914030/1013412267376775168

Either this is an error in the documentation or this is an error in cpython's implementation of the feature. There is also a general inconsistency in how these two cases are handled:

>>> match False:
...     case int(0):
...         print("this is bad")
...     case _:
...         print("this is good")
... 
this is bad
>>> match 0:
...     case bool(False):
...         print("this is bad")
...     case _:
...         print("this is good")
... 
this is good

The semantics of the feature changes depending upon whether the False or 0 is the value or the pattern.

So apparently 0 is not False, but False is 0. The latter case behaves along the lines of the documentation as it stands today.

@L3viathan
Copy link
Contributor Author

@m-rutter No, that behavior is as expected, because isinstance(False, int) but not isinstance(0, bool).

@m-rutter
Copy link

@L3viathan why the special case for float then and not for bool?

without int()

>>> match 0.0:
...     case 0:
...         print("this is bad")
...     case _:
...         print("this is good")
... 
this is bad

with int()

>>> match 0.0:
...     case int(0):
...         print("this is bad")
...     case _:
...         print("this is good")
... 
this is good

Why is it more likely to be a documentation error and not an implementation error?

@L3viathan
Copy link
Contributor Author

In the first case, 0 == 0.0 is checked, which is True. In the second case, isinstance(0.0, int) is checked first, which is False.

TL;DR: bool is a subclass of int, but float isn't.

@m-rutter
Copy link

m-rutter commented Aug 28, 2022

Perhaps builtins should be and were intended to be a special case here. In fact, the docs themselves say they are a special case.

Not to treat them as a special case introduces unnecessary footguns into the language. Not that you can ever fully correct the mistake of having bool extend int.

@kaya3
Copy link
Contributor

kaya3 commented Aug 28, 2022

So apparently 0 is not False, but False is 0

What's apparent is that 0 does not match bool(False), because 0 is not an instance of bool, but False does match int(0), because False is an instance of int and is equal to 0.

Perhaps builtins should be and were intended to be a special case here. In fact, the docs themselves say they are a special case.

The docs say they are a special case, but not in the respect you are describing. According to PEP 634:

A class pattern fails if the subject is not an instance of name_or_attr. This is tested using isinstance().
...
For a number of built-in types (specified below), a single positional subpattern is accepted which will match the entire subject. (Keyword patterns work as for other types here.)

That is, the special case is for how positional subpatterns are handled, not for how the instance test is done. There's nothing in the spec to suggest any other behaviour was intended for the instance test for built-in types.

Not to treat them as a special case introduces unnecessary footguns into the language

If anything, the footgun is that bool is a subtype of int at all; you'll get the same potential bug if you write if isinstance(x, int): ... as if you write match x: case int(): ....

But it's a bit late for this argument now, since PEP 634 has been accepted and what you're proposing would be a backwards-incompatible change. For what it's worth, I disagree that the additional special case you're proposing would be a good idea.

@rhettinger
Copy link
Contributor

@L3viathan Your suggested wording looks correct to me. Please submit a PR and assign it to Brandt for review.

@m-rutter
Copy link

m-rutter commented Aug 28, 2022

@kaya3 thanks for the explanation. I understand its impossible to change things like bool extending int and I can understand the reasoning behind wanting instanceof checks being consistent. I also will not argue that int | bool is good type design no matter the language.

However, it is a shame that we have the tools to distinguish between float and int, but our only real option for int and bool is to remember to order your match arms correctly and happen to know that 20+ years ago someone decided to make bool extend int. Its undoubtedly a footgun that has been propagated to a brand new feature.

@L3viathan
Copy link
Contributor Author

@rhettinger It doesn't look like I'm able to assign a reviewer, but I made a PR.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 28, 2022
…honGH-96361)

(cherry picked from commit 3d3a86e)

Co-authored-by: Jonathan Oberländer <github@l3vi.de>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 28, 2022
…honGH-96361)

(cherry picked from commit 3d3a86e)

Co-authored-by: Jonathan Oberländer <github@l3vi.de>
miss-islington added a commit that referenced this issue Aug 29, 2022
(cherry picked from commit 3d3a86e)

Co-authored-by: Jonathan Oberländer <github@l3vi.de>
miss-islington added a commit that referenced this issue Aug 29, 2022
(cherry picked from commit 3d3a86e)

Co-authored-by: Jonathan Oberländer <github@l3vi.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

No branches or pull requests

4 participants