Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mypy] Missing positional argument "x" in call to "C" #18

Open
dosisod opened this issue Oct 21, 2021 · 6 comments
Open

[mypy] Missing positional argument "x" in call to "C" #18

dosisod opened this issue Oct 21, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@dosisod
Copy link
Contributor

dosisod commented Oct 21, 2021

I am getting mypy errors when using the inject decorator on classes:

# file.py

from kink import di, inject

di["x"] = 42

@inject
class C:
    def __init__(self, x: int) -> None:
        print(x)


c = C()

Running mypy with:

$ mypy file.py
file.py:13: error: Missing positional argument "x" in call to "C"
Found 1 error in 1 file (checked 1 source file)

Version info:

$ pip freeze
kink==0.6.2
mypy==0.910
mypy-extensions==0.4.3
toml==0.10.2
typing-extensions==3.10.0.2

$ python --version
Python 3.9.7

The only workaround I can see is this:

class C:
    def __init__(self, x: int = di["x"]) -> None:

Which means the inject decorator isn't needed.

@dkraczkowski
Copy link
Contributor

hi @dosisod, what is the usecase for initialisation C directly?

I would recommend getting it from di instead, like in the example below:

c = di[C]

The reason for that is container is quite capable of instantiating complex dependencies with nested structures and by using automated initialisation you possibly cannot make any mistake.

Valid reason to instantiate dependencies manually is testing, but in this scenario you should pass all the required dependencies whether they are mocked dependencies or not and probably you shouldn't use DI in that scenario or at least you should mock it.

@dosisod
Copy link
Contributor Author

dosisod commented Oct 23, 2021

I did not know that services that where injected got added to the di object. The issue now is that c will be type Any, and I will need to cast it:

from kink import di, inject

di["x"] = 42

@inject
class C:
    def __init__(self, x: int) -> None:
        print(x)

c = di[C]
reveal_type(c)

c2: C = di[C]
reveal_type(c2)
$ mypy file.py
file.py:11: note: Revealed type is "Any"
file.py:14: note: Revealed type is "file.C"

@dkraczkowski
Copy link
Contributor

dkraczkowski commented Oct 25, 2021

@dosisod There are two possible solutions for now:

  • add mypy plugin that adds kink support (and I would really appreciate help on that)
  • second is obviously not perfect and you have already provided it.

The way how we use this library is mostly in controllers and dependencies are injected as function's arguments and those have type annotations so mypy handles those gracefully. Our controllers are either executed by framework code or lambda layer in AWS, so again this is outside our project code.

I assume creating a mypy plugin to fix this issue shouldn't be a big deal, but at the moment I have couple other priorities so I would appreciate support on this. Eventually you can give it a time and I believe in next couple weeks I might sort it out.

@dkraczkowski dkraczkowski added the enhancement New feature or request label Oct 25, 2021
@and3rson
Copy link

and3rson commented Oct 8, 2022

First of all, thanks for the awesome library!

I've tried writing a plugin for mypy which will modify signatures of functions decorated with @inject, but there is a problem: it's not possible to differentiate which specific arguments are going to be injected and which are going to be provided by the caller.

I've outlined several possible solutions here: #30 (comment). Please let me know you thoughts!

@skewty
Copy link

skewty commented Aug 16, 2023

This issue is now very much related to #36

@dkraczkowski
Copy link
Contributor

@skewty I have just seen the issue. I think I will need to spend some time with mypy plugin system, once I get a bit of my life back ^^. I will keep you in the loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants