[red-knot] Instance attributes: type inference clarifications#15512
[red-knot] Instance attributes: type inference clarifications#15512
Conversation
sharkdp
requested review from
AlexWaygood,
MichaReiser and
carljm
as code owners
| # TODO: After this assignment to the attribute within this scope, we may eventually want to | ||
| # narrow the `bool` type (see above) for this instance variable to `Literal[False]` here. | ||
| # This is unsound in general (we don't know what else happened to `c_instance` between the | ||
| # assignment and the use here), but pyright supports this, presumably to provide features |
There was a problem hiding this comment.
| # assignment and the use here), but pyright supports this, presumably to provide features | |
| # assignment and the use here), but pyright and mypy support this, presumably to provide features |
I can't remember where pyre ended up on this; @carljm can refresh my memory here :-) I believe they started off being very strict about soundness here, but it resulted in a lot of user confusion and not a significant increase in type safety, so then loosened their rules somewhat. But I don't know exactly where they ended up at the end of the day. When we were chatting to the pyre developers at PyCon, my recollection is that they said they regretted ever going down that road, because users had such a strong expectation of type narrowing in code like this.
There was a problem hiding this comment.
I think (and Pyre playground testing just now confirmed) Pyre does the narrowing but then reverts it after an expression that seems likely to execute arbitrary code elsewhere (e.g. a call expression). This is also very much a heuristic, and one that results in behavior that is surprising to users ("why did adding this call that doesn't involve variable c_instance suddenly change the inferred type of c_instance.pure_instance_variable4??"), so I'm not particularly fond of that approach either.
I agree that we will probably just want to follow mypy and pyright here.
There was a problem hiding this comment.
@AlexWaygood In which sense does mypy support this? I don't see a narrowed type here: https://mypy-play.net/?mypy=latest&python=3.13&gist=4cc6d2748f364c614965b72becf538e7
Do I need to use an example that does not involve literals?
There was a problem hiding this comment.
Do I need to use an example that does not involve literals?
Yes. Yes, I do. For unions, it works.
There was a problem hiding this comment.
Yes, mypy is very sparing in its willingness to infer literals. If you try an optional type like int | None for an attribute, then assign 3 to it, mypy will narrow to int.
There was a problem hiding this comment.
Yeah, mypy never narrows instance types to literal types, but you can see that it narrows the types of attributes in exactly the same way it allows you narrow the types of local variables https://mypy-play.net/?mypy=latest&python=3.13&gist=c218144f43fd07fd2a2c93c142471b8f
|
3 participants
Summary
Some clarifications in the instance-attributes tests, mostly regarding type inference behavior following this discussion