Should concrete implementations be required to implement protocol?

[pauleveritt]

• Are you reporting a bug, or opening a feature request?

Bug

• Please insert below the code you are checking with mypy,
  or a mock-up repro if the source is private. We would appreciate
  if you try to simplify your case to a minimal repro.

from typing import Protocol


class Person(Protocol):
    first_name: str

    def greeting(self, message: str) -> str:
        ...


class FrenchPerson(Person):
    prenom: str

    def __init__(self, name: str):
        self.prenom = name

    def salutation(self, message: str) -> str:
        return f"{message} {self.prenom}"


def print_person(person: Person) -> str:
    return person.greeting(message="Je m'appelle")


def main():
    fp = FrenchPerson(name='Henri')
    response = print_person(fp)
    print(response)

• What is the actual behavior/output?

PEP 544 says you can state that an implementation supports a protocol by
subclassing the protocol.

It also says: "If one omits Protocol in the base class list, this would
be a regular (non-protocol) class that must implement Sized."

• What is the behavior/output you expect?

I would hope mypy would, either at declaration time or in the print_person function, spot that something claimed to be a Person but had neither the protocol variable name nor the method greeting.

It's also a shame that FrenchPerson got the greeting method added to it from the subclassing.

• What are the versions of mypy and Python you are using?

mypy 0.770 and Python 3.8.1

• What are the mypy flags you are using? (For example --strict-optional)

None.

[louwers]

I ran into exactly this today. Mypy doesn't recognize the problem. It should!

[JukkaL]

Yeah, this behavior is not what one would expect. Note that if main was type checked (with a -> None annotation), the missing attribute definition would be caught when constructing an instance of FrenchPerson. However, we should probably report the missing attribute already in the class definition.

There is a question about when exactly should we require a method to be implemented in the concrete derived class. The method greeting has a body, but since it's ..., it should not be treated as a default implementation, I think.

@ilevkivskyi What do you think?

[nickgaya]

This behavior seems to be as-expected according to the PEP.

I would hope mypy would, either at declaration time or in the print_person function, spot that something claimed to be a Person but had neither the protocol variable first_name nor the method greeting.

I don't see any reason for a type error in the class declaration or the print_person() function. MyPy will correctly raise an error when you try to instantiate FrenchPerson due to the missing first_name property.

error: Cannot instantiate abstract class 'FrenchPerson' with abstract attribute 'first_name'

It's also a shame that FrenchPerson got the greeting method added to it from the subclassing.

That's expected due to how class inheritance works. If you want to make Person.greeting() abstract, annotate the method with @abstractmethod and optionally raise a NotImplementedError in the body.

The method greeting has a body, but since it's ..., it should not be treated as a default implementation, I think.

That's how it will be treated at runtime, so the type checker should reflect this. Maybe there could be a warning if the body is ... for a non-abstract method, as this is likely to indicate programmer error.

[pauleveritt]

I don't see any reason for a type error in the class declaration or the print_person() function. MyPy will correctly raise an error when you try to instantiate FrenchPerson due to the missing first_name property.

When the PEP says: "To explicitly declare that a certain class implements a given protocol, it can be used as a regular base class. " ... IMO, it's a statement about the class, not the instance.

If it's about the instance, then that doesn't fit (again, IMO) with the zen of Python type hinting. Why warn me about a function's return type not matching the type of the return, and not just say "I'm not going to help until you use the function somewhere."

But I recognize that's a small part of the PEP and I doubt it really reflects the intent. I confess I'm trying to shoe-horn zope.interface into protocols.

[nickgaya]

It seems like with the current behavior, a class that extends a protocol class without implementing its abstract members is treated like an abstract class. Defining an abstract class isn't a type error, but attempting to instantiate one is.

If MyPy were to require protocol subclasses to implement all abstract protocol members, this would rule out defining an abstract subclass of a protocol, other than a subprotocol.

class AbstractPerson(Person):
    @abstractmethod
    def do_something(self) -> None:
        raise NotImplementedError

I can see a couple potential issues with this.

1. It might break type-checking for existing code that uses the protocols defined in the typing module.

2. It imposes a restriction on protocol subclasses at type-checking time which has no basis in actual runtime behavior.

One possible solution would be if there were a way to explicitly mark a class as non-abstract, which would signal to the type-checker that all abstract members should be implemented. (This could be useful for ABCs as well.)

For practical purposes, you can do something like this (with or without explicitly subclassing the protocol):

if typing.TYPE_CHECKING:
    # Check that FrenchPerson is a concrete implementation of the Person protocol
    _: Person = FrenchPerson()

Note, however, that the PEP explicitly discourages this pattern.

[ktbarrett]

I also ran into this testing out the following snippet:

class A(Protocol):
    def a(self) -> int: ...

class B(A): ...

b = B()
b.a()

mypy sees no problem with this, which is wrong. At runtime, b.b() returns None, not an int.

Adding an incorrect implementation of b in the B implementation class is also not correctly checked. Changing out B for the following also passes mypy.

class B(A):
    def b(self):
        return "not an int"

[nickgaya]

@ktbarrett Your first example passes type-checking due to a quirk of MyPy unrelated to protocols. MyPy doesn't warn about methods/functions whose body is ... or pass. See
#9772 for more discussion. So A.a passes type-checking, and B inherits this method.

In your second example it looks like you meant to override a in the protocol. Also, since the method is not annotated, it is considered untyped and ignored by default. If you change the method name to a and annotate it, MyPy will detect errors as expected. This is the same regardless of whether A is a protocol.

[ktbarrett]

due to a quirk of MyPy unrelated to protocols. MyPy doesn't warn about methods/functions whose body is ... or pass

That's quite the "quirk".

In your second example it looks like you meant to override a in the protocol

I did. Transliteration error.

since the method is not annotated, it is considered untyped and ignored by default ... This is the same regardless of whether A is a protocol.

Ugh, mypy complains if you provide an incompatible type in a subclass, but just ignores the definition in the base class if you don't annotate? That's not consistent or intuitive IMO. I really like how all these "expected" corner cases team up and seemingly misbehave. There are FAR too many gotchas in mypy. It should be convention to avoid them.

[tmke8]

There were two previous issues for the same problem: #8005 and #8395 but this one has the most discussion, so it probably makes sense to close the other two and keep this one.

I was thinking that a method in a protocol should be treated as abstract if all of the following holds:

1. The function body only consists of pass, ..., and/or a docstring.
2. The definition of the protocol is not from a stub file (because stubs never have function bodies).
3. Return type is not None or Any (because for those return types, an empty function body is valid).
4. Function is not annotated with @overload.

You could extend the first rule to also cover functions that only raise a NotImplementedError but this maybe gets tricky.

[ktbarrett]

This is not a good idea. The relevant part of PEP544.

1. abstract methods should be marked with abstractmethod. All the examples in the PEP use it.
2. Protocols, like ABCs, might only be partially implemented by a class, so really this should only be checked upon instantiation, or if the class is marked final (I don't remember off the top of my head if this is supported yet). The PEP states "The semantics of @abstractmethod is not changed" in the second sentence of the last paragraph.
3. Method implementations are already checked for compatibility

I think the primary issue from the OP, myself, and others is the assumption of the opposite of 2. You see this in other languages like Java's interfaces and Rust's traits when you "implement" them. However, inheritance here isn't "implementing" the interface, it's more like using the Protocol as an ABC. ABCs allow for partial implementation. Deciding that when inheriting from Protocol the user must fully implement the interface will likely break a lot of users and changes the semantics of abstractmethod, violating the PEP. You could argue that the semantics of abstractmethod should have been different, which would make this change valid, but I think that ship sailed long ago...