Refactor schedule observer (#118)

* refactor ScheduleObserver

jsdlink: avoid syncing with the backend and update the target widget directly
in the front-end.

* fix schedule unlink / unobserve with transport

* fix schedule_dlink

* fix/update tests

* update release notes

docs/release_notes.rst

@@ -8,6 +8,13 @@ Release Notes
 v0.6.0 (unreleased)
 -------------------
 
+Bug fixes
+~~~~~~~~~
+
+- Fixed ``schedule_jsdlink`` so it doesn't synchronize state with the backend
+  and fix ``unlink`` / ``unobserve`` with ``transport=True`` (:issue:`116`,
+  :pull:`118`).
+
 Maintenance
 ~~~~~~~~~~~
 

ipytone/observe.py

@@ -1,5 +1,5 @@
 import ipywidgets
-from ipywidgets import widget_serialization
+from ipywidgets import Widget, widget_serialization
 from traitlets import (
     Bool,
     Dict,
@@ -19,6 +19,11 @@
 from .serialization import data_array_serialization
 
 
+class DummyLink:
+    def unlink(self):
+        pass
+
+
 class ToneDirectionalLink:
     def __init__(self, observer, link):
         self.observer = observer
@@ -64,6 +69,11 @@ class ScheduleObserver(ToneWidgetBase):
         sync=True
     )
 
+    target_widget = Instance(Widget, help="target widget instance (jsdlink)", allow_none=True).tag(
+        sync=True, **widget_serialization
+    )
+    target_trait = Unicode(allow_none=True).tag(sync=True)
+
     time = Float(0.0, help="current observed time", read_only=True).tag(sync=True)
 
     value = Union(
@@ -145,7 +155,9 @@ def schedule_dlink(self, target, update_interval, transport, js=False):
         self.schedule_repeat(update_interval, transport, draw=draw)
 
         if js:
-            link = ipywidgets.jsdlink((self, self.observed_trait), target)
+            # bypass ipywidget.jsdlink (target is updated directly by this
+            # widget in the front-end)
+            link = DummyLink()
         else:
             link = ipywidgets.dlink((self, self.observed_trait), target)
 
@@ -253,7 +265,7 @@ def schedule_observe(
             )
 
         observer = ScheduleObserver(
-            observed_widget=self, observed_trait=name, observe_time=observe_time
+            observed_widget=self, observed_trait=name, observe_time=observe_time, target_trait=name
         )
         observer.schedule_observe(handler, update_interval, transport)
 
@@ -274,7 +286,24 @@ def schedule_unobserve(self, handler):
     def _schedule_dlink(self, target, update_interval, transport, name, js=False):
         name = self._validate_trait_name(name)
 
-        observer = ScheduleObserver(observed_widget=self, observed_trait=name)
+        if not isinstance(target, tuple) or len(target) != 2:
+            raise ValueError("target must be a (widget_obj, trait_name) 2-length tuple")
+
+        if js:
+            target_widget, target_trait = target
+        else:
+            # the target won't be used directly ; instead the observer instance
+            # created below will sync with the backend and be linked with the
+            # target.
+            target_widget = None
+            target_trait = name
+
+        observer = ScheduleObserver(
+            observed_widget=self,
+            observed_trait=name,
+            target_widget=target_widget,
+            target_trait=target_trait,
+        )
         return observer.schedule_dlink(target, update_interval, transport, js=js)
 
     def schedule_dlink(self, target, update_interval=1, transport=False, name=None):

ipytone/tests/test_observe.py

@@ -67,6 +67,7 @@ def test_schedule_observe(widget, trait_name, handler):
         observer = widget._observers[key]
         assert observer.observed_widget is widget
         assert observer.observed_trait == trait_name
+        assert observer.target_widget is None
 
         # test observe already observed with same handler
         with pytest.raises(ValueError, match="this handler is already used"):
@@ -97,6 +98,12 @@ def test_schedule_dlink(widget, trait_name, target_widget, mocker):
             assert link.observer.observed_widget is widget
             assert link.observer.observed_trait == trait_name
 
+            if method is widget.schedule_dlink:
+                assert link.observer.target_widget is None
+            elif method is widget.schedule_jsdlink:
+                assert link.observer.target_widget is target_widget
+                assert link.observer.target_trait == trait_name
+
             # test unlink
             mocker.patch.object(link.link, "unlink")
             mocker.patch.object(link.observer, "schedule_cancel")
@@ -111,6 +118,7 @@ def test_schedule_observer():
 
     assert observer.observed_widget is sig
     assert observer.observe_time is False
+    assert observer.target_widget is None
     assert observer.time == 0.0
     assert observer.value == 0
     assert observer.state == "stopped"
@@ -149,7 +157,6 @@ def test_schedule_observer_schedule_observe(mocker, handler):
 
 def test_schedule_observer_schedule_link(mocker, target_widget):
     dlink_patch = mocker.patch("ipywidgets.dlink")
-    jsdlink_patch = mocker.patch("ipywidgets.jsdlink")
 
     sig = Signal()
     observer = ScheduleObserver(observed_widget=sig)
@@ -168,4 +175,3 @@ def test_schedule_observer_schedule_link(mocker, target_widget):
     observer.send.assert_called_with(
         {"event": "scheduleRepeat", "update_interval": 1, "transport": True, "draw": True}
     )
-    jsdlink_patch.assert_called_with((observer, "value"), (target_widget, "value"))

src/widget_observe.ts

@@ -1,4 +1,8 @@
-import { ISerializers, unpack_models } from '@jupyter-widgets/base';
+import {
+  ISerializers,
+  unpack_models,
+  WidgetModel,
+} from '@jupyter-widgets/base';
 
 import * as tone from 'tone';
 
@@ -39,6 +43,8 @@ export class ScheduleObserverModel extends ToneWidgetModel {
       observed_widget: null,
       observed_trait: 'value',
       observe_time: false,
+      target_widget: null,
+      target_trait: 'value',
       time: 0.0,
       value: 0.0,
       state: 'stopped',
@@ -52,6 +58,8 @@ export class ScheduleObserverModel extends ToneWidgetModel {
   }
 
   private event: ObserveEvent;
+  private _observedWidget: ObservableModel;
+  private _targetWidget: WidgetModel;
 
   initialize(
     attributes: Backbone.ObjectHash,
@@ -60,10 +68,21 @@ export class ScheduleObserverModel extends ToneWidgetModel {
     super.initialize(attributes, options);
 
     this.event = { id: 0, transport: false };
+
+    // cache unserialized widget models for fast access in schedule loop
+    this._observedWidget = this.get('observed_widget');
+    this._targetWidget = this.get('target_widget');
+    if (this._targetWidget === null) {
+      this._targetWidget = this;
+    }
   }
 
+  /*
+   * Returns the observed ipytone widget model (e.g., audio node, signal, param
+   * or transport)
+   */
   get observedWidget(): ObservableModel {
-    return this.get('observed_widget');
+    return this._observedWidget;
   }
 
   get observedTrait(): string {
@@ -74,35 +93,52 @@ export class ScheduleObserverModel extends ToneWidgetModel {
     return this.get('observe_time');
   }
 
-  private setObservedTrait(time: tone.Unit.Seconds, transport: boolean): void {
-    const model = this.observedWidget;
-    const traitName = this.observedTrait;
-    let traitValue: TraitValue = 0;
+  /*
+   * Returns either this observer model instance (observe or dlink) or another
+   * widget model instance (jsdlink)
+   */
+  get targetWidget(): WidgetModel {
+    return this._targetWidget;
+  }
 
-    if (traitName === 'array') {
+  get targetTrait(): string {
+    return this.get('target_trait');
+  }
+
+  private setTargetTrait(time: tone.Unit.Seconds, transport: boolean): void {
+    const observedWidget = this.observedWidget;
+    const observedTrait = this.observedTrait;
+    const targetWidget = this.targetWidget;
+    const targetTrait = this.targetTrait;
+    let value: TraitValue = 0;
+
+    if (observedTrait === 'array') {
       // bug when array elements aren't changing, then it's never
       // synced again even if it gets updated again later?
       // -> reset array value silently
-      this.set('array', new Float32Array([0]), { silent: true });
+      targetWidget.set('array', new Float32Array([0]), { silent: true });
     }
 
-    if (traitName === 'time') {
-      traitValue = time;
+    if (observedTrait === 'time') {
+      value = time;
     } else {
       if (transport) {
-        traitValue = model.getValueAtTime(traitName, time);
+        value = observedWidget.getValueAtTime(observedTrait, time);
       } else {
-        traitValue = model.getValue(traitName);
+        value = observedWidget.getValue(observedTrait);
       }
     }
 
     if (this.observeTime) {
-      this.set('time_value', [time, traitValue]);
+      targetWidget.set('time_value', [time, value]);
     } else {
-      this.set(traitName, traitValue);
+      targetWidget.set(targetTrait, value);
     }
 
-    this.save_changes();
+    if (targetWidget === this) {
+      // sync with backend unless target is another widget (jsdlink).
+      targetWidget.save_changes();
+    }
   }
 
   private scheduleRepeat(
@@ -116,16 +152,16 @@ export class ScheduleObserverModel extends ToneWidgetModel {
       eid = tone.Transport.scheduleRepeat((time) => {
         if (draw) {
           tone.Draw.schedule(() => {
-            this.setObservedTrait(time, transport);
+            this.setTargetTrait(time, transport);
           }, time);
         } else {
-          this.setObservedTrait(time, transport);
+          this.setTargetTrait(time, transport);
         }
       }, updateInterval);
     } else {
       eid = setInterval(
         () => {
-          this.setObservedTrait(tone.now(), transport);
+          this.setTargetTrait(tone.now(), transport);
         },
         (updateInterval as number) * 1000,
       );
@@ -136,7 +172,7 @@ export class ScheduleObserverModel extends ToneWidgetModel {
 
   private scheduleCancel(): void {
     if (this.event.transport) {
-      tone.Transport.cancel(this.event.id as number);
+      tone.Transport.clear(this.event.id as number);
     } else {
       clearInterval(this.event.id as ReturnType<typeof setInterval>);
     }
@@ -161,6 +197,7 @@ export class ScheduleObserverModel extends ToneWidgetModel {
   static serializers: ISerializers = {
     ...ToneWidgetModel.serializers,
     observed_widget: { deserialize: unpack_models as any },
+    target_widget: { deserialize: unpack_models as any },
     array: dataarray_serialization,
   };