Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document why having a user defined protocol return type is often a problem #5031

Closed
ilevkivskyi opened this issue May 14, 2018 · 9 comments
Closed

Comments

@ilevkivskyi
Copy link
Member

I have seen this proposed sometimes. We should explicitly say that protocols are good for argument types, but are bad for return types, because protocol is never a subtype of a concrete type (with an example). Typically even a union is better than protocol for a return type (which is itself quite bad, see #1693)

@JelleZijlstra
Copy link
Member

Why is it bad? It is pretty common to annotate a return type like Iterable[int] when really you are returning a generator or some other concrete type. This generally doesn't cause problems in the calling code.

Union return types are problematic because callers will almost always have to use isinstance before they can do anything with the return value, even though in practice they usually know the precise type. I don't see a similar problem with protocol return types.

@ilevkivskyi
Copy link
Member Author

Builtin protocols are actually OK. But I can't think of a good reason to return a user defined protocol. User (almost) always knows what type is actually returned in function body. Even if there may be few concrete (nominal) types, it is still precise and will fail less often than a protocol.

@ilevkivskyi ilevkivskyi changed the title Document why having a protocol return type is often a problem Document why having a user defined protocol return type is often a problem May 14, 2018
@Michael0x2a
Copy link
Collaborator

I think it could potentially be ok if the user is trying to establish and use a strong set of abstractions within their program? For example, if I'm trying to write a GUI app with a plugin system or something, I might want certain methods to return arbitrary objects so long as they implement a 'render' method -- so, returning a 'Renderable' protocol might make sense.

I think this ends up being pretty similar to how it can potentially be useful to return a base class from methods rather then specific subtypes: just like with protocols, certain base classes will never end up being subtypes of a (useful) concrete type.

(Slightly unrelatedly: I wonder what would happen if somebody tried typing an entire codebase using just structural typing with little to nominal typing -- went all in with duck types? In that case, returning protocols would be a natural choice.)

@ilevkivskyi
Copy link
Member Author

I wonder what would happen if somebody tried typing an entire codebase using just structural typing with little to nominal typing

I think people who are brave enough to do this are unlikely to listen to such recommendations :-)

There may be cases when returning a protocol is OK, and when returning a union is OK. The point is this should be a conscious choice, understanding the consequences (especially when annotating a public library).

@glyph
Copy link

glyph commented Aug 6, 2019

What about this use-case:

class X(Protocol):
    "some methods, not important"

class XFactory(Protocol):
    def make_x(self) -> X:
        ...

Surely I'd want to use a user-defined protocol as a return type here?

@JukkaL
Copy link
Collaborator

JukkaL commented Aug 7, 2019

I think that it's totally fine to have protocol return type in an abstract method. It's probably less useful in a non-abstract method, but I'm not sure why we should discourage this. Is this a common source of user confusion? This seems like a design/style issue to me.

Also, using a protocol or ABC return type allows the concrete return type to be changed without changing the signature, which seems like a potentially useful thing as it can hide implementation details.

@ilevkivskyi
Copy link
Member Author

Is this a common source of user confusion? This seems like a design/style issue to me.

I have seen this few times. We might not need to discourage this, but it is probably a good idea to document pros and cons of such approach.

@glyph
Copy link

glyph commented Aug 10, 2019

I'm not sure why we should discourage this.

Glad it’s not just me 😅

@hauntsaninja
Copy link
Collaborator

Seems like there is neither consensus nor action here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants