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

Listen to a field change in a sibling magicclass #129

Open
multimeric opened this issue Nov 3, 2023 · 10 comments
Open

Listen to a field change in a sibling magicclass #129

multimeric opened this issue Nov 3, 2023 · 10 comments

Comments

@multimeric
Copy link

multimeric commented Nov 3, 2023

If I have classes like this:

@magicclass
class A:
    @magicclass
    class B:
        b = vfield(...)
    @magicclass
    class C:
        def on_b_change(self, value):
            ...

How can I implement on_b_change so that it will trigger when B.b actually changes? I don't think that @wraps works here, but I could be wrong. Also I tried using __magicclass_parent__ in the __post_init__ to search up the tree for the field, but the parent relationship doesn't seem to exist at that point.

@hanjinliu
Copy link
Owner

I think you can just define the callback in the parent class.
If you need some methods or attributes defined in C, they are available via self.C.

from magicclass import magicclass, vfield

@magicclass
class A:
    @magicclass
    class B:
        b = vfield(int)
    @magicclass
    class C:
        pass

    @B.b.connect
    def _on_b_change(self, value):
        print(self.C, value)
ui = A()
ui.show()

Do you think this works for your case?

@multimeric
Copy link
Author

This mostly works, although my nested magicclasses are mostly inside vfield, for reasons discussed in #110, which makes connecting to the right event handler impossible. At least I can listen for top level changes to the nested classes though.

@hanjinliu
Copy link
Owner

Is this example similar to your case?

from magicclass import magicclass, field

@magicclass
class A:
    x = field(int)

@magicclass
class Main:
    a = field(A)
    # A.x is not accessible here

ui = Main()
ui.show()

@multimeric
Copy link
Author

Yes basically. I have

@magicclass
class A:
    x = field(int)

@magicclass
class Main:
    a = vfield(A)

    # This works
    @a.connect()
    def _on_a_changed():
        ...

    # This doesn't work
    @a.x.connect()
    def _on_x_changed():
        ...

@hanjinliu
Copy link
Owner

I got it.
There are two ways to do that.

1. Define a field in the parent.

Since last release, location=... argument was introduced to unify the parent/children connection of methods using @wraps and the field's equivalent one (this feature is not documented yet).
NOTE: Using abstractapi is a safer way to do this pre-definition things, as described here. It essentially does nothing.

from magicclass import magicclass, field, abstractapi

@magicclass
class A:
    x = abstractapi()

@magicclass
class Main:
    a = field(A)
    x = field(int, location=A)

    @x.connect
    def _on_x_changed(self):
        print(self, "on x changed")

ui = Main()
ui.show()

2. Use find_ancestor method.

All magicclasses have find_ancestor method (see here). You can keep the class structure of Main tidy this way (you can even define class A in a separate file).

from magicclass import magicclass, field

@magicclass
class A:
    x = field(int)
    
    @x.connect
    def _on_x_changed(self):
        main = self.find_ancestor(Main)
        print(main, "on x changed")

@magicclass
class Main:
    a = field(A)

ui = Main()
ui.show()

@multimeric
Copy link
Author

multimeric commented Nov 9, 2023

Thanks, but how can I now put these ideas together to listen to a sibling's event? I can move the field into the parent using location, but the child can't @connect to its parent anyway.

@magicclass
class A:
    x = field(int)

@magicclass
class B:
    # How to implement this?
    def on_x_changed(...): ...

@magicclass
class Main:
    a = vfield(A)
    b = vfield(B)

@hanjinliu
Copy link
Owner

In that case you'll have to rely on find_ancestor.

@magicclass
class A:
    x = field(int)

    @x.connect
    def _on_x_changed(self):
        self.find_ancestor(Main).b._on_x_changed()

@magicclass
class B:
    def _on_x_changed(self):
        print(self)

@magicclass
class Main:
    a = vfield(A)
    b = vfield(B)

ui = Main()
ui.show()

Regarding to #110, can you check if directly nesting magicclasses works?

@magicclass
class Main:
    @magicclass
    class A:
        x = field(int)
    @magicclass
    class B:
        pass
    @A.x.connect
    def _do_something_in_b(self):
        print(self.B)

Magic classes treat child magic classes differently from other simple widgets. You may not have to hide classes in fields if not needed.

@multimeric
Copy link
Author

Thanks. I've already tried removing the vfield and it indeed triggers the RuntimeError: wrapped C/C++ object of type QLabel has been deleted bug when I run my test suite, which creates and destroys objects very rapidly.

@multimeric
Copy link
Author

I wonder if it would be helpful to allow child classes to listen to parent events in some way. Because if this were possible then I could decouple everything properly: the first child class being changed could fire an event which the parent captures and then it triggers its own event which the other is connected to.

@hanjinliu
Copy link
Owner

I've already tried removing the vfield and it indeed triggers the RuntimeError: wrapped C/C++ object of type QLabel has been deleted bug when I run my test suite, which creates and destroys objects very rapidly.

Can you show me how you put Label widget in your class? I'm curious when this problem happens.

I wonder if it would be helpful to allow child classes to listen to parent events in some way.

You can still manually connect signals in the top-level class as below.

from magicclass import magicclass, field, vfield

@magicclass
class A:
    x = field(int)

@magicclass
class B:
    def _on_x_changed(self):
        print(self)

@magicclass
class Main:
    a = vfield(A)
    b = vfield(B)
    
    def __post_init__(self):
        # signal connection(s).
        self.a.x.changed.connect(self.b._on_x_changed)

ui = Main()
ui.show()

However, I don't think it's a good idea to connect parent events to all the children by default. It means that any event in the parent class has to be recursively passed to all its offsprings, which causes very bad performance for complicated widgets. The reset_choices method of magicgui does this, but it's comparatively rare to be called.

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

No branches or pull requests

2 participants