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

Better narrows bool in if statements #11187

Closed
wants to merge 1 commit into from
Closed

Better narrows bool in if statements #11187

wants to merge 1 commit into from

Conversation

sobolevn
Copy link
Member

For some reason previously bool was narrowed like this:

y: bool
if y is True:
    reveal_type(y)  # N: Revealed type is "Literal[True]"
else:
    reveal_type(y)  # N: Revealed type is "bool"

Which is not ideal, when we can do better:

y: bool
if y is True:
    reveal_type(y)  # N: Revealed type is "Literal[True]"
else:
    reveal_type(y)  # N: Revealed type is "Literal[False]"

@bluetech
Copy link
Contributor

I wonder how this relates to #6113 (which also points to an open PR #10389)?

@sobolevn
Copy link
Member Author

@bluetech yes, this looks like a very related issue. My approach is quite minimalistic compared to #10389 (which looks like a more full-featured solution).

Let's leave it @JukkaL to decide what to do.

@hauntsaninja
Copy link
Collaborator

My hesitation with this PR is that if bool_var is True is a somewhat unidiomatic check (I'd only recommend it for variables of type Optional[bool]), and I suspect solving the more general if bool_var case (as #10389 attempts to), will solve this case for free.

@sobolevn sobolevn closed this Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants