Feature: 'anytrait' for observe

[mdickinson]

This PR adds anytrait-handling for the 'observe' framework, with mini-language support via the "*" character. Specifically, it adds:

• The symbol '*' to the mini language; this can be used wherever a trait name can be used.
• The anytrait function for use in the expression language. This is exposed in traits.observation.api.
• An anytrait method to ObserverExpression.

Examples: after

>>> from traits.api import *
>>> class Child(HasTraits):
...     age = Int()
...     name = Str()
... 
>>> class Parent(HasTraits):
...     children = List(Instance(Child))
...     dexterity = Int()
... 
>>> parent = Parent()

We can print all changes to children's traits by doing:

>>> parent.observe(print, "children:items:*")

Then we'll see results like:

>>> child = Child(age=123, name="Unova")
>>> parent.children.append(child)
>>> child.age = 124
TraitChangeEvent(object=<__main__.Child object at 0x135f19e50>, name='age', old=123, new=124)
>>> child.name = "Squintifego"
TraitChangeEvent(object=<__main__.Child object at 0x135f19e50>, name='name', old='Unova', new='Squintifego')

More simply, we can just listen to all traits on the parent:

>>> parent.observe(print, "*")
>>> parent.dexterity += 1
TraitChangeEvent(object=<__main__.Parent object at 0x135f2e090>, name='dexterity', old=0, new=1)

Checklist

• Tests
• Update API reference (docs/source/traits_api_reference)
• Update User manual (docs/source/traits_user_manual)
• [ ] Update type annotation hints in traits-stubs Not applicable

Fixes #1435

[mdickinson]

Inplementation note: there's a substantial difference from the way that anytrait listening is implemented for on_trait_change. For on_trait_change, we register object-level listeners rather than trait-level, while for the observe machinery we register the listener for each and every trait.

In particular, this means that on_trait_change automatically does the right thing when traits are added or removed, while there's currently no support for that for observe. It also likely means that on_trait_change is more efficient.

[corranwebster]

Good to see this - it's a needed feature.

High-level comments before I look more closely:

• some care may be needed because this is a change in behaviour compared to the old mini-language: there + effectively meant anytrait and * meant recursive matching. I'm not too worried about that because I have only seen * used a handful of times (usually in tree structures)
• I'm worried by the efficiency concerns, but this might be good enough for now and we can improve if efficiency becomes an issue

[corranwebster]

Actual implementation looks fine: does what it says, and cleanly.

[corranwebster]

One thought on this:

on_trait_change automatically does the right thing when traits are added or removed, while there's currently no support for that for observe

It seems like that might be an issue that is more general than "anytrait": should we have code that when traits are added (and perhaps more importantly, removed) runs through the observers and check for matches. There are events we can hook into for this, IIRC.

[corranwebster]

Final thought: do we want to have anytrait as a synonym for * in this?

[mdickinson]

It seems like that might be an issue that is more general than "anytrait":

Yep. This got discussed when we originally implemented observe, and I think there's already an issue for this somewhere. I'll dig it out, or open a new one if there isn't one.

There are events we can hook into for this, IIRC.

Kinda sorta. We have trait_added, but there's no corresponding trait_removed. For a complete solution we'd probably need that trait_removed event. In theory that's a backwards incompatible change, since there may be existing HasTraits subclasses out there that already have a trait named trait_removed, but I think it's fairly safe to assume that won't happen. (There's an issue somewhere for documenting that any trait name that starts with trait should be reserved.)

[mdickinson]

Final thought: do we want to have anytrait as a synonym for * in this?

I'd rather not - we currently don't have any trait name "keywords" - names that would prevent the use of those names as traits. ("items" comes close, but it's okay because it can never be used on an instance trait - only on a list, set or dict).

[mdickinson]

• some care may be needed because this is a change in behaviour compared to the old mini-language: there + effectively meant anytrait and * meant recursive matching. I'm not too worried about that because I have only seen * used a handful of times (usually in tree structures)

Yes. I've gone back and forth on this, but I think this is the right compromise. In particular, recent work on the on_trait_change parser has reminded me just how much complication the recursive * adds, so I'm strongly inclined not to try to add an equivalent for observe. If we do decide to, it shouldn't be too big a deal to find a new character that we can use for that.

I also dislike the use of + in the old language for both metadata and anytrait; the * feels more natural to me for anytrait.

[rahulporuri]

LGTM with one minor comment.

[mdickinson]

I'm having some trouble with uses of add_trait. The following script gives ValueError: Trait named 'name' not found on 'child'. on this branch. I think this is orthogonal to this PR, but I'm going to do some digging before merging this.

from traits.api import HasTraits, Instance, Str


class Child(HasTraits):
    name = Str()


class Parent(HasTraits):
    pass


foo = Parent()
foo.observe(print, "*:name")
foo.add_trait("child", Instance(Child))

[mdickinson]

I think this is orthogonal to this PR, but I'm going to do some digging before merging this.

Confirmed that the issue is separate from the changes in this PR. I've opened #1503 for this.

[mdickinson]

From investigating #1503, the behaviour of * anywhere other than at the end of an expression isn't terribly useful: that is, listening to children:items:* is fine, but listening to *:updated only works if every single trait is an Instance trait whose type has an updated trait.

I'm now wondering whether to restrict the grammar for now so that * is only legal at the end of an expression. For a * other than at the end of an expression, I think we may want to change the behaviour to make it more permissive, and making that behaviour change post-release could be problematic.

[mdickinson]

Another potentially fun case: listening to *:items where some traits are Instance traits and others might be List traits. I'm beginning to think that we should either make items an illegal trait name, or introduce an alternative syntax for list items.

[corranwebster]

I am curious now what happens when you filter on metadata not as the last item (eg. +foo_meta.updated). Does that also not work unless every trait which matches the metadata also is an Instance with an updated trait?

[mdickinson]

I am curious now what happens when you filter on metadata not as the last item (eg. +foo_meta.updated).

I haven't checked (will do that shortly), but I'd expect it not to fail for matching traits that don't have an updated trait.

[mdickinson]

I'd expect it not to fail for matching traits that don't have an updated trait.

Yep, it's perfectly happy. Here's the test script:

from traits.api import Event, HasTraits, Instance

class HasUpdated(HasTraits):
    updated = Event()

class LacksUpdated(HasTraits):
    pass

class A(HasTraits):
    foo = Instance(HasUpdated, fancy=True)
    bar = Instance(LacksUpdated, fancy=True)

def print_event(obj, name, old, new):
    print("Change event:", obj, name, old, new)

a = A(foo=HasUpdated(), bar=LacksUpdated())
a.on_trait_change(print_event, "+fancy:updated")
a.foo.updated = True  # Prints event as expected

And the output:

(traits) mdickinson@mirzakhani traits % python test.py                                     
Change event: <__main__.HasUpdated object at 0x10645e400> updated <undefined> True

The observe equivalent fails, as expected:

from traits.api import Event, HasTraits, Instance
from traits.observation.api import match

class HasUpdated(HasTraits):
    updated = Event()

class LacksUpdated(HasTraits):
    pass

class A(HasTraits):
    foo = Instance(HasUpdated, fancy=True)
    bar = Instance(LacksUpdated, fancy=True)

def matcher(name, trait):
    return getattr(trait, "fancy", None) is not None

a = A(foo=HasUpdated(), bar=LacksUpdated())

a.observe(print, match(matcher, notify=False).trait("updated"))
a.foo.updated = True

Output:

(traits) mdickinson@mirzakhani traits % python test.py
Traceback (most recent call last):
  File "/Users/mdickinson/Enthought/ETS/traits/test.py", line 19, in <module>
    a.observe(print, match(matcher, notify=False).trait("updated"))
  File "/Users/mdickinson/Enthought/ETS/traits/traits/has_traits.py", line 2381, in observe
    observe_api.observe(
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/observe.py", line 54, in observe
    add_or_remove_notifiers(
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 54, in add_or_remove_notifiers
    callable_()
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 94, in __call__
    step()
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 126, in _add_or_remove_children_notifiers
    add_or_remove_notifiers(
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 54, in add_or_remove_notifiers
    callable_()
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 94, in __call__
    step()
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_observe.py", line 163, in _add_or_remove_notifiers
    for observable in self.graph.node.iter_observables(self.object):
  File "/Users/mdickinson/Enthought/ETS/traits/traits/observation/_named_trait_observer.py", line 87, in iter_observables
    raise ValueError(
ValueError: Trait named 'updated' not found on <__main__.LacksUpdated object at 0x1088b0860>.

It's not obvious what the right thing to do is here. In general, the lack of tolerance of observe is a feature, in that it makes sure that misspellings like some_trait:udpated don't pass silently.

But for a * not in a tail position to be useful, the expectation would surely have to be that whatever follows it would be applied non-strictly.

For this PR, I think I'm going to change the grammar to only allow * in a trailing position, so that we can change the behaviour later if we want to without backwards compatibility concerns.

[mdickinson]

For this PR, I think I'm going to change the grammar to only allow * in a trailing position, so that we can change the behaviour later if we want to without backwards compatibility concerns.

Change of plans; to avoid confusion, I'll merge this as-is, and create a new PR for the above change.