PEP 544 indicates that a type checker should generate an error if a class that explicitly derives from the protocol attempts to call a method through super() if that method is unimplemented in the protocol.
class Proto(Protocol):
def method(self) -> None:
...
class Impl(Proto):
def method(self) -> None:
super().method() # Type checker error
This makes sense because the method in the protocol is effectively abstract. Currently, pyright doesn’t emit an error here, but PEP 544 says that it should. Mypy properly emits an error.
My question to the community… Should the same logic apply to methods that are explicitly decorated with @abstractmethod in an ABC? There are two cases to consider:
Case 1: Calling an abstract classmethod or staticmethod.
class BaseClass(ABC):
@abstractmethod
@classmethod
def method(cls) -> None:
pass
# Should this be an error?
# Pyright currently generates error here, but mypy doesn't.
BaseClass.method()
Case 2: Calling an abstract instance method through a super() call:
class BaseClass(ABC):
@abstractmethod
def method(self) -> None:
pass
class Subclass(BaseClass):
def method(self) -> None:
# Should this be an error?
# Currently, neither pyright nor mypy generate an error here.
super().method()
I’m inclined to think they both should be errors (although this isn’t a high confidence view and I’d be interested in seeing primer — I’m not a big user of abstract classes or inheritance).
Actually, I’m surprised mypy doesn’t error on the second one. Taking a closer look, mypy does complain in the second case if the return type is not a supertype of None. See mypy Playground (Call to abstract method "method" of "BC2" with trivial body via super() is unsafe)
No, that Case 2 example shouldn’t be an error. If the programmer wants that to be an error, they can put raise NotImplementedError in the ABC’s implementation. I have heard of examples where Base Classes are the base class of a complex inheritances hierarchy and each subclass should call to super().method to make sure their sibling classes can do the correct work, although I don’t have any real examples of that.
Here a small mock example:
class Base(ABC):
@abstractmethod
def method(self) -> None:
pass
class A(Base):
def method(self) -> None:
super().method()
print("A")
class B(Base):
def method(self) -> None:
super().method()
print("B")
class C(A, B):
def method(self) -> None:
super().method()
print("C")
Type checkers should have no complaints about this.
My takeaway is that an @abstractmethod can be implemented or not implemented. If it’s implemented, it’s safe to call. If it’s not implemented, then a type checker should generate an error if it’s called.
A “not implemented” method is one whose body consists of either:
An ellipsis
The statement raise NotImplementedError
(Either of these can be preceded by a docstring.)
class BaseClass(ABC):
@abstractmethod
def method1(self) -> None:
""" This method is unimplemented """
...
@abstractmethod
def method2(self) -> None:
""" This method is unimplemented """
raise NotImplementedError
@abstractmethod
def method3(self) -> None:
""" This method is implemented """
pass
@abstractmethod
@classmethod
def cls_method1(cls) -> None:
""" This class method is unimplemented """
...
@abstractmethod
@classmethod
def cls_method2(cls) -> None:
""" This class method is implemented """
pass
class Subclass(BaseClass):
def method1(self) -> None:
super().method1() # Type checker error
def method2(self) -> None:
super().method2() # Type checker error
def method3(self) -> None:
super().method3() # OK
BaseClass.cls_method1() # Type checker error
BaseClass.cls_method2() # OK
Hm, that is indeed a well-known pattern for multiple inheritance, but I would argue that the base class method should not be declared abstract, since it is clearly expected to be called.
I am surprised that mypy makes a distinction between an abstract superclass method returning None or object and one returning something else. I suspect this is the result of a careful compromise based on observation of a large production code base. (It’s unlikely to be needed by typeshed, otherwise I’d put that in as well.) Unfortunately it appears that the active mypy developers have chosen to ignore this forum.
PS: I seem to be behind the times. I didn’t realize there was a difference between ... and pass as the body of a “trivial” function/method.
OTOH, there is a clear expectation that it should be overwritten and a subclass not doing that is doing the wrong thing . An alternative solution would be that super().method() somehow doesn’t try to call an abstract method, but that doesn’t sound like a good idea either.
I am also not sure if I like that. This seems like something that will have to be explained again and again… But I can’t think of a better solution either.
I’ve changed pyright to generate an error when calling an unimplemented abstract method (and similarly, an unimplemented method within a protocol). That brings pyright in conformance with the existing typing spec in this respect. It also brings the behavior of pyright and mypy closer together.
There’s still an open question about what precisely constitutes an “unimplemented function”, but that debate can be deferred. I think it may be somewhat contentious. I find mypy’s current behavior to be inconsistent and difficult to justify, which is why I intentionally deviated from it in pyright. I’ve added this to a long list of issues that I think we should eventually tackle in the typing spec. I’m trying to be very deliberate about which issues we tackle first, and this doesn’t strike me as one of the higher-priority items.
If I am understanding your comment correctly, it would mean that mypy would then complain about the example I provided above. How would that example be written in a way such that mypy doesn’t complain?
So then there is a difference between empty/.../pass and return, despite all resulting in (almost) identical bytecode. IMO treating pass as non-trivial seems like a cleaner solution. Yes, then there is a difference between empty/... and pass, but that at least seems to be an easier to explain idiom than having an empty return. But I don’t have much of a stake in that discussion tbh.
Yeah, fair enough — the main advantage is that it wouldn’t change well-established behaviour for mypy users. I also think both ... and pass read a little bit more like “dummy code” than return does, but that’s a more marginal and subjective argument.
Thank you! Can you please share an ETA/which pyright version is expected to intercept this change?
Looks like it has already been addressed in 1.1.345 , thank you.
. An alternative solution would be that