From 34fc5531e985b59cc9a04f853d560fc0c1791901 Mon Sep 17 00:00:00 2001 From: WU Leizhi Date: Mon, 5 Oct 2026 04:09:12 +0800 Subject: [PATCH] fix: avoid comparing unchanged linked trait values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent assistance: Codex 🤖🤖. --- docs/source/utils.rst | 7 ++++ tests/test_traitlets.py | 79 +++++++++++++++++++++++++++++++++++++++++ traitlets/traitlets.py | 14 ++++++-- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/docs/source/utils.rst b/docs/source/utils.rst index 76c8b4191..60f4a0e88 100644 --- a/docs/source/utils.rst +++ b/docs/source/utils.rst @@ -40,6 +40,13 @@ Example: Links ----- +After a link writes the other trait, ``link`` uses the protected +``_should_update`` method to check that the original trait still has the value +from the change event. The default accepts identical objects and otherwise +uses inequality. Subclasses can override this post-propagation consistency +check for values with custom comparison behavior; it does not suppress the +write to the other trait. + .. autoclass:: link .. autoclass:: directional_link diff --git a/tests/test_traitlets.py b/tests/test_traitlets.py index 3fbb676aa..96a934179 100644 --- a/tests/test_traitlets.py +++ b/tests/test_traitlets.py @@ -2054,6 +2054,83 @@ def _value_validate(self, proposal): class TestLink(TestCase): + def test_non_scalar_identity_comparison_preserves_observer_chain(self): + class AmbiguousComparison: + def __bool__(self): + raise ValueError("ambiguous comparison") + + class Value: + __hash__ = None + + def __eq__(self, other): + return AmbiguousComparison() + + __ne__ = __eq__ + + class A(HasTraits): + value = Instance(Value) + + a = A(value=Value()) + b = A(value=Value()) + c = link((a, "value"), (b, "value")) + observed = [] + a.observe(lambda change: observed.append("a"), names="value") + b.observe(lambda change: observed.append("b"), names="value") + + a.value = Value() + b.value = Value() + + self.assertEqual(observed, ["b", "a", "a", "b"]) + self.assertIs(a.value, b.value) + self.assertFalse(c.updating) + + def test_should_update_can_be_overridden(self): + class Value: + __hash__ = None + + def __init__(self, value): + self.value = value + + def __eq__(self, other): + return isinstance(other, Value) and self.value == other.value + + def __ne__(self, other): + return not self == other + + class A(HasTraits): + value = Instance(Value) + + class IdentityLink(link): + def _should_update(self, old, new): + return old is not new + + def replace_with_equal_distinct(owner, change): + if not replaced: + replaced.append(True) + owner.value = Value(change.new.value) + + for link_type, raises in ((link, False), (IdentityLink, True)): + for owner_name in ("source", "target"): + source = A(value=Value(1)) + target = A(value=Value(1)) + replaced = [] + owner = source if owner_name == "source" else target + original = source if owner_name == "source" else target + opposite = target if owner_name == "source" else source + opposite.observe( + lambda change, original=original: replace_with_equal_distinct(original, change), + names="value", + ) + connection = link_type((source, "value"), (target, "value")) + replaced.clear() + + if raises: + with self.assertRaises(TraitError): + owner.value = Value(2) + else: + owner.value = Value(2) + self.assertFalse(connection.updating) + def test_connect_same(self): """Verify two traitlets of the same type can be linked together using link.""" @@ -2203,6 +2280,7 @@ def another_update(self, change): mc = MyClass() l = link((mc, "i"), (mc, "j")) self.assertRaises(TraitError, setattr, mc, "i", 2) + self.assertFalse(l.updating) def test_link_broken_at_target(self): class MyClass(HasTraits): @@ -2216,6 +2294,7 @@ def another_update(self, change): mc = MyClass() l = link((mc, "i"), (mc, "j")) self.assertRaises(TraitError, setattr, mc, "j", 2) + self.assertFalse(l.updating) class TestDirectionalLink(TestCase): diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py index 2cc239c14..c3ce8dba5 100644 --- a/traitlets/traitlets.py +++ b/traitlets/traitlets.py @@ -357,12 +357,22 @@ def _busy_updating(self) -> t.Any: finally: self.updating = False + def _should_update(self, old: t.Any, new: t.Any) -> bool: + """Return whether two values should be considered different. + + Subclasses can override this method when a trait value has comparison + semantics that cannot be reduced to a boolean with ``!=``. + """ + if old is new: + return False + return bool(old != new) + def _update_target(self, change: t.Any) -> None: if self.updating: return with self._busy_updating(): setattr(self.target[0], self.target[1], self._transform(change.new)) - if getattr(self.source[0], self.source[1]) != change.new: + if self._should_update(getattr(self.source[0], self.source[1]), change.new): raise TraitError( f"Broken link {self}: the source value changed while updating the target." ) @@ -372,7 +382,7 @@ def _update_source(self, change: t.Any) -> None: return with self._busy_updating(): setattr(self.source[0], self.source[1], self._transform_inv(change.new)) - if getattr(self.target[0], self.target[1]) != change.new: + if self._should_update(getattr(self.target[0], self.target[1]), change.new): raise TraitError( f"Broken link {self}: the target value changed while updating the source." )