Pylance complaining about missing parameters with injected classes

[kodyVS]

When using @inject I have issues with pylance getting mad that there are missing parameters for the classes I inject in. Is there a fix for this?

[dkraczkowski]

@kodyVS, Could you please provide a whole code snippet? It is hard to understand what is going on just by looking at the part of a screenshot.

[kodyVS]

main.py:

from kink import inject, di
di[TemperatureService] = TemperatureService()

Temperature service:

@inject
class TemperatureService(object):
    def __init__(self,
                 temperature_sensor_reader: TemperatureSensorReader,
                 temperature_repository: TemperatureRepository,
                 socketio_service: SocketIO):
        self.temperature_sensor_reader = temperature_sensor_reader
        self.temperature_repository = temperature_repository
        self.socketio = socketio_service

[dkraczkowski]

@kodyVS

If you annotate a class with the @inject decorator, it is automatically registered in the di. This means di[TemperatureService] = TemperatureService() is superfluous.

To test this you can replace the line with a temp_service = di[TemperatureService], and you will find out that temp_service variable holds the instance of Temperature service.

[and3rson]

@dkraczkowski I think the author means that the linter is complaining about missing arguments.

For example:

from kink import di, inject

class User:
    def __init__(self, name):
        self.name = name

class Repository:
    def save(self, user: User):
        pass

di[Repository] = Repository()

# ...

class Service:
    @inject
    def create_user(self, name: str, repository: Repository):
        user = User(name=name)
        repository.save(user)

# pylint complains here, too: "No value for argument 'repository' in method call (no-value-for-parameter)"
Service().create_user('Foo Bar')

This issue is common across other checkers as well, e. g. mypy: they are not aware that kink will inject those arguments during call.

I wonder what would be the best solution to this (without disabling the entire check like # pylint: disable=no-value-for-parameter).

Temporary workaround would be to add default stub values for injected arguments (e. g. repository: Repository = IRepository()), but this will fail if IRepository extends, say, typing.Protocol (which cannot be instantiated).

The bigger problem with static typing for methods with injected arguments

I've tried writing a plugin for mypy but had some other priorities so had to ditch the idea. What I've learned is that it's not possible to determine (at static check time) which arguments will be injected with @inject during invocation. Consider this function:

@inject
def do_work(user: User, repository: Repository, storage: Storage):
    # Which arguments are injected by `kink` and which are provided by the caller?
    pass

I wish there was some kind of an explicit generic class to "mark" arguments as injectable, for example:

@inject
def do_work(user: User, repository: Injectable[Repository], storage: Injectable[Storage]):
    # Okay, now we can update method signature from within `mypy` plugin
    # since we know which arguments are going to be injected!
    pass

...or maybe even:

@inject('repository', 'storage')  # Tell static checker that these specific arguments are optional
def do_work(user: User, repository: Repository, storage: Storage):
    pass

What do you people think?

[dkraczkowski]

@and3rson I think you are right and that's the problem that the author has. But my point is you should not be doing this- you should not be instantiating class without arguments. Instead, you should request it from the DI.

Instead of doing this:

di[TemperatureService] = TemperatureService()

the Author should just do this:

instance = di[TemperatureService]

There is no need to instantiate TemperatureService manually when you can simply request it from the di, and if mypy will complain that it does not know what the instance is simply tell it, like below:

instance: TemperatureService = di[TemperatureService]

Your example after refactoring

from kink import di, inject

class User:
    def __init__(self, name):
        self.name = name

@inject
class UserRepository:
    def save(self, user: User) -> None:
        pass

# ...

 @inject
class UserService:
    def  __init__(self, repository: UserRepository):
        self.repository = repository

    def create_user(self, name: str) -> User: 
        user = User(name=name)
        self.repository.save(user)
        return user

# pylint  no longer complains here

service: UserService = di[UserService]
service.create_user("Foo Bar")

How it works

When you use the @inject decorator, it injects dependencies and registers the class itself in the di container. This means every class annotated with @inject can be requested from the DI container (kink.di), without the need to define a factory for them. Please consider the following example:

from kink import di, inject

@inject
class B:
    def __init__(self):
        ...

@inject
class A:
    def __init__(self, b: B):
        self.b = b

assert isinstance(di[A], A)
assert isinstance(di[B], B)

Hope this helps

[dkraczkowski]

@and3rson Btw. I like the idea around the Injectable generic, and might use it just for the type-hint support :)

My problem with exposing too many symbols from the library is the following consequences:

• complexity increases (more use-case scenarios to support)
• more maintenance
• pollution in userland

The third one is the biggest from my perspective. My idea was to build a tiny and easy-to-use library which does not pollute your codebase. Of course, I have no power over how people are using the library, and I am open to PRs and suggestions but would like to keep it very simple.

[and3rson]

@dkraczkowski Thanks for an awesome explanation! I agree that this solves the issues with container parameters: we "hide" the instantiation from the static checker, so that works.

However, injecting arguments on a per-method basis is where this breaks. We're heavily using interfaces (typing.Protocol) in the following way:

# interface
class IRepository(Protocol):
    def save_user(self, user):
        raise NotImplementedError()

# implementation
@inject(alias=IRepository)
class DynamoDBRepository(IRepository):
    def save_user(self, user):
        boto3.resource('dynamodb').etc.etc()

# service
class UserService:
    @inject  # Notice that this is not a constructor, but a method
    def register_new_user(self, username, repo: IRepository):
        user = User(username)
        repo.save_user(user)

# controller
def handle_request():
    user_service = UserService()
    user_service.register_new_user(request.data['username'])  # Checkers complain about "missing argument 'repo'"

Technically, kink does a great job here: it solves the DI problem perfectly. The issue is that static analysis works only as long as the injection happens into the constructor. Once @inject is moved to a method, checkers start complaining about missing args.

In my situation, the only solution is to use @inject with classes instead of methods. This seems to be the recommended solution, but we really liked that we could decorate specific methods only: it made the method's dependencies more explicit and did not rely on the instance state, making it possible to write a code that's more "pure". Consider the following code:

# Current usage
@inject
class Foo:
    def __init__(self, a: A, b: B, c: C, d: D, e: E, f: F):
        # store all 6 in self to use later (boilerplate assignments)
    def do_stuff_involving_a_and_b(self):
        self.a.x()
        self.b.y()
    def do_stuff_involving_a_c_and_d(self):
        self.a.x()
        self.c.z()
        self.d.w()
    def do_other_stuff(self):
        print('OK')

# Desired usage - cleaner and more explicit, but breaks static analyzers due to auto-injection.
# This also simplifies refactoring: a method can be moved out of the class somewhere else
# and will still work due to not being dependent on the instance state.
class Foo:
    @inject
    def do_stuff_involving_a_and_b(self, a: A, b: B):  # Explicit dependencies for this method
        a.x()
        b.y()
    @inject
    def do_stuff_involving_a_c_and_d(self, a: A, c: C, d: D):  # Ditto
        a.x()
        c.z()
        d.w()
    def do_other_stuff(self):  # No @inject here, so no explicit dependencies
        print('OK')

Let me know what you think. Maybe I'm missing something. :)

My problem with exposing too many symbols from the library is the following consequences:

Totally agree: introducing more symbols would make it more confusing. My suggestion with Injectable was my attempt to find a "pythonic" solution for static analysis of methods but I'm not sure how really pythonic will it be. Maybe it will just overcomplicate things.

[dkraczkowski]

@and3rson I can prepare an injectable symbol in the library with my PR, so you can try it out and give me your feedback. Would also like to see how mypy plugin can be build for it.

[and3rson]

@dkraczkowski sounds great! Writing a mypy plugin will be pretty easy with this, since the plugin will know which fields will be injected. The algorithm for the plugin will be as follows:

• Register a hook for kink.inject.inject function
• When the hook is called and the decorated object is mypy.types.Callable, inspect its arguments
• Mark the arguments of type Injectable[X] as optional, effectively making mypy ignore them.

Does this make sense? I'm open to any other suggestions!

[dkraczkowski]

@and3rson That Makes perfect sense, I will have PR ready this afternoon.

[and3rson]

Here's a demo of the plugin I've tried to make. It still has some issues, but I'm working on it.

from mypy.nodes import ARG_OPT
from mypy.plugin import FunctionContext, Plugin
from mypy.types import CallableType, Instance

# Set this to contain full names of corresponding items
INJECT_FULLNAME = 'kink.inject.inject'
INJECTABLE_FULLNAME = 'kink.inject.Injectable'

class KinkPlugin(Plugin):
    def get_function_hook(self, fullname: str):
        if fullname == INJECT_FULLNAME:
            return self._inject_hook
        return super().get_function_hook(fullname)

    def _inject_hook(self, ctx: FunctionContext):
        try:
            func = ctx.arg_types[0][0]
        except IndexError:
            # FIXME: This is not an `@inject` form, but `@inject()`.
            # Do nothing since we don't have the function signature yet.
            return ctx.default_return_type
        for i, (arg_name, arg_type) in enumerate(zip(func.arg_names, func.arg_types)):
            # Check if argument is an instance of `Injectable[T]`
            if (
                arg_name not in ('self', 'cls')
                and isinstance(arg_type, Instance)
                and arg_type.type.fullname == INJECTABLE_FULLNAME
            ):
                # Mark as optional
                func.arg_kinds[i] = ARG_OPT
        return func

def plugin(version: str):
    return KinkPlugin

Small follow-up explanation:

This plugin may feel like a dumb automation for adding = None to injected arguments, but it's not: adding = None makes the argument's type Union[X, None] and gets mypy angry about the developer not checking it for None every time they use it.

The plugin approach, on the other hand, handles this well and tells mypy that "this argument is optional, but it does not default to None" (as confusing as it sounds).

This works because (as it turns out) every argument has default value AND kind, and these are NOT equal. Kind is what defines if the value is optional:

# Excerpt from mypy/nodes.py
ARG_POS: Final = ArgKind.ARG_POS
ARG_OPT: Final = ArgKind.ARG_OPT
ARG_STAR: Final = ArgKind.ARG_STAR
ARG_NAMED: Final = ArgKind.ARG_NAMED
ARG_STAR2: Final = ArgKind.ARG_STAR2
ARG_NAMED_OPT: Final = ArgKind.ARG_NAMED_OPT

On the other hand, default value syntax (= xxx) is something that sets the default value AND sets the argument's kind to ARG_OPT. TL;DR: Mypy plugin allows us to mark the argument as optional WITHOUT providing a default value for it, which is exactly what we want to achieve.

[dkraczkowski]

@and3rson please check https://github.com/kodemore/kink/pull/31#pullrequestreview-1141254634