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

Cannot connect event callbacks to MagicFactory #155

Closed
tlambert03 opened this issue Feb 18, 2021 · 8 comments · Fixed by #159
Closed

Cannot connect event callbacks to MagicFactory #155

tlambert03 opened this issue Feb 18, 2021 · 8 comments · Fixed by #159
Labels
bug Something isn't working

Comments

@tlambert03
Copy link
Member

As noticed by @uschmidt83, the @magic_factory decorator is not compatible with the current callback connection approach (widget.param.changed.connect). That's a big bummer and needs to be fixed ASAP. One possibility would be to allow for MagicFactory to accept a widget_init function, that would be called with the widget instance each time a new one is created...

def init(widget):
    widget.x.changed.connect(print)

@magic_factory(init=init)
def func(x: int):
    ...
@tlambert03 tlambert03 added the bug Something isn't working label Feb 18, 2021
@jni
Copy link
Contributor

jni commented Feb 18, 2021

@tlambert03 I'm confused... Why doesn't the namespace mangling you did for me in #117 work here? ie I would have expected this to work, since func refers (should refer??) to the instance within func:

from magicgui import magic_factory

@magic_factory
def func(x: int):
    func.x.changed.connect(lambda *args: print(args))


widget = func()
widget.show(run=True)

Indeed this doesn't work but I don't understand why.

@tlambert03
Copy link
Member Author

that doesn't work because the function itself hasn't been called yet. widget = func() calls the factory, creating an instance of FunctionGui ... but until the functiongui itself has been called func hasn't been called and nothing has been hooked up:

from magicgui import magic_factory

@magic_factory
def func(x: int):
    func.x.changed.connect(lambda *args: print(args))


widget = func()
widget()  # this would make it work ... but now we're connecting it _every_ time
widget.show(run=True)

@tlambert03
Copy link
Member Author

for the sake of completeness, I'll mention something here that I mentioned on zulip:
Because the purpose of @magic_factory is to return something that makes instances of FunctionGui when called, one could also just subclass FunctionGui directly here:

from magicgui.widgets import FunctionGui

def my_function(...):
    ...

class MyGui(FunctionGui):
    def __init__(self):
        param_options = {
            ...
        }
        super().__init__(my_function, call_button=True, layout='vertical', param_options=param_options)
        # do your connections here with self.

@uschmidt83
Copy link
Contributor

uschmidt83 commented Feb 19, 2021

Maybe that's a stupid question, but what's the purpose of a dedicated @magic_factory decorator?
What can it do that can't be done with a simple wrapper (as @tlambert03 suggested on zulip):

from magicgui import magicgui, magic_factory

@magic_factory()
def func(x: int):
    pass

def func_wrapper():
    @magicgui()
    def func(x: int):
        pass
    # func.x.changed.connect(lambda *args: print(args))
    return func

print(func())
print(func_wrapper())

@tlambert03
Copy link
Member Author

tlambert03 commented Feb 19, 2021

It's just a convenience function. Think of it like functools.partial. It is indeed very much like that simple wrapper, but it will remember all of the kwargs passed to it, but also allow you to override them when you call the factory. For instance:

@magic_factory(call_button=True)
def my_factory(x: int):
    ...

widget = my_factory()
widget2 = my_factory(call_button=False, x={'widget_type': 'Slider'})

We originally added it for @jni's use case in affinder (where it actually was quite convenient because he didn't need to create event callbacks before the function itself was actually called).

@tlambert03
Copy link
Member Author

tlambert03 commented Feb 19, 2021

Keep in mind that magicgui is being developed with the hope/intention that it be useful beyond just napari. So it's really this special case with provide_dock_widget where the user isn't (yet) provided a way to do anything with the widget once it's been instantiated that this feels a bit silly. So there, we need to provide a fully declarative-like API, where everything is setup once the widget is created.

Outside of napari, you can definitely imagine someone using this factory, and then being able to connect events to the widget instances, in which case the factory is a convenient way to create multiple widgets based on a single template (the original function), which are possibly a bit different from each other (based on the arguments to the factory call itself)

@uschmidt83
Copy link
Contributor

...it will remember all of the kwargs passed to it, but also allow you to override them when you call the factory.
...in which case the factory is a convenient way to create multiple widgets based on a single template...

That makes sense, thanks for the explanation.
But it then probably needs some sort of init function as you suggested in the first post of this issue.

@tlambert03
Copy link
Member Author

yeah, I'm thinking that's a nice way to keep the functional interface. Will add it soon. Thanks for revealing the need!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants