From 936f1ae183b7490e33ea38a7e642ae9d25557aab Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Mon, 23 Jun 2025 01:30:16 -0400 Subject: [PATCH 1/4] Respect priority between one- and two-argument callbacks. --- echo/callback_container.py | 21 +++++++++++++++++---- echo/core.py | 16 ++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/echo/callback_container.py b/echo/callback_container.py index 9620d82c..7e9545c7 100644 --- a/echo/callback_container.py +++ b/echo/callback_container.py @@ -72,8 +72,13 @@ def _contains(self, value, priority=None): else: return False - def __iter__(self): - for callback in sorted(self.callbacks, key=lambda x: x[-1], reverse=True): + def iterate(self, use_priority=True, with_priority=False): + if use_priority: + iterator = sorted(self.callbacks, key=lambda x: x[-1], reverse=True) + else: + iterator = self.callbacks + + for callback in iterator: if len(callback) == 3: func = callback[0]() inst = callback[1]() @@ -82,9 +87,17 @@ def __iter__(self): # just check here that the weakrefs were resolved if func is None or inst is None: continue - yield partial(func, inst) + result = partial(func, inst) else: - yield callback[0] + result = callback[0] + if with_priority: + yield result, callback[-1] + else: + yield result + + def __iter__(self): + for callback in self.iterate(): + yield callback def __len__(self): return len(self.callbacks) diff --git a/echo/core.py b/echo/core.py index 4e910ba0..6635887f 100644 --- a/echo/core.py +++ b/echo/core.py @@ -1,6 +1,7 @@ import weakref from weakref import WeakKeyDictionary from contextlib import contextmanager +from itertools import chain from .callback_container import CallbackContainer @@ -128,10 +129,17 @@ def notify(self, instance, old, new): """ if not self.enabled(instance): return - for cback in self._callbacks.get(instance, []): - cback(new) - for cback in self._2arg_callbacks.get(instance, []): - cback(old, new) + iterators = [] + if (container := self._callbacks.get(instance, None)): + iterators.append((cb, priority, 1) for cb, priority in container.iterate(with_priority=True, use_priority=False)) + if (container := self._2arg_callbacks.get(instance, None)): + iterators.append((cb, priority, 2) for cb, priority in container.iterate(with_priority=True, use_priority=False)) + + for callback, _priority, args in sorted(chain(*iterators), key=lambda x: x[1], reverse=True): + if args == 1: + callback(new) + else: + callback(old, new) def _validate(self, instance, old, new): """ From 5de87e686f02fa62ddefc104524dd4e3b80f5469 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Mon, 23 Jun 2025 10:19:43 -0400 Subject: [PATCH 2/4] Rename iteration method and parameters. --- echo/callback_container.py | 8 ++++---- echo/core.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/echo/callback_container.py b/echo/callback_container.py index 7e9545c7..9e336f31 100644 --- a/echo/callback_container.py +++ b/echo/callback_container.py @@ -72,8 +72,8 @@ def _contains(self, value, priority=None): else: return False - def iterate(self, use_priority=True, with_priority=False): - if use_priority: + def iterator(self, sort=True, priority=False): + if sort: iterator = sorted(self.callbacks, key=lambda x: x[-1], reverse=True) else: iterator = self.callbacks @@ -90,13 +90,13 @@ def iterate(self, use_priority=True, with_priority=False): result = partial(func, inst) else: result = callback[0] - if with_priority: + if priority: yield result, callback[-1] else: yield result def __iter__(self): - for callback in self.iterate(): + for callback in self.iterator(): yield callback def __len__(self): diff --git a/echo/core.py b/echo/core.py index 6635887f..8800f4b6 100644 --- a/echo/core.py +++ b/echo/core.py @@ -131,9 +131,9 @@ def notify(self, instance, old, new): return iterators = [] if (container := self._callbacks.get(instance, None)): - iterators.append((cb, priority, 1) for cb, priority in container.iterate(with_priority=True, use_priority=False)) + iterators.append((cb, priority, 1) for cb, priority in container.iterator(priority=True, sort=False)) if (container := self._2arg_callbacks.get(instance, None)): - iterators.append((cb, priority, 2) for cb, priority in container.iterate(with_priority=True, use_priority=False)) + iterators.append((cb, priority, 2) for cb, priority in container.iterator(priority=True, sort=False)) for callback, _priority, args in sorted(chain(*iterators), key=lambda x: x[1], reverse=True): if args == 1: From eaf4e1175fd7816bde871b7b4c205da392a35ef7 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 22 Oct 2025 11:04:49 -0400 Subject: [PATCH 3/4] Add docstring for iterator. --- echo/callback_container.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/echo/callback_container.py b/echo/callback_container.py index 9e336f31..ac7df1b5 100644 --- a/echo/callback_container.py +++ b/echo/callback_container.py @@ -73,6 +73,20 @@ def _contains(self, value, priority=None): return False def iterator(self, sort=True, priority=False): + """ + Returns an iterator over the callback functions inside the container, skipping + and removing callbacks where the underlying instance has already been garbage + collected. + + Parameters + --------- + sort: bool, optional + Whether the yielded callbacks will be sorted (by priority). + Defaults to `False`. + priority: bool, optional + If `True`, each iteration will yield a tuple (callback, priority). + If `False` (the default), only the callback is yielded. + """ if sort: iterator = sorted(self.callbacks, key=lambda x: x[-1], reverse=True) else: From 583efc6da0cb028ff3fe1b09df7a594b9eaea4cf Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 22 Oct 2025 11:08:15 -0400 Subject: [PATCH 4/4] Add test of one-vs-two argument callback priority. --- echo/tests/test_core.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/echo/tests/test_core.py b/echo/tests/test_core.py index a1de1f9b..75910972 100644 --- a/echo/tests/test_core.py +++ b/echo/tests/test_core.py @@ -1,5 +1,5 @@ import pytest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, call from echo import (CallbackProperty, add_callback, remove_callback, delay_callback, @@ -710,3 +710,32 @@ def x(self, value): foo.x = 1 assert test.call_count == 1 + + +def test_priority_order_twoargs(): + + mock = MagicMock() + + def one_arg(value): + mock(value) + + def two_arg(old, new): + mock(old, new) + + state = State() + state.add_callback('a', one_arg, priority=10) + state.add_callback('a', two_arg, echo_old=True, priority=1000) + + state.a = 2 + calls = [call(None, 2), call(2)] + mock.assert_has_calls(calls, any_order=False) + + mock.reset_mock() + + state2 = State() + state2.add_callback('a', one_arg, priority=1000) + state2.add_callback('a', two_arg, echo_old=True, priority=10) + + state2.a = 7 + calls = [call(7), call(None, 7)] + mock.assert_has_calls(calls, any_order=False)