enthought / enthought/traits

Listeners on extended trait change handlers firing unexpectedly on List trait items

Open
#537 3 comments 0 reactions 0 assignees View on GitHub
topic: traits listener rework type: bug
Dominant language
Python
Stars
462
Forks
90
PR merge metrics
No merged PRs in 30d

Description

Given the following code:
```
from traits.api import HasTraits, List, Instance, Int

class Child(HasTraits):
values = List(Int)

class Parent(HasTraits):
child = Instance(Child)
values = List(Int)

p = Parent(
child=Child(values=[1, 2, 3]),
values=[4, 5, 6],
)

def printer(object, name, old, new):
print(object, name, old, new)

p.on_trait_change(printer, 'values')
print("Change list items, listener doesn't fire values_items, as expected")
p.values[1] = 100

p.on_trait_change(printer, 'child.values')
print("Change list items on an instance attr, listener fires child.values_items!")
p.child.values[1] = 100
```
and based on the documentation, we don't expect the trait change handler for `child.values` to fire when the `_items` fire. Based on the documentation, this should only happen in the case where the trait change handler takes no arguments (see https://docs.enthought.com/traits/traits_user_manual/notification.html#dynamic-handler-special-cases).

This behaviour is the root cause for the TraitsUI bugs described in https://github.com/enthought/traitsui/issues/403 and https://github.com/enthought/traitsui/issues/680 where the unexpected firing of a trait change handler is causing an unwanted refresh of the UI.

Looking into the way that the listeners are set up, it looks like it is the following line of code that is causing the `_items` change handler to be connected: https://github.com/enthought/traits/blob/94308a28e4cb1e071cfc1e107c8e19081089f88c/traits/traits_listener.py#L772-L780

Further, it appears that the issue may be that the value of `self.type` may not be being assigned correctly;

- The `type` trait is derived from the signature of the handler on the `ListenerNotifyWrapper` class here: https://github.com/enthought/traits/blob/94308a28e4cb1e071cfc1e107c8e19081089f88c/traits/traits_listener.py#L1367
- This is then applied to the listener object here: https://github.com/enthought/traits/blob/master/traits/has_traits.py#L2814-L2821 (by `type = lnw.type`) along with a number of other properties (`handler`, `dispatch`, etc.)
- Most of the other properties have a `__changed` handler` on the `ListenerItem` which push the changed values through the listener structures recursively: see the methods starting here https://github.com/enthought/traits/blob/94308a28e4cb1e071cfc1e107c8e19081089f88c/traits/traits_listener.py#L606
- The `type` trait _doesn't_ have a change handler, and so doesn't get propagated recursively.

Adding the following code to `ListenerItem` resolves the issues described above:
```
def _type_changed(self, type):
""" Handles the ``type`` trait being changed.
"""
if self.next is not None:
self.next.type = type
```

However, this appears to be a very long-standing bug (12+ years) so some caution may need to be taken when fixing it that there may be code that relies on the current behaviour.

Edit: simplified the example to remove some unused code.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.