Fix type annotations for .provides

[thiromi]

I'm using this package for a project that relies on mypy static typing checks and bumped into a stub that has a strange behavior:

When I define let's say a Singleton[int] provider and want to access the class type via the property Singleton.cls, mypy complains the type is an instance of int rather than Type[int]

And as Singleton.cls is an alias to Singleton.provides, I revised the types from that property as well to check if their types were correctly defined

thank you for any feedback provided 🙏

[rmk135]

@thiromi Wow! That's great! Thanks for the PR. I'll make a review later today

[rmk135]

Didn't get to the review today. Will take a look tomorrow

[rmk135]

So in that case we better return _Callable[..., T]. Attribute .provides should return what was provided in .__init__() or .set_provides(). Could you, please, update it?

[thiromi]

Sure, I can. But If I could then propose here, because we can have the providers declared in two ways (they're semantically correct):

provider10 = providers.Callable(Cat)

and this

provider10 = providers.Callable[Animal](Cat)

mypy complains about the latter expecting that Cat (Type[Cat]) is not _Callable[..., T].

what do you think of using Union[Type[T], _Callable[..., T]]?

I was going to propose that in another PR, but as you brought up this point, I thought it would be a pertinent question 😅

[rmk135]

Yep, that makes sense. Go for it

[thiromi]

Well, that's embarrassing... I added the tests using the explicit type on typevar

provider10 = providers.Callable[Animal](Cat)

and mypy didn't complain 😅

I might have done something wrong on my project 🤔

anyway, changed the code as you suggested at 47cf4c2

[rmk135]

Like in the case with Callable provider we better return _Callable[..., T] here. With the current design Factory and Singleton providers can take not only classes but any callables. This change design change was made to support factory method and functions like sqlite.connect(), logging.getLogger(), etc.

[thiromi]

done 47cf4c2

[rmk135]

At the same time, Factory provider still has a strong use case when it is used with a class. When you need to access factory class, attribute .cls is better from the readability and understandability perspectives. Even despite .cls and .provides are now aliases and have the same implementation, I think we should treat them differently. With that said, I think that's ok if we type .cls -> Type[T], and .provides -> _Callable[..., T].

[thiromi]

okidoki 👍

once we resolve the conversation at https://github.com/ets-labs/python-dependency-injector/pull/491/files#r692507266, I'll do the changes 🙏

[thiromi]

done 47cf4c2

[rmk135]

@thiromi

Looking at this PR I started think about proper typing (and behavior) for .provides. It currently can return None if undefined and that leads to typing it's return type as optional:

class Object(Provider[T]):
    ...
    @property
    def provides(self) -> Optional[T]: ...
    ...

This might be inconvenient from typing perspective cause every time when you use .provides you need to check it agains None before further usage (that's right from the perspective of current implementation).

Considering that I'm thinking about two changes:

1. Change typing for .provides and .cls attributes to exclude Optional
2. Make sure None is never returned by adding raising an exception when you're trying to access uninitialized provider.

Appreciate your thoughts on this.

[thiromi]

@rmk135

I think your suggestion on ensuring the provider is initialized is great! That would improve the cleanliness of the type hinting of .provides and .cls 👍

[rmk135]

@rmk135

I think your suggestion on ensuring the provider is initialized is great! That would improve the cleanliness of the type hinting of .provides and .cls 👍

@thiromi ok, we can make a followup PR then. Thanks.

[rmk135]

Merged the PR in develop branch. Next release to the PyPI will be on Wednesday August 25th.

[rmk135]

Published in 4.36.0.