A new trick to avoid dead reference errors

[theodox]

I've been toying with an idea for getting rid of dead ref errors without needing users to make classes.

Basically it looks like this:

    with BindingWindow(title = 'example window') as test_window:
        bound = ObservableCollection("A", "B")

        with VerticalForm() as main:
            Text(label = "The following items don't have vertex colors")
            list_view = VerticalList()
            list_view.bind.collection < bind() < bound
            with HorizontalStretchForm('buttons'):
                refresh = Button('refresh', l='Refresh')
                close = Button('close', l='Close')

    def close_window(*_, **__):
        cmds.deleteUI(test_window)

    def refresh_window(*_, **__):
        list_view.redraw()

    test_window.refresh = refresh_window
    test_window.close = close_window

    refresh.command += test_window.refresh
    close.command +=  test_window.close

by making test_window.refresh and test_window.close members of the object test_window, they don't fall out of scope but they don't need to be regular instance methods with self. I was thinking we could extend the handler syntax so it automatically did this for the user, so maybe:

 refresh.command *= refresh_window  

would add the static function refresh_window as a member variable (on refresh maybe instead of on the window), keeping it alive as long as the widget lived.

Thoughts?

[theodox]

from mGui.gui import *
from mGui.forms import *
from mGui.lists import *
from mGui.observable import *
from mGui.bindings import bind

def stash (widget, callback):
     if not hasattr(widget, 'static'):
         setattr(widget, 'static', [])
     widget.static.append(callback)
     widget.command += callback

def test():

    with BindingWindow(title = 'example window') as test_window:
        bound = ObservableCollection("A", "B")

        with VerticalForm() as main:
            Text(label = "The following items don't have vertex colors")
            list_view = VerticalList()
            list_view.bind.collection < bind() < bound
            with HorizontalStretchForm('buttons'):
                refresh = Button('refresh', l='Refresh')
                close = Button('close', l='Close')

    def close_window(*_, **__):
        cmds.deleteUI(test_window)

    def refresh_window(*_, **__):
        list_view.redraw()

    stash(refresh, refresh_window)
    stash(close, close_window)    

    test_window.show()

test()

[bob-white]

I like it. Ran into the same problem as you're second example. Was defining a quick dialog inside a function with sub-functions that were dropping out of scope.

Another option would be to add a second set to the event, this one would hold strong references to the functions, which would persist as long as the event does.

class Event(object):

    def __init__(self, **data):
        self._weak_handlers = set()
        self._strong_handlers = set()
        self.data = data
        self.data['event'] = self

    def _add_weak_handler(self, handler):
        if not callable(handler):
            raise ValueError("%s is not callable", handler)
        self._weak_handlers.add(get_weak_reference(handler))
        return self

    def _add_strong_handler(self, handler):
        if not callable(handler):
            raise ValueError('{!s} is not callable'.format(handler))
        self._strong_handlers.add(handler)
        return self

    def _remove_weak_handler(self, handler):
        wr = get_weak_reference(handler)
        delenda = [h for h in self._weak_handlers if h == wr]
        self._weak_handlers = self._weak_handlers.difference(set(delenda))
        return self

    def _remove_strong_handler(self, handler):
        self._strong_handlers.discard(handler)
        return self

    def metadata(self, kwargs):
        md = {}
        md.update(self.data)
        md.update(kwargs)
        return md

    def _fire(self, *args, **kwargs):
        delenda = []
        for handler in self._weak_handlers:
            try:
                handler(*args, **self.metadata(kwargs))
            except DeadReferenceError:
                delenda.append(handler)
        self._weak_handlers = self._weak_handlers.difference(set(delenda))
        for handler in self._strong_handlers:
            handler(*args, **self.metadata(kwargs))

    def _handler_count(self):
        return len([i for i in self._weak_handlers])

    __call__ = _fire
    __len__ = _handler_count
    __iadd__ = _add_weak_handler
    __isub__ = _remove_weak_handler

    __imul__ = _add_strong_handler
    __idiv__ = _remove_strong_handler

    def __del__(self):
        print 'event expired'

[theodox]

Both solutions have a risk of leaking handlers.

If we hard-reference in the handler and don't manually remove, deleted widget handlers will still be hanging around . Stashing them into the widgets will solve that problem, although the widgets themselves could be left around for a variety of reasons. The whole weakref thing is intended to prevent leaks, but it has obvious costs too :)

[bob-white]

Yeah, it also occurred to me that we'd never be able to remove something from the _strong_handler set without keeping some kind of easily accessible reference around, at which point, why not bind it with the weakref method.

I like the stash / static example, so I thought about how we could apply the *=, /= syntax to it. And it doesn't look like it would be to hard.

class Event(object):
    def __init__(self, **data):
        self._handlers = set()
        self.data = data
        self.data['event'] = self

    def _add_handler(self, handler):
        if not callable(handler):
            raise ValueError("%s is not callable", handler)

        self._handlers.add(get_weak_reference(handler))
        return self

    def _remove_handler(self, handler):
        wr = get_weak_reference(handler)
        delenda = [h for h in self._handlers if h == wr]
        self._handlers = self._handlers.difference(set(delenda))
        return self

    def metadata(self, kwargs):
        md = {}
        md.update(self.data)
        md.update(kwargs)
        return md

    def _fire(self, *args, **kwargs):
        delenda = []
        for handler in self._handlers:
            try:
                handler(*args, **self.metadata(kwargs))
            except DeadReferenceError:
                delenda.append(handler)
        self._handlers = self._handlers.difference(set(delenda))

    def _handler_count(self):
        return len([i for i in self._handlers])

    def _stash_handler(self, handler):
        sender = self.data['sender']
        if not hasattr(sender, 'static'):
            setattr(sender, 'static', defaultdict(list))
        sender.static[sender.key].append(handler)
        return self._add_handler(handler)

    def _unstash_handler(self, handler):
        sender = self.data['sender']
        sender.static.get(sender.key, []).remove(handler)
        return self._remove_handler(handler)

    __call__ = _fire
    __len__ = _handler_count
    __iadd__ = _add_handler
    __isub__ = _remove_handler
    __imul__ = _stash_handler
    __idiv__ = _unstash_handler

    def __del__(self):
        print 'event expired'

[theodox]

That's pretty much what i was figuring. I think we might want to make the key name a static field in the Event class (for clarity) and give it private name like '_events' or '_mGui_events' .

Or maybe we ought to explictly add it to all Control classes? We could put it in __slots__ for efficiency. That way we're not adding properties to any old object willy-nilly

[bob-white]

Added #44 as an actual reference instead of knocking code around in the issues. Nothing set in stone, and it still needs updated tests, but felt like a good idea to have some common code to work with. Biggest change from my last idea was changing the static stash to a dict of sets instead of dict of lists, keeps it more inline with the weakrefs set.

Also added a more direct reference to the sender proxy on the events, mostly so we didn't have to keep pulling it from the data dict. As its still a proxy this still avoids creating a direct cycle.

Also fixed Control to clear the static dict when forget gets triggered, keeping it inline with the callbacks dict.

[theodox]

Added a comment - is it safe to rely on sender being present?

[bob-white]

I don't think it is. The event system itself doesn't rely on it, though mGui uses it pervasively. Its a convention, not a requirement.

I updated the PR making the whole thing more opt-in, by passing in an optional stash parameter when initializing the Event, if no stash object has been provided, and someone tries to use the *= or /= syntax an exception gets raised. Not to fond of the names I've come up with for things, but the functionality feels better.

[bob-white]

Closing this now that #69 has been merged.