From 8803a053bab32cf33999dad5b24fbe99c1421d8c Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 8 Mar 2017 12:46:53 +0100 Subject: [PATCH 01/21] Any descendant of the state can be a binary_type now, usefull for serializing numpy arrays, in addition to shape, dtype etc --- ipywidgets/widgets/widget.py | 47 +++++++++++++++++++++++++------- jupyter-js-widgets/src/widget.ts | 11 ++++++-- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index a23e8873d0..98a4295a6c 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -213,7 +213,7 @@ def __del__(self): def open(self): """Open a comm to the frontend if one isn't already open.""" if self.comm is None: - state, buffer_keys, buffers = self._split_state_buffers(self.get_state()) + state, state_with_buffers, buffer_keys, buffers = self._split_state_buffers(self.get_state()) args = dict(target_name='jupyter.widget', data=state) if self._model_id is not None: @@ -259,14 +259,41 @@ def close(self): self._ipython_display_ = None def _split_state_buffers(self, state): - """Return (state_without_buffers, buffer_keys, buffers) for binary message parts""" + """Return (state_without_buffers, state_with_buffers, buffer_keys, buffers) for binary message parts + + state_with_buffers is a dict where any of it's decendents is is a binary_type. + """ + def seperate_buffers(substate, path, buffer_keys, buffers): + # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} + # where are ar* are binary types + # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] + # instead of removing elements from the list, this will make replacing the buffers on the js side much easier + if isinstance(substate, (list, tuple)): + for i, v in enumerate(substate): + if isinstance(v, (dict, list, tuple)): + seperate_buffers(v, path + [i], buffer_keys, buffers) + if isinstance(v, _binary_types): + substate[i] = None + buffers.append(v) + buffer_keys.append(path + [i]) + elif isinstance(substate, dict): + for k, v in list(substate.items()): # we need to copy to a list since substate will be modified + if isinstance(v, (dict, list, tuple)): + seperate_buffers(v, path + [k], buffer_keys, buffers) + if isinstance(v, _binary_types): + substate.pop(k) + buffers.append(v) + buffer_keys.append(path + [k]) + else: + raise ValueError("expected state to be a list or dict, not %r" % substate) buffer_keys, buffers = [], [] - for k, v in list(state.items()): - if isinstance(v, _binary_types): - state.pop(k) - buffers.append(v) - buffer_keys.append(k) - return state, buffer_keys, buffers + seperate_buffers(state, [], buffer_keys, buffers) + state_with_buffers = {} + # any part of the state that has buffers needs to be treated seperately + for key in set([k[0] for k in buffer_keys]): + state_with_buffers[key] = state[key] + del state[key] + return state, state_with_buffers, buffer_keys, buffers def send_state(self, key=None): """Sends the widget state, or a piece of it, to the front-end. @@ -277,8 +304,8 @@ def send_state(self, key=None): A single property's name or iterable of property names to sync with the front-end. """ state = self.get_state(key=key) - state, buffer_keys, buffers = self._split_state_buffers(state) - msg = {'method': 'update', 'state': state, 'buffers': buffer_keys} + state, state_with_buffers, buffer_keys, buffers = self._split_state_buffers(state) + msg = {'method': 'update', 'state': state, 'buffers': buffer_keys, 'state_with_buffers': state_with_buffers} self._send(msg, buffers=buffers) def get_state(self, key=None, drop_defaults=False): diff --git a/jupyter-js-widgets/src/widget.ts b/jupyter-js-widgets/src/widget.ts index d1efafc47c..aead27fa4c 100644 --- a/jupyter-js-widgets/src/widget.ts +++ b/jupyter-js-widgets/src/widget.ts @@ -172,11 +172,18 @@ class WidgetModel extends Backbone.Model { case 'update': this.state_change = this.state_change .then(() => { - var state = msg.content.data.state || {}; + var state = _.extend({}, msg.content.data.state || {}, msg.content.data.state_with_buffers); var buffer_keys = msg.content.data.buffers || []; var buffers = msg.buffers || []; for (var i=0; i { From 035e75ebbd3b1e09d9b04b694c580eaf0b117147 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 8 Mar 2017 20:45:46 +0100 Subject: [PATCH 02/21] serializing of nested buffer supported --- ipywidgets/widgets/widget.py | 42 +++++++++++++++--------- jupyter-js-widgets/src/widget.ts | 56 ++++++++++++++++++++++---------- 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 98a4295a6c..76de768508 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -213,7 +213,7 @@ def __del__(self): def open(self): """Open a comm to the frontend if one isn't already open.""" if self.comm is None: - state, state_with_buffers, buffer_keys, buffers = self._split_state_buffers(self.get_state()) + state, state_with_buffers, buffer_paths, buffers = self._split_state_buffers(self.get_state()) args = dict(target_name='jupyter.widget', data=state) if self._model_id is not None: @@ -223,6 +223,10 @@ def open(self): if buffers: # FIXME: workaround ipykernel missing binary message support in open-on-init # send state with binary elements as second message + # TODO: if this gets fixed, _split_state_buffers does not need to have a seperate + # state_with_buffers, this is needed since first the object is created without sending + # the buffers, then on the js side, it tries to serialize, while the buffer have not been + # patched in yet, see also widget.ts:_handle_comm_msg self.send_state() @observe('comm') @@ -259,11 +263,11 @@ def close(self): self._ipython_display_ = None def _split_state_buffers(self, state): - """Return (state_without_buffers, state_with_buffers, buffer_keys, buffers) for binary message parts + """Return (state_without_buffers, state_with_buffers, buffer_paths, buffers) for binary message parts state_with_buffers is a dict where any of it's decendents is is a binary_type. """ - def seperate_buffers(substate, path, buffer_keys, buffers): + def seperate_buffers(substate, path, buffer_paths, buffers): # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} # where are ar* are binary types # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] @@ -271,29 +275,30 @@ def seperate_buffers(substate, path, buffer_keys, buffers): if isinstance(substate, (list, tuple)): for i, v in enumerate(substate): if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [i], buffer_keys, buffers) + seperate_buffers(v, path + [i], buffer_paths, buffers) if isinstance(v, _binary_types): substate[i] = None buffers.append(v) - buffer_keys.append(path + [i]) + buffer_paths.append(path + [i]) elif isinstance(substate, dict): for k, v in list(substate.items()): # we need to copy to a list since substate will be modified if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [k], buffer_keys, buffers) + seperate_buffers(v, path + [k], buffer_paths, buffers) if isinstance(v, _binary_types): substate.pop(k) buffers.append(v) - buffer_keys.append(path + [k]) + buffer_paths.append(path + [k]) else: raise ValueError("expected state to be a list or dict, not %r" % substate) - buffer_keys, buffers = [], [] - seperate_buffers(state, [], buffer_keys, buffers) + buffer_paths, buffers = [], [] + seperate_buffers(state, [], buffer_paths, buffers) state_with_buffers = {} # any part of the state that has buffers needs to be treated seperately - for key in set([k[0] for k in buffer_keys]): + # since of a issue as indicated in .open(..) + for key in set([k[0] for k in buffer_paths]): state_with_buffers[key] = state[key] del state[key] - return state, state_with_buffers, buffer_keys, buffers + return state, state_with_buffers, buffer_paths, buffers def send_state(self, key=None): """Sends the widget state, or a piece of it, to the front-end. @@ -304,8 +309,8 @@ def send_state(self, key=None): A single property's name or iterable of property names to sync with the front-end. """ state = self.get_state(key=key) - state, state_with_buffers, buffer_keys, buffers = self._split_state_buffers(state) - msg = {'method': 'update', 'state': state, 'buffers': buffer_keys, 'state_with_buffers': state_with_buffers} + state, state_with_buffers, buffer_paths, buffers = self._split_state_buffers(state) + msg = {'method': 'update', 'state': state, 'buffers': buffer_paths, 'state_with_buffers': state_with_buffers} self._send(msg, buffers=buffers) def get_state(self, key=None, drop_defaults=False): @@ -479,9 +484,16 @@ def _handle_msg(self, msg): if method == 'backbone': if 'sync_data' in data: # get binary buffers too + print(data) sync_data = data['sync_data'] - for i,k in enumerate(data.get('buffer_keys', [])): - sync_data[k] = msg['buffers'][i] + if 'buffer_paths' in data: + for path, buffer in zip(data['buffer_paths'], msg['buffers']): + # we'd like to set say sync_data['x'][0]['y'] = buffer + # where path in this example would be ['x', 0, 'y'] + obj = sync_data + for key in path[:-1]: + obj = obj[key] + obj[path[-1]] = buffer self.set_state(sync_data) # handles all methods # Handle a state request. diff --git a/jupyter-js-widgets/src/widget.ts b/jupyter-js-widgets/src/widget.ts index aead27fa4c..543864c0e8 100644 --- a/jupyter-js-widgets/src/widget.ts +++ b/jupyter-js-widgets/src/widget.ts @@ -172,18 +172,18 @@ class WidgetModel extends Backbone.Model { case 'update': this.state_change = this.state_change .then(() => { + // see Widget.open/_split_state_buffers about why we need state_with_buffers var state = _.extend({}, msg.content.data.state || {}, msg.content.data.state_with_buffers); - var buffer_keys = msg.content.data.buffers || []; + var buffer_paths = msg.content.data.buffers || []; var buffers = msg.buffers || []; - for (var i=0; i { @@ -406,23 +406,45 @@ class WidgetModel extends Backbone.Model { // get binary values, then send var keys = Object.keys(state); var buffers = []; - var buffer_keys = []; - for (var i=0; i { this.pending_msgs--; From 01e03ca10a9fe5f4cc4c466e12921a94190a19fa Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Thu, 9 Mar 2017 09:33:07 +0100 Subject: [PATCH 03/21] better docstring for _split_state_buffers --- ipywidgets/widgets/widget.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 76de768508..174a0994c9 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -266,6 +266,12 @@ def _split_state_buffers(self, state): """Return (state_without_buffers, state_with_buffers, buffer_paths, buffers) for binary message parts state_with_buffers is a dict where any of it's decendents is is a binary_type. + + As an example: + >>> state = {'plain': [0, 'text'], 'x': {'ar': memoryview(ar1)}, 'y': {shape: (10,10), data: memoryview(ar2)}} + >>> widget._split_state_buffers(state) + ({'plain': [0, 'text']}, {'x': {}, 'y': {'shape': (10, 10)}}, [['x', 'ar'], ['y', 'data']], + [, ]) """ def seperate_buffers(substate, path, buffer_paths, buffers): # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} From 7866d2c5a1fd615611018a3c185d6381dc07ac9c Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 19:54:30 +0100 Subject: [PATCH 04/21] fix: although state is a copy, it's ancestors may refer to original data of a model, so clone objects is their members are changed, also avoid enless loops when objects ancestors refer to theirselves --- jupyter-js-widgets/src/widget.ts | 73 ++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/jupyter-js-widgets/src/widget.ts b/jupyter-js-widgets/src/widget.ts index 543864c0e8..6a1d021307 100644 --- a/jupyter-js-widgets/src/widget.ts +++ b/jupyter-js-widgets/src/widget.ts @@ -407,40 +407,77 @@ class WidgetModel extends Backbone.Model { var keys = Object.keys(state); var buffers = []; var buffer_paths = []; + // keep track of what we visited, to avoid endless loops, when an object ancester + // refers to itself, we cannot use an object, since using objects for keys don't work + var visited_set = []; // this function goes through lists and object and removes arraybuffers // they will be transferred seperately, since they don't json'ify // on the python side the inverse happens + // if we need to remove an object from a list, we need to clone that list, otherwise we may modify + // the internal state of the widget model + // however, we do not want to clone everything, for performance function seperate_buffers(obj, path) { - if(_.isArray(obj)) { - for(var i = 0; i < obj.length; i++) { + if(visited_set.indexOf(obj) != -1) { // we already visited this object + return obj; + } + visited_set.push(obj); + if (_.isArray(obj)) { + var is_cloned = false; + for (var i = 0; i < obj.length; i++) { var value = obj[i]; - if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { - buffers.push(value); - buffer_paths.push(path.concat([i])); - // easier to just keep the array, but clear the entry, otherwise we have to think - // about array length - obj[i] = null; - } else { - seperate_buffers(value, path.concat([i])); + if(value) { + if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { + if(!is_cloned) { + obj = _.map(obj, _.identity); + is_cloned = true; + } + buffers.push(value); + buffer_paths.push(path.concat([i])); + // easier to just keep the array, but clear the entry, otherwise we have to think + // about array length + obj[i] = null; + } + else { + var new_value = seperate_buffers(value, path.concat([i])); + if((new_value != value) && !is_cloned) { // only clone when we have to + obj = _.map(obj, _.identity); + is_cloned = true; + obj[i] = new_value; + } + } } } } else if(_.isObject(obj)) { for (var key in obj) { - if (obj.hasOwnProperty(key)) { + var is_cloned = false; + if (!obj.hasOwnProperty || obj.hasOwnProperty(key)) { var value = obj[key]; - if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { - buffers.push(value); - buffer_paths.push(path.concat([key])); - delete obj[key]; // for objects/dicts we just delete them - } else { - seperate_buffers(value, path.concat([key])); + if(value) { + if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { + if(!is_cloned) { + obj = _.mapObject(obj, _.identity); // clone only once for performance + is_cloned = true; + } + buffers.push(value); + buffer_paths.push(path.concat([key])); + delete obj[key]; // for objects/dicts we just delete them + } + else { + var new_value = seperate_buffers(value, path.concat([key])); + if((new_value != value) && !is_cloned) { // only clone when we have to + obj = _.mapObject(obj, _.identity); + is_cloned = true; + obj[key] = new_value; + } + } } } } } + return obj; } - seperate_buffers(state, []) + state = seperate_buffers(state, []); // could return a clone this.comm.send({ method: 'backbone', sync_data: state, From 542940dd28e47fa6d42de20c98ed13bbaf317efc Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 19:55:17 +0100 Subject: [PATCH 05/21] if a top level key enters twice, don't try to remove it twice --- ipywidgets/widgets/widget.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 174a0994c9..5f9fb307e7 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -302,8 +302,9 @@ def seperate_buffers(substate, path, buffer_paths, buffers): # any part of the state that has buffers needs to be treated seperately # since of a issue as indicated in .open(..) for key in set([k[0] for k in buffer_paths]): - state_with_buffers[key] = state[key] - del state[key] + if key in state: # if we didn't remove it allready + state_with_buffers[key] = state[key] + del state[key] return state, state_with_buffers, buffer_paths, buffers def send_state(self, key=None): @@ -490,7 +491,6 @@ def _handle_msg(self, msg): if method == 'backbone': if 'sync_data' in data: # get binary buffers too - print(data) sync_data = data['sync_data'] if 'buffer_paths' in data: for path, buffer in zip(data['buffer_paths'], msg['buffers']): From 761b62fc5670716ce1514a74b763cf093eab40e0 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 20:18:48 +0100 Subject: [PATCH 06/21] added unittest, make _split_state_buffer a function instead of method of Widget --- ipywidgets/widgets/tests/test_traits.py | 20 +++++ ipywidgets/widgets/widget.py | 97 +++++++++++++------------ 2 files changed, 70 insertions(+), 47 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index 6d5b27702a..27b4b62be6 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -2,11 +2,13 @@ # Distributed under the terms of the Modified BSD License. """Test trait types of the widget packages.""" +import array from unittest import TestCase from traitlets import HasTraits from traitlets.tests.test_traitlets import TraitTestBase from ipywidgets import Color +from ipywidgets.widgets import _split_state_buffers class ColorTrait(HasTraits): @@ -19,3 +21,21 @@ class TestColor(TraitTestBase): _good_values = ["blue", "#AA0", "#FFFFFF"] _bad_values = ["vanilla", "blues"] + +class TestBuffers(TestCase): + def test_state_with_buffers(self): + ar1 = array.array('d', [1.0, 2.0, 3.14]) + ar2 = array.array('d', [1.0, 2.0, 3.14]) + mv1 = memoryview(ar1) + mv2 = memoryview(ar2) + state = {'plain': [0, 'text'], 'x': {'ar': mv1}, 'y': {'shape': (10, 10), 'data': mv1}, 'z': [mv1, mv2]} + state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) + print("executed", state, state_with_buffers, buffer_paths, buffers) + self.assertIn('plain', state) + self.assertNotIn('x', state) + self.assertNotIn('y', state) + self.assertNotIn('z', state) + for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv2), (['z', 0], mv1), (['z', 1], mv2)]: + self.assertIn(path, buffer_paths, "%r not in path" % path) + index = buffer_paths.index(path) + self.assertEqual(buffer, buffers[index]) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 5f9fb307e7..86298bf7e6 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -50,6 +50,54 @@ def _json_to_widget(x, obj): else: _binary_types = (memoryview, buffer) + +def _split_state_buffers(state): + """Return (state_without_buffers, state_with_buffers, buffer_paths, buffers) for binary message parts + + state_with_buffers is a dict where any of it's decendents is is a binary_type. + + As an example: + >>> state = {'plain': [0, 'text'], 'x': {'ar': memoryview(ar1)}, 'y': {'shape': (10,10), 'data': memoryview(ar2)}} + >>> widget._split_state_buffers(state) + ({'plain': [0, 'text']}, {'x': {}, 'y': {'shape': (10, 10)}}, [['x', 'ar'], ['y', 'data']], + [, ]) + """ + + def seperate_buffers(substate, path, buffer_paths, buffers): + # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} + # where are ar* are binary types + # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] + # instead of removing elements from the list, this will make replacing the buffers on the js side much easier + if isinstance(substate, (list, tuple)): + for i, v in enumerate(substate): + if isinstance(v, (dict, list, tuple)): + seperate_buffers(v, path + [i], buffer_paths, buffers) + if isinstance(v, _binary_types): + substate[i] = None + buffers.append(v) + buffer_paths.append(path + [i]) + elif isinstance(substate, dict): + for k, v in list(substate.items()): # we need to copy to a list since substate will be modified + if isinstance(v, (dict, list, tuple)): + seperate_buffers(v, path + [k], buffer_paths, buffers) + if isinstance(v, _binary_types): + substate.pop(k) + buffers.append(v) + buffer_paths.append(path + [k]) + else: + raise ValueError("expected state to be a list or dict, not %r" % substate) + + buffer_paths, buffers = [], [] + seperate_buffers(state, [], buffer_paths, buffers) + state_with_buffers = {} + # any part of the state that has buffers needs to be treated seperately + # since of a issue as indicated in .open(..) + for key in set([k[0] for k in buffer_paths]): + state_with_buffers[key] = state[key] + del state[key] + return state, state_with_buffers, buffer_paths, buffers + + class CallbackDispatcher(LoggingConfigurable): """A structure for registering and running callbacks""" callbacks = List() @@ -213,7 +261,7 @@ def __del__(self): def open(self): """Open a comm to the frontend if one isn't already open.""" if self.comm is None: - state, state_with_buffers, buffer_paths, buffers = self._split_state_buffers(self.get_state()) + state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(self.get_state()) args = dict(target_name='jupyter.widget', data=state) if self._model_id is not None: @@ -262,51 +310,6 @@ def close(self): self.comm = None self._ipython_display_ = None - def _split_state_buffers(self, state): - """Return (state_without_buffers, state_with_buffers, buffer_paths, buffers) for binary message parts - - state_with_buffers is a dict where any of it's decendents is is a binary_type. - - As an example: - >>> state = {'plain': [0, 'text'], 'x': {'ar': memoryview(ar1)}, 'y': {shape: (10,10), data: memoryview(ar2)}} - >>> widget._split_state_buffers(state) - ({'plain': [0, 'text']}, {'x': {}, 'y': {'shape': (10, 10)}}, [['x', 'ar'], ['y', 'data']], - [, ]) - """ - def seperate_buffers(substate, path, buffer_paths, buffers): - # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} - # where are ar* are binary types - # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] - # instead of removing elements from the list, this will make replacing the buffers on the js side much easier - if isinstance(substate, (list, tuple)): - for i, v in enumerate(substate): - if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [i], buffer_paths, buffers) - if isinstance(v, _binary_types): - substate[i] = None - buffers.append(v) - buffer_paths.append(path + [i]) - elif isinstance(substate, dict): - for k, v in list(substate.items()): # we need to copy to a list since substate will be modified - if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [k], buffer_paths, buffers) - if isinstance(v, _binary_types): - substate.pop(k) - buffers.append(v) - buffer_paths.append(path + [k]) - else: - raise ValueError("expected state to be a list or dict, not %r" % substate) - buffer_paths, buffers = [], [] - seperate_buffers(state, [], buffer_paths, buffers) - state_with_buffers = {} - # any part of the state that has buffers needs to be treated seperately - # since of a issue as indicated in .open(..) - for key in set([k[0] for k in buffer_paths]): - if key in state: # if we didn't remove it allready - state_with_buffers[key] = state[key] - del state[key] - return state, state_with_buffers, buffer_paths, buffers - def send_state(self, key=None): """Sends the widget state, or a piece of it, to the front-end. @@ -316,7 +319,7 @@ def send_state(self, key=None): A single property's name or iterable of property names to sync with the front-end. """ state = self.get_state(key=key) - state, state_with_buffers, buffer_paths, buffers = self._split_state_buffers(state) + state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) msg = {'method': 'update', 'state': state, 'buffers': buffer_paths, 'state_with_buffers': state_with_buffers} self._send(msg, buffers=buffers) From 1fa8e081aba9f6f991b740a237c4ccefaf715796 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 20:47:13 +0100 Subject: [PATCH 07/21] fix: toplevel buffers triggered a bug, since items were already removed from the state at the top level --- ipywidgets/widgets/tests/test_traits.py | 6 +++--- ipywidgets/widgets/widget.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index 27b4b62be6..dd125f743f 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -8,7 +8,7 @@ from traitlets import HasTraits from traitlets.tests.test_traitlets import TraitTestBase from ipywidgets import Color -from ipywidgets.widgets import _split_state_buffers +from ipywidgets.widgets.widget import _split_state_buffers class ColorTrait(HasTraits): @@ -28,14 +28,14 @@ def test_state_with_buffers(self): ar2 = array.array('d', [1.0, 2.0, 3.14]) mv1 = memoryview(ar1) mv2 = memoryview(ar2) - state = {'plain': [0, 'text'], 'x': {'ar': mv1}, 'y': {'shape': (10, 10), 'data': mv1}, 'z': [mv1, mv2]} + state = {'plain': [0, 'text'], 'x': {'ar': mv1}, 'y': {'shape': (10, 10), 'data': mv1}, 'z': [mv1, mv2], 'top': mv1} state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) print("executed", state, state_with_buffers, buffer_paths, buffers) self.assertIn('plain', state) self.assertNotIn('x', state) self.assertNotIn('y', state) self.assertNotIn('z', state) - for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv2), (['z', 0], mv1), (['z', 1], mv2)]: + for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv2), (['z', 0], mv1), (['z', 1], mv2), (['top'], mv1)]: self.assertIn(path, buffer_paths, "%r not in path" % path) index = buffer_paths.index(path) self.assertEqual(buffer, buffers[index]) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 86298bf7e6..9f28ab5d98 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -92,7 +92,8 @@ def seperate_buffers(substate, path, buffer_paths, buffers): state_with_buffers = {} # any part of the state that has buffers needs to be treated seperately # since of a issue as indicated in .open(..) - for key in set([k[0] for k in buffer_paths]): + # also remove top level elements if they contain nested buffers (len(k) > 1) + for key in set([k[0] for k in buffer_paths if len(k) > 1]): state_with_buffers[key] = state[key] del state[key] return state, state_with_buffers, buffer_paths, buffers From caadc228377636fd4f15c6cfeef95f6ba73baf33 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 21:17:13 +0100 Subject: [PATCH 08/21] test: using byte/string for memoryview for python27 compat --- ipywidgets/widgets/tests/test_traits.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index dd125f743f..6f59021d1e 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -24,10 +24,8 @@ class TestColor(TraitTestBase): class TestBuffers(TestCase): def test_state_with_buffers(self): - ar1 = array.array('d', [1.0, 2.0, 3.14]) - ar2 = array.array('d', [1.0, 2.0, 3.14]) - mv1 = memoryview(ar1) - mv2 = memoryview(ar2) + mv1 = memoryview(b'test1') + mv2 = memoryview(b'test2') state = {'plain': [0, 'text'], 'x': {'ar': mv1}, 'y': {'shape': (10, 10), 'data': mv1}, 'z': [mv1, mv2], 'top': mv1} state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) print("executed", state, state_with_buffers, buffer_paths, buffers) From 1fccf080b9595b5176c2ce5744d1d1f261839844 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Wed, 15 Mar 2017 21:35:39 +0100 Subject: [PATCH 09/21] fix unittest --- ipywidgets/widgets/tests/test_traits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index 6f59021d1e..2a2c875316 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -33,7 +33,7 @@ def test_state_with_buffers(self): self.assertNotIn('x', state) self.assertNotIn('y', state) self.assertNotIn('z', state) - for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv2), (['z', 0], mv1), (['z', 1], mv2), (['top'], mv1)]: + for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv1), (['z', 0], mv1), (['z', 1], mv2), (['top'], mv1)]: self.assertIn(path, buffer_paths, "%r not in path" % path) index = buffer_paths.index(path) self.assertEqual(buffer, buffers[index]) From d5740b445f49434501951a813c1778ed166be101 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Thu, 16 Mar 2017 12:50:21 +0100 Subject: [PATCH 10/21] better docstring --- ipywidgets/widgets/widget.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 9f28ab5d98..8a12e6b02b 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -64,8 +64,8 @@ def _split_state_buffers(state): """ def seperate_buffers(substate, path, buffer_paths, buffers): - # remove binary types from dicts and lists, and keep there key, e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]} - # where are ar* are binary types + # remove binary types from dicts and lists, but keep track of their paths + # e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]}, where ar/ar2/ar3 are binary types # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] # instead of removing elements from the list, this will make replacing the buffers on the js side much easier if isinstance(substate, (list, tuple)): From 9d896f76d01b23b3b9fb60a2948141f77da769ef Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Thu, 16 Mar 2017 20:10:05 +0100 Subject: [PATCH 11/21] not using a seperate state with keeps the state which have buffer, much cleaner code, better unittets, buffer splitting/joining code went into utils.ts --- ipywidgets/widgets/tests/test_traits.py | 26 ++++-- ipywidgets/widgets/widget.py | 29 ++----- jupyter-js-widgets/src/manager-base.ts | 10 ++- jupyter-js-widgets/src/utils.ts | 107 ++++++++++++++++++++++++ jupyter-js-widgets/src/widget.ts | 91 ++------------------ 5 files changed, 146 insertions(+), 117 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index 2a2c875316..c07e89b3ed 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -26,14 +26,26 @@ class TestBuffers(TestCase): def test_state_with_buffers(self): mv1 = memoryview(b'test1') mv2 = memoryview(b'test2') - state = {'plain': [0, 'text'], 'x': {'ar': mv1}, 'y': {'shape': (10, 10), 'data': mv1}, 'z': [mv1, mv2], 'top': mv1} - state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) - print("executed", state, state_with_buffers, buffer_paths, buffers) + state = {'plain': [0, 'text'], + 'x': {'ar': mv1}, + 'y': {'shape': (10, 10), 'data': mv1}, + 'z': [mv1, mv2], + 'top': mv1, + 'deep': {'a': 1, 'b':[0,{'deeper':mv2}]}} + state, buffer_paths, buffers = _split_state_buffers(state) + print("executed", state, buffer_paths, buffers) self.assertIn('plain', state) - self.assertNotIn('x', state) - self.assertNotIn('y', state) - self.assertNotIn('z', state) - for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv1), (['z', 0], mv1), (['z', 1], mv2), (['top'], mv1)]: + self.assertIn('shape', state['y']) + self.assertNotIn('ar', state['x']) + self.assertNotIn('data', state['x']) + self.assertNotIn(mv1, state['z']) + self.assertNotIn(mv1, state['z']) + self.assertNotIn('top', state) + self.assertIn('deep', state) + self.assertIn('b', state['deep']) + self.assertNotIn('deeper', state['deep']['b'][1]) + for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv1), (['z', 0], mv1), (['z', 1], mv2),\ + (['top'], mv1), (['deep', 'b', 1, 'deeper'], mv2)]: self.assertIn(path, buffer_paths, "%r not in path" % path) index = buffer_paths.index(path) self.assertEqual(buffer, buffers[index]) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 8a12e6b02b..e8728a3476 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -52,9 +52,7 @@ def _json_to_widget(x, obj): def _split_state_buffers(state): - """Return (state_without_buffers, state_with_buffers, buffer_paths, buffers) for binary message parts - - state_with_buffers is a dict where any of it's decendents is is a binary_type. + """Return (state_without_buffers, buffer_paths, buffers) for binary message parts As an example: >>> state = {'plain': [0, 'text'], 'x': {'ar': memoryview(ar1)}, 'y': {'shape': (10,10), 'data': memoryview(ar2)}} @@ -90,13 +88,7 @@ def seperate_buffers(substate, path, buffer_paths, buffers): buffer_paths, buffers = [], [] seperate_buffers(state, [], buffer_paths, buffers) state_with_buffers = {} - # any part of the state that has buffers needs to be treated seperately - # since of a issue as indicated in .open(..) - # also remove top level elements if they contain nested buffers (len(k) > 1) - for key in set([k[0] for k in buffer_paths if len(k) > 1]): - state_with_buffers[key] = state[key] - del state[key] - return state, state_with_buffers, buffer_paths, buffers + return state, buffer_paths, buffers class CallbackDispatcher(LoggingConfigurable): @@ -262,21 +254,14 @@ def __del__(self): def open(self): """Open a comm to the frontend if one isn't already open.""" if self.comm is None: - state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(self.get_state()) + state, buffer_paths, buffers = _split_state_buffers(self.get_state()) - args = dict(target_name='jupyter.widget', data=state) + args = dict(target_name='jupyter.widget', data={'state': state, 'buffer_paths': buffer_paths}) if self._model_id is not None: args['comm_id'] = self._model_id + args['buffers'] = buffers self.comm = Comm(**args) - if buffers: - # FIXME: workaround ipykernel missing binary message support in open-on-init - # send state with binary elements as second message - # TODO: if this gets fixed, _split_state_buffers does not need to have a seperate - # state_with_buffers, this is needed since first the object is created without sending - # the buffers, then on the js side, it tries to serialize, while the buffer have not been - # patched in yet, see also widget.ts:_handle_comm_msg - self.send_state() @observe('comm') def _comm_changed(self, change): @@ -320,8 +305,8 @@ def send_state(self, key=None): A single property's name or iterable of property names to sync with the front-end. """ state = self.get_state(key=key) - state, state_with_buffers, buffer_paths, buffers = _split_state_buffers(state) - msg = {'method': 'update', 'state': state, 'buffers': buffer_paths, 'state_with_buffers': state_with_buffers} + state, buffer_paths, buffers = _split_state_buffers(state) + msg = {'method': 'update', 'state': state, 'buffer_paths': buffer_paths} self._send(msg, buffers=buffers) def get_state(self, key=None, drop_defaults=False): diff --git a/jupyter-js-widgets/src/manager-base.ts b/jupyter-js-widgets/src/manager-base.ts index 5ab6a6ca7a..af7735eedd 100644 --- a/jupyter-js-widgets/src/manager-base.ts +++ b/jupyter-js-widgets/src/manager-base.ts @@ -185,12 +185,14 @@ abstract class ManagerBase { * Handle when a comm is opened. */ handle_comm_open(comm: shims.services.Comm, msg: services.KernelMessage.ICommOpenMsg): Promise { + var data = (msg.content.data as any); + utils.put_buffers(data.state, data.buffer_paths, msg.buffers) return this.new_model({ - model_name: msg.content.data['_model_name'], - model_module: msg.content.data['_model_module'], - model_module_version: msg.content.data['_model_module_version'], + model_name: data.state['_model_name'], + model_module: data.state['_model_module'], + model_module_version: data.state['_model_module_version'], comm: comm - }, msg.content.data).catch(utils.reject('Could not create a model.', true)); + }, data.state).catch(utils.reject('Could not create a model.', true)); }; /** diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index 4213d118f6..dc50db2717 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -1,6 +1,8 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import * as _ from 'underscore'; + // TODO: ATTEMPT TO KILL THIS MODULE USING THIRD PARTY LIBRARIES WHEN IPYWIDGETS // IS CONVERTED TO NODE COMMONJS. @@ -169,3 +171,108 @@ function escape_html(text: string): string { esc.textContent = text; return esc.innerHTML; }; + +/** + * Takes an object 'state' and fills in buffer[i] at 'path' buffer_paths[i] + * where buffer_paths[i] is a list indicating where in the object buffer[i] should + * be placed + * Example: state = {a: 1, b: {}, c: [0, null]} + * buffers = [array1, array2] + * buffer_paths = [['b', 'data'], ['c', 1]] + * Will lead to {a: 1, b: array1, c: [0, array2]} + */ +export +function put_buffers(state, buffer_paths, buffers) { + for (var i=0; i { // see Widget.open/_split_state_buffers about why we need state_with_buffers - var state = _.extend({}, msg.content.data.state || {}, msg.content.data.state_with_buffers); - var buffer_paths = msg.content.data.buffers || []; + var state = msg.content.data.state; + var buffer_paths = msg.content.data.buffer_paths || []; var buffers = msg.buffers || []; - for (var i=0; i { this.set_state(state); @@ -405,84 +397,15 @@ class WidgetModel extends Backbone.Model { utils.resolvePromisesDict(attrs).then((state) => { // get binary values, then send var keys = Object.keys(state); - var buffers = []; - var buffer_paths = []; - // keep track of what we visited, to avoid endless loops, when an object ancester - // refers to itself, we cannot use an object, since using objects for keys don't work - var visited_set = []; // this function goes through lists and object and removes arraybuffers // they will be transferred seperately, since they don't json'ify // on the python side the inverse happens - // if we need to remove an object from a list, we need to clone that list, otherwise we may modify - // the internal state of the widget model - // however, we do not want to clone everything, for performance - function seperate_buffers(obj, path) { - if(visited_set.indexOf(obj) != -1) { // we already visited this object - return obj; - } - visited_set.push(obj); - if (_.isArray(obj)) { - var is_cloned = false; - for (var i = 0; i < obj.length; i++) { - var value = obj[i]; - if(value) { - if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { - if(!is_cloned) { - obj = _.map(obj, _.identity); - is_cloned = true; - } - buffers.push(value); - buffer_paths.push(path.concat([i])); - // easier to just keep the array, but clear the entry, otherwise we have to think - // about array length - obj[i] = null; - } - else { - var new_value = seperate_buffers(value, path.concat([i])); - if((new_value != value) && !is_cloned) { // only clone when we have to - obj = _.map(obj, _.identity); - is_cloned = true; - obj[i] = new_value; - } - } - } - } - } - else if(_.isObject(obj)) { - for (var key in obj) { - var is_cloned = false; - if (!obj.hasOwnProperty || obj.hasOwnProperty(key)) { - var value = obj[key]; - if(value) { - if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { - if(!is_cloned) { - obj = _.mapObject(obj, _.identity); // clone only once for performance - is_cloned = true; - } - buffers.push(value); - buffer_paths.push(path.concat([key])); - delete obj[key]; // for objects/dicts we just delete them - } - else { - var new_value = seperate_buffers(value, path.concat([key])); - if((new_value != value) && !is_cloned) { // only clone when we have to - obj = _.mapObject(obj, _.identity); - is_cloned = true; - obj[key] = new_value; - } - } - } - } - } - } - return obj; - } - state = seperate_buffers(state, []); // could return a clone + var split = utils.remove_buffers(state); this.comm.send({ method: 'backbone', - sync_data: state, - buffer_paths: buffer_paths - }, callbacks, {}, buffers); + sync_data: split.state, + buffer_paths: split.buffer_paths + }, callbacks, {}, split.buffers); }).catch((error) => { this.pending_msgs--; return (utils.reject('Could not send widget sync message', true))(error); From cca211048e03290bfeefe44d16be33f824aeacd6 Mon Sep 17 00:00:00 2001 From: maartenbreddels Date: Sat, 18 Mar 2017 20:19:13 +0100 Subject: [PATCH 12/21] _split_state_buffer renamed for ts/js consistency, _remove_buffers/_put_buffers for py/ts codes are equivalent, unittest for py side covers more cases also putting it back together, bugfix (covered by python test) for _remove_buffers, typo corrections --- ipywidgets/widgets/tests/test_traits.py | 40 +++++++--- ipywidgets/widgets/widget.py | 101 +++++++++++++++--------- jupyter-js-widgets/src/manager-base.ts | 2 +- jupyter-js-widgets/src/utils.ts | 31 +++++--- jupyter-js-widgets/src/widget.ts | 2 +- 5 files changed, 115 insertions(+), 61 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index c07e89b3ed..28fd4c2b13 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -8,7 +8,7 @@ from traitlets import HasTraits from traitlets.tests.test_traitlets import TraitTestBase from ipywidgets import Color -from ipywidgets.widgets.widget import _split_state_buffers +from ipywidgets.widgets.widget import _remove_buffers, _put_buffers class ColorTrait(HasTraits): @@ -23,20 +23,27 @@ class TestColor(TraitTestBase): class TestBuffers(TestCase): - def test_state_with_buffers(self): + def test_remove_and_put_buffers(self): mv1 = memoryview(b'test1') mv2 = memoryview(b'test2') - state = {'plain': [0, 'text'], - 'x': {'ar': mv1}, + state = {'plain': [0, 'text'], # should not get removed + 'x': {'ar': mv1}, # should result in an empty dict 'y': {'shape': (10, 10), 'data': mv1}, - 'z': [mv1, mv2], - 'top': mv1, - 'deep': {'a': 1, 'b':[0,{'deeper':mv2}]}} - state, buffer_paths, buffers = _split_state_buffers(state) - print("executed", state, buffer_paths, buffers) + 'z': (mv1, mv2), # tests tuple assigment + 'top': mv1, # test a top level removal + 'deep': {'a': 1, 'b':[0,{'deeper':mv2}]}} # deeply nested + plain = state['plain'] + x = state['x'] + y = state['y'] + y_shape = y['shape'] + state_before = state + state, buffer_paths, buffers = _remove_buffers(state) + + # check if buffers are removed self.assertIn('plain', state) self.assertIn('shape', state['y']) self.assertNotIn('ar', state['x']) + self.assertFalse(state['x']) # should be empty self.assertNotIn('data', state['x']) self.assertNotIn(mv1, state['z']) self.assertNotIn(mv1, state['z']) @@ -44,8 +51,23 @@ def test_state_with_buffers(self): self.assertIn('deep', state) self.assertIn('b', state['deep']) self.assertNotIn('deeper', state['deep']['b'][1]) + + # check that items that didn't need change aren't touched + self.assertIsNot(state, state_before) + self.assertIs(state['plain'], plain) + self.assertIsNot(state['x'], x) + self.assertIsNot(state['y'], y) + self.assertIs(state['y']['shape'], y_shape) + + # check that the buffers paths really points to the right buffer for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv1), (['z', 0], mv1), (['z', 1], mv2),\ (['top'], mv1), (['deep', 'b', 1, 'deeper'], mv2)]: self.assertIn(path, buffer_paths, "%r not in path" % path) index = buffer_paths.index(path) self.assertEqual(buffer, buffers[index]) + + # and check that we can put it back together again + _put_buffers(state, buffer_paths, buffers) + # we know that tuples get converted to list, so help the comparison by changing the tuple to a list + state_before['z'] = list(state_before['z']) + self.assertEqual(state_before, state) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index e8728a3476..24c3894e82 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -50,44 +50,75 @@ def _json_to_widget(x, obj): else: _binary_types = (memoryview, buffer) +def _put_buffers(state, buffer_paths, buffers): + """The inverse of _remove_buffers, except here we modify the existing dict/lists. + Modifying should be fine, since this is used when state comes from the wire. + """ + for buffer_path, buffer in zip(buffer_paths, buffers): + # we'd like to set say sync_data['x'][0]['y'] = buffer + # where buffer_path in this example would be ['x', 0, 'y'] + obj = state + for key in buffer_path[:-1]: + obj = obj[key] + obj[buffer_path[-1]] = buffer + +def _seperate_buffers(substate, path, buffer_paths, buffers): + """For internal, see _remove_buffers""" + # remove binary types from dicts and lists, but keep track of their paths + # any part of the dict/list that needs modification will be cloned, so the original stays untouched + # e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]}, where ar/ar2/ar3 are binary types + # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] + # instead of removing elements from the list, this will make replacing the buffers on the js side much easier + if isinstance(substate, (list, tuple)): + is_cloned = False + for i, v in enumerate(substate): + if isinstance(v, _binary_types): + if not is_cloned: + substate = list(substate) # shallow clone list/tuple + is_cloned = True + substate[i] = None + buffers.append(v) + buffer_paths.append(path + [i]) + elif isinstance(v, (dict, list, tuple)): + vnew = _seperate_buffers(v, path + [i], buffer_paths, buffers) + if v is not vnew: # only assign when value changed + if not is_cloned: + substate = list(substate) # clone list/tuple + is_cloned = True + substate[i] = vnew + elif isinstance(substate, dict): + is_cloned = False + for k, v in substate.items(): + if isinstance(v, _binary_types): + if not is_cloned: + substate = dict(substate) # shallow clone dict + is_cloned = True + del substate[k] + buffers.append(v) + buffer_paths.append(path + [k]) + elif isinstance(v, (dict, list, tuple)): + vnew = _seperate_buffers(v, path + [k], buffer_paths, buffers) + if v is not vnew: # only assign when value changed + if not is_cloned: + substate = dict(substate) # clone list/tuple + is_cloned = True + substate[k] = vnew + else: + raise ValueError("expected state to be a list or dict, not %r" % substate) + return substate + -def _split_state_buffers(state): +def _remove_buffers(state): """Return (state_without_buffers, buffer_paths, buffers) for binary message parts As an example: >>> state = {'plain': [0, 'text'], 'x': {'ar': memoryview(ar1)}, 'y': {'shape': (10,10), 'data': memoryview(ar2)}} - >>> widget._split_state_buffers(state) + >>> _remove_buffers(state) ({'plain': [0, 'text']}, {'x': {}, 'y': {'shape': (10, 10)}}, [['x', 'ar'], ['y', 'data']], [, ]) """ - - def seperate_buffers(substate, path, buffer_paths, buffers): - # remove binary types from dicts and lists, but keep track of their paths - # e.g. {'x': {'ar': ar}, 'y': [ar2, ar3]}, where ar/ar2/ar3 are binary types - # will result in {'x': {}, 'y': [None, None]}, [ar, ar2, ar3], [['x', 'ar'], ['y', 0], ['y', 1]] - # instead of removing elements from the list, this will make replacing the buffers on the js side much easier - if isinstance(substate, (list, tuple)): - for i, v in enumerate(substate): - if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [i], buffer_paths, buffers) - if isinstance(v, _binary_types): - substate[i] = None - buffers.append(v) - buffer_paths.append(path + [i]) - elif isinstance(substate, dict): - for k, v in list(substate.items()): # we need to copy to a list since substate will be modified - if isinstance(v, (dict, list, tuple)): - seperate_buffers(v, path + [k], buffer_paths, buffers) - if isinstance(v, _binary_types): - substate.pop(k) - buffers.append(v) - buffer_paths.append(path + [k]) - else: - raise ValueError("expected state to be a list or dict, not %r" % substate) - buffer_paths, buffers = [], [] - seperate_buffers(state, [], buffer_paths, buffers) - state_with_buffers = {} + state = _seperate_buffers(state, [], buffer_paths, buffers) return state, buffer_paths, buffers @@ -254,7 +285,7 @@ def __del__(self): def open(self): """Open a comm to the frontend if one isn't already open.""" if self.comm is None: - state, buffer_paths, buffers = _split_state_buffers(self.get_state()) + state, buffer_paths, buffers = _remove_buffers(self.get_state()) args = dict(target_name='jupyter.widget', data={'state': state, 'buffer_paths': buffer_paths}) if self._model_id is not None: @@ -305,7 +336,7 @@ def send_state(self, key=None): A single property's name or iterable of property names to sync with the front-end. """ state = self.get_state(key=key) - state, buffer_paths, buffers = _split_state_buffers(state) + state, buffer_paths, buffers = _remove_buffers(state) msg = {'method': 'update', 'state': state, 'buffer_paths': buffer_paths} self._send(msg, buffers=buffers) @@ -482,13 +513,7 @@ def _handle_msg(self, msg): # get binary buffers too sync_data = data['sync_data'] if 'buffer_paths' in data: - for path, buffer in zip(data['buffer_paths'], msg['buffers']): - # we'd like to set say sync_data['x'][0]['y'] = buffer - # where path in this example would be ['x', 0, 'y'] - obj = sync_data - for key in path[:-1]: - obj = obj[key] - obj[path[-1]] = buffer + _put_buffers(sync_data, data['buffer_paths'], msg['buffers']) self.set_state(sync_data) # handles all methods # Handle a state request. diff --git a/jupyter-js-widgets/src/manager-base.ts b/jupyter-js-widgets/src/manager-base.ts index af7735eedd..946ca135d7 100644 --- a/jupyter-js-widgets/src/manager-base.ts +++ b/jupyter-js-widgets/src/manager-base.ts @@ -185,7 +185,7 @@ abstract class ManagerBase { * Handle when a comm is opened. */ handle_comm_open(comm: shims.services.Comm, msg: services.KernelMessage.ICommOpenMsg): Promise { - var data = (msg.content.data as any); + var data = (msg.content.data as any); utils.put_buffers(data.state, data.buffer_paths, msg.buffers) return this.new_model({ model_name: data.state['_model_name'], diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index dc50db2717..3b71957d35 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -179,18 +179,19 @@ function escape_html(text: string): string { * Example: state = {a: 1, b: {}, c: [0, null]} * buffers = [array1, array2] * buffer_paths = [['b', 'data'], ['c', 1]] - * Will lead to {a: 1, b: array1, c: [0, array2]} + * Will lead to {a: 1, b: {data: array1}, c: [0, array2]} */ export function put_buffers(state, buffer_paths, buffers) { for (var i=0; i Date: Mon, 20 Mar 2017 16:07:11 -0400 Subject: [PATCH 13/21] Support object toJSON serialization in js This removes the need for the visited node list. --- jupyter-js-widgets/src/utils.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index 3b71957d35..6095042a90 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -207,17 +207,15 @@ export function remove_buffers(state) { var buffers = []; var buffer_paths = []; - // keep track of what we visited, to avoid endless loops, when an object ancester - // refers to itself, we cannot use an object, since using objects for keys don't work - var visited_set = []; // if we need to remove an object from a list, we need to clone that list, otherwise we may modify // the internal state of the widget model // however, we do not want to clone everything, for performance function remove(obj, path) { - if(visited_set.indexOf(obj) != -1) { // we already visited this object - return obj; + if (obj.toJSON) { + // We need to get the JSON form of the object before recursing. + // See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON()_behavior + obj = obj.toJSON(); } - visited_set.push(obj); if (Array.isArray(obj)) { var is_cloned = false; for (var i = 0; i < obj.length; i++) { From 926d40a9900ede3144044ce44eea66c511c8cbbf Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:03:53 -0400 Subject: [PATCH 14/21] != -> !== --- jupyter-js-widgets/src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index 6095042a90..b45583ceb4 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -235,7 +235,7 @@ function remove_buffers(state) { else { var new_value = remove(value, path.concat([i])); // only assigned when the value changes, we may serialize objects that don't support assignment - if(new_value != value) { + if(new_value !== value) { if(!is_cloned) { obj = _.clone(obj); is_cloned = true; @@ -264,7 +264,7 @@ function remove_buffers(state) { else { var new_value = remove(value, path.concat([key])); // only assigned when the value changes, we may serialize objects that don't support assignment - if(new_value != value) { + if(new_value !== value) { if(!is_cloned) { obj = _.clone(obj); is_cloned = true; From 6ff791aaf631d551a84790dab6efc2009ef0215b Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:04:38 -0400 Subject: [PATCH 15/21] We can assume hasOwnProperty in our supported browsers. --- jupyter-js-widgets/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index b45583ceb4..586b51ebdd 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -249,7 +249,7 @@ function remove_buffers(state) { else if(_.isObject(obj)) { for (var key in obj) { var is_cloned = false; - if (!obj.hasOwnProperty || obj.hasOwnProperty(key)) { + if (obj.hasOwnProperty(key)) { var value = obj[key]; if(value) { if (value.buffer instanceof ArrayBuffer || value instanceof ArrayBuffer) { From 03323a655429dbd979753631bfa9fa1d0b86775e Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:04:46 -0400 Subject: [PATCH 16/21] whitespace changes --- jupyter-js-widgets/src/utils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index 586b51ebdd..cf290deedb 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -231,8 +231,7 @@ function remove_buffers(state) { // easier to just keep the array, but clear the entry, otherwise we have to think // about array length, much easier this way obj[i] = null; - } - else { + } else { var new_value = remove(value, path.concat([i])); // only assigned when the value changes, we may serialize objects that don't support assignment if(new_value !== value) { @@ -245,8 +244,7 @@ function remove_buffers(state) { } } } - } - else if(_.isObject(obj)) { + } else if(_.isObject(obj)) { for (var key in obj) { var is_cloned = false; if (obj.hasOwnProperty(key)) { From 91e849741cadd6399bbc486be5121617ee7cdf61 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:06:38 -0400 Subject: [PATCH 17/21] fix some unit tests --- ipywidgets/widgets/tests/test_traits.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipywidgets/widgets/tests/test_traits.py b/ipywidgets/widgets/tests/test_traits.py index 28fd4c2b13..08b1071c1e 100644 --- a/ipywidgets/widgets/tests/test_traits.py +++ b/ipywidgets/widgets/tests/test_traits.py @@ -43,8 +43,8 @@ def test_remove_and_put_buffers(self): self.assertIn('plain', state) self.assertIn('shape', state['y']) self.assertNotIn('ar', state['x']) - self.assertFalse(state['x']) # should be empty - self.assertNotIn('data', state['x']) + self.assertEqual(state['x'], {}) + self.assertNotIn('data', state['y']) self.assertNotIn(mv1, state['z']) self.assertNotIn(mv1, state['z']) self.assertNotIn('top', state) @@ -59,7 +59,7 @@ def test_remove_and_put_buffers(self): self.assertIsNot(state['y'], y) self.assertIs(state['y']['shape'], y_shape) - # check that the buffers paths really points to the right buffer + # check that the buffer paths really point to the right buffer for path, buffer in [(['x', 'ar'], mv1), (['y', 'data'], mv1), (['z', 0], mv1), (['z', 1], mv2),\ (['top'], mv1), (['deep', 'b', 1, 'deeper'], mv2)]: self.assertIn(path, buffer_paths, "%r not in path" % path) From abe3ee87de634df9bdb821c73f618d33efb51a6d Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:10:05 -0400 Subject: [PATCH 18/21] seperate -> separate --- ipywidgets/widgets/widget.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 24c3894e82..950454b268 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -62,7 +62,7 @@ def _put_buffers(state, buffer_paths, buffers): obj = obj[key] obj[buffer_path[-1]] = buffer -def _seperate_buffers(substate, path, buffer_paths, buffers): +def _separate_buffers(substate, path, buffer_paths, buffers): """For internal, see _remove_buffers""" # remove binary types from dicts and lists, but keep track of their paths # any part of the dict/list that needs modification will be cloned, so the original stays untouched @@ -80,7 +80,7 @@ def _seperate_buffers(substate, path, buffer_paths, buffers): buffers.append(v) buffer_paths.append(path + [i]) elif isinstance(v, (dict, list, tuple)): - vnew = _seperate_buffers(v, path + [i], buffer_paths, buffers) + vnew = _separate_buffers(v, path + [i], buffer_paths, buffers) if v is not vnew: # only assign when value changed if not is_cloned: substate = list(substate) # clone list/tuple @@ -97,7 +97,7 @@ def _seperate_buffers(substate, path, buffer_paths, buffers): buffers.append(v) buffer_paths.append(path + [k]) elif isinstance(v, (dict, list, tuple)): - vnew = _seperate_buffers(v, path + [k], buffer_paths, buffers) + vnew = _separate_buffers(v, path + [k], buffer_paths, buffers) if v is not vnew: # only assign when value changed if not is_cloned: substate = dict(substate) # clone list/tuple @@ -118,7 +118,7 @@ def _remove_buffers(state): [, ]) """ buffer_paths, buffers = [], [] - state = _seperate_buffers(state, [], buffer_paths, buffers) + state = _separate_buffers(state, [], buffer_paths, buffers) return state, buffer_paths, buffers From c3e13ec8f2f0f3f7c7ba166ae8bc39be312bfc30 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:10:39 -0400 Subject: [PATCH 19/21] Add buffers to args dict on creation. --- ipywidgets/widgets/widget.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ipywidgets/widgets/widget.py b/ipywidgets/widgets/widget.py index 950454b268..baedd885a8 100644 --- a/ipywidgets/widgets/widget.py +++ b/ipywidgets/widgets/widget.py @@ -287,10 +287,11 @@ def open(self): if self.comm is None: state, buffer_paths, buffers = _remove_buffers(self.get_state()) - args = dict(target_name='jupyter.widget', data={'state': state, 'buffer_paths': buffer_paths}) + args = dict(target_name='jupyter.widget', + data={'state': state, 'buffer_paths': buffer_paths}, + buffers=buffers) if self._model_id is not None: args['comm_id'] = self._model_id - args['buffers'] = buffers self.comm = Comm(**args) From 0a1dffbe9c7c70efbc60da15371521cd590dedbd Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Mon, 20 Mar 2017 17:12:32 -0400 Subject: [PATCH 20/21] var -> let --- jupyter-js-widgets/src/utils.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index cf290deedb..afa48dd660 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -183,12 +183,12 @@ function escape_html(text: string): string { */ export function put_buffers(state, buffer_paths, buffers) { - for (var i=0; i Date: Mon, 20 Mar 2017 17:13:39 -0400 Subject: [PATCH 21/21] add newline --- jupyter-js-widgets/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter-js-widgets/src/utils.ts b/jupyter-js-widgets/src/utils.ts index afa48dd660..fd5de01ca8 100644 --- a/jupyter-js-widgets/src/utils.ts +++ b/jupyter-js-widgets/src/utils.ts @@ -278,4 +278,4 @@ function remove_buffers(state) { } let new_state = remove(state, []); return {state: new_state, buffers: buffers, buffer_paths: buffer_paths} -} \ No newline at end of file +}