nocarryr/python-dispatch

observable collection item removal

Opened this issue · 1 comments

It seems the event for item removal doesnt include the removed item.

from pydispatch import Dispatcher
from pydispatch.properties import DictProperty

#old_dict = sys.modules
old_dict = {}

class MyDictListener(object):
    def on_new_data(self, emitter, dict_, **kwargs):
        new_keys = kwargs.get('keys')
        if new_keys:
            for key in new_keys:
                print('new item: {}={}'.format(key, dict_[key]))
        else:
            print('unknown', kwargs)

class MyDictWrapper(Dispatcher):
    new_dict = DictProperty(old_dict)

listener = MyDictListener()
dict_wrapper = MyDictWrapper()
dict_wrapper.bind(new_dict=listener.on_new_data)

new_dict = dict_wrapper.new_dict

new_dict['a'] = 1
del new_dict['a']
new item: a=1
unknown {'old': None, 'property': <<class 'pydispatch.properties.DictProperty'>: new_dict>}

I saw I can have old populated, and then I could do a diff in the listener, but that seems wasteful as the underlying collection knows which item is being removed.

@jayvdb my apologies for lack of response, I'm not sure how this issue got unnoticed.

That behavior was intentional at the time for consistency. The keys argument is meant to indicate items that were either added or altered in the container (for both ListProperty and DictProperty) to help the callback handler deal with the container's current state. (and not have to worry about handling KeyError exceptions)

This is somewhat documented, but I'll admit that it could be more clear.

It would be trivial to provide the removed keys, but I think it would be best to have them in a separate argument (something like removed_keys).

And yes, using copy_on_change to get the old value can be quite wasteful since it has to create a copy of the container for every callback