hanjinliu / magic-class

Generate multifunctional and macro recordable GUIs from Python classes
https://hanjinliu.github.io/magic-class/
BSD 3-Clause "New" or "Revised" License
39 stars 5 forks source link

Listen to a field change in a sibling magicclass #129

Open multimeric opened 10 months ago

multimeric commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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.