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

Register a processor which has injections #50

Closed
davidbrochart opened this issue Feb 25, 2023 · 5 comments
Closed

Register a processor which has injections #50

davidbrochart opened this issue Feb 25, 2023 · 5 comments

Comments

@davidbrochart
Copy link
Contributor

  • in-n-out version: 0.1.6
  • Python version: 3.11
  • Operating System: Ubuntu 22.04

Description

Consider the following code which registers providers and processors:

import in_n_out as ino

COUNTER = 0

class Dummy:
    pass

class Thing:
    def __init__(self):
        self.list = []

def dummy_provider() -> Dummy:
    return Dummy()

@ino.inject
@ino.inject_processors
def thing_provider(dummy: Dummy) -> Thing:
    return Thing()

ino.register_provider(dummy_provider)
ino.register_provider(thing_provider)

def add_item(thing: Thing):
    global COUNTER
    thing.list.append(COUNTER)
    COUNTER += 1

ino.inject_processors(add_item)

ino.register_processor(add_item)
ino.register_processor(add_item)
ino.register_processor(add_item)

@ino.inject
def func(thing: Thing):
    print(thing.list)

func()
# prints [0, 1, 2]

Now if I want to inject a Dummy object in add_item:

@ino.inject
def add_item(dummy: Dummy, thing: Thing):
    global COUNTER
    thing.list.append(COUNTER)
    COUNTER += 1

It doesn't work anymore, as dummy and thing can be injected and the signature doesn't match with a Thing. This works:

def add_item(thing: Thing):
    @ino.inject
    def inner(dummy: Dummy):
        global COUNTER
        thing.list.append(COUNTER)
        COUNTER += 1
    inner()

But I was wondering if there could be a better solution?

@tlambert03
Copy link
Member

tlambert03 commented Feb 26, 2023

thanks @davidbrochart
can you slightly reword or recode that last bit where "it doesn't work anymore", perhaps in the form of an assertion statement? I think there's something here, but there's a lot going on in this example and I'm not entirely sure I'm following. Here's a test I wrote to confirm that processors can indeed themselves have injection requirements, and it currently passes on main ... but I suspect I've missed the crux of the issue here.

perhaps you could modify this test a bit to make it fail according to your expectations?

def test_processors_with_injections() -> None:
    """Test that we can use processors with injections."""

    class ThingToProcess:
        ...

    class InjectedDep:
        ...

    dep = InjectedDep()
    t2p = ThingToProcess()
    mock = Mock()

    # this function processes a ThingToProcess, but also requires
    # an InjectedDep to be injected before it will work
    @ino.register_processor
    @ino.inject
    def processor_that_has_injections(p0: ThingToProcess, p1: InjectedDep) -> None:
        mock(p0, p1)

    # "inject processors" of this function... 
    # meaning: when `return_a_thing ` is called, the t2p return value should be processed
    #  (as a side effect of calling the function)
    @ino.inject_processors
    def return_a_thing() -> ThingToProcess:
        return t2p

    # at the moment, our `processor_that_has_injections` function still doesn't have a
    # p1 InjectedDep, so `return_a_thing`'s return value won't be processed successfully
    with pytest.warns(UserWarning, match="Processor .* failed to process result"):
        return_a_thing()

    # but if we register a something that provides an InjectedDep...
    @ino.register_provider
    def provide_other_thing() -> InjectedDep:
        return dep

    # then the processor should work as expected ...
    return_a_thing()
    mock.assert_called_once_with(t2p, dep)

@davidbrochart
Copy link
Contributor Author

Sorry, I could probably simplify the example again, but here is a test that fails (the two commented blocks work fine):

import in_n_out as ino

def test_processors_with_injections() -> None:

    class Dummy:
        pass

    class Thing:
        def __init__(self):
            self.list = []

    def dummy_provider() -> Dummy:
        return Dummy()

    @ino.inject
    @ino.inject_processors
    def thing_provider(dummy: Dummy) -> Thing:
        return Thing()

    ino.register_provider(dummy_provider)
    ino.register_provider(thing_provider)

    # def add_item(thing: Thing):
    #     thing.list.append("item")

    @ino.inject
    def add_item(dummy: Dummy, thing: Thing):
        thing.list.append("item")

    # def add_item(thing: Thing):
    #     @ino.inject
    #     def inner(dummy: Dummy):
    #         thing.list.append("item")
    #     inner()

    ino.register_processor(add_item)
    ino.register_processor(add_item)
    ino.register_processor(add_item)

    @ino.inject
    def func(thing: Thing):
        return thing.list

    assert func() == ["item"] * 3

I think it's the normal behavior, because with:

@ino.inject
    def add_item(dummy: Dummy, thing: Thing):
        ...

One cannot say that thing is the thing to process.

@tlambert03
Copy link
Member

tlambert03 commented Feb 26, 2023

ahhh, I see! sorry, I missed that. So, to restate the problem slightly, you want to specify that add_item is a processor of the second argument in its signature (the Thing not the Dummy). In that case, you can use the type_hint argument for register_processor. By default, processors with multiple arguments are assumed to process the first argument in their signature, but you can override that inference with:

ino.register_processor(add_item, type_hint=Thing)

(... this is documented, but it's a bit hard to find at the moment when you use the global ino.register_processor command)

type_hint : object
A type or type hint that `processor` can handle.

however! ... doing that in your then reveals a RecursionError in your test. So let me look into that a bit more

@tlambert03
Copy link
Member

OK, looks like I just didn't prepare for processor functions that process anything other than their first argument:

for processor in self.iter_processors(type_hint): # type: ignore
try:
processor(result)
except Exception as e: # pragma: no cover
if raise_exception:

I think I could fix this by additionally storing the name of the function parameter that the processor processes ('thing' in your case), and then calling the processor with processor(**{param_name: result})

@tlambert03
Copy link
Member

in summary, this boiled down to 2 things:

  1. while processors may override the associated type that they process (using the type_hint argument when registering), they must process their first argument. A "feature" was implemented in feat: allow registration of a processor function that doesn't process its first argument #52, but opted against at this time
  2. there was a latent recursion error when a provider has its output processed. that is was fixed in fix: avoid recursion when a provider also uses processors #51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants