python-injector/injector

Binding of Annotated types does not work as one would expect

hmoniz opened this issue · 8 comments

First of all, my apologies is this issue has already been raised. I've been trying to incorporate the injector framework into one of my projects, but I have hit a snag that I'm not sure how to address. Very succinctly, I have the need to bind the same type to different instances in different parts of the code.

For instance, let's say I have an abstract base class Shape and I have two different places in the code in which I want to inject an implementation:

class Shape(ABC):
    pass

@inject
@dataclass
class A:
    first_shape: Shape
    second_shape: Shape  

I want first_shape to be an instance of Square and second_shape to be an instance of Circle, both of which inherit from Shape. To me, the most natural way to do this would be to annotate these types using typing.Annotated and create the respective bindings, like such:

@inject
@dataclass
class A:
    first_shape: Annotated[Shape, 'first']
    second_shape: Annotated[Shape, 'second']  

and then create the bindings like this:

def configure(self, binder):
    binder.bind(Annotated[Shape, 'first'], to=Circle)
    binder.bind(Annotated[Shape, 'second'], to=Square)

This, unfortunately, doesn't work. When I call injector.get(A), I get the following error:

injector.CallError: Call to ABCMeta.__new__() failed: Can't instantiate abstract class Shape with abstract method draw (injection stack: ['__main__'])

I also tried using provider methods:

    @provider
    def provide_first(self) -> Annotated[Shape, 'first']:
        return Circle()

    @provider
    def provide_second(self) -> Annotated[Shape, 'second']:
        return Square()

But this also results in an error:

injector.UnknownProvider: couldn't determine provider for typing.Annotated[__main__.Shape, 'first'] to None

At this point, my question is whether this is the intended behaviour, and, if so, what is the idiomatic way of binding separate declarations of the same type to different instances?

I just to add one more data point. Interestingly, if I have the binding configured like this:

def configure(self, binder):
    binder.bind(Annotated[Shape, 'first'], to=Circle)
    binder.bind(Annotated[Shape, 'second'], to=Square)

and I get the objects directly from the injector, it works:

injector = Injector(Mod())
injector.get(Annotated[Shape, 'first'])
injector.get(Annotated[Shape, 'second'])

This means that the following provider method that manually constructs the A object also works:

@provider
def provide_a_from_injector(self, injector: Injector) -> A:
    return A(
        injector.get(Annotated[Shape, 'first']),
        injector.get(Annotated[Shape, 'second'])
    )

While this is a potential workaround, I feel like the overall behaviour is too inconsistent to warrant a production use of the framework.

I haven't had the time to get into all the details of this issue, but have you tried using NewType instead of Annotated? There's an example in the tests.

I have, but unfortunately it doesn't work in situations where I want to inject a ClassAssistedBuilder. For example, let's say I define a type FirstShape = NewType('FirstShape', Shape). Then, in class A, I inject a ClassAssistedBuilder[FirstShape]. When I call build(), I get:

injector.CallError: Call to NewType.__new__() failed: object.__new__(X): X is not a type object (NewType) (injection stack: [])

Would the the regular AssistedBuilder work for you here? I haven't tried it myself, but it uses the existing binds so it might work better.

jriddy commented

I've been looking at this an I'm thinking about generalizing this pattern. If PRs are welcome I might give it a go to try to cook something up here.

I'm trying to evaluate an ad-hoc injection framework I can use or modify, but the limitation of needing to inject a type is kinda frustrating. It basically pushes you in the direction of using DI solely for creating more complex types that are worth constructing, whereas I often want to encourage devs to do things like express getting an environment variable as a dependency.

FastAPI has a DI implementation that is a good example of this, where it's injecting query strings and stuff from having parsed an HTTP request, but often the underlying types are simple types like str and int. It uses Annotated to declare this metadata and then extracts that to create the binding.

While the design there is a bit different, since FastAPI is an application framework instead of a general-purpose library, I think this pattern could be applied here, such as in this putative code:

from dataclasses import dataclass
from injector import inject, Injector, Name
from typing import Annotated
import os

@inject
@dataclass
def Login:
    username: Annotated[str, Name("login_username")]
    password: Annotated[str, Name("login_password")]


def configure_from_env(binder):
    binder.bind(str, named="login_username", to=lambda: os.environ["LOGIN_USERNAME"])
    binder.bind(str, named="login_password", to=lambda: os.environ["LOGIN_PASSWORD"])


injector = Injector([configure_from_env])
injector.get(Login)

If there's interest I could look into adding this

jriddy commented

I haven't had the time to get into all the details of this issue, but have you tried using NewType instead of Annotated? There's an example in the tests.

NewType works sorta, but it leads to an annoying situation where you have to convert everything to your NewType where you use them. And unlike Haskell or Rust or languages like that where this idea is really prevalent, using NewTypes in Python don't get compiled away and do incur a (small) runtime penalty.

PRs are welcome, and it seems to me like a natural evolution of Injector to support this.

But regarding the API, don't you think it would be better to bind the Annotated[...] type instead of adding the named argument?

# ...

LoginUsername = Annotated[str, Name("login_username")]
LoginPassword = Annotated[str, Name("login_password")]

@dataclass
def Login:
    username: LoginUsername
    password: LoginPassword


def configure_from_env(binder):
    binder.bind(LoginUsername, to=lambda: os.environ["LOGIN_USERNAME"])
    binder.bind(LoginPassword, to=lambda: os.environ["LOGIN_PASSWORD"])

I hope the performance hit of NewType is negligible for things that run on Python, but the usability improvement of Annotated is appealing.

ljnsn commented

I just opened a PR a that adds support for Annotated. Sorry @jriddy , I kind of needed this and it seemed pretty straightforward (unless I missed something). Would definitely value your input!