Skip to content

Conversation

@dmoisset
Copy link
Contributor

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good, but it would be good to get an implementation in at least one type checker. It should be trivial, but as a general principle I'd like the spec to only specify behavior that has actually been implemented.

While reviewing the change I was thinking about whether we should specify what + means. My conclusion is that it's fine to leave it implicit that it means the same thing as it means at runtime.

JelleZijlstra added a commit to JelleZijlstra/mypy that referenced this pull request Jan 2, 2024
Implements python/typing#1550

Fixes python#16727 (a trivial bug I found while implementing this feature)
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Jan 2, 2024

I wrote an implementation for mypy, which was trivial as expected: python/mypy#16729.

hauntsaninja pushed a commit to python/mypy that referenced this pull request Jan 2, 2024
Implements python/typing#1550

Fixes #16727 (a trivial bug I found while implementing this feature)
@erictraut
Copy link
Collaborator

Oh, one thing I just thought about. Should a type checker explicitly reject redundant unary + or - operators in a literal type? Example: Literal[+++++-----1]. Do we care one way or another?

@JelleZijlstra
Copy link
Member

Good point. I'd say any number of +/- should be accepted as it's unambiguous, easy to implement, and matches the runtime behavior.

Copy link
Collaborator

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it's fine to leave the edge case of whether multiple +/- are allowed unspecified.

@JelleZijlstra JelleZijlstra merged commit 8fd71d2 into python:main Jan 12, 2024
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.

5 participants