You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
support narrowing enum values using == and != #10915
fromtypingimportNoReturn, Literalfromenumimportauto, Enumdeffoo(value: Literal["foo","bar"]) ->None:
ifvalue=="foo":
reveal_type(value) #note: Revealed type is "Literal['foo']"elifvalue!="bar":
reveal_type(value) #error: Statement is unreachable [unreachable]classFoo(Enum):
foo=auto()
bar=auto()
defbar(value: Foo) ->None:
ifvalue==Foo.foo:
reveal_type(value) #Revealed type is "__main__.Foo"elifvalue!=Foo.bar:
reveal_type(value) #no error, even though this is unreachable
I don't know why mypy handles is and is not but not == and != in this case. I'm guessing that the former is more common than the latter. I think both patterns are reasonable. I can't think of a reason why == and != would be an anti-pattern. FWIW, pyright supports type narrowing for both pairs of operators.
Reacted by KotlinIsland and Felipe S. S. Schneider
Yeah, should be something like that. You'll want to restrict to when the comparison is against an Enum that has a last_known_value. Let me play around and see...
coerce_only_in_literal_context = False if all(isinstance(p := get_proper_type(t), Instance) and p.type.is_enum for t in expr_types) else True
I wrote this, but it looks pretty cringe.
Probably fine to omit that check, since coerce_to_literal won't do anything if last_known_value is None (actually that's not true, it'll narrow singleton enums as well, so probably good to omit it, barring some chaining weirdness).
Yeah, the tests are pretty explicit about why they test what they test:
# So strictly speaking, we ought to do the same thing with 'is' comparisons
I'm not convinced that those tests are necessarily the behaviour we want, but seems like whoever wrote them collected some data / spent more time thinking about this specific issue than I have, so just saying I'd need to think about it too! :-)
Yeah, I'm inclined to agree (although literal types always make things murkier in practice). Although the test case specifically tests enums, it's possible that the real world false positives alluded to were more often narrowing to literal strs or something. I can make the PR and we'll see what other mypy maintainers have to say.
Last I checked (which was all the way back in version 0.790), the behavior was even stranger than presented here. ==did narrow narrow Enums in if-else blocks, but only if they followed an is. See #9786
Activity
erictraut commentedon Aug 3, 2021
If you use the
isandis notoperators rather than==and!=, mypy will perform narrowing for enum values.KotlinIsland commentedon Aug 3, 2021
Why? is it an anti pattern to use
==/!=on enum values? if so should a warning be raised when they are used?.erictraut commentedon Aug 3, 2021
I don't know why mypy handles
isandis notbut not==and!=in this case. I'm guessing that the former is more common than the latter. I think both patterns are reasonable. I can't think of a reason why==and!=would be an anti-pattern. FWIW, pyright supports type narrowing for both pairs of operators.[-]support narrowing enum values[/-][+]support narrowing enum values using `==` and `!=`[/+]hauntsaninja commentedon Aug 3, 2021
If you want to give implementing this a shot take a look at #7000, in particular #7000 (comment)
KotlinIsland commentedon Nov 11, 2021
@hauntsaninja Would this be as simple as changing:
mypy/mypy/checker.py
Line 4476 in 421fcc2
To be
Falseif the comparison is against anEnum?hauntsaninja commentedon Nov 11, 2021
Yeah, should be something like that. You'll want to restrict to when the comparison is against an Enum that has a last_known_value. Let me play around and see...
KotlinIsland commentedon Nov 11, 2021
coerce_only_in_literal_context = False if all(isinstance(p := get_proper_type(t), Instance) and p.type.is_enum for t in expr_types) else TrueI wrote this, but it looks pretty cringe.
hauntsaninja commentedon Nov 11, 2021
This is what I'm playing with:
We do have test cases that explicitly test against this behaviour though :-( most notably
testNarrowingEqualityFlipFlop.KotlinIsland commentedon Nov 11, 2021
Were they added when enums
iscomparisons were added.To make sure that they wouldn't work on
==due to complications.KotlinIsland commentedon Nov 11, 2021
But now that we are implementing it correctly we should change the tests.
KotlinIsland commentedon Nov 11, 2021
and (typ.last_known_value is not None)When would it be an enum and
None?hauntsaninja commentedon Nov 11, 2021
It'd be the case in
Probably fine to omit that check, since
coerce_to_literalwon't do anything iflast_known_valueis None (actually that's not true, it'll narrow singleton enums as well, so probably good to omit it, barring some chaining weirdness).Yeah, the tests are pretty explicit about why they test what they test:
mypy/test-data/unit/check-narrowing.test
Line 720 in d41e34a
I'm not convinced that those tests are necessarily the behaviour we want, but seems like whoever wrote them collected some data / spent more time thinking about this specific issue than I have, so just saying I'd need to think about it too! :-)
KotlinIsland commentedon Nov 11, 2021
That looks like a scenario like:
Sus!
KotlinIsland commentedon Nov 11, 2021
What's the policy on narrowing mutable variables? seems to me like that example with narrowing to
Trueis the same as that test?hauntsaninja commentedon Nov 11, 2021
Yeah, I'm inclined to agree (although literal types always make things murkier in practice). Although the test case specifically tests enums, it's possible that the real world false positives alluded to were more often narrowing to literal strs or something. I can make the PR and we'll see what other mypy maintainers have to say.
Allow narrowing enum values using ==
parsons-kyle-89 commentedon Apr 1, 2022
Last I checked (which was all the way back in version 0.790), the behavior was even stranger than presented here.
==did narrow narrow Enums in if-else blocks, but only if they followed anis. See #9786KotlinIsland commentedon Feb 9, 2023
This makes
matchon enums really annoying.hauntsaninja commentedon Feb 9, 2023
I don't really use
match. Could you paste a semi-realistic example where merging #11521 would help?KotlinIsland commentedon Feb 9, 2023
Good point, I don't know why I used a
match...But I could imagine if anyone ever does use a
matchthat this will be annoying.Allow narrowing enum values using == (#11521)