From b3ced003403f6cec072af6f202c2bee7052d6f37 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 28 Oct 2020 18:32:23 +0000 Subject: [PATCH 01/14] Support @observe("name:items") for ExtensionPoint This is achieved by caching the value of ExtensionPoint and make it impossible to mutate the list directly. --- envisage/extension_point.py | 337 ++++++++++++++++-- envisage/plugin.py | 2 +- envisage/tests/test_extension_point.py | 105 +++++- .../tests/test_extension_point_changed.py | 263 +++++++++++--- 4 files changed, 638 insertions(+), 69 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index cd976a637..6b726d758 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -12,10 +12,12 @@ # Standard library imports. import inspect +import warnings import weakref # Enthought library imports. from traits.api import List, TraitType, Undefined, provides +from traits.trait_list_object import TraitList # Local imports. from .i_extension_point import IExtensionPoint @@ -162,14 +164,14 @@ def __repr__(self): def get(self, obj, trait_name): """ Trait type getter. """ + cache_name = self._get_cache_name(trait_name) + if cache_name not in obj.__dict__: + self._update_cache(obj, trait_name) - extension_registry = self._get_extension_registry(obj) - - # Get the extensions to this extension point. - extensions = extension_registry.get_extensions(self.id) - - # Make sure the contributions are of the appropriate type. - return self.trait_type.validate(obj, trait_name, extensions) + value = obj.__dict__[cache_name] + # validate again + self.trait_type.validate(obj, trait_name, value[:]) + return value def set(self, obj, name, value): """ Trait type setter. """ @@ -181,8 +183,6 @@ def set(self, obj, name, value): # for exxample ;^). extension_registry.set_extensions(self.id, value) - return - ########################################################################### # 'ExtensionPoint' interface. ########################################################################### @@ -200,23 +200,41 @@ def connect(self, obj, trait_name): """ def listener(extension_registry, event): - """ Listener called when an extension point is changed. """ - - # If an index was specified then we fire an '_items' changed event. + """ Listener called when an extension point is changed. + + Parameters + ---------- + extension_registry : IExtensionRegistry + Registry that maintains the extensions. + event : ExtensionPointChangedEvent + Event for created for the change. + If the event.index is None, this means the entire extensions + is set to a new value. If the event.index is not None, some + portion of the list has been modified. + """ if event.index is not None: - name = trait_name + "_items" - old = Undefined - new = event + # We know where in the list is changed. - # Otherwise, we fire a normal trait changed event. - else: - name = trait_name - old = event.removed - new = event.added + # Mutate the _ExtensionPointValue to fire ListChangeEvent + # expected from observing item change. + getattr(obj, trait_name)._sync_values(event) - obj.trait_property_changed(name, old, new) + # For on_trait_change('name_items') + obj.trait_property_changed( + trait_name + "_items", Undefined, event + ) - return + else: + # The entire list has changed. We reset the cache and fire a + # normal trait changed event. + self._update_cache(obj, trait_name) + + # In case the cache was created first and the registry is then mutated + # before this ``connect``` is called, the internal cache would be in + # an inconsistent state. This also has the side-effect of firing + # another change event, hence allowing future changes to be observed + # without having to access the trait first. + self._update_cache(obj, trait_name) extension_registry = self._get_extension_registry(obj) @@ -264,3 +282,278 @@ def _get_extension_registry(self, obj): ) return extension_registry + + def _get_cache_name(self, trait_name): + """ Return the cache name for the extension point value associated + with a given trait. + """ + return "__envisage_{}".format(trait_name) + + def _update_cache(self, obj, trait_name): + """ Update the internal cached value for the extension point and + fire change event. + + Parameters + ---------- + obj : HasTraits + The object on which an ExtensionPoint is defined. + trait_name : str + The name of the trait for which ExtensionPoint is defined. + """ + cache_name = self._get_cache_name(trait_name) + old = obj.__dict__.get(cache_name, Undefined) + new = ( + _ExtensionPointValue( + _get_extensions(obj, trait_name), + object=obj, + name=trait_name, + ) + ) + obj.__dict__[cache_name] = new + obj.trait_property_changed(trait_name, old, new) + + +class _ExtensionPointValue(TraitList): + """ _ExtensionPointValue is the list being returned while retrieving the + attribute value for an ExtensionPoint trait. + + This list returned for an ExtensionPoint acts as a proxy to query + extensions in an ExtensionRegistry for a given extension point id. Users of + ExtensionPoint expect to handle a list-like object, and expect to be able + to listen to "mutation" on the list. The ExtensionRegistry remains to be + the source of truth as to what extensions are available for a given + extension point ID. + + Users are not expected to mutate the list directly. All mutations to + extensions are expected to go through the extension registry to maintain + consistency. With that, all methods for mutating the list are nullified, + unless it is used internally. + + The requirement to support ``observe("name:items")`` means this list, + associated with `name`, cannot be a property that gets recomputed on every + access (enthought/traits/#624), it needs to be cached. As with any + cached quantity, it needs to be synchronized with the ExtensionRegistry. + + The assumption on the internal values being synchronized with the registry + breaks down if the extension registry is mutated before listeners + are hooked up between the extension point and the registry. This sequence + of events is difficult to enforce. Therefore we always resort to the + extension registry for querying values. + + Parameters + ---------- + iterable : iterable + Iterable providing the items for the list + obj : HasTraits + The object on which an ExtensionPoint is defined. + trait_name : str + The name of the trait for which ExtensionPoint is defined. + """ + + def __init__(self, iterable=(), *, object, name): + """ Reimplemented TraitList.__init__ + + Parameters + ---------- + object : HasTraits + The object on which an ExtensionPoint is defined. + trait_name : str + The name of the trait for which ExtensionPoint is defined. + """ + super().__init__(iterable) + + # Flag to control access for mutating the list. Only internal + # code can mutate the list. See _sync_values + self._internal_use = False + + self._object = object + self._name = name + + def __eq__(self, other): + if self._internal_use: + return super().__eq__(other) + return _get_extensions(self._object, self._name) == other + + def __getitem__(self, key): + if self._internal_use: + return super().__getitem__(key) + return _get_extensions(self._object, self._name)[key] + + def __len__(self): + if self._internal_use: + return super().__len__() + return len(_get_extensions(self._object, self._name)) + + def _sync_values(self, event): + """ Given an ExtensionPointChangedEvent, modify the values in this list + to match. This is an internal method only used by Envisage code. + + Parameters + ---------- + event : ExtenstionPointChangedEvent + Event being fired for extension point values changed (typically + via the extension registry) + """ + self._internal_use = True + try: + if isinstance(event.index, slice): + if event.added: + self[event.index] = event.added + else: + del self[event.index] + else: + slice_ = slice( + event.index, event.index + len(event.removed) + ) + self[slice_] = event.added + finally: + self._internal_use = False + + # Reimplement TraitList interface to avoid any mutation. + # The original implementation of __setitem__ and __delitem__ can be used + # by internal code. + + def __delitem__(self, key): + """ Reimplemented TraitList.__delitem__ """ + + # This is used by internal code + + if not self._internal_use: + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + return + + super().__delitem__(key) + + def __iadd__(self, value): + """ Reimplemented TraitList.__iadd__ """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + return self[:] + + def __imul__(self, value): + """ Reimplemented TraitList.__imul__ """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + return self[:] + + def __setitem__(self, key, value): + """ Reimplemented TraitList.__setitem__ """ + + # This is used by internal code + + if not self._internal_use: + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + return + + super().__setitem__(key, value) + + def append(self, object): + """ Reimplemented TraitList.append """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def clear(self): + """ Reimplemented TraitList.clear """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def extend(self, iterable): + """ Reimplemented TraitList.extend """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def insert(self, index, object): + """ Reimplemented TraitList.insert """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def pop(self, index=-1): + """ Reimplemented TraitList.pop """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def remove(self, value): + """ Reimplemented TraitList.remove """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def reverse(self): + """ Reimplemented TraitList.reverse """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + def sort(self, *, key=None, reverse=False): + """ Reimplemented TraitList.sort """ + # We should not need it for internal use either. + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=2, + ) + + +def _get_extensions(object, name): + """ Return the extensions reported by the extension registry for the + given object and the name of a trait whose type is an ExtensionPoint. + + Parameters + ---------- + object : HasTraits + Object on which an ExtensionPoint is defined + name : str + Name of the trait whose trait type is an ExtensionPoint. + + Returns + ------- + extensions : list + All the extensions for the extension point. + """ + extension_point = object.trait(name).trait_type + extension_registry = extension_point._get_extension_registry(object) + + # Get the extensions to this extension point. + return extension_registry.get_extensions(extension_point.id) diff --git a/envisage/plugin.py b/envisage/plugin.py index f58647299..0a70c35ec 100644 --- a/envisage/plugin.py +++ b/envisage/plugin.py @@ -327,7 +327,7 @@ def _anytrait_changed(self, trait_name, old, new): else: added = new removed = old - index = slice(0, max(len(old), len(new))) + index = slice(0, len(old)) # Let the extension registry know about the change. self._fire_extension_point_changed( diff --git a/envisage/tests/test_extension_point.py b/envisage/tests/test_extension_point.py index 745c6f42d..f642c2581 100644 --- a/envisage/tests/test_extension_point.py +++ b/envisage/tests/test_extension_point.py @@ -12,6 +12,8 @@ # Standard library imports. import unittest +from traits.api import Undefined + # Enthought library imports. from envisage.api import Application, ExtensionPoint from envisage.api import ExtensionRegistry @@ -120,7 +122,7 @@ def test_mutate_extension_point_no_effect(self): registry.add_extension_point(self._create_extension_point("my.ep")) # Set the extensions. - registry.set_extensions("my.ep", [1, 2, 3]) + registry.set_extensions("my.ep", [1, 2, 3, 0, 5]) # Declare a class that consumes the extension. class Foo(TestBase): @@ -128,13 +130,48 @@ class Foo(TestBase): # when f = Foo() - f.x.append(42) + + with self.assertWarns(RuntimeWarning): + f.x.append(42) + + with self.assertWarns(RuntimeWarning): + f.x.clear() + + with self.assertWarns(RuntimeWarning): + f.x.extend((100, 101)) + + with self.assertWarns(RuntimeWarning): + f.x.insert(0, 1) + + with self.assertWarns(RuntimeWarning): + f.x.pop() + + with self.assertWarns(RuntimeWarning): + f.x.remove(1) + + with self.assertWarns(RuntimeWarning): + f.x[0] = 99 + + with self.assertWarns(RuntimeWarning): + f.x *= 99 + + with self.assertWarns(RuntimeWarning): + f.x += [9] + + with self.assertWarns(RuntimeWarning): + del f.x[0:2] + + with self.assertWarns(RuntimeWarning): + f.x.reverse() + + with self.assertWarns(RuntimeWarning): + f.x.sort() # 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]) + self.assertEqual(registry.get_extensions("my.ep"), [1, 2, 3, 0, 5]) + self.assertEqual(f.x.copy(), [1, 2, 3, 0, 5]) def test_untyped_extension_point(self): """ untyped extension point """ @@ -205,6 +242,31 @@ class Foo(TestBase): with self.assertRaises(TraitError): getattr(f, "x") + def test_invalid_extension_point_after_mutation(self): + """ Test extension point becomes invalid later. """ + + registry = self.registry + + # Add an extension point. + registry.add_extension_point(self._create_extension_point("my.ep")) + + # Declare a class that consumes the extension. + class Foo(TestBase): + x = ExtensionPoint(List(Int), id="my.ep") + + # Make sure we get a trait error because the type of the extension + # doesn't match that of the extension point. + f = Foo() + + # This is okay, the list is empty. + f.x + + registry.set_extensions("my.ep", "xxx") + + # Now this should fail. + with self.assertRaises(TraitError): + getattr(f, "x") + def test_extension_point_with_no_id(self): """ extension point with no Id """ @@ -253,6 +315,41 @@ class Foo(TestBase): self.assertEqual([42], registry.get_extensions("my.ep")) + def test_set_typed_extension_point_emit_change(self): + """ Test change event is emitted for setting the extension point """ + + registry = self.registry + + # Add an extension point. + registry.add_extension_point(self._create_extension_point("my.ep")) + + # Declare a class that consumes the extension. + class Foo(TestBase): + x = ExtensionPoint(List(Int), id="my.ep") + + on_trait_change_events = [] + + def on_trait_change_handler(*args): + on_trait_change_events.append(args) + + observed_events = [] + + f = Foo() + f.on_trait_change(on_trait_change_handler, "x") + f.observe(observed_events.append, "x") + + # when + ExtensionPoint.connect_extension_point_traits(f) + + # then + self.assertEqual(len(on_trait_change_events), 1) + self.assertEqual(len(observed_events), 1) + event, = observed_events + self.assertEqual(event.object, f) + self.assertEqual(event.name, "x") + self.assertEqual(event.old, Undefined) + self.assertEqual(event.new, []) + def test_extension_point_str_representation(self): """ test the string representation of the extension point """ ep_repr = "ExtensionPoint(id={!r})" diff --git a/envisage/tests/test_extension_point_changed.py b/envisage/tests/test_extension_point_changed.py index 38025682b..07eab6269 100644 --- a/envisage/tests/test_extension_point_changed.py +++ b/envisage/tests/test_extension_point_changed.py @@ -51,18 +51,59 @@ def test_mutate_extension_point_no_events(self): """ Mutation will not emit change event for name_items """ a = PluginA() + b = PluginB() + c = PluginC() + a.on_trait_change(listener, "x_items") + events = [] + a.observe(events.append, "x:items") + + application = TestApplication(plugins=[a, b, c]) + application.start() + + # when + with self.assertWarns(RuntimeWarning): + a.x.append(42) + + # then + self.assertIsNone(listener.obj) + self.assertEqual(len(events), 0) + + def test_mutate_extension_point_then_modify_from_registry(self): + """ Mutating the extension point does nothing and should not cause + subsequent change event information to become inconsistent. + """ + a = PluginA() b = PluginB() c = PluginC() + a.on_trait_change(listener, "x_items") + events = [] + a.observe(events.append, "x:items") + application = TestApplication(plugins=[a, b, c]) application.start() # when - a.x.append(42) + with self.assertWarns(RuntimeWarning): + a.x.clear() # then self.assertIsNone(listener.obj) + self.assertEqual(len(events), 0) + + # when + # Append a contribution. + b.x.append(4) + + # then + self.assertEqual(a.x, [1, 2, 3, 4, 98, 99, 100]) + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 3) + self.assertEqual(event.added, [4]) + self.assertEqual(event.removed, []) def test_append(self): """ append """ @@ -75,11 +116,6 @@ def test_append(self): application = TestApplication(plugins=[a, b, c]) application.start() - # fixme: If the extension point has not been accessed then the - # provider extension registry can't work out what has changed, so it - # won't fire a changed event. - self.assertEqual([1, 2, 3, 98, 99, 100], a.x) - # Append a contribution. b.x.append(4) @@ -105,6 +141,30 @@ def test_append(self): self.assertEqual([], listener.new.removed) self.assertEqual(3, listener.new.index) + def test_append_with_observe(self): + """ append with observe """ + + a = PluginA() + b = PluginB() + c = PluginC() + + events = [] + a.observe(events.append, "x:items") + + application = TestApplication(plugins=[a, b, c]) + application.start() + + # Append a contribution. + b.x.append(4) + + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 3) + self.assertEqual(event.added, [4]) + self.assertEqual(event.removed, []) + def test_remove(self): """ remove """ @@ -116,11 +176,6 @@ def test_remove(self): application = TestApplication(plugins=[a, b, c]) application.start() - # fixme: If the extension point has not been accessed then the - # provider extension registry can't work out what has changed, so it - # won't fire a changed event. - self.assertEqual([1, 2, 3, 98, 99, 100], a.x) - # Remove a contribution. b.x.remove(3) @@ -146,6 +201,30 @@ def test_remove(self): self.assertEqual([3], listener.new.removed) self.assertEqual(2, listener.new.index) + def test_remove_with_observe(self): + """ remove with observing items change. """ + + a = PluginA() + b = PluginB() + c = PluginC() + + events = [] + a.observe(events.append, "x:items") + + application = TestApplication(plugins=[a, b, c]) + application.start() + + # Remove a contribution. + b.x.remove(3) + + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 2) + self.assertEqual(event.added, []) + self.assertEqual(event.removed, [3]) + def test_assign_empty_list(self): """ assign empty list """ @@ -157,11 +236,6 @@ def test_assign_empty_list(self): application = TestApplication(plugins=[a, b, c]) application.start() - # fixme: If the extension point has not been accessed then the - # provider extension registry can't work out what has changed, so it - # won't fire a changed event. - self.assertEqual([1, 2, 3, 98, 99, 100], a.x) - # Assign an empty list to one of the plugin's contributions. b.x = [] @@ -188,37 +262,29 @@ def test_assign_empty_list(self): self.assertEqual(0, listener.new.index.start) self.assertEqual(3, listener.new.index.stop) - def test_assign_empty_list_no_event(self): - """ assign empty list no event """ + def test_assign_empty_list_with_observe(self): + """ assign an empty list to a plugin triggers a list change event.""" a = PluginA() - a.on_trait_change(listener, "x_items") b = PluginB() c = PluginC() + events = [] + a.observe(events.append, "x:items") + application = TestApplication(plugins=[a, b, c]) application.start() # Assign an empty list to one of the plugin's contributions. b.x = [] - # Make sure we pick up the correct contribution via the application. - extensions = application.get_extensions("a.x") - extensions.sort() - - self.assertEqual(3, len(extensions)) - self.assertEqual([98, 99, 100], extensions) - - # Make sure we pick up the correct contribution via the plugin. - extensions = a.x[:] - extensions.sort() - - self.assertEqual(3, len(extensions)) - self.assertEqual([98, 99, 100], extensions) - - # We shouldn't get a trait event here because we haven't accessed the - # extension point yet! - self.assertEqual(None, listener.obj) + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.added, []) + self.assertEqual(event.removed, [1, 2, 3]) + self.assertEqual(event.index, 0) def test_assign_non_empty_list(self): """ assign non-empty list """ @@ -231,11 +297,6 @@ def test_assign_non_empty_list(self): application = TestApplication(plugins=[a, b, c]) application.start() - # fixme: If the extension point has not been accessed then the - # provider extension registry can't work out what has changed, so it - # won't fire a changed event. - self.assertEqual([1, 2, 3, 98, 99, 100], a.x) - # Assign a non-empty list to one of the plugin's contributions. b.x = [2, 4, 6, 8] @@ -260,7 +321,31 @@ def test_assign_non_empty_list(self): self.assertEqual([2, 4, 6, 8], listener.new.added) self.assertEqual([1, 2, 3], listener.new.removed) self.assertEqual(0, listener.new.index.start) - self.assertEqual(4, listener.new.index.stop) + self.assertEqual(3, listener.new.index.stop) + + def test_assign_non_empty_list_with_observe(self): + """ assign non-empty list """ + + a = PluginA() + b = PluginB() + c = PluginC() + + events = [] + a.observe(events.append, "x:items") + + application = TestApplication(plugins=[a, b, c]) + application.start() + + # Assign a non-empty list to one of the plugin's contributions. + b.x = [2, 4, 6, 8] + + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 0) + self.assertEqual(event.added, [2, 4, 6, 8]) + self.assertEqual(event.removed, [1, 2, 3]) def test_add_plugin(self): """ add plugin """ @@ -313,6 +398,31 @@ def test_add_plugin(self): self.assertEqual([], listener.new.removed) self.assertEqual(3, listener.new.index) + def test_add_plugin_with_observe(self): + """ add plugin with observe """ + + a = PluginA() + b = PluginB() + c = PluginC() + + events = [] + a.observe(events.append, "x:items") + + # Start off with just two of the plugins. + application = TestApplication(plugins=[a, b]) + application.start() + + # Now add the other plugin. + application.add_plugin(c) + + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 3) + self.assertEqual(event.added, [98, 99, 100]) + self.assertEqual(event.removed, []) + def test_remove_plugin(self): """ remove plugin """ @@ -363,6 +473,75 @@ def test_remove_plugin(self): self.assertEqual([1, 2, 3], listener.new.removed) self.assertEqual(0, listener.new.index) + def test_remove_plugin_with_observe(self): + """ remove plugin with observe """ + + a = PluginA() + b = PluginB() + c = PluginC() + + events = [] + a.observe(events.append, "x:items") + + # Start off with just two of the plugins. + application = TestApplication(plugins=[a, b, c]) + application.start() + + # Now remove one plugin. + application.remove_plugin(b) + + # then + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 0) + self.assertEqual(event.added, []) + self.assertEqual(event.removed, [1, 2, 3]) + + def test_race_condition(self): + """ Test the extension point being modified before the application + starts, changes before starting the application are not notified. + """ + a = PluginA() + b = PluginB() + c = PluginC() + application = TestApplication(plugins=[a, b, c]) + + events = [] + a.observe(events.append, "x:items") + + # This sets the cache. + self.assertEqual(a.x, [1, 2, 3, 98, 99, 100]) + + # Now we mutate the registry, but the application has not started. + b.x = [4, 5, 6] + + # then + self.assertEqual(a.x, [4, 5, 6, 98, 99, 100]) + # application has not started, no events. + self.assertEqual(len(events), 0) + + # Now we start the application, which connects the listener. + application.start() + + # Change the value again. + b.x = [1, 2] + + # then + self.assertEqual(a.x, [1, 2, 98, 99, 100]) + + # The mutation occurred before application starting is not reported. + self.assertEqual(len(events), 1) + event, = events + self.assertEqual(event.object, a.x) + self.assertEqual(event.index, 0) + self.assertEqual(event.added, [1, 2]) + self.assertEqual(event.removed, [4, 5, 6]) + + +class TestExtensionPointChangedEvent(unittest.TestCase): + """ Test ExtensionPointChangedEvent object.""" + def test_extension_point_change_event_str_representation(self): """ test string representation of the ExtensionPointChangedEvent class """ From ab81199680941ee7f3874e19e74cfa6d948f0ab7 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 29 Oct 2020 09:50:01 +0000 Subject: [PATCH 02/14] Make sure an object with ExtensionPoint is garbage collectable after disconnecting listeners --- envisage/extension_point.py | 8 ++++---- envisage/tests/test_extension_point.py | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 6b726d758..aab1430d3 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -366,23 +366,23 @@ def __init__(self, iterable=(), *, object, name): # code can mutate the list. See _sync_values self._internal_use = False - self._object = object + self._object_ref = weakref.ref(object) self._name = name def __eq__(self, other): if self._internal_use: return super().__eq__(other) - return _get_extensions(self._object, self._name) == other + return _get_extensions(self._object_ref(), self._name) == other def __getitem__(self, key): if self._internal_use: return super().__getitem__(key) - return _get_extensions(self._object, self._name)[key] + return _get_extensions(self._object_ref(), self._name)[key] def __len__(self): if self._internal_use: return super().__len__() - return len(_get_extensions(self._object, self._name)) + return len(_get_extensions(self._object_ref(), self._name)) def _sync_values(self, event): """ Given an ExtensionPointChangedEvent, modify the values in this list diff --git a/envisage/tests/test_extension_point.py b/envisage/tests/test_extension_point.py index f642c2581..04be4498c 100644 --- a/envisage/tests/test_extension_point.py +++ b/envisage/tests/test_extension_point.py @@ -350,6 +350,29 @@ def on_trait_change_handler(*args): self.assertEqual(event.old, Undefined) self.assertEqual(event.new, []) + def test_object_garbage_collectable(self): + import weakref + registry = self.registry + + # Add an extension point. + registry.add_extension_point(self._create_extension_point("my.ep")) + + # Declare a class that consumes the extension. + class Foo(TestBase): + x = ExtensionPoint(List(Int), id="my.ep") + + f = Foo() + object_ref = weakref.ref(f) + + # when + ExtensionPoint.connect_extension_point_traits(f) + f.x = [1] + ExtensionPoint.disconnect_extension_point_traits(f) + del f + + # then + self.assertIsNone(object_ref()) + def test_extension_point_str_representation(self): """ test the string representation of the extension point """ ep_repr = "ExtensionPoint(id={!r})" From 4a2a27ff7ea58fd712cb8c26b7c3635cdd892a1d Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 29 Oct 2020 09:51:52 +0000 Subject: [PATCH 03/14] Clean up test and add docstring --- envisage/tests/test_extension_point.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/envisage/tests/test_extension_point.py b/envisage/tests/test_extension_point.py index 04be4498c..bb26a6b66 100644 --- a/envisage/tests/test_extension_point.py +++ b/envisage/tests/test_extension_point.py @@ -11,6 +11,7 @@ # Standard library imports. import unittest +import weakref from traits.api import Undefined @@ -351,7 +352,7 @@ def on_trait_change_handler(*args): self.assertEqual(event.new, []) def test_object_garbage_collectable(self): - import weakref + """ object can be garbage collected after disconnecting listeners.""" registry = self.registry # Add an extension point. @@ -366,7 +367,6 @@ class Foo(TestBase): # when ExtensionPoint.connect_extension_point_traits(f) - f.x = [1] ExtensionPoint.disconnect_extension_point_traits(f) del f From 867edcbbe990231296044681925f20caf60208c0 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 29 Oct 2020 11:11:39 +0000 Subject: [PATCH 04/14] Remove reference back to the object The reference only serves to support some edge cases where the extension point values become out-of-sync because the listeners are not connected yet. Once the listeners are connected the values are refreshed. So all in all it is not worth keeping the reference, as one will then need to use weakref to avoid cycle references, and special handling in the pickling logic to handle that weakref. --- envisage/extension_point.py | 39 ++----------------- envisage/tests/test_extension_point.py | 33 +++++++++++++++- .../tests/test_extension_point_changed.py | 7 +++- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index aab1430d3..68cb2f57f 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -304,9 +304,7 @@ def _update_cache(self, obj, trait_name): old = obj.__dict__.get(cache_name, Undefined) new = ( _ExtensionPointValue( - _get_extensions(obj, trait_name), - object=obj, - name=trait_name, + _get_extensions(obj, trait_name) ) ) obj.__dict__[cache_name] = new @@ -344,46 +342,15 @@ class _ExtensionPointValue(TraitList): ---------- iterable : iterable Iterable providing the items for the list - obj : HasTraits - The object on which an ExtensionPoint is defined. - trait_name : str - The name of the trait for which ExtensionPoint is defined. """ - def __init__(self, iterable=(), *, object, name): - """ Reimplemented TraitList.__init__ - - Parameters - ---------- - object : HasTraits - The object on which an ExtensionPoint is defined. - trait_name : str - The name of the trait for which ExtensionPoint is defined. - """ - super().__init__(iterable) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) # Flag to control access for mutating the list. Only internal # code can mutate the list. See _sync_values self._internal_use = False - self._object_ref = weakref.ref(object) - self._name = name - - def __eq__(self, other): - if self._internal_use: - return super().__eq__(other) - return _get_extensions(self._object_ref(), self._name) == other - - def __getitem__(self, key): - if self._internal_use: - return super().__getitem__(key) - return _get_extensions(self._object_ref(), self._name)[key] - - def __len__(self): - if self._internal_use: - return super().__len__() - return len(_get_extensions(self._object_ref(), self._name)) - def _sync_values(self, event): """ Given an ExtensionPointChangedEvent, modify the values in this list to match. This is an internal method only used by Envisage code. diff --git a/envisage/tests/test_extension_point.py b/envisage/tests/test_extension_point.py index bb26a6b66..1a96e4d72 100644 --- a/envisage/tests/test_extension_point.py +++ b/envisage/tests/test_extension_point.py @@ -10,6 +10,7 @@ """ Tests for extension points. """ # Standard library imports. +import pickle import unittest import weakref @@ -17,8 +18,8 @@ # Enthought library imports. from envisage.api import Application, ExtensionPoint -from envisage.api import ExtensionRegistry -from traits.api import HasTraits, Int, List, TraitError +from envisage.api import ExtensionRegistry, IExtensionRegistry +from traits.api import HasTraits, Instance, Int, List, TraitError class TestBase(HasTraits): @@ -27,6 +28,16 @@ class TestBase(HasTraits): extension_registry = None +class ClassWithExtensionPoint(HasTraits): + """ Class with an ExtensionPoint for testing purposes. + Defined at the module level for pickability. + """ + + extension_registry = Instance(IExtensionRegistry) + + x = ExtensionPoint(List(Int), id="my.ep") + + class ExtensionPointTestCase(unittest.TestCase): """ Tests for extension points. """ @@ -258,6 +269,7 @@ class Foo(TestBase): # Make sure we get a trait error because the type of the extension # doesn't match that of the extension point. f = Foo() + ExtensionPoint.connect_extension_point_traits(f) # This is okay, the list is empty. f.x @@ -373,6 +385,23 @@ class Foo(TestBase): # then self.assertIsNone(object_ref()) + def test_object_pickability(self): + # Add an extension point. + self.registry.add_extension_point(ExtensionPoint(id="my.ep")) + + # An object is created, connected to the registry and have the + # extension point created. + f = ClassWithExtensionPoint(extension_registry=self.registry) + ExtensionPoint.connect_extension_point_traits(f) + self.registry.set_extensions("my.ep", [1, 2, 3]) + self.assertEqual(f.x, [1, 2, 3]) + + # then + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + serialized = pickle.dumps(f.x, protocol=protocol) + deserialized = pickle.loads(serialized) + self.assertEqual(deserialized, [1, 2, 3]) + def test_extension_point_str_representation(self): """ test the string representation of the extension point """ ep_repr = "ExtensionPoint(id={!r})" diff --git a/envisage/tests/test_extension_point_changed.py b/envisage/tests/test_extension_point_changed.py index 07eab6269..af3785742 100644 --- a/envisage/tests/test_extension_point_changed.py +++ b/envisage/tests/test_extension_point_changed.py @@ -517,13 +517,18 @@ def test_race_condition(self): b.x = [4, 5, 6] # then - self.assertEqual(a.x, [4, 5, 6, 98, 99, 100]) + # The values are not synchronized. + self.assertEqual(a.x, [1, 2, 3, 98, 99, 100]) + # application has not started, no events. self.assertEqual(len(events), 0) # Now we start the application, which connects the listener. application.start() + # then + self.assertEqual(a.x, [4, 5, 6, 98, 99, 100]) + # Change the value again. b.x = [1, 2] From 723ef9cf4c46717733640a48066530e26bbf587c Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 29 Oct 2020 11:15:18 +0000 Subject: [PATCH 05/14] Fix comment after removing query method reimplementation --- envisage/extension_point.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 68cb2f57f..c82999a0e 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -329,14 +329,11 @@ class _ExtensionPointValue(TraitList): The requirement to support ``observe("name:items")`` means this list, associated with `name`, cannot be a property that gets recomputed on every - access (enthought/traits/#624), it needs to be cached. As with any + access (enthought/traits#624), it needs to be cached. As with any cached quantity, it needs to be synchronized with the ExtensionRegistry. - The assumption on the internal values being synchronized with the registry - breaks down if the extension registry is mutated before listeners - are hooked up between the extension point and the registry. This sequence - of events is difficult to enforce. Therefore we always resort to the - extension registry for querying values. + Note that the list can only be synchronized with the extension registry + when the listeners are connected (see ``ExtensionPoint.connect``). Parameters ---------- From fc880be1970e3a63d24e4dabc175a5d9722ab178 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Thu, 29 Oct 2020 11:20:31 +0000 Subject: [PATCH 06/14] Move functions out of ExtensionPoint to avoid cycle reference between listeners and trait types --- envisage/extension_point.py | 69 ++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index c82999a0e..6926a7261 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -164,9 +164,9 @@ def __repr__(self): def get(self, obj, trait_name): """ Trait type getter. """ - cache_name = self._get_cache_name(trait_name) + cache_name = _get_cache_name(trait_name) if cache_name not in obj.__dict__: - self._update_cache(obj, trait_name) + _update_cache(obj, trait_name) value = obj.__dict__[cache_name] # validate again @@ -227,14 +227,14 @@ def listener(extension_registry, event): else: # The entire list has changed. We reset the cache and fire a # normal trait changed event. - self._update_cache(obj, trait_name) + _update_cache(obj, trait_name) # In case the cache was created first and the registry is then mutated # before this ``connect``` is called, the internal cache would be in # an inconsistent state. This also has the side-effect of firing # another change event, hence allowing future changes to be observed # without having to access the trait first. - self._update_cache(obj, trait_name) + _update_cache(obj, trait_name) extension_registry = self._get_extension_registry(obj) @@ -283,33 +283,6 @@ def _get_extension_registry(self, obj): return extension_registry - def _get_cache_name(self, trait_name): - """ Return the cache name for the extension point value associated - with a given trait. - """ - return "__envisage_{}".format(trait_name) - - def _update_cache(self, obj, trait_name): - """ Update the internal cached value for the extension point and - fire change event. - - Parameters - ---------- - obj : HasTraits - The object on which an ExtensionPoint is defined. - trait_name : str - The name of the trait for which ExtensionPoint is defined. - """ - cache_name = self._get_cache_name(trait_name) - old = obj.__dict__.get(cache_name, Undefined) - new = ( - _ExtensionPointValue( - _get_extensions(obj, trait_name) - ) - ) - obj.__dict__[cache_name] = new - obj.trait_property_changed(trait_name, old, new) - class _ExtensionPointValue(TraitList): """ _ExtensionPointValue is the list being returned while retrieving the @@ -521,3 +494,37 @@ def _get_extensions(object, name): # Get the extensions to this extension point. return extension_registry.get_extensions(extension_point.id) + + +def _get_cache_name(trait_name): + """ Return the attribute name on the object for storing the cached + extension point value associated with a given trait. + + Parameters + ---------- + trait_name : str + The name of the trait for which ExtensionPoint is defined. + """ + return "__envisage_{}".format(trait_name) + + +def _update_cache(obj, trait_name): + """ Update the internal cached value for the extension point and + fire change event. + + Parameters + ---------- + obj : HasTraits + The object on which an ExtensionPoint is defined. + trait_name : str + The name of the trait for which ExtensionPoint is defined. + """ + cache_name = _get_cache_name(trait_name) + old = obj.__dict__.get(cache_name, Undefined) + new = ( + _ExtensionPointValue( + _get_extensions(obj, trait_name) + ) + ) + obj.__dict__[cache_name] = new + obj.trait_property_changed(trait_name, old, new) From 208a055606beaffe3073c79f39ef0618e2b23824 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Tue, 3 Nov 2020 10:26:39 +0000 Subject: [PATCH 07/14] Improve docstring for exent --- envisage/extension_point.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 6926a7261..7d5761c81 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -207,10 +207,10 @@ def listener(extension_registry, event): extension_registry : IExtensionRegistry Registry that maintains the extensions. event : ExtensionPointChangedEvent - Event for created for the change. + Event created for the change. If the event.index is None, this means the entire extensions - is set to a new value. If the event.index is not None, some - portion of the list has been modified. + list is set to a new value. If the event.index is not None, + some portion of the list has been modified. """ if event.index is not None: # We know where in the list is changed. From e998fbed33181e104d6e0d438e3deb9fe6ccc1df Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Mon, 30 Nov 2020 14:15:38 +0000 Subject: [PATCH 08/14] Refactoring warning --- envisage/extension_point.py | 74 +++++++++---------------------------- 1 file changed, 18 insertions(+), 56 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 7d5761c81..af8b66e6d 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -356,11 +356,7 @@ def __delitem__(self, key): # This is used by internal code if not self._internal_use: - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() return super().__delitem__(key) @@ -368,21 +364,13 @@ def __delitem__(self, key): def __iadd__(self, value): """ Reimplemented TraitList.__iadd__ """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() return self[:] def __imul__(self, value): """ Reimplemented TraitList.__imul__ """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() return self[:] def __setitem__(self, key, value): @@ -391,11 +379,7 @@ def __setitem__(self, key, value): # This is used by internal code if not self._internal_use: - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() return super().__setitem__(key, value) @@ -403,73 +387,51 @@ def __setitem__(self, key, value): def append(self, object): """ Reimplemented TraitList.append """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def clear(self): """ Reimplemented TraitList.clear """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def extend(self, iterable): """ Reimplemented TraitList.extend """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def insert(self, index, object): """ Reimplemented TraitList.insert """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def pop(self, index=-1): """ Reimplemented TraitList.pop """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def remove(self, value): """ Reimplemented TraitList.remove """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def reverse(self): """ Reimplemented TraitList.reverse """ # We should not need it for internal use either. - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=2, - ) + self._warn_mutation() def sort(self, *, key=None, reverse=False): """ Reimplemented TraitList.sort """ # We should not need it for internal use either. + self._warn_mutation() + + def _warn_mutation(self): + """ Emit a warning when attempts to mutating this list is made + externally. + """ warnings.warn( "Extension point cannot be mutated directly.", RuntimeWarning, - stacklevel=2, + stacklevel=3, ) From b6dfe8e4ea579309832af93361fddac91ff340b5 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 6 Jan 2021 16:59:53 +0000 Subject: [PATCH 09/14] Fix unpickling ExtensionPointValue in newer versions of Python --- envisage/extension_point.py | 131 ++++++++----------------- envisage/tests/test_extension_point.py | 6 -- 2 files changed, 42 insertions(+), 95 deletions(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index b548117d3..907157588 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -11,6 +11,7 @@ # Standard library imports. +from functools import wraps import inspect import warnings import weakref @@ -274,6 +275,26 @@ def _get_extension_registry(self, obj): return extension_registry +def _warn_if_not_internal(func): + """ Decorator for instance methods of _ExtensionPointValue such that its + effect is nullified if the function is not called with the _internal_use + flag set to true. + """ + + @wraps(func) + def decorated(object, *args, **kwargs): + if not object._internal_use: + warnings.warn( + "Extension point cannot be mutated directly.", + RuntimeWarning, + stacklevel=3, + ) + return func(TraitList(iter(object)), *args, **kwargs) + return func(object, *args, **kwargs) + + return decorated + + class _ExtensionPointValue(TraitList): """ _ExtensionPointValue is the list being returned while retrieving the attribute value for an ExtensionPoint trait. @@ -304,11 +325,18 @@ class _ExtensionPointValue(TraitList): Iterable providing the items for the list """ - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + def __new__(cls, *args, **kwargs): + # Methods such as 'append' or 'extend' may be called during unpickling. + # Initialize internal flag to true which gets changed back to false + # in __init__. + self = super().__new__(cls) + self._internal_use = True + return self + def __init__(self, *args, **kwargs): # Flag to control access for mutating the list. Only internal # code can mutate the list. See _sync_values + super().__init__(*args, **kwargs) self._internal_use = False def _sync_values(self, event): @@ -336,93 +364,18 @@ def _sync_values(self, event): finally: self._internal_use = False - # Reimplement TraitList interface to avoid any mutation. - # The original implementation of __setitem__ and __delitem__ can be used - # by internal code. - - def __delitem__(self, key): - """ Reimplemented TraitList.__delitem__ """ - - # This is used by internal code - - if not self._internal_use: - self._warn_mutation() - return - - super().__delitem__(key) - - def __iadd__(self, value): - """ Reimplemented TraitList.__iadd__ """ - # We should not need it for internal use either. - self._warn_mutation() - return self[:] - - def __imul__(self, value): - """ Reimplemented TraitList.__imul__ """ - # We should not need it for internal use either. - self._warn_mutation() - return self[:] - - def __setitem__(self, key, value): - """ Reimplemented TraitList.__setitem__ """ - - # This is used by internal code - - if not self._internal_use: - self._warn_mutation() - return - - super().__setitem__(key, value) - - def append(self, object): - """ Reimplemented TraitList.append """ - # We should not need it for internal use either. - self._warn_mutation() - - def clear(self): - """ Reimplemented TraitList.clear """ - # We should not need it for internal use either. - self._warn_mutation() - - def extend(self, iterable): - """ Reimplemented TraitList.extend """ - # We should not need it for internal use either. - self._warn_mutation() - - def insert(self, index, object): - """ Reimplemented TraitList.insert """ - # We should not need it for internal use either. - self._warn_mutation() - - def pop(self, index=-1): - """ Reimplemented TraitList.pop """ - # We should not need it for internal use either. - self._warn_mutation() - - def remove(self, value): - """ Reimplemented TraitList.remove """ - # We should not need it for internal use either. - self._warn_mutation() - - def reverse(self): - """ Reimplemented TraitList.reverse """ - # We should not need it for internal use either. - self._warn_mutation() - - def sort(self, *, key=None, reverse=False): - """ Reimplemented TraitList.sort """ - # We should not need it for internal use either. - self._warn_mutation() - - def _warn_mutation(self): - """ Emit a warning when attempts to mutating this list is made - externally. - """ - warnings.warn( - "Extension point cannot be mutated directly.", - RuntimeWarning, - stacklevel=3, - ) + __delitem__ = _warn_if_not_internal(TraitList.__delitem__) + __iadd__ = _warn_if_not_internal(TraitList.__iadd__) + __imul__ = _warn_if_not_internal(TraitList.__imul__) + __setitem__ = _warn_if_not_internal(TraitList.__setitem__) + append = _warn_if_not_internal(TraitList.append) + clear = _warn_if_not_internal(TraitList.clear) + extend = _warn_if_not_internal(TraitList.extend) + insert = _warn_if_not_internal(TraitList.insert) + pop = _warn_if_not_internal(TraitList.pop) + remove = _warn_if_not_internal(TraitList.remove) + reverse = _warn_if_not_internal(TraitList.reverse) + sort = _warn_if_not_internal(TraitList.sort) def _get_extensions(object, name): diff --git a/envisage/tests/test_extension_point.py b/envisage/tests/test_extension_point.py index 54a1000dc..82b969a6f 100644 --- a/envisage/tests/test_extension_point.py +++ b/envisage/tests/test_extension_point.py @@ -164,12 +164,6 @@ class Foo(TestBase): with self.assertWarns(RuntimeWarning): f.x[0] = 99 - with self.assertWarns(RuntimeWarning): - f.x *= 99 - - with self.assertWarns(RuntimeWarning): - f.x += [9] - with self.assertWarns(RuntimeWarning): del f.x[0:2] From 3765d1eb793a71a9c66cf301a1df46c2225badd7 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 6 Jan 2021 17:05:41 +0000 Subject: [PATCH 10/14] Add a comment for the stand-in replacement --- envisage/extension_point.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 907157588..9014f1caf 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -289,6 +289,8 @@ def decorated(object, *args, **kwargs): RuntimeWarning, stacklevel=3, ) + # This restores the existing behavior where the operation + # is acted on a list object that is not persisted. return func(TraitList(iter(object)), *args, **kwargs) return func(object, *args, **kwargs) From b2166e54bab16a18d71d70c74cc36a233967e78d Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 6 Jan 2021 17:12:46 +0000 Subject: [PATCH 11/14] Restore comment location --- envisage/extension_point.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 9014f1caf..f49e63594 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -336,9 +336,10 @@ def __new__(cls, *args, **kwargs): return self def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Flag to control access for mutating the list. Only internal # code can mutate the list. See _sync_values - super().__init__(*args, **kwargs) self._internal_use = False def _sync_values(self, event): From 6f412ceca61e40dab296e148aa631bc81d53038c Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 6 Jan 2021 17:25:06 +0000 Subject: [PATCH 12/14] Restore stacklevel in warning --- envisage/extension_point.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index f49e63594..1f98a42eb 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -287,7 +287,7 @@ def decorated(object, *args, **kwargs): warnings.warn( "Extension point cannot be mutated directly.", RuntimeWarning, - stacklevel=3, + stacklevel=2, ) # This restores the existing behavior where the operation # is acted on a list object that is not persisted. From 08f48bc974aeffa820c70136711f7f09e2d7f8f1 Mon Sep 17 00:00:00 2001 From: Kit Choi Date: Wed, 13 Jan 2021 10:27:10 +0000 Subject: [PATCH 13/14] Remove a redundant back :tick: Co-authored-by: Poruri Sai Rahul --- envisage/extension_point.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envisage/extension_point.py b/envisage/extension_point.py index 1f98a42eb..835edb8cc 100644 --- a/envisage/extension_point.py +++ b/envisage/extension_point.py @@ -225,7 +225,7 @@ def listener(extension_registry, event): _update_cache(obj, trait_name) # In case the cache was created first and the registry is then mutated - # before this ``connect``` is called, the internal cache would be in + # before this ``connect`` is called, the internal cache would be in # an inconsistent state. This also has the side-effect of firing # another change event, hence allowing future changes to be observed # without having to access the trait first. From c34c15fdc5a771c57a0a4ad987dab17e76bb9a75 Mon Sep 17 00:00:00 2001 From: Kit Yan Choi Date: Wed, 13 Jan 2021 11:52:37 +0000 Subject: [PATCH 14/14] Bump traits requirement to 6.1 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ac2388714..0884e5e8c 100644 --- a/setup.py +++ b/setup.py @@ -309,7 +309,7 @@ def get_long_description(): "demo_examples = envisage.examples._etsdemo_info:info", ], }, - install_requires=["apptools", "setuptools", "traits"], + install_requires=["apptools", "setuptools", "traits>=6.1"], extras_require={ "docs": ["enthought-sphinx-theme", "Sphinx>=2.1.0,!=3.2.0"], "ipython": ["ipykernel", "tornado"],