Repository navigation
Should we always prohibit "abstract" attributes? #4426
Description
Activity
- Honestly I think it should be allowed in both cases.…On Jan 3, 2018 10:41, "Ivan Levkivskyi" ***@***.***> wrote: Currently, instantiating explicit subclasses of protocols with "abstract" attributes is prohibited: class P(Protocol): x: int # Note, no r.h.s. class C(P): passclass D(P): def __init__(self) -> None: self.x = 42 C() # E: Cannot instantiate abstract class 'C' with abstract attribute 'x' D() # OK This is feature appeared with little discussion, in particular should this be specific only to protocols or also to "normal" ABCs? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4426>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACwrMjC08CvwNnrUkQflbGJ_B-RVdgVoks5tG8ncgaJpZM4RSGqh> .
Honestly I think it should be allowed in both cases.
Note that allowing this will lead to runtime unsafety (although there is similar unsafety with nominal classes). IIRC additional argument that Jukka mentioned is that sometimes it can be a desirable feature (like
@abstractmethod) so that subclasses will be forced to "implement" the attribute (currently this also can be done by@abstractpropertybut it looks more verbose).IIUC there are many cases where mypy can't verify whether a variable or attribute actually has a value, so it only checks that you use the type correctly. That's totally acceptable to me. (The only case where I think mypy should be held to higher standards is for local variables, where flow analysis is usually pretty straightforward.)
IIUC there are many cases where mypy can't verify whether a variable or attribute actually has a value, so it only checks that you use the type correctly. That's totally acceptable to me.
Yes, there are many cases where we can't detect detect this. But there are some ideas how to "tighten" this, see #4019
@JukkaL What do you think about the check for abstractness of variables in protocol subclasses?
A primary use case for explicitly subclassing a protocol is to get mypy to verify that everything in the protocol is actually implemented by the class. If mypy doesn't require attributes to be explicitly defined in the subclass, this use case is significantly less useful.
Also, if an attribute is declared in a class body but it gets never assigned a value in the body, mypy should perhaps complain about this:
class A: success: bool # Maybe complain that 'success' is never initialized in "A" def __init__(self) -> None: self.succes = True # Typo!
I'd actually go even further into this direction -- I think that mypy should complain also about missing implementations of methods that don't have non-trivial bodies. For example, mypy allows this but I'd rather have it rejected:
from typing_extensions import Protocol class P(Protocol): def f(self) -> int: ... class C(P): pass C() # No error, even though there is a missing implementation for 'f'
There are other potential ways to approach this, such as requiring
@abstractmethodforfinP.(Similarly, mypy should probably complain about empty method bodies more generally in strict optional mode, but that's a separate issue.)
Reacted by Enrico Guiraud, Daniel Davis, Kaleb Barrett, Pyprohly and Adrian Garcia BadaraccoA primary use case for explicitly subclassing a protocol is to get mypy to verify that everything in the protocol is actually implemented by the class. If mypy doesn't require attributes to be explicitly defined in the subclass, this use case is significantly less useful.
+1. This use case is already mentioned in PEP 544.
Similarly, mypy should probably complain about empty method bodies more generally in strict optional mode, but that's a separate issue.
Yes, I think we have a separate issue for this.
Yes, I think we have a separate issue for this.
Could you point me to this issue?
In the docs it currently says:
Explicitly including a protocol as a base class is also a way of documenting that your class implements a particular protocol, and it forces mypy to verify that your class implementation is actually compatible with the protocol.
However, I can't seem to get this functionality to work. For example, MyPy seems to accept subclasses of protocols that don't implement that protocol:
import typing import typing_extensions class TestProtocol(typing_extensions.Protocol): def some_method(self) -> int: ... class TestClass(TestProtocol): pass
Is there any way I can tell MyPy to assert that a class follows a protocol?
For example, MyPy seems to accept subclasses of protocols that don't implement that protocol:
I don't see this in your example. The
TestClassdoes havesome_method()inherited fromTestProtocol, i.e. the implementation in the protocol acts as a default implementation. If you want an abstract method, then simply declare it as such.Unfortunately, mypy allows literal
...andpassas valid bodies for any function signature, but this is a separate issue #2350Ah I didn't realize that
...was interpreted as a method implementation. Thanks for the pointer to the other issue.So you mean decorating the method in the protocol with
abc.abstractmethod? I tried it with and without the metaclass and this still seems to type fine:import abc import typing import typing_extensions class TestProtocol(typing_extensions.Protocol, metaclass=abc.ABCMeta): @abc.abstractmethod def some_method(self) -> int: ... class TestClass(TestProtocol): pass
Instantiating
TestClasswill fail (both statically and at runtime). Also you don't need the metaclass there,Protocolis already an instance ofABCMeta.Finally, for questions and help please use the Gitter chat, this question is off-topic for this issue.
Reacted by Saul ShanabrookCould mypy also detect errors in the implementation of protocols before even trying to instantiate the class? To protect myself from not implementing my own interfaces correctly I currently use the following decorator to check a class against a
ProtocolT = TypeVar('T', bound=Type) def check_class_implements(t: T) -> Callable[[T], T]: def check(cls: T) -> T: return cls return check
Given a
Protocol, for instance:class GreeterProtocol(Protocol): @abstractmethod def greet(self, name: str) -> str: ...
it is used like this:
@check_class_implements(GreeterProtocol) class HelloWorld: def greet(self, name: str) -> str: return f'{name}; Hello world' @check_class_implements(GreeterProtocol) class Foo: def greet(self) -> str: return 'FOOO!'
When a class is not compatible with the given Protocol, as is the case for
Foo, mypy issues an error oncheck_class_implements:Argument 1 has incompatible type "Type[Foo]"; expected "Type[GreeterProtocol]"mypy(error)Ideally mypy would check if a class is compatible with the given Protocol when one simply inherits from it so that the above would be just the following with the same error as before:
class Foo(GreeterProtocol): def greet(self) -> str: # mypy complains about the signature return 'FOOO!'
More detailed error messages on what exactly went wrong (such as
greetnot having the right signature) would be nice to have but are imho not essential. Maybe the error message would be clearer already if it spelled something likeInstances of class "Foo" do not implement "GreeterProtocol".
Note that in this case mypy already complains about the wrong signature ofgreet; this has nothing to do withProtocolhowever and ifgreetwere missing no error would be shown:class Bar(GreeterProtocol): pass # typechecks, even though implementation is missing
Similar behavior would be useful with abstract classes that are final (so that there is no way to ever implement any of the abstract methods or propertys), for instance:
class Base(ABC): @abstractproperty def foo(self) -> str: ... @final class Derived(Base): pass
Are there valid use cases for abstract classes that are final?
A primary use case for explicitly subclassing a protocol is to get mypy to verify that everything in the protocol is actually implemented by the class. If mypy doesn't require attributes to be explicitly defined in the subclass, this use case is significantly less useful.
100% agree. Specifying attributes in a protocol definition is a natural way to express that classes implementing the protocol should have the attribute. The alternative is using abstract (read-write) properties, which is definitely not as simple/elegant.
Reacted by Sascha Desch, Daniel Davis, Pyprohly and Sid KapurI'd also suggest to make this as strict as possible, allowing more relaxed checking only with explicit declarations, for example with a decorator, putting ABC/Protocol in the class hierarchy again or by redefining the abstract method/attributes:
class Base(Protocol): foo: int @abc.abstractmethod def bar(self) -> int: ... # extra decorator to show that this class is still abstract and not a implementation of Base @mixin class MixinA(Base): def bar(self): return 42 # declare class to be abstract by directly putting ABC or Protocol into the hierarchy: class MixinB(Base, Protocol): def bar(self): return 23 # this declaration should imho fail, as it does not set the attr 'foo' class Bar(MixinA): pass # OK: if you really set the attribute foo by an external mean, you can re-declare it in the class to state this explicitly class Foo(MixinB): foo: int
Regarding this example by @JukkaL :
class A: success: bool # Maybe complain that 'success' is never initialized in "A" def __init__(self) -> None: self.succes = True # Typo!
I implemented such a check in #13797 but maybe it's too broad (there didn't seem to be much interest in the PR). If we cared about absolute correctness, then I think we'd go for pyre's behavior (see the end of this comment to see what pyre does), but that might be too aggressive for mypy. Maybe as a less invasive change, mypy could just treat uninitialized instance variables on abstract classes as they are treated on protocols? (Which was the original suggestion at the top of this issue here.) The idea being that if you declare an attribute on an abstract class (and don't initialize it in the
__init__) then you probably expect subclasses to initialize it. Doing this only for abstract classes also has the advantage that we maybe avoid edge cases like using__new__that can't be analyzed well in terms of which attributes they initialize.Also, it should probably not be a new flag, but a new error code that's disabled by default?
what pyre does:
from abc import ABC class Sized(ABC): # OK (because it's abstract) len: int def __len__(self) -> int: return self.len class A(Sized): # OK def __init__(self) -> None: self.len = 10 class B(Sized): # err: Uninitialized attribute `len` def __init__(self) -> None: self.ln = 10 class C: # err: Uninitialized attribute `len` len: int def __len__(self) -> int: return self.len
pyre output:
main.py:12:0 Uninitialized attribute [13]: Attribute `len` inherited from abstract class `Sized` in class `B` to have type `int` but is never initialized. main.py:16:0 Uninitialized attribute [13]: Attribute `len` is declared in class `C` to have type `int` but is never initialized.
Currently, instantiating explicit subclasses of protocols with "abstract" attributes is prohibited:
This is feature appeared with little discussion, in particular should this be specific only to protocols or also to "normal" ABCs?