Repository navigation
Add guide to type narrowing - #1798
Conversation
Some of the text derives from the "How to teach this" section of PEP 742. The asyncio example is based on a sample written by Liz King in https://discuss.python.org/t/problems-with-typeis/55410.
erictraut
left a comment
erictraut
left a comment
There was a problem hiding this comment.
Thanks for writing this! Looks good overall. I added a couple of minor suggestions. Feel free to use or ignore as you see fit.
| In addition to narrowing local variables, type checkers usually also support | ||
| narrowing instance attributes and sequence members, such as | ||
| ``if x.some_attribute is not None`` or ``if x[0] is not None``. |
There was a problem hiding this comment.
I would probably not mention this. I believe there are many situations where other type checkers will happily understand an instance attribute or sequence/dictionary lookup as being narrowed, but Pyre will not, because it cannot safely determine that the stored value has not been reset to a new value by some other function in between two accesses of that value. Moreover, I believe even the more limited narrowing that Pyre allows is not strictly type-safe if you have multithreaded code.
In the name of practicality, I personally prefer the approach mypy and pyright have taken here. But it's an area in which type checkers differ, and mypy's/pyright's behaviour isn't strictly safe. So I think it's perhaps better to leave it unsaid here.
There was a problem hiding this comment.
I'm not sure why this shouldn't be mentioned. It's something users commonly need and ask for. While it is somewhat more obviously unsafe than some other narrowing patterns, there's few kinds of type narrowing that are 100% safe.
There was a problem hiding this comment.
Pyre seems to do type narrowing here too:
Correct, but if you insert a single intermediate function call, it decides that it can no longer assume that the attribute wasn't modified in between the two accesses by the intermediate function call, and no longer does the type narrowing: https://pyre-check.org/play?input=%23%20pyre-strict%0A%0Afrom%20typing%20import%20*%0A%0Afrom%20dataclasses%20import%20dataclass%0A%0A%40dataclass%0Aclass%20X%3A%0A%20%20%20%20a%3A%20int%20%7C%20None%0A%0Adef%20do_something_else()%20-%3E%20None%3A%0A%20%20%20%20pass%0A%0Adef%20f(x%3A%20X)%20-%3E%20None%3A%0A%20%20%20%20if%20x.a%20is%20not%20None%3A%0A%20%20%20%20%20%20%20%20do_something_else()%0A%20%20%20%20%20%20%20%20reveal_type(x.a)%0A%20%20%20%20%20%20%20%20print(x.a%20%2B%201)%0A%20%20%20%20print(x.a%20%2B%201)
I'm not sure why this shouldn't be mentioned.
Well, I just worry about documenting behaviour here that isn't standardised between all major type checkers. It might be pretty confusing for users of type checkers with differing behaviour.
There was a problem hiding this comment.
The text is already pretty clear that the exact type narrowing behaviors differ among type checkers, but I'll add another line to clarify that.
I think it's important to mention this behavior because it's a common pattern in practice, so people are likely to run into this.
There was a problem hiding this comment.
Thanks, I think the extra line definitely helps
Thanks Alex! Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
This looks good to me! Thanks for writing it up.
| are useful if you want to reuse a more complicated check in multiple places, or | ||
| you use a check that the type checker doesn't understand. In these cases, you | ||
| can define a ``TypeIs`` or ``TypeGuard`` function to perform the check and allow type checkers | ||
| to use it to narrow the type of a variable. Among the two, ``TypeIs`` usually |
There was a problem hiding this comment.
nit: for only two items, I would usually prefer "Between" over "Among"
| to use it to narrow the type of a variable. Among the two, ``TypeIs`` usually | |
| to use it to narrow the type of a variable. Between the two, ``TypeIs`` usually |
|
Oh, it might also be useful to mention |
| # Incorrect: does not return True for all ints | ||
| def is_positive_int(x: object) -> TypeIs[int]: | ||
| return isinstance(x, int) and x > 0 | ||
|
|
||
| # Incorrect: returns True for some non-ints | ||
| def is_real_number(x: object) -> TypeIs[int]: | ||
| return isinstance(x, (int, float)) | ||
|
|
There was a problem hiding this comment.
These two examples would also be good to have in the TypeIs documentation at docs.python.org. Also, the TypeIs and TypeGuard docs could link to this document when it gets merged.
srittau
left a comment
srittau
left a comment
There was a problem hiding this comment.
LGTM, with or without the changes suggested by others.
|
@carljm good call, added a section on |
|
Please move to the guides directory when resolving conflicts! |
6 participants
Some of the text derives from the "How to teach this" section of PEP 742.
The asyncio example is based on a sample written by Liz King in
https://discuss.python.org/t/problems-with-typeis/55410.