-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement ObserverGraph with NamedTraitObserver #976
Merged
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
24ab695
ObserverPath with basic handling of equality
kitchoi 87ed1c5
Handle more cases for equality check
kitchoi 7e8b69b
Add a test that fails due to loops
kitchoi 0a70631
Handling of loops in path
kitchoi 03a2d23
Flake8
kitchoi 780d205
Rename cycle -> loop
kitchoi 59e01da
Check the hash as well
kitchoi dbc4d29
Expand comment and docstring
kitchoi 4d31c91
Add one observer so it can be used with an ObserverPath
kitchoi 658812b
Make NamedTraitObserver immutable
kitchoi 63a7363
Expand docstring for name
kitchoi 7608bbd
Implement hashing on NamedTraitObserver
kitchoi 2748837
Clean up and renaming
kitchoi 918fa43
Move the location of integration test
kitchoi 96cbeb1
Flake8
kitchoi 7e159be
Refine docstring
kitchoi 4e5d7a1
Flake8
kitchoi c9da46b
Remove unnecessary dependency on helper function in test
kitchoi 2343b10
Require keyword arguments
kitchoi b166dd0
Expand a comment
kitchoi b281326
Rename modules
kitchoi e31b3b0
Remove notify from IObserver for now as it is not relevant for Observ…
kitchoi fdf2796
Remove redundant length check
kitchoi 1697d70
Remove loops for now
kitchoi 5928716
Rename _interfaces to _i_observer
kitchoi 606027c
Make IObserver an ABC
kitchoi 1ee2139
Move Parameters section up
kitchoi 036e76a
Combine the equality checks
kitchoi 25552f2
Rename nexts -> children
kitchoi 396d9b6
Rename module from _observer_path to _observer_graph
kitchoi 8afbe83
Rename ObserverPath to ObserverGraph
kitchoi 5939e54
Remove negative assertions on hashes
kitchoi 0203663
Fix error messages mentioning subclasses
kitchoi a712224
Remove a redundant hashing
kitchoi 3bd2323
Make children ordered, but still compare regardless of ordering
kitchoi 54d667e
Rename path -> graph
kitchoi 32648b3
Flake8
kitchoi 05889d5
Raise early
kitchoi 9255074
Rename the test module from test_observer_path -> test_observer_graph
kitchoi df7b604
Add a newline between copyright header and imports
kitchoi 07b141f
Use register as a class decorator
kitchoi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
import abc | ||
|
||
|
||
class IObserver(abc.ABC): | ||
""" Interface for all observers. | ||
|
||
Each instance of ``IObserver`` can be a node in the | ||
``ObserverGraph``. These objects are considered | ||
low-level objects not to be instantiated directly by the | ||
user. In order to support equality and hashing on the | ||
``ObserverGraph``, ``IObserver`` needs to be hashable | ||
and it needs to support comparison for equality. | ||
""" | ||
|
||
def __hash__(self): | ||
""" Return a hash of this object.""" | ||
raise NotImplementedError("__hash__ must be implemented.") | ||
|
||
def __eq__(self, other): | ||
""" Return true if this observer is equal to the given one.""" | ||
raise NotImplementedError("__eq__ must be implemented.") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
from traits.observers._i_observer import IObserver | ||
|
||
|
||
@IObserver.register | ||
class NamedTraitObserver: | ||
""" Observer for observing changes on a named trait | ||
on an instance of HasTraits. | ||
""" | ||
|
||
def __init__(self, *, name, notify): | ||
""" Initializer. | ||
Once this observer is defined, it should not be mutated. | ||
|
||
Parameters | ||
---------- | ||
name : str | ||
Name of the trait to be observed. | ||
notify : boolean | ||
Whether to notify for changes. | ||
""" | ||
self._name = name | ||
self._notify = notify | ||
|
||
@property | ||
def name(self): | ||
""" Name of trait to observe on any given HasTraits object.""" | ||
return self._name | ||
|
||
@property | ||
def notify(self): | ||
""" A boolean for whether this observer will notify | ||
for changes. | ||
""" | ||
return self._notify | ||
|
||
def __hash__(self): | ||
return hash((type(self), self.name, self.notify)) | ||
mdickinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def __eq__(self, other): | ||
return ( | ||
type(self) is type(other) | ||
and self.name == other.name | ||
and self.notify == other.notify | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
|
||
class ObserverGraph: | ||
""" An ``ObserverGraph`` is an object for describing what traits are being | ||
observed on an instance of ``HasTraits``. | ||
|
||
The most basic unit in a graph is a node, which is a context specific | ||
observer. For example, a node can be an observer specialized in | ||
changes on a named trait, an observer specialized in | ||
changes on a number of traits matching a certain criteria, an observer | ||
specialized in mutations on a list, etc. | ||
|
||
The most basic example is an ``ObserverGraph`` that contains only one node, | ||
e.g. for observing changes on a named trait. | ||
|
||
An ``ObserverGraph`` can have branches, e.g. to observe more than one trait | ||
on a nested object. The attribute ``children`` represents these branches. | ||
Each item in ``children`` is another ``ObserverGraph``. | ||
|
||
In order to (1) avoid hooking up a user callback with the same observer | ||
twice, and (2) remove an observer when they are not needed, once an | ||
``ObserverGraph`` object is constructed (e.g. after mutating ``children`` | ||
for constructing branches) and is ready to be used against an instance | ||
of ``HasTraits``, it should not be mutated again. | ||
|
||
For the same reason, ``ObserverGraph`` implements ``__hash__`` and | ||
``__eq__`` and requires its nodes to also support these methods. | ||
|
||
An ``ObserverGraph`` does not keep states regarding the HasTraits instances | ||
and the user callbacks it was used with. An ``ObserverGraph`` can be | ||
reused multiple times on different ``HasTraits`` instance and with | ||
different user callback. | ||
|
||
This object is considered a low-level object for the observer mechanism. | ||
It is not intended to be instantiated by users directly. Users will be | ||
given higher-level wrappers for creating ``ObserverGraph`` objects. | ||
|
||
Parameters | ||
---------- | ||
node : any | ||
A context specific observer. | ||
It must be a hashable object. In practice, this will be | ||
an instance that implements ``IObserver``. | ||
children : iterable of ObserverGraph, optional | ||
Branches on this graph. All children must be unique. | ||
|
||
Raises | ||
------ | ||
ValueError | ||
If not all children are unique. | ||
""" | ||
|
||
def __init__(self, *, node, children=None): | ||
|
||
if children is not None and len(set(children)) != len(children): | ||
raise ValueError("Not all children are unique.") | ||
|
||
self.node = node | ||
self.children = list(children) if children is not None else [] | ||
|
||
def __hash__(self): | ||
""" Return the hash of this ObserverGraph.""" | ||
return hash( | ||
(type(self), self.node, frozenset(self.children)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously I had |
||
) | ||
|
||
def __eq__(self, other): | ||
""" Return true if another object is an ObserverGraph with the | ||
same content. The order of children is not taken into account | ||
in the comparison. | ||
""" | ||
return ( | ||
type(self) is type(other) | ||
and self.node == other.node | ||
and set(self.children) == set(other.children) | ||
) |
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
import unittest | ||
from unittest import mock | ||
|
||
from traits.observers._named_trait_observer import ( | ||
NamedTraitObserver, | ||
) | ||
from traits.observers._observer_graph import ObserverGraph | ||
|
||
|
||
class TestNamedTraitObserver(unittest.TestCase): | ||
|
||
def test_not_equal_notify(self): | ||
observer1 = NamedTraitObserver(name="foo", notify=True) | ||
observer2 = NamedTraitObserver(name="foo", notify=False) | ||
self.assertNotEqual(observer1, observer2) | ||
|
||
def test_not_equal_name(self): | ||
observer1 = NamedTraitObserver(name="foo", notify=True) | ||
observer2 = NamedTraitObserver(name="bar", notify=True) | ||
self.assertNotEqual(observer1, observer2) | ||
|
||
def test_equal_observers(self): | ||
observer1 = NamedTraitObserver(name="foo", notify=True) | ||
observer2 = NamedTraitObserver(name="foo", notify=True) | ||
self.assertEqual(observer1, observer2) | ||
self.assertEqual(hash(observer1), hash(observer2)) | ||
|
||
def test_not_equal_type(self): | ||
observer = NamedTraitObserver(name="foo", notify=True) | ||
imposter = mock.Mock() | ||
imposter.name = "foo" | ||
imposter.notify = True | ||
self.assertNotEqual(observer, imposter) | ||
|
||
def test_name_not_mutable(self): | ||
observer = NamedTraitObserver(name="foo", notify=True) | ||
with self.assertRaises(AttributeError) as exception_context: | ||
observer.name = "bar" | ||
self.assertEqual( | ||
str(exception_context.exception), "can't set attribute") | ||
|
||
def test_notify_not_mutable(self): | ||
observer = NamedTraitObserver(name="foo", notify=True) | ||
with self.assertRaises(AttributeError) as exception_context: | ||
observer.notify = False | ||
self.assertEqual( | ||
str(exception_context.exception), "can't set attribute") | ||
|
||
|
||
class TestObserverGraphIntegrateNamedTraitObserver(unittest.TestCase): | ||
""" Test integrating ObserverGraph with NamedTraitObserver as nodes. | ||
""" | ||
|
||
def test_observer_graph_hash_with_named_listener(self): | ||
# Test equality + hashing using set passes. | ||
|
||
path1 = ObserverGraph( | ||
node=NamedTraitObserver(name="foo", notify=True), | ||
children=[ | ||
ObserverGraph( | ||
node=NamedTraitObserver(name="bar", notify=True), | ||
), | ||
], | ||
) | ||
path2 = ObserverGraph( | ||
node=NamedTraitObserver(name="foo", notify=True), | ||
children=[ | ||
ObserverGraph( | ||
node=NamedTraitObserver(name="bar", notify=True), | ||
), | ||
], | ||
) | ||
# This tests __eq__ and __hash__ | ||
self.assertEqual(path1, path2) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
# (C) Copyright 2005-2020 Enthought, Inc., Austin, TX | ||
# All rights reserved. | ||
# | ||
# This software is provided without warranty under the terms of the BSD | ||
# license included in LICENSE.txt and may be redistributed only under | ||
# the conditions described in the aforementioned license. The license | ||
# is also available online at http://www.enthought.com/licenses/BSD.txt | ||
# | ||
# Thanks for using Enthought open source! | ||
|
||
import unittest | ||
|
||
from traits.observers._observer_graph import ObserverGraph | ||
|
||
|
||
def graph_from_nodes(*nodes): | ||
nodes = nodes[::-1] | ||
graph = ObserverGraph(node=nodes[0]) | ||
for node in nodes[1:]: | ||
graph = ObserverGraph(node=node, children=[graph]) | ||
return graph | ||
|
||
|
||
class TestObserverGraph(unittest.TestCase): | ||
""" Test generic functions on ObserverGraph.""" | ||
|
||
def test_equality(self): | ||
graph1 = graph_from_nodes(1, 2, 3) | ||
graph2 = graph_from_nodes(1, 2, 3) | ||
self.assertEqual(graph1, graph2) | ||
self.assertEqual(hash(graph1), hash(graph2)) | ||
|
||
def test_equality_different_type(self): | ||
graph1 = graph_from_nodes(1, 2, 3) | ||
self.assertNotEqual(graph1, 1) | ||
|
||
def test_equality_different_length_children(self): | ||
graph1 = ObserverGraph( | ||
node=1, | ||
children=[ | ||
ObserverGraph(node=2), | ||
ObserverGraph(node=3), | ||
], | ||
) | ||
graph2 = ObserverGraph( | ||
node=1, | ||
children=[ | ||
ObserverGraph(node=2), | ||
], | ||
) | ||
self.assertNotEqual(graph1, graph2) | ||
|
||
def test_equality_order_of_children(self): | ||
# The order of items in children does not matter | ||
graph1 = ObserverGraph( | ||
node=1, | ||
children=[ | ||
ObserverGraph(node=2), | ||
ObserverGraph(node=3), | ||
], | ||
) | ||
graph2 = ObserverGraph( | ||
node=1, | ||
children=[ | ||
ObserverGraph(node=3), | ||
ObserverGraph(node=2), | ||
], | ||
) | ||
self.assertEqual(graph1, graph2) | ||
self.assertEqual(hash(graph1), hash(graph2)) | ||
|
||
def test_children_ordered(self): | ||
child_graph = ObserverGraph(node=2) | ||
graph = ObserverGraph( | ||
node=1, | ||
children=[ | ||
child_graph, | ||
ObserverGraph(node=3), | ||
], | ||
) | ||
self.assertIs(graph.children[0], child_graph) | ||
|
||
def test_children_unique(self): | ||
child_graph = ObserverGraph(node=2) | ||
|
||
with self.assertRaises(ValueError) as exception_cm: | ||
ObserverGraph( | ||
node=1, | ||
children=[ | ||
child_graph, | ||
ObserverGraph(node=2), | ||
], | ||
) | ||
|
||
self.assertEqual( | ||
str(exception_cm.exception), "Not all children are unique.") |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a style nitpick: could we keep a blank line separating the copyright header from the module body?