injector icon indicating copy to clipboard operation
injector copied to clipboard

Binding to interface style class Protocols

Open davebirch opened this issue 3 years ago • 6 comments

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 avatar Feb 02 '21 14:02 davebirch

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.

davebirch avatar Feb 02 '21 14:02 davebirch

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.

jstasiak avatar Feb 02 '21 19:02 jstasiak

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.

davebirch avatar Feb 18 '21 10:02 davebirch

Are the errors you're getting similar to https://github.com/alecthomas/injector/issues/143 (https://github.com/python/mypy/issues/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. :)

jstasiak avatar Feb 18 '21 21:02 jstasiak

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,

davebirch avatar Feb 18 '21 22:02 davebirch

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.)

pauleveritt avatar Feb 23 '21 19:02 pauleveritt