traits icon indicating copy to clipboard operation
traits copied to clipboard

Relax requirement on the signature of handler for traits.observation.observe.observe

Open kitchoi opened this issue 3 years ago • 3 comments

For traits.observation.observe.observe, the handler argument signature can be relaxed so long as dispatcher can collaborate with it: https://github.com/enthought/traits/blob/72b1fe35ff1618fd01554102243266bd35b2ca95/traits/observation/observe.py#L29-L49

Proposed new documentation:

handler : callable(*args, **kwargs)
         User-defined callable to invoke when there is a change event.
         A weak reference is held for the handler such that it can be garbage collected.
...
dispatcher : callable(handler, event)
         Callable for invoking the user-defined handler.
         The first argument is the `handler` given to this function.
         The second argument is an event object representing the change.
         Its type and content depends on the change. 

So long as dispatcher can handle the signature of handler, it does not matter what handler call signature is.

I believe this behaviour is already true. We just need to document and test it in order to formally support it.

The motivation for this issue comes from an attempt to reimplement Envisage's ExtensionRegistry. If the signature is relaxed, we can call observe(..., handler=listener, ..., dispatcher=dispatcher) where listener has a signature of (extension_registry, extension_point_event) and dispatcher can be responsible for calling listener with the appropriate values. We want to avoid holding a strong reference to the listener. observe already does that for handler.

This proposal does NOT apply to the handler signature on HasTraits.observe. This is because HasTraits.observe offers a convenient mapping from str to dispatcher where the dispatcher requires the handler(event) signature.

kitchoi avatar Oct 23 '20 12:10 kitchoi

Proposed new documentation:

handler : callable(*args, **kwargs) User-defined callable to invoke when there is a change event. A weak reference is held for the handler such that it can be garbage collected.

There is an error in the proposed sentence: Currently for observe, a weak reference is only held if the handler is an instance method. For Envisage, however, a weakref is used even for non-instance-method. I am investigating whether that is strictly necessary in that context.

kitchoi avatar Oct 23 '20 17:10 kitchoi

I am investigating whether that is strictly necessary in that context.

Looks like that is not justified. I tried to find usage where the handler is a non-method and I could only find one, and in that occurrence, a strong reference to the handler is added immediately to counter the weakref: https://github.com/enthought/envisage/blob/aadc73d48784eafc1dca6c293a11a7d175a28bda/envisage/extension_point.py#L223-L229

kitchoi avatar Oct 26 '20 11:10 kitchoi

The work in this PR motivates this issue: https://github.com/enthought/envisage/pull/347

kitchoi avatar Oct 26 '20 15:10 kitchoi