Add warning while observing container items with comparison mode set to equality

traits/observers/_has_traits_helpers.py

@@ -11,10 +11,14 @@
 """ Module for HasTraits and CTrait specific utility functions for observers.
 """
 
+import warnings
+
+from traits.constants import ComparisonMode
 from traits.ctraits import CHasTraits
-from traits.trait_base import Undefined, Uninitialized
 from traits.observers._exceptions import NotifierNotFound
 from traits.observers._observe import add_or_remove_notifiers
+from traits.trait_base import Undefined, Uninitialized
+from traits.trait_types import Dict, List, Set
 
 
 #: List of values not to be passed onto the next observers because they are
@@ -27,6 +31,59 @@
 ]
 
 
+def _has_container_trait(ctrait):
+    """ Return true if the CTrait trait type contains container trait
+    at the top-level.
+
+    Parameters
+    ----------
+    ctrait : CTrait
+    """
+    container_types = (Dict, List, Set)
+    is_container = ctrait.is_trait_type(container_types)
+    if is_container:
+        return True
+    # Try inner traits, e.g. to support Union
+    return any(
+        trait.is_trait_type(container_types)
+        for trait in ctrait.inner_traits
+    )
+
+
+def warn_comparison_mode(object, name):
+    """ Check if the trait is a Dict/List/Set with comparison_mode set to
+    equality (or higher). If so, warn about the fact that observers will be
+    lost in the event of reassignment.
+
+    Parameters
+    ----------
+    object : HasTraits
+        Object where a trait is defined
+    name : str
+        Name of a trait to check if warning needs to be issued for it.
+    """
+    ctrait = object.traits()[name]
+
+    if (not ctrait.is_property
+            and _has_container_trait(ctrait)
+            and ctrait.comparison_mode == ComparisonMode.equality):
+        warnings.warn(
+            "Trait {name!r} (trait type: {trait_type}) on class {class_name} "
+            "is defined with comparison_mode={current_mode!r}. "
+            "Mutations and extended traits cannot be observed if a new "
+            "container compared equally to the old one is set. Redefine the "
+            "trait with {trait_type}(..., comparison_mode={new_mode!r}) "
+            "to avoid this.".format(
+                name=name,
+                class_name=object.__class__.__name__,
+                current_mode=ctrait.comparison_mode,
+                trait_type=ctrait.trait_type.__class__.__name__,
+                new_mode=ComparisonMode.identity,
+            ),
+            RuntimeWarning,
+        )
+
+
 def object_has_named_trait(object, name):
     """ Return true if a trait with the given name is defined on the object.
 
@@ -66,6 +123,7 @@ def iter_objects(object, name):
         The value of the trait, if it is defined and is not one of those
         skipped values.
     """
+    warn_comparison_mode(object, name)
     value = object.__dict__.get(name, Undefined)
     if value not in UNOBSERVABLE_VALUES:
         yield value

traits/observers/tests/test_has_traits_helpers.py

@@ -9,11 +9,15 @@
 # Thanks for using Enthought open source!
 
 import unittest
+from unittest import mock
+import warnings
 
 from traits.api import (
-    Bool, HasTraits, List, Instance, Int, Property,
+    Bool, Dict, HasTraits, List, Instance, Int, Property, Set, Union,
 )
 from traits.observers import _has_traits_helpers as helpers
+from traits.observers import expression
+from traits.observers.observe import observe
 
 
 class Bar(HasTraits):
@@ -131,3 +135,161 @@ def test_iter_objects_does_not_trigger_property(self):
         foo = Foo()
         list(helpers.iter_objects(foo, "property_value"))
         self.assertEqual(foo.property_n_calculations, 0)
+
+
+class ObjectWithEqualityComparisonMode(HasTraits):
+    """ Class for supporting TestHasTraitsHelpersWarning """
+
+    list_values = List(comparison_mode=2)
+    dict_values = Dict(comparison_mode=2)
+    set_values = Set(comparison_mode=2)
+    property_list = Property(List(comparison_mode=2))
+    container_in_union = Union(
+        None,
+        Set(comparison_mode=1),
+        comparison_mode=2,
+    )
+
+
+class TestHasTraitsHelpersWarning(unittest.TestCase):
+
+    def test_warn_list_explicit_equality_comparison_mode(self):
+        # Test a warning is emitted if the comparison mode is explicitly set to
+        # equality. Note that iter_objects is for providing values for the
+        # next observers, hence imitates the use case when membership in a
+        # container is observed.
+        instance = ObjectWithEqualityComparisonMode()
+
+        name_to_type = {
+            "list_values": "List",
+            "dict_values": "Dict",
+            "set_values": "Set",
+            "container_in_union": "Union",
+        }
+        for trait_name, type_name in name_to_type.items():
+            with self.subTest(trait_name=trait_name, type_name=type_name):
+
+                with self.assertWarns(RuntimeWarning) as warn_context:
+                    list(helpers.iter_objects(instance, trait_name))
+
+                self.assertIn(
+                    "Redefine the trait with {}(..., comparison_mode".format(
+                        type_name
+                    ),
+                    str(warn_context.warning)
+                )
+
+    def test_union_equality_comparison_mode_prevent_change_event(self):
+        # Justification for the warning: Reassess if the warning is still
+        # needed if this test fails.
+        instance = ObjectWithEqualityComparisonMode()
+        instance.container_in_union = {1}
+        handler = mock.Mock()
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            observe(
+                object=instance,
+                expression=expression.trait("container_in_union").set_items(),
+                handler=handler,
+            )
+
+        # New set, but equals to the previous
+        instance.container_in_union = {1}
+        handler.reset_mock()
+
+        # when
+        instance.container_in_union.add(2)
+
+        # The expected value is 1.
+        # If this fails with 1 != 0, consider removing the warning.
+        self.assertEqual(handler.call_count, 0)
+
+    def test_list_equality_comparison_mode_prevent_change_event(self):
+        # Justification for the warning: Reassess if the warning is still
+        # needed if this test fails.
+        instance = ObjectWithEqualityComparisonMode()
+        instance.list_values = [1]
+        handler = mock.Mock()
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            observe(
+                object=instance,
+                expression=expression.trait("list_values").list_items(),
+                handler=handler,
+            )
+
+        # New list, but equals to the previous
+        instance.list_values = [1]
+        handler.reset_mock()
+
+        # when
+        instance.list_values.append(2)
+
+        # The expected value is 1.
+        # If this fails with 1 != 0, consider removing the warning.
+        self.assertEqual(handler.call_count, 0)
+
+    def test_dict_equality_comparison_mode_prevent_change_event(self):
+        # Justification for the warning: Reassess if the warning is still
+        # needed if this test fails.
+        instance = ObjectWithEqualityComparisonMode()
+        instance.dict_values = {"1": 1}
+        handler = mock.Mock()
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            observe(
+                object=instance,
+                expression=expression.trait("dict_values").dict_items(),
+                handler=handler,
+            )
+
+        # New dict, but equals to the previous
+        instance.dict_values = {"1": 1}
+        handler.reset_mock()
+
+        # when
+        instance.dict_values.pop("1")
+
+        # The expected value is 1.
+        # If this fails with 1 != 0, consider removing the warning.
+        self.assertEqual(handler.call_count, 0)
+
+    def test_set_equality_comparison_mode_prevent_change_event(self):
+        # Justification for the warning: Reassess if the warning is still
+        # needed if this test fails.
+        instance = ObjectWithEqualityComparisonMode()
+        instance.set_values = {1}
+        handler = mock.Mock()
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            observe(
+                object=instance,
+                expression=expression.trait("set_values").set_items(),
+                handler=handler,
+            )
+
+        # New set, but equals to the previous
+        instance.set_values = {1}
+        handler.reset_mock()
+
+        # when
+        instance.set_values.add(2)
+
+        # The expected value is 1.
+        # If this fails with 1 != 0, consider removing the warning.
+        self.assertEqual(handler.call_count, 0)
+
+    def test_no_warn_for_property(self):
+        # property is computed and downstream observers are not useful.
+        # they do exist in the wild. Do not warn for those.
+
+        instance = ObjectWithEqualityComparisonMode()
+
+        # almost equivalent to assertNoWarns...
+        with self.assertRaises(AssertionError):
+            with self.assertWarns(RuntimeWarning):
+                list(helpers.iter_objects(instance, "property_list"))