injector icon indicating copy to clipboard operation
injector copied to clipboard

Unable to bind Annotated to instances of origin

Open pstatix opened this issue 4 years ago • 6 comments

Originally stemming from interest in #133 , I too thought to replicate Guice's @Named convention. In doing so, I originally used the typing.NewType (prior to having even seen the aforementioned issue) and succeeded. However, instantiating a type for each simple alias, I thought typing.Annotated (albeit, using typing_extensions.Annotated) would be more idiomatic.

Given a simple example:

import pathlib
import injector

ConfigFile = injector.Annotated[pathlib.WindowsPath, 'config']

cfg_file = pathlib.Path.home() / 'settings.cfg'


class AppModule(injector.Module):

    def configure(self, binder):
        binder.bind(ConfigFile, to=cfg_file) # Guice uses annotatedWith here

class App:

    @injector.inject
    def __init__(self, cfg: ConfigFile):
        self.config = cfg

app = injector.Injector(AppModule).get(App)

...throws...

Traceback (most recent call last):
  File "E:\Programming\test\test_injector\test.py", line 20, in <module>
    app = injector.Injector(AppModule).get(App)
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 913, in __init__
    self.binder.install(module)
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 577, in install
    instance(self)
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 860, in __call__
    self.configure(binder)
  File "E:\Programming\test\test_injector\test.py", line 12, in configure
    binder.bind(ConfigFile, to=cfg_file) # Guice uses annotatedWith here
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 479, in bind
    self._bindings[interface] = self.create_binding(interface, to, scope)
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 582, in create_binding
    provider = self.provider_for(interface, to)
  File "C:\Python37\lib\site-packages\injector\__init__.py", line 644, in provider_for
    raise UnknownProvider('couldn\'t determine provider for %r to %r' % (interface, to))
injector.UnknownProvider: couldn't determine provider for typing_extensions.Annotated[pathlib.WindowsPath, 'config'] to WindowsPath('C:/Users/Paul/settings.cfg')

My belief is that provider_for does not adequately check to see if the interface is an _AnnotatedAlias or not. The origin variable resolves to the base interface correctly, in this case pathlib.WindowsPath, but since base_type is typing_extenstions.Annotated, not check is made given that case.

pstatix avatar Jan 05 '21 14:01 pstatix

Ah yeah, I haven't considered handling typing.Annotated in this context yet and it's totally unsupported.

It's probably worth exploring what possible uses of Annotated could there be other than this one (a kinda-new-type opaque to Injector), but this seems like a reasonable use case.

jstasiak avatar Jan 05 '21 14:01 jstasiak

One can, however, use typing.Annotated interfaces so long as the @provider decoration is used in the injector.Module:

import pathlib
import injector

ConfigFile = injector.Annotated[pathlib.WindowsPath, 'config']

cfg_file = pathlib.Path.home() / 'settings.cfg'


class AppModule(injector.Module):

    @singleton
    @prodiver
    def provides_config_file(self) -> ConfigFile:
        return cfg_file

class App:

    @injector.inject
    def __init__(self, cfg: ConfigFile):
        self.config = cfg

app = injector.Injector(AppModule).get(App)
print(app.config)

# C:\Users\<me>\settings.cfg

However, for simply annotating multiple instances of the same type (e.g. a file interface [app config, user config, network config, etc.]), using the bind() API would be idiomatic and closer to the Guice semantics.

pstatix avatar Jan 05 '21 14:01 pstatix

Oh, really? That's interesting, those should have the same effect. Also I have a feeling that due to Annotated-stripping in the Injector internals this won't distinguish between typing.Annotated[pathlib.Path] and plain pathlib.Path. I seem to be correct:

% cat asd.py
import pathlib
import injector

ConfigFile = injector.Annotated[pathlib.Path, 'config']
OtherConfigFile = injector.Annotated[pathlib.Path, 'other_config']

cfg_file = pathlib.Path.home() / 'settings.cfg'
other_cfg_file = pathlib.Path.home() / 'settings.cfg' / 'other'


class AppModule(injector.Module):

    @injector.singleton
    @injector.provider
    def provides_config_file(self) -> ConfigFile:
        return cfg_file

    @injector.singleton
    @injector.provider
    def provides_other_config_file(self) -> OtherConfigFile:
        return other_cfg_file

class App:
    @injector.inject
    def __init__(self, cfg: ConfigFile, other_cfg: OtherConfigFile) -> None:
        self.cfg = cfg
        self.other_cfg = other_cfg

app = injector.Injector(AppModule).get(App)
print(app.cfg)
print(app.other_cfg)
% python asd.py
/Users/user/settings.cfg/other
/Users/user/settings.cfg/other

With this in mind I'm tempted to add a big warning to the documentation to inform people about this for the time being.

Bonus: Injector(...).get(ConfigFile) will fail with the same error you provided in the original post.

(edit: originally both provided methods returned ConfigFile, this was a mistake, the example is still correct)

jstasiak avatar Jan 05 '21 15:01 jstasiak

Correct. Using typing.NewType however results in "aligned" semantics to @Named (which one would think is easily achievable with typing.Annotated):

import typing
import pathlib
import injector

UserFile = typing.NewType('UserFile', pathlib.WindowsPath)
ConfigFile = typing.NewType('ConfigFile', pathlib.WindowsPath)

usr_file = pathlib.Path.home() / 'user.dat'
cfg_file = pathlib.Path.home() / 'settings.cfg'


class AppModule(injector.Module):

    def configure(self, binder):
        binder.bind(UserFile, to=usr_file)
        binder.bind(ConfigFile, to=cfg_file)

class App:

    @injector.inject
    def __init__(self, cfg: ConfigFile, usr: UserFile):
        self.user = usr
        self.config = cfg

app = injector.Injector(AppModule).get(App)
print(app.config)
print(app.user)

# C:\Users\<me>\settings.cfg
# C:\Users\<me>\user.dat

I believe such integration is achievable still in the provider_for method as base_type maintains the reference to the _AnnotatedAlias. The _is_specialized method could also be extended since it already has conditionals for annotations.

pstatix avatar Jan 05 '21 15:01 pstatix

Yes, absolutely. I'd like to consider other options before allowing arbitrary, opaque Annotated instances in context like this, mixing with Inject/NoInject etc., the design space here is not that small possibly.

jstasiak avatar Jan 05 '21 15:01 jstasiak

FYI, calling out a technically nit that I mentioned in #133 about NewType vs type aliases (https://github.com/python-injector/injector/issues/133#issuecomment-1480067511). I don't think it's right to equate the behavior of @named and NewType. NewType ends up being a different key because it's actually a new type so it would break cases where a caller wants instances of the same type but annotated differently.

For instance, in the example above (I know it was just meant as a motivating example) the binder.bind(UserFile, to=usr_file) passes type checking because to:Any when it really shouldn't since you're binding a pathlib.Path to a UserFile

strangemonad avatar Mar 28 '23 17:03 strangemonad