Potentially undefined attributes
Hi! I want to start a conversation about undefined attributes in Python. Recently, I was working on a PR in typeshed that led me to this problem: some attributes in Python classes are not always defined. Simplest example: ```python class Some: x: int def set_x(self): self.x = 1 Some().x # boom! but type checker is happy ``` Right now we don't have any ways of expressing this in our type system. My view on Python's type system is quite practical: we should be able to express things that might ruin someone's program. So, here's my conversation starter piece (please, don't consider it as a real PEP draft for now): https://docs.google.com/document/d/1KC6y5QVQetM2s6pS4R9eu-icLP3r9mNKNRjghN-O... From my knowledge of mypy internals, implementing something like this should not be very hard. However, I cannot speak for other type checkers. Feedback is appreciated: 1. Other cases where this pattern is common? Have you seen any problems with such attributes? 2. Possible problems it can cause? Like supporting a bad practice (which "potentially undefined attributes" might be to some judgement) or bloating type system 3. How do other dynamic type systems / languages handle this? (Notes on TS are inside the doc) 4. Other suggestions / ideas? Have a great weekend! Best, Nikita Sobolev
If a Python developer is interested in writing robust and maintainable code, I would recommend that they not leave attributes uninitialized within their class implementations. This seems to me like an anti-pattern, not something that we should support and promote in the type system. The type system should guide developers toward patterns that lead to robust and maintainable code, and this proposal does the opposite in my opinion. I agree with Nikita that this is a potential source of bugs that can go uncaught by current type checkers, so it's a worthwhile problem to solve. Let's consider what we can do to detect this class of bug while guiding developers to a better solution. Pyright implements an optional check called reportUninitializedInstanceVariable (https://github.com/microsoft/pyright/issues/1924). (A similar feature was requested in the mypy issue tracker here: https://github.com/python/mypy/issues/10552, and I think PyCharm also implements a similar check.) When enabled, this check enforces that all instance variables, including those that are declared in the class body, must be initialized -- either by direct assignment in the class body or in the `__init__` method. This check detects errors like the one in the code sample above. Perhaps it would be better for us to formalize the rules for a check like this and enable it across all Python type checkers. This would help guide developers to a pattern that leads to more robust code. -- Eric Traut Contributor to Pyright & Python Microsoft
Pyre also requires attributes to be initialized in either the class body or in the constructor (playground link <https://pyre-check.org/play?input=%23%20Pyre%20is%20being%20run%20in%20stric...>). This check is on by default, since Pyre aims to be sound. That prevents runtime errors like the above. On Sun, Sep 11, 2022 at 9:51 AM Eric Traut <eric@traut.com> wrote:
-- S Pradeep Kumar
If a Python developer is interested in writing robust and maintainable code, I would recommend that they not leave attributes uninitialized within their class implementations. This seems to me like an anti-pattern, not something that we should support and promote in the type system. The type system should guide developers toward patterns that lead to robust and maintainable code, and this proposal does the opposite in my opinion. I agree with Nikita that this is a potential source of bugs that can go uncaught by current type checkers, so it's a worthwhile problem to solve. Let's consider what we can do to detect this class of bug while guiding developers to a better solution. Pyright implements an optional check called reportUninitializedInstanceVariable (https://github.com/microsoft/pyright/issues/1924). (A similar feature was requested in the mypy issue tracker here: https://github.com/python/mypy/issues/10552, and I think PyCharm also implements a similar check.) When enabled, this check enforces that all instance variables, including those that are declared in the class body, must be initialized -- either by direct assignment in the class body or in the `__init__` method. This check detects errors like the one in the code sample above. Perhaps it would be better for us to formalize the rules for a check like this and enable it across all Python type checkers. This would help guide developers to a pattern that leads to more robust code. -- Eric Traut Contributor to Pyright & Python Microsoft
Pyre also requires attributes to be initialized in either the class body or in the constructor (playground link <https://pyre-check.org/play?input=%23%20Pyre%20is%20being%20run%20in%20stric...>). This check is on by default, since Pyre aims to be sound. That prevents runtime errors like the above. On Sun, Sep 11, 2022 at 9:51 AM Eric Traut <eric@traut.com> wrote:
-- S Pradeep Kumar
participants (3)
-
Eric Traut -
S Pradeep Kumar -
Никита Соболев