Skip to content

Conversation

@pavalso
Copy link

@pavalso pavalso commented Dec 10, 2025

Copy link
Contributor

@ZipFile ZipFile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, but we need test coverage for this new feature.

@pavalso pavalso requested a review from ZipFile December 10, 2025 14:53

@inject
def test_provided_instance_call_with_args(
some_value: int = Provide[Container.service_with_callable.provided.method_with_args.call(1, 2)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we test for string ids? I.e. Provide["service_with_callable", provided().method_with_args.call(1, 2)]?

Comment on lines +8 to +9
self.method_with_args = lambda x, y: x + y
self.method_with_kwargs = lambda **kwargs: kwargs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to have them as proper methods


class ServiceWithCallable:
def __init__(self):
self.foo = CallableDict({"bar": lambda: 10})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.foo = CallableDict({"bar": lambda: 10})
self.foo = {"bar": lambda *args, **kwargs: {"args": args, "kwargs": kwargs}}

Does it need to be separate dict subclass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants