Add annotated support for the provider and added named provider #285
Conversation
davidparsson
left a comment
davidparsson
left a comment
There was a problem hiding this comment.
Thanks for your contribution! Please see my comments below.
| scope_ = getattr(function, '__scope__', None) | ||
| try: | ||
| annotations = get_type_hints(function) | ||
| annotations = get_type_hints(function, include_extras=True) |
There was a problem hiding this comment.
Is this what allows for a @provider to return things like Annotated[str, 'first']?
There was a problem hiding this comment.
Yes, adding include_extras=True ensures that metadata inside Annotated types is preserved when fetching type hints. Without it, only the base type (e.g., str) was returned, causing the internal _punch_through_alias check for _AnnotatedAlias to never trigger.
Example:
from typing import Annotated, get_type_hints
def f() -> Annotated[str, 'first']:
...
print(get_type_hints(f))
# {'return': <class 'str'>}
print(get_type_hints(f, include_extras=True))
# {'return': typing.Annotated[str, 'first']}The internal logic in _punch_through_alias already handles checking for _AnnotatedAlias:
elif isinstance(type_, _AnnotatedAlias) and getattr(type_, '__metadata__', None) is not None:
return type_.__origin__but the metadata was being stripped before, so this change fixes that.
| def named_provider(name: str) -> CallableT: | ||
| """Decorator for :class:`Module` methods, creating annoted type a of a type. | ||
| >>> class MyModule(Module): | ||
| ... @named_provider("first") | ||
| ... def provide_name(self) -> str: | ||
| ... return 'Bob' | ||
| """ | ||
|
|
||
| def decorator(function: CallableT): | ||
| _mark_named_provider_function(function, name, allow_multi=False) | ||
| return function | ||
|
|
||
| return decorator |
There was a problem hiding this comment.
I'm not convinced that this alternative way of declaring a provider is worth the extra maintenance it introduces. Do you have any arguments behind introducing it?
There was a problem hiding this comment.
The reason for adding this was to replicate the @Named annotated functionality from Guice — allowing us to bind and inject a specific instance among multiple instances of the same interface.
Also, considering how @multiprovider works, I wanted to avoid using a dictionary lookup pattern to retrieve the instance by key.
There was a problem hiding this comment.
Right. I'm still not convinced though.
It has been a while since Java was my primary language, but isn't the presence of @Named in Guice motivated by the absence of a language supported way of declaring annotated types? Since that is supported natively in Python, I think this only adds a less intuitive way to declare the same thing.
I however still happy to merge the other fix in this PR if you're willing to split it.
There was a problem hiding this comment.
Right, the only purpose this would serve is to replicate the same pattern used in Guice, which can already be achieved using Annotated. So, there isn’t much of a use case for adding this.
We can close this PR.
You can check the split PR created for the other fix here:
#286
|
Closing in favor of #286 |
2 participants
Why
This change fixes an issue where annotated return types were not properly handled by the provider.
Additionally, it introduces support for named_provider, allowing specific instances to be bound for a given type — particularly useful when multiple providers exist for the same type.