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

Binding to interface style class Protocols #177

Open
davebirch opened this issue Feb 2, 2021 · 6 comments
Open

Binding to interface style class Protocols #177

davebirch opened this issue Feb 2, 2021 · 6 comments

Comments

@davebirch
Copy link

Separated out from #168 - specifically to deal with binding to a class level Protocol usage (i.e normal methods defined in the protocol rather than the special __call__() usage.

This use case for Protocol is about as close as I've found in python to a java interface, thus also feels like it would be a pretty desirable use case to support in an injection framework. In fact, if you ignore the MyPy warnings, this use case seems to work with the injector code on master. (based on a very quick unit test!)

With usage of @runtime_checkable on the Protocol class, you can use isinstance() on it, which might ease some of the implementation pain?

I think with some sensible use of @overload on the binder.bind method, it might be possible to isolate these use cases and allow some type checking that things aren't entirely wrong, if your concerns are around pushing a load of checking to run-time?

If only the class level Protocol is being supported at this stage, we should ensure that any attempt to bind to a protocol with a __call__ method fails at runtime.

@davebirch davebirch changed the title Support of Interface style Binding to interface style class Protocols Feb 2, 2021
@davebirch
Copy link
Author

Already have some initial unit tests for this as described above, am pretty flat out until tomorrow evening, but will try and get an initial PR with some sample unit tests up then to see if I have the rough use-cases covered in my head.

@jstasiak
Copy link
Collaborator

jstasiak commented Feb 2, 2021

So this should already work to a large degree I think (as you already found out). Injector specifically doesn't do too much, if any, runtime checking if you're not doing some "wrong" things typing-wise so I don't think any extra checks are required (or even desired) unless there are some good cases for them. My position on this front is: in ideal case, if mypy is unhappy with the way the client code uses Injector all bets are off and the client code has no guarantees it's doing anything reasonable. Granted, there may be some false positives and we probably need to document those.

The support for partially applied injected-into functions was removed because there was no correctly express that in the type system I think.

What you're saying sounds good overall, looking forward to seeing the use cases you mention.

@davebirch
Copy link
Author

So, I had a go at this a little while back. Seems to be blocked on MyPy not allowing Protocol to be used as a type. So it appears there is no way of having a generic TypeVar which bounds on Protocol.

Utterly snowed under with work stuff at the moment, but will try and raise something over mypy side and see if I can get that changed, suspect it's something to do with the special case treatment for Protocol classes.

Anyhow, this would appear to be a non-starter until that behaviour changes.

@jstasiak
Copy link
Collaborator

Are the errors you're getting similar to #143 (python/mypy#4717)?

Does your thing work at runtime though? If yes maybe it's worth sharing it in the current state to judge what's already there. :)

@davebirch
Copy link
Author

davebirch commented Feb 18, 2021

Exactly that issue, same problem as with other abstract types. Given you had said that the MyPy checking was a fairly ideal, it seemed a non-starter so I mostly just played with some type signatures and then left it at that!

Checking back, all I had written were some unit tests, which seem to pass (even for the injection on a method call style protocol.

Only problem, same as with ABCs, abstractmethod et al is the mypy issue with trying to type check user calls to the bind method,

@pauleveritt
Copy link

FWIW I had a long effort at this with my injector and came to the same conclusion on protocols and mypy. I'm on the PyCharm team, spent some time pair-programming with the team lead who is big in Python typing, and he convinced me protocols were a dead end for what I wanted. (I'm an old zope.interface guy and he convinced me that Java interfaces are part of nominal typing, not structural.)

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

No branches or pull requests

3 participants