Support for observe in ExtensionPoint

[kitchoi]

Traits 6.1 introduces a new trait notification/observation framework: observe. The mini-language for this framework has changed for observing mutations in a container such as a list.

Currently ExtensionPoint emits property change event using a trait named name and name_items. This allows users to use on_trait_change as if an ExtensionPoint was a List and listen to "mutations" to it even though the list is never cached. In other words, it behaves as if it was two Property combined. Changing on_trait_change("name_items") to observe("name:items") won't work as expected. This issue is about how to make ExtensionPoint be compatible with observe.

I might be able to put together a small example and post here later.

[rahulporuri]

I think we need to explore this for the 5.0.0 release (doesn't have to be merged into master) because it might bring up potential issues when we move to using traits observe. Adding this issue to the 5.0.0 milestone.

[kitchoi]

For reference, I think observe("name_items") should still work, but the event object may not be what you think it is. name_items is just another trait, it is named as such to imitate on_trait_change mini-language.

[kitchoi]

I have so far attempted two approaches, neither were successful:

(1) Reimplement ExtensionRegistry and friends with observe
This is futile because in order to support @observe("name:items") where name is defined as an ExtensionPoint, we need the name to refer to a persisted TraitList or an instance of HasTraits that has a trait named "items". But that can't happen, see below.

(2) Caching the TraitListObject returned by ExtensionPoint.get.
While this supports @observe("name:items"), this would be a wrong fix because the list returned by the ExtensionPoint is NOT supposed to be mutated. If I try to cache the list to support @observe("name:items"), this test I added in #346 will fail:

envisage/envisage/tests/test_extension_point.py

Lines 114 to 137 in 71a913c

	def test_mutate_extension_point_no_effect(self):
	""" Extension point is recomputed so mutation has no effect. """
	registry = self.registry
	# Add an extension point.
	registry.add_extension_point(self._create_extension_point("my.ep"))
	# Set the extensions.
	registry.set_extensions("my.ep", [1, 2, 3])
	# Declare a class that consumes the extension.
	class Foo(TestBase):
	x = ExtensionPoint(List(Int), id="my.ep")
	# when
	f = Foo()
	f.x.append(42)
	# then
	# The registry is not changed, and the extension point is still the
	# same as before
	self.assertEqual(registry.get_extensions("my.ep"), [1, 2, 3])
	self.assertEqual(f.x, [1, 2, 3])

In particular, it would fail on the very last line with [1, 2, 3, 42] != [1, 2, 3]. Much of envisage code assumes that changes to the extension points must go through the registry mechanism. Allowing the extension point to be mutated directly will bleach that assumption.

As I said in #346, I think the naming of "name_items" is unfortunate: It is not the same as "name_items" in Traits's on_trait_change framework (as is demonstrated by this other test in #346). Had it been called something else, one would not have tried to convert it to "name:items" when migrating to observe. We should still find a solution to support such a tempting migration, as the name attribute is never truly set, @observe("name:items") will simply do nothing and it will be very hard to detect unless one has good test coverage on the logic.

My next idea is for ExtensionPoint.get to return an instance of HasTraits that also implements the interface of a sequence, and then we can define a trait called "items" on the instance to continue with this API.

[kitchoi]

My next idea is for ExtensionPoint.get to return an instance of HasTraits that also implements the interface of a sequence, and then we can define a trait called "items" on the instance to continue with this API.

That idea also does not work because although an event can be fired for @observe("name:items"), the event is not a ListChangeEvent but a TraitChangeEvent. Developers using this API is going to expect a list-like behaviour and expect a ListChangeEvent like one would if the value is actually a TraitList.

At the end, I have to subclass TraitList and have it cached on the object, and at the same time, prevent mutations to occur directly on the list. PR coming...

[mdickinson]

We had to revert #354; re-opening this issue accordingly.