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

Use Literal for MatchSingleton #5590

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Use Literal for MatchSingleton #5590

merged 1 commit into from
Jun 8, 2021

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Jun 8, 2021

MatchSingleton can only ever be True, False, or None.
The current Optional[bool] is inaccurate for that as it also allows implicit booleans: 1 != 2.

https://docs.python.org/3.10/library/ast.html#ast.MatchSingleton

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2021

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit bad2fea into python:master Jun 8, 2021
@cdce8p cdce8p deleted the patterns branch June 8, 2021 11:28
@Akuli
Copy link
Collaborator

Akuli commented Jun 8, 2021

So why should match_singleton.value = (foo != bar) be an error in code that manipulates AST?

@srittau
Copy link
Collaborator

srittau commented Jun 8, 2021

It should basically not matter whether something is annotated bool or Literal[True, False], both should be treated as equivalent. (Although I think at least mypy is buggy in that regard.) So this change was a no-op (minus type checker bugs).

@Akuli
Copy link
Collaborator

Akuli commented Jun 8, 2021

I am assuming the current "buggy" mypy, where Literal[True, False] is a subtype of bool, and expressions generally result in type bool.

@srittau
Copy link
Collaborator

srittau commented Jun 8, 2021

Here's the corresponding mypy issue: python/mypy#6113.

@srittau
Copy link
Collaborator

srittau commented Jun 8, 2021

@Akuli Just to clarify: I'm not married to this change. If it causes problem we can roll this back. I was just assuming that the current implementation was causing problems for @cdce8p, so this would be better.

@cdce8p
Copy link
Contributor Author

cdce8p commented Jun 9, 2021

I had the issue the other way round. Got an error when I tried to assign Optional[bool] to Literal[True, False, None]. It seems this does indeed needs a fix in mypy. Another reason for Literal was that it's a bit more expressive especially for hints in IDEs.

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