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

Widget observer framework #733

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Widget observer framework #733

wants to merge 3 commits into from

Conversation

corranwebster
Copy link
Contributor

This adds the infrastructure for simple widget observer configuration via trait metadata.

This adds code to the MWidget implementation of the current private API methods _add_event_listeners and _remove_event_listeners to automatically hook up methods to trait observers where the widget_observer metadata holds the name of the method to handle the calls.

Some changes are needed to the various Widget classes to make sure that things are always called correctly: the addition of several super() calls, and the addition of a base implementation of dispose on MWidget.

Other than in a test, this does not change any widget to use the framework, and so should have no impact on behaviour.

This is the first step of #732.

@kitchoi
Copy link
Contributor

kitchoi commented Oct 12, 2020

I believe Traits is already dropping Python 3.5, which means we can use descriptors with __set_name__ to do things that used to require metaclasses.

If I understand the intention here, we can alternatively have a decorator @observe_later that retains most of the features of @observe but allows us to defer setting up the observers until later (much later than post_init):


class Person(HasStrictTraits):
    name = Str()

# see the implementation of Mixin in the details block below
class SubClass(Mixin, HasStrictTraits):
    people = List(Instance(Person), comparison_mode=1)

    @observe_later("people.items.name")
    def greeting3(self, event):
        """ people.items.name change. """
        print("people.items.name changed", event)


s = SubClass()
another = SubClass()

# -------------------------------------
# Nothing is hooked up yet.
# Nothing are printed for the following.

s.people = [Person()]
another.people = [Person()]
# --------------------------------------

s.start_observing()
print("Started")

# --------------------------------------
# Only observers for `s` are hooked up.
# Changes on the observed traits on `s` are called.
# Changes on `another` are not observed.

s.people[0].name = "Mary"
another.people[0].name = "Chris"
# --------------------------------------

s.stop_observing()
print("Stopped")

# --------------------------------------
# Back to how it was.
# Nothing should be printed

s.people[0].name = "Ben"
another.people[0].name = "Holly"
Implementation for `@observe_later`

from functools import wraps

class Mixin:
    """ A mixin for a subclass which has the ``observe`` method
    for dynamically adding traits observer.

    e.g. the subclass is a subclass of HasTraits / HasStrictTraits /... as
    well.
    """

    def start_observing(self):
        for name, expression, dispatch in self.__class__._deferred_observers:
            self.observe(
                handler=getattr(self, name),
                expression=expression,
                dispatch=dispatch,
            )

    def stop_observing(self):
        for name, expression, dispatch in self.__class__._deferred_observers:
            self.observe(
                handler=getattr(self, name),
                expression=expression,
                dispatch=dispatch,
                remove=True,
            )


class _Descriptor:

    def __init__(self, func, expression, dispatch):
        self.func = func
        self.expression = expression
        self.dispatch = dispatch

    def __get__(self, instance, owner):
        return self.func.__get__(instance, owner)

    def __set_name__(self, owner, name):
        if not hasattr(owner, "_deferred_observers"):
            owner._deferred_observers = []
        owner._deferred_observers.append(
            (name, self.expression, self.dispatch)
        )


def observe_later(expression, *, dispatch="same"):
    """ Defer observe until the 'start_observing' method on an instance of
    Mixin is called. Allow the observer to be removed when the 'stop_observing'
    method is called.
    """

    def decorator(func):
        return wraps(func)(_Descriptor(func, expression, dispatch))

    return decorator

The benefit is that the name of the method is irrelevant, it can support more than one handlers for observing the same trait, it can observe extended traits and the side-effect intended is colocated/transparent.

One point to note that if the method used as the change handler is to be overridden by subclasses, the @observe_later needs to be used in subclasses as well. This is concern for a forward/backward compatibility though (which probably applies to the approach here too).

What do you think about this?

@corranwebster
Copy link
Contributor Author

What do you think about this?

Had some time to digest the approach and think about it a bit. It's interesting, and generically useful, and I could see some system like this having a place in Traits. I think it would want a little generalization to allow grouping: perhaps using a dictionary of handlers instead of a list, and do start_observing('foo') and stop_observing('foo'). Or you could imagine building this into the observe framework - it already has logic for connecting/disconnecting listeners so you could have conditionals in observer expressions that turn the expression off based on the state of another trait.

In terms of capabilities, the only real improvement of this approach over the one using metadata is the ability to observe expressions rather than single traits - multiple observers could be handled by having a list of handlers in the metadata, and the first implementation of this allowed any callable as a handler in addition to named methods. But I can see the utility in being able to handle foo.items listeners within this infrastructure.

All of that said, I also realised that most of the desired goals from dynamically turning observers on or off could be obtained by writing a decorator that returns immediately if the control is None, something like:

def is_not_none(trait_name):
    def _decorator(func):
        @wraps(func)
        def _guarded_func(self, event):
            if getattr(self, trait_name, None) is not None:
                func(self, event)
        return _guarded_func
    return _decorator

class FooWidget(...):
    text = Str()

    @is_not_none('control')
    def _text_changed(self):
        self.control.setText(self.text)

or some variation thereof. Coupled with almost all use-cases being effectively covered by #737, I think that this PR might not be quite the right thing in many cases.

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.

None yet

2 participants