From ee901763145bb8bec1688962009245e455d98b31 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 16:24:30 +0200 Subject: [PATCH 1/7] Avoid events boomeranging from frontend --- panel/reactive.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/panel/reactive.py b/panel/reactive.py index 626c2e5835..8bb7da55d0 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -138,6 +138,9 @@ def __init__(self, **params): # A dictionary of bokeh property changes being processed self._changing = {} + # A dictionary of parameter changes being processed + self._in_process__events = {} + # Whether the component is watching the stylesheets self._watching_stylesheets = False @@ -326,7 +329,12 @@ def _update_model( ) -> None: ref = root.ref['id'] self._changing[ref] = attrs = [] - for attr, value in msg.items(): + for attr, value in dict(msg).items(): + # Do not apply model change that is in flight + if attr in self._events or (attr in self._in_process__events and value is self._in_process__events[attr]): + del msg[attr] + continue + # Bokeh raises UnsetValueError if the value is Undefined. try: model_val = getattr(model, attr) @@ -336,10 +344,6 @@ def _update_model( if not model.lookup(attr).property.matches(model_val, value): attrs.append(attr) - # Do not apply model change that is in flight - if attr in self._events: - del self._events[attr] - try: model.update(**msg) finally: @@ -416,7 +420,8 @@ def _process_events(self, events: dict[str, Any]) -> None: if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): state._busy_counter += 1 - params = self._process_property_change(dict(events)) + self._in_process__events = events = dict(events) + params = self._process_property_change(events) try: with edit_readonly(self): self_params = {k: v for k, v in params.items() if '.' not in k} From 1bb1339c0a048bf045868ffb0100273830bcb1e5 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 16:33:07 +0200 Subject: [PATCH 2/7] Small fix --- panel/reactive.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/panel/reactive.py b/panel/reactive.py index 8bb7da55d0..8ffa75edc9 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -331,7 +331,11 @@ def _update_model( self._changing[ref] = attrs = [] for attr, value in dict(msg).items(): # Do not apply model change that is in flight - if attr in self._events or (attr in self._in_process__events and value is self._in_process__events[attr]): + if attr in self._in_process__events and value is self._in_process__events[attr]: + del self._in_process__events[attr] + del msg[attr] + continue + elif attr in self._events: del msg[attr] continue From 74f7e6bb4542124a64710437a72f4ce588bf8cba Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 16:49:51 +0200 Subject: [PATCH 3/7] More fixes --- panel/reactive.py | 8 ++++++-- panel/tests/test_param.py | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/panel/reactive.py b/panel/reactive.py index 8ffa75edc9..ba9ade3264 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -330,13 +330,17 @@ def _update_model( ref = root.ref['id'] self._changing[ref] = attrs = [] for attr, value in dict(msg).items(): - # Do not apply model change that is in flight if attr in self._in_process__events and value is self._in_process__events[attr]: + # Do not apply change that originated directly from + # the frontend since this may cause boomerang if a + # new property value is already in-flight del self._in_process__events[attr] del msg[attr] continue elif attr in self._events: - del msg[attr] + # Do not override a property value that was just changed + # on the frontend + del self._events[attr] continue # Bokeh raises UnsetValueError if the value is Undefined. diff --git a/panel/tests/test_param.py b/panel/tests/test_param.py index e3801ec98d..7ea286e55d 100644 --- a/panel/tests/test_param.py +++ b/panel/tests/test_param.py @@ -574,7 +574,7 @@ class Test(param.Parameterized): assert mb.value != 3 assert test.b == 3 - test_pane._widgets['b']._process_events({'value': 4}) + mb.value = 4 assert test.b == 3 assert mb.value == 4 @@ -616,7 +616,7 @@ class Test(param.Parameterized): assert mb.value != '3' assert test.b == '3' - test_pane._widgets['b']._process_events({'value': '4'}) + mb.value = '4' assert test.b == '3' assert mb.value == '4' @@ -873,7 +873,7 @@ class Test(param.Parameterized): assert number.value != 3 assert test.a == 3 - pane._widgets['a']._process_events({'value': 4}) + number.value = 4 assert test.a == 3 assert number.value == 4 From 8cc1e7815280d74a01e27baf7e3580b453048420 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 17:44:54 +0200 Subject: [PATCH 4/7] Fix issue with busy counter --- panel/reactive.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/panel/reactive.py b/panel/reactive.py index ba9ade3264..fc1fafc911 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -334,7 +334,6 @@ def _update_model( # Do not apply change that originated directly from # the frontend since this may cause boomerang if a # new property value is already in-flight - del self._in_process__events[attr] del msg[attr] continue elif attr in self._events: @@ -455,6 +454,7 @@ def _process_events(self, events: dict[str, Any]) -> None: log.exception(f'Callback failed for object named {self.name!r} {msg_end}') raise finally: + self._in_process_events = {} self._log('finished processing events %s', events) if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): From c4fee503aa24eb054401a0065e7e194ae4faec79 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 17:53:18 +0200 Subject: [PATCH 5/7] Small fix --- panel/reactive.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/panel/reactive.py b/panel/reactive.py index fc1fafc911..c527f761f5 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -427,7 +427,7 @@ def _process_events(self, events: dict[str, Any]) -> None: if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): state._busy_counter += 1 - self._in_process__events = events = dict(events) + self._in_process__events.update(events) params = self._process_property_change(events) try: with edit_readonly(self): @@ -454,7 +454,7 @@ def _process_events(self, events: dict[str, Any]) -> None: log.exception(f'Callback failed for object named {self.name!r} {msg_end}') raise finally: - self._in_process_events = {} + self._in_process__events.clear() self._log('finished processing events %s', events) if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): From 830131e255a4641d942312d4ce74d55c8e997584 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 19:52:19 +0200 Subject: [PATCH 6/7] Ensure boomerang checks handle different threads and multiple views --- panel/reactive.py | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/panel/reactive.py b/panel/reactive.py index c527f761f5..7558cb3750 100644 --- a/panel/reactive.py +++ b/panel/reactive.py @@ -307,21 +307,36 @@ def _update_manual(self, *events: param.parameterized.Event) -> None: else: cb() + def _scheduled_update_model( + self, events: dict[str, param.parameterized.Event], msg: dict[str, Any], + root: Model, model: Model, doc: Document, comm: Optional[Comm], + curdoc_events: dict[str, Any] + ) -> None: + # + self._in_process__events[doc] = curdoc_events + try: + self._update_model(events, msg, root, model, doc, comm) + finally: + del self._in_process__events[doc] + def _apply_update( self, events: dict[str, param.parameterized.Event], msg: dict[str, Any], model: Model, ref: str - ) -> None: + ) -> bool: if ref not in state._views or ref in state._fake_roots: - return + return True viewable, root, doc, comm = state._views[ref] if comm or not doc.session_context or state._unblocked(doc): with unlocked(): self._update_model(events, msg, root, model, doc, comm) if comm and 'embedded' not in root.tags: push(doc, comm) + return True else: - cb = partial(self._update_model, events, msg, root, model, doc, comm) + curdoc_events = self._in_process__events.pop(doc, {}) + cb = partial(self._scheduled_update_model, events, msg, root, model, doc, comm, curdoc_events) doc.add_next_tick_callback(cb) + return False def _update_model( self, events: dict[str, param.parameterized.Event], msg: dict[str, Any], @@ -329,8 +344,9 @@ def _update_model( ) -> None: ref = root.ref['id'] self._changing[ref] = attrs = [] - for attr, value in dict(msg).items(): - if attr in self._in_process__events and value is self._in_process__events[attr]: + curdoc_events = self._in_process__events.get(doc, {}) + for attr, value in msg.copy().items(): + if attr in curdoc_events and value is curdoc_events[attr]: # Do not apply change that originated directly from # the frontend since this may cause boomerang if a # new property value is already in-flight @@ -416,18 +432,25 @@ async def _watch_stylesheets(self): def _param_change(self, *events: param.parameterized.Event) -> None: named_events = {event.name: event for event in events} + applied = False for ref, (model, _) in self._models.copy().items(): properties = self._update_properties(*events, doc=model.document) if not properties: return - self._apply_update(named_events, properties, model, ref) + applied &= self._apply_update(named_events, properties, model, ref) + if ref not in state._views: + continue + doc = state._views[ref][2] + if applied and doc in self._in_process__events: + del self._in_process__events[doc] def _process_events(self, events: dict[str, Any]) -> None: self._log('received events %s', events) if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): state._busy_counter += 1 - self._in_process__events.update(events) + if events: + self._in_process__events[state.curdoc] = events params = self._process_property_change(events) try: with edit_readonly(self): @@ -454,7 +477,6 @@ def _process_events(self, events: dict[str, Any]) -> None: log.exception(f'Callback failed for object named {self.name!r} {msg_end}') raise finally: - self._in_process__events.clear() self._log('finished processing events %s', events) if any(e for e in events if e not in self._busy__ignore): with edit_readonly(state): From 7e20bc87833f0207a98b0d84ccb876affbe05431 Mon Sep 17 00:00:00 2001 From: Philipp Rudiger Date: Wed, 7 Aug 2024 20:37:30 +0200 Subject: [PATCH 7/7] Add test --- panel/tests/test_reactive.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/panel/tests/test_reactive.py b/panel/tests/test_reactive.py index 232a86e491..9d1bc6f4f9 100644 --- a/panel/tests/test_reactive.py +++ b/panel/tests/test_reactive.py @@ -15,6 +15,7 @@ from bokeh.models import Div from panel.depends import bind, depends +from panel.io.state import set_curdoc from panel.layout import Tabs, WidgetBox from panel.pane import Markdown from panel.reactive import Reactive, ReactiveHTML @@ -370,6 +371,19 @@ def test_text_input_controls_explicit(): text_input.placeholder = "Test placeholder..." assert placeholder.value == "Test placeholder..." +def test_property_change_does_not_boomerang(document, comm): + text_input = TextInput(value='A') + + model = text_input.get_root(document, comm) + + assert model.value == 'A' + model.value = 'B' + with set_curdoc(document): + text_input._process_events({'value': 'C'}) + + assert model.value == 'B' + assert text_input.value == 'C' + def test_reactive_html_basic(): class Test(ReactiveHTML):