WIP Eventful dict and custom events

[SylvainCorlay]

from traitlets import *

class Foo(HasTraits):
    bar = EDict()

    @observe('bar', type='element_set')
    def o_change(self, change):
        print(change)

    @observe('bar', type='element_del')
    def o_changes(self, change):
        print(change)

foo = Foo()

foo.bar['key'] = 'value'
del foo.bar['key']

{'owner': <__main__.Foo object at 0x107e65510>, 'new': 'value', 'type': 'element_set', 'name': 'bar', 'key': 'key'}
{'owner': <__main__.Foo object at 0x107e65510>, 'type': 'element_del', 'name': 'bar', 'key': 'key'}

The basic change event dictionary now contains the method to rollback the change which is not hardcoded anymore, so that this event type is not special.

This is backward incompatible for people who were calling notify_change directly with the old requirements for the dict content since traitlets now expect the rollback method as part of the dict.

[rmorshea]

@Carreau mentioned rwatch for Py>=3.5.

[SylvainCorlay]

@minrk I would love to have your opinion on this approach. The eventful dict is observable through observe via type=element_set.

Changes are rolled back in case of cross-validation error when exiting hold_trait_notifications.

[SylvainCorlay]

cc @jdfreder

[SylvainCorlay]

@minrk at the moment, the pre-set and pre-del call the trait validation prior to accepting a change to the dict.

However this raises a question: traitlets validation not only raises error for invalid input but may coerce it... this is an issue in that in this implementation, I am not using the return value of _validate...

[minrk]

@SylvainCorlay can you use the return value, then?

[SylvainCorlay]

@SylvainCorlay can you use the return value, then?

I would not want to assign the new value since it is supposed to be an element change initially. Although this would probably work.

The element set / del could tap into their own validation logic (such as @validate_element)... Or I could apply a diff of the return value from the validate with the current value...

[SylvainCorlay]

@ellisonbg you might be interested in that one.

[rmorshea]

@SylvainCorlay implementing edict with spectate

spectate represents a generic way to establish callbacks for any method, on any type of instance.

This does not replace eventful trait types, and would instead be the internal foundation for them.

[SylvainCorlay]

@SylvainCorlay implementing edict with spectate
spectate represents a generic way to establish callbacks for any method, on any type of instance.
This does not replace eventful trait types, and would instead be the internal foundation for them.

Does this provide pre / post hooks for validation / observe methods?
I only wrap two methods in edict: (set and del) and override clear to use them. Do you think that there is a need for a library to do this?

[rmorshea]

@SylvainCorlay, yes. There are pre / post callbacks.

There are some small advantages:

• Pre-callbacks pass a value to their respective post-callback, which removes the need for caching, and keeps them thread-safe.
• The base methods docstring's are retained for introspection.
• Being able to register multiple callbacks for a single method.

However the overall usefulness of spectate depends on whether this kind of custom notification would be useful:

class A(HasTraits):
    e = Eventful(Instance(MyClass))

    @custom_event(type='custom', on='e.my_method', before=True)
    def my_callback(self, call):
        return my_bunch

    @observe('e', type='custom')
    def my_observer(self, my_bunch):
        # do something special

# trigger the event and subsiquent observer
A().e.my_method(*args, **kwargs)

[rmorshea]

Also "spectate" isn't a seperate library we'd have to support. It's just a single file module I wrote.

[rmorshea]

This will fail in the case that a default value was defined via TraitType.__init__ since those are only assigned to the HasTraits instance during instance_init. I'm not sure this can be resolved given the current way default values are handled. However with #332, popping off of _trait_values would work.

[rmorshea]

Issue resolved - #322 was merged.

[rmorshea]

Thoughts on whether this should be included in 5.0 (if at all)?

I've moved my alternative proposal to jupyter-widgets/ipywidgets#1071 under the assumption that it would have more freedom, and exposure. Once in a more stable state it could be merged upstream into traitlets.

Either way, IMO this should wait for a release > 5.0.

cc: @minrk, @SylvainCorlay, @ellisonbg

[minrk]

It makes a lot more sense to me for this to be in widgets (at least to start).

[Carreau]

Tagging as closed-PR, and closing as this is seem to have stalled a bit.
This is just to keep the PR queue a bit shorter and focusing on what is actually worked on.
Feel free to reopen, (or ask for reopening if you don't have enough right to do so).
If you were not the original author, and want to revive this, please feel free to do so.

Also, we are doing our best to enable people to learn how to contribute to the project, and get commits in. If you are missing time or have any other issues that prevent you to work on that, please feel free to tell us, we can try to build on top of what you did.

Thanks a lot and looking forward to see this moving forward again.

[ellisonbg]

Thanks Matthias! I am in favor of closing issues that have gone inactive like this - they can always be reopened as needed in the future.

…

On Tue, Jul 16, 2019 at 2:23 PM Matthias Bussonnier < ***@***.***> wrote: Tagging as closed-PR, and closing as this is seem to have stalled a bit. This is just to keep the PR queue a bit shorter and focusing on what is actually worked on. Feel free to reopen, (or ask for reopening if you don't have enough right to do so). If you were not the original author, and want to revive this, please feel free to do so. Also, we are doing our best to enable people to learn how to contribute to the project, and get commits in. If you are missing time or have any other issues that prevent you to work on that, please feel free to tell us, we can try to build on top of what you did. Thanks a lot and looking forward to see this moving forward again. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#278?email_source=notifications&email_token=AAAGXUEGMR5XRGDCM32UW7DP7Y35BA5CNFSM4CM3LRN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2CGKTY#issuecomment-511993167>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAGXUGNMQ4NDXSRCJUYHJTP7Y35BANCNFSM4CM3LRNQ> .

-- Brian E. Granger Principal Technical Program Manager, AWS AI Platform ([email protected]) On Leave - Professor of Physics and Data Science, Cal Poly @ellisonbg on GitHub

[Carreau]

Thanks Matthias!

Thanks much appreciate; closing old PRs like this is always hard as we care about all contributions; I also don't think it should be done by a bot.