-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PEP 586: Small wording changes from review; expanded example in Abstract #993
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
Conversation
…stract The additions to the Abstract example may be controversial, I'm happy to change these -- or anything else -- if you're uncomfortable with my suggestions.
pep-0586.rst
Outdated
accepts_only_four(19) # Rejected | ||
|
||
four = 4 | ||
accepts_only_four(four) # Rejected (not a literal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential wording is "Can be rejected" or "May be rejected" to emphasize that type checkers are allowed to reject this (but are not obliged to reject).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But they would be required to reject it if there was intervening code that could possibly assign a different value to four
. I don't think we need to split such hairs in this initial example.
Here are some (slightly) more substantial comments on the PEP:
|
pep-0586.rst
Outdated
accepts_only_four(four) # Rejected (not a literal) | ||
|
||
four: Final = 4 # See PEP 591 | ||
accepts_only_four(four) # OK if PEP 591 is accepted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although mypy currently doesn't accept this yet.
I also prefer
TBH, I didn't like this example since beginning. It is not incorrect, but may be misleading.
I could imagine a similar (maybe even more realistic) function that accepts any object (like @overload
def is_int_like(x: Union[int, List[int]]) -> Literal[True]: ...
@overload
def is_int_like(x: object) -> bool: ... There is a subtle point that using such functions for restricting subtype away is not type safe in presence of multiple inheritance (IIUC there is no such problem in TypeScript). But on the other hands many things are already technically unsafe because of multiple inheritance and there were not many complains about this.
I don't think this should be specified in any strict way. Type inference can never affect type safety, it can just annoy a user more or less if a type checker (in-)correctly guesses the type arguments. In Python, type inference is probably more important than in other languages because we don't have syntax for type application for functions (i.e. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the first example goes against the "let's not mandate a particular inference strategy" approach we decided on. At least based on talking to Mark, it seemed like the Pyre team wanted to retain the ability to infer that four
is of type Literal[4]
instead of int
in cases like those.
So, I think we should either remove that example or modify the comment to make it clear whether that example is accepted or rejected is implementation-specific.
The second example is sort of similar, but I think the inference strategy we're proposing there is pretty straightforward and non-controversial, so I'm ok with keeping that example.
Everything else looks good to me -- I'm happy merging once we either remove or change the first new example.
Hm, then I think it's better to remove both new examples (esp. since mypy doesn't support either one). |
Yeah, I think it's probably fine to not care -- like you said, it's sort of hard to detect. (And also, probably not too many people are going to write code like this, and it's fine even if they do -- the syntax doesn't really imply anything "misleading".)
Good point; will do.
I agree, but I also think we probably want to keep the Literal and Final PEPs mostly decoupled from each other. My current plan is to submit a PR adding a short section about how Literal and Final interact, then add a short note here linking people to that section.
Hmm, it seems like I left out a constructor in that example. I think something roughly like this would work:
But yeah, the Matrix example is sort of an attractive nuisance at this point. I'm considering replacing it: the only think stopping me is that I'm having some difficulty thinking of different example using Literals + generics that's at least somewhat realistic.
I believe the resolution we came to here is that type checkers should preserve backwards compatibility on a best-effort basis, instead of a mandatory basis. I just need to get around to making a PR for it...
We could do that, yeah. I guess just my main worry is that sort of check might end up being inconvenient to some users. For example, mypy currently doesn't do any exhaustiveness checks when we do an if-elif-...-else against Unions in general. I'm guessing there was some reason for that? Or maybe what we could do is make mypy start reporting errors if any branches containing There's also the "assert never" pattern discussed here: python/mypy#5818
None of this in master yet. I've been making some some experimental tweaks to the binder over the past few weeks that sort of make this stuff work, but they all feel pretty janky atm.
Hmm, good point. This section should probably be renamed. Maybe something like "Cool stuff you can do with Literals" but more professional?
Good point, I'll change that.
Typo -- 'x' should say "status".
I can change this to Ivan's new example above. |
I think mypy can handle the second example? E.g. it works as expected on this program: from typing_extensions import Literal, Final
def accepts_only_four(x: Literal[4]) -> None: pass
four: Final = 4
seven: Final = 7
accepts_only_four(four) # OK
accepts_only_four(seven) # Error |
Ah, so it does. Well, I already merged my PR without either example. Anyway, I just added it because I thought it would be a nice contrast to the first example I added, but since that's out, it's probably not essential to have it in the Abstract.
Does mypy do any value-based exhaustiveness checks? I guess not. And I didn't realize how subtle all this was. I know the binder does do exhaustiveness checks on types (using
Yeah, I honestly don't believe why exhaustiveness should be so closely tied to Literal types. Is it because the binder mostly looks at types, not values? I guess there's also a concern about subclassing? (The section in exhaustiveness in PEP 484 specifically mentions that enums can't be subclassed.) I guess I have gotten myself thoroughly confused at this point. :-( Maybe you can attempt to rewrite this section in a separate PR so we can discuss it separately from the other clarifications and improvements you're planning. For everything I'm not commenting on I am trusting the plans you outlined in your previous comments. |
This commit adjusts two sections of this PEP that are related to enums. First, it removes the sections regarding the interaction between enums, imports, and Any. I wasn't aware that the import behavior described in that section was mypy-only and isn't codified in PEP 484. So, I decided to just remove that section entirely -- it didn't feel there was much I could salvage there. Instead, I opted to adjust the "invalid parameters" section to explain in a little more detail why `Literal[Any]` is not allowed. Second, I split up the section about type narrowing into two. The first new section is a reminder that PEP 484 requires type checkers to support certain kinds of exhaustibility checks when working with enums. To make this more clear, I adjusted the example to be more closer to what is used in the spec and removed any mention of reachability -- it felt like a distraction. The second section focuses back on some neat tricks using Literals that type checkers may optionally implement. I also tweaked some of the examples here as suggested in python#993.
This commit adjusts two sections of this PEP that are related to enums. First, it removes the sections regarding the interaction between enums, imports, and Any. I wasn't aware that the import behavior described in that section was mypy-only and isn't codified in PEP 484. So, I decided to just remove that section entirely -- it didn't feel there was much I could salvage there. Instead, I opted to adjust the "invalid parameters" section to explain in a little more detail why `Literal[Any]` is not allowed. Second, I split up the section about type narrowing into two. The first new section is a reminder that PEP 484 requires type checkers to support certain kinds of exhaustibility checks when working with enums. To make this more clear, I adjusted the example to be more closer to what is used in the spec and removed any mention of reachability -- it felt like a distraction. The second section focuses back on some neat tricks using Literals that type checkers may optionally implement. I also tweaked some of the examples here as suggested in #993.
This commit adjusts two sections of this PEP that are related to enums. First, it removes the sections regarding the interaction between enums, imports, and Any. I wasn't aware that the import behavior described in that section was mypy-only and isn't codified in PEP 484. So, I decided to just remove that section entirely -- it didn't feel there was much I could salvage there. Instead, I opted to adjust the "invalid parameters" section to explain in a little more detail why `Literal[Any]` is not allowed. Second, I split up the section about type narrowing into two. The first new section is a reminder that PEP 484 requires type checkers to support certain kinds of exhaustibility checks when working with enums. To make this more clear, I adjusted the example to be more closer to what is used in the spec and removed any mention of reachability -- it felt like a distraction. The second section focuses back on some neat tricks using Literals that type checkers may optionally implement. I also tweaked some of the examples here as suggested in python#993.
@Michael0x2a please review
The additions to the Abstract example may be controversial, I'm happy
to change these -- or anything else -- if you're uncomfortable with my
suggestions.