injectable
injectable copied to clipboard
Comply with PEP-593 and support typing.Annotated
PEP-593 defines the a standard for annotation metadata other than type hinting through the use of typing.Annotated
. This framework should comply with this recommendation and give full support to typing.Annotated
.
This was originally brought to attention by @Euraxluo in https://github.com/allrod5/injectable/issues/107#issuecomment-905180618
Initial Considerations
Currently Autowired(T)
already works well with most type checkers and linters since it evaluates to T
but this is not in line with the recommendation from PEP-593 to leave annotations primary to type hinting and other metadata to extras.
We should decide how best to incorporate these changes in Injectable. I intend to lay down some possible paths we can go from here in this issue thread so anyone can share their thoughts if any and then eventually I'll commit to an implementation.
While typing.Annotated
was introduced in Python 3.9, this frameworks currently supports all Python versions since 3.6. So we should think in a way that will deliver the best possible experience for all possible users and use cases of Injectable.
Current State
When Python version < 3.9 then this is the only possible way to use Injectable:
@autowired
def foo(service: Autowired(Service): ...
When typing.Annotated is available (Python version >= 3.9), then this is also possible:
@autowired
def foo(service: Annotated[Autowired(Service), ...]): ...
Therefore, since Python 3.9 one can probably use other libs which rely on annotations and fully support PEP-593 without undesired interactions or further problems with the use of Injectable.
Proposals
- ~[rejected] Proposal A: Keep things the way they are~
- [accepted] Proposal B: Make
Autowired
an alias oftyping.Annotated
- ~[rejected] Proposal C: Move
Autowired
from the annotations to the default values of arguments~ - ~[rejected] Proposal D: denote autowired arguments in the
@autowired
decorator, dispensing the use ofAutowired
~
[REJECTED] Proposal A: Keep things the way they are
The way Autowired
is implemented doesn't directly conflict with typing.Annotated
and theoretically we could keep Injectable as is and everything would still work.
Use with Python versions < 3.9 remains the same:
@autowired
def foo(service: Autowired(Service)): ...
Using it with typing.Annotated
would work only if the Autowired
annotation is the first one:
@autowired
def foo(service: Annotated[Autowired(Service), ...]): ...
Pros
- No need for changes
- Less potential bugs when there are no changes
Cons
- Not pythonic not to follow the PEP recommendation
- It may be impeditive to work with some other libs that expect PEP-593 compliant behaviour
- It will still be a costly and sloppy maintenance for
Autowired
to keep working well with linters and type checks - This may break expectations of new users which are accustomed to PEP-593 recommended behavior
Conclusion
I'm rejecting this approach since there seems to be no real benefits going on here and lots of problems.
[ACCEPTED] Proposal B: Make Autowired
an alias of typing.Annotated
All these uses would be correct:
@autowired
def foo(service: Autowired(Service)): ...
@autowired
def foo(service: Annotated[Autowired(Service), foo, bar]): ...
@autowired
def foo(service: Annotated[foo, Autowired(Service), bar]): ...
For backwards compatibility with Python versions prior to 3.9 we'll use the typing_extensions
module which provides the Annotated
generic for Python versions 3.5.3+ which is just perfect since we only support Python 3.6+.
Pros
- We keep the
Autowired
use the way it already is while also being compliant to the PEP and allowing other uses - We need no more to care about every possible linter and type checker
- PyCharm type inference would get along with
Autowired
better this way - We will never need to worry about interactions and conflicts with other libs which use annotations
Cons
- None?
Conclusion
All other proposals seem inferior to this one at the moment so I'll carry with the implementation of this proposal.
[REJECTED] Proposal C: Move Autowired
from the annotations to the default values of arguments
The use of Autowired
would now be like this:
@autowired
def foo(service: Service = Autowired(Service)): ...
This change can be made backwards compatible but it would cause confusion since there would be two ways to use the framework and would be painful to set and push forward a deprecation policy for the use of Autowired
in annotations.
Pros
- This simplifies some requirements of where it is allowed to declare autowired arguments (which currently is always after non-autowired args)
- This would make the function's signature clearer and less magical (currently it is a little awkward to call
foo()
and see it work without passing parameters when its signature is `def foo(a, b, c):```) - For many cases this implementation can be more intuitive to read and use
Cons
- Not entirely backwards compatible
- The behavior of state can be confusing since the default value will not be actually coming from the class and be static but rather created upon object initialization
- If one annotates a concrete type but autowire with a qualifier maybe linters will comply about
Autowired
's return type - It can be tedious to repeat the class in the type annotation and in the
Autowired
call; and without the repetition of the class it wouldn't work well with linters...
Conclusion
As I do not intend to change how injectable works right now and since Proposal B seems easier to implement and also to be more aligned to PEP-593 I'll reject this proposal in favor of Proposal B.
[REJECTED] Proposal D: denote autowired arguments in the @autowired
decorator, dispensing the use of Autowired
This alternative was already considered and rejected at the very beginning of this project as it has too many drawbacks but it is worth mentioning that it was given though and that this could be a way to comply with the PEP.
@autowired("service")
def foo(service: Service): ...
Pros
- No direct interference with annotations and default values
Cons
- Not backwards compatible
- Passing autowiring params would be messy (
@autowired({"service": {params**}})
???) - This wouldn't be refactoring friendly if someone is changing the parameter name
- Since the parameter name must be specified in string for this would make this process prone to typos
- Tedious repetition of the parameters names
- This is the uglier implementation of all, mainly when there are many args
- Autowiring wouldn't be easily spotted at the function definition no more
Conclusion
I'm rejecting this proposal for the same reasons it was rejected in the past.