Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kernel is the source of truth: support multiple clients and fixes out of sync bug #3195

Merged
merged 15 commits into from
Dec 22, 2021
Merged
106 changes: 97 additions & 9 deletions python/ipywidgets/ipywidgets/widgets/tests/test_set_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def test_set_state_simple():
c=[False, True, False],
))

assert w.comm.messages == []
assert len(w.comm.messages) == 1


def test_set_state_transformer():
Expand All @@ -105,7 +105,7 @@ def test_set_state_data():
a=True,
d={'data': data},
))
assert w.comm.messages == []
assert len(w.comm.messages) == 1


def test_set_state_data_truncate():
Expand All @@ -124,7 +124,7 @@ def test_set_state_data_truncate():
data=dict(
buffer_paths=[['d', 'data']],
method='update',
state=dict(d={})
state=dict(d={}, a=True)
)))

# Sanity:
Expand All @@ -144,8 +144,8 @@ def test_set_state_numbers_int():
i = 3,
ci = 4,
))
# Ensure no update message gets produced
assert len(w.comm.messages) == 0
# Ensure one update message gets produced
assert len(w.comm.messages) == 1


def test_set_state_numbers_float():
Expand All @@ -156,8 +156,8 @@ def test_set_state_numbers_float():
cf = 2.0,
ci = 4.0
))
# Ensure no update message gets produced
assert len(w.comm.messages) == 0
# Ensure one update message gets produced
assert len(w.comm.messages) == 1


def test_set_state_float_to_float():
Expand All @@ -167,8 +167,8 @@ def test_set_state_float_to_float():
f = 1.2,
cf = 2.6,
))
# Ensure no update message gets produced
assert len(w.comm.messages) == 0
# Ensure one message gets produced
assert len(w.comm.messages) == 1


def test_set_state_cint_to_float():
Expand Down Expand Up @@ -235,6 +235,7 @@ def _propagate_value(self, change):
# this mimics a value coming from the front end
widget.set_state({'value': 42})
assert widget.value == 42
assert widget.stop is True

# we expect no new state to be sent
calls = []
Expand Down Expand Up @@ -268,3 +269,90 @@ def _propagate_value(self, change):

calls = [call42]
widget._send.assert_has_calls(calls)



def test_echo():
# we always echo values back to the frontend
class ValueWidget(Widget):
value = Float().tag(sync=True)

widget = ValueWidget(value=1)
assert widget.value == 1

widget._send = mock.MagicMock()
# this mimics a value coming from the front end
widget.set_state({'value': 42})
assert widget.value == 42

# we expect this to be echoed
msg = {'method': 'update', 'state': {'value': 42.0}, 'buffer_paths': []}
call42 = mock.call(msg, buffers=[])

calls = [call42]
widget._send.assert_has_calls(calls)


def test_echo_single():
# we always echo multiple changes back in 1 update
class ValueWidget(Widget):
value = Float().tag(sync=True)
square = Float().tag(sync=True)
@observe('value')
def _square(self, change):
self.square = self.value**2

widget = ValueWidget(value=1)
assert widget.value == 1

widget._send = mock.MagicMock()
# this mimics a value coming from the front end
widget._handle_msg({
'content': {
'data': {
'method': 'update',
'state': {
'value': 8,
}
}
}
})
assert widget.value == 8
assert widget.square == 64

# we expect this to be echoed
msg = {'method': 'update', 'state': {'square': 64, 'value': 8.0}, 'buffer_paths': []}
call = mock.call(msg, buffers=[])

calls = [call]
widget._send.assert_has_calls(calls)


def test_no_echo():
# in cases where values coming fromt the frontend are 'heavy', we might want to opt out
class ValueWidget(Widget):
value = Float().tag(sync=True, no_echo=True)

widget = ValueWidget(value=1)
assert widget.value == 1

widget._send = mock.MagicMock()
# this mimics a value coming from the front end
widget._handle_msg({
'content': {
'data': {
'method': 'update',
'state': {
'value': 42,
}
}
}
})
assert widget.value == 42

# widget._send.assert_not_called(calls)
widget._send.assert_not_called()

# a regular set should sync to the frontend
widget.value = 43
widget._send.assert_has_calls([mock.call({'method': 'update', 'state': {'value': 43.0}, 'buffer_paths': []}, buffers=[])])
74 changes: 25 additions & 49 deletions python/ipywidgets/ipywidgets/widgets/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ def get_view_spec(self):
def _default_keys(self):
return [name for name in self.traits(sync=True)]

_property_lock = Dict()
_holding_sync = False
_holding_sync_from_frontend_update = False
_states_to_send = Set()
_msg_callbacks = Instance(CallbackDispatcher, ())

Expand Down Expand Up @@ -497,10 +497,6 @@ def send_state(self, key=None):
"""
state = self.get_state(key=key)
if len(state) > 0:
if self._property_lock: # we need to keep this dict up to date with the front-end values
for name, value in state.items():
if name in self._property_lock:
self._property_lock[name] = value
state, buffer_paths, buffers = _remove_buffers(state)
msg = {'method': 'update', 'state': state, 'buffer_paths': buffer_paths}
self._send(msg, buffers=buffers)
Expand Down Expand Up @@ -549,10 +545,8 @@ def _compare(self, a, b):

def set_state(self, sync_data):
"""Called when a state is received from the front-end."""
# The order of these context managers is important. Properties must
# be locked when the hold_trait_notification context manager is
# released and notifications are fired.
with self.hold_sync(), self._lock_property(**sync_data), self.hold_trait_notifications():
# maybe self.hold_sync()
with self._hold_sync_frontend(), self.hold_trait_notifications():
for name in sync_data:
if name in self.keys:
from_json = self.trait_metadata(name, 'from_json',
Expand Down Expand Up @@ -598,10 +592,14 @@ def notify_change(self, change):
# Send the state to the frontend before the user-registered callbacks
# are called.
name = change['name']
if self.comm is not None and self.comm.kernel is not None:
# Make sure this isn't information that the front-end just sent us.
if name in self.keys and self._should_send_property(name, getattr(self, name)):
# Send new state to front-end
if self.comm is not None and self.comm.kernel is not None and name in self.keys:
if self._holding_sync:
# if we're holding a sync, we will only record which trait was changed
# but we skip those traits marked no_echo, during an update from the frontend
if not (self._holding_sync_from_frontend_update and self.trait_metadata(name, 'no_echo')):
self._states_to_send.add(name)
else:
# otherwise we send it directly
self.send_state(key=name)
super().notify_change(change)

Expand All @@ -612,21 +610,6 @@ def __repr__(self):
# Support methods
#-------------------------------------------------------------------------

@contextmanager
def _lock_property(self, **properties):
"""Lock a property-value pair.

The value should be the JSON state of the property.

NOTE: This, in addition to the single lock for all state changes, is
flawed. In the future we may want to look into buffering state changes
back to the front-end."""
self._property_lock = properties
try:
yield
finally:
self._property_lock = {}

@contextmanager
def hold_sync(self):
"""Hold syncing any state until the outermost context manager exits"""
Expand All @@ -641,27 +624,19 @@ def hold_sync(self):
self.send_state(self._states_to_send)
self._states_to_send.clear()

def _should_send_property(self, key, value):
"""Check the property lock (property_lock)"""
to_json = self.trait_metadata(key, 'to_json', self._trait_to_json)
if key in self._property_lock:
# model_state, buffer_paths, buffers
split_value = _remove_buffers({ key: to_json(value, self)})
split_lock = _remove_buffers({ key: self._property_lock[key]})
# A roundtrip conversion through json in the comparison takes care of
# idiosyncracies of how python data structures map to json, for example
# tuples get converted to lists.
if (jsonloads(jsondumps(split_value[0])) == split_lock[0]
and split_value[1] == split_lock[1]
and _buffer_list_equal(split_value[2], split_lock[2])):
if self._holding_sync:
self._states_to_send.discard(key)
return False
if self._holding_sync:
self._states_to_send.add(key)
return False
@contextmanager
def _hold_sync_frontend(self):
"""Same as hold_sync, but will not sync back traits tagged as no_echo"""
if self._holding_sync_from_frontend_update is True:
with self.hold_sync():
yield
else:
return True
try:
self._holding_sync_from_frontend_update = True
with self.hold_sync():
yield
finally:
self._holding_sync_from_frontend_update = False

# Event handlers
@_show_traceback
Expand All @@ -684,7 +659,8 @@ def _handle_msg(self, msg):
# Handle a custom msg from the front-end.
elif method == 'custom':
if 'content' in data:
self._handle_custom_msg(data['content'], msg['buffers'])
with self._hold_sync_frontend():
self._handle_custom_msg(data['content'], msg['buffers'])

# Catch remainder.
else:
Expand Down
2 changes: 1 addition & 1 deletion python/ipywidgets/ipywidgets/widgets/widget_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class FileUpload(DescriptionWidget, ValueWidget, CoreWidget):
style = InstanceDict(ButtonStyle).tag(sync=True, **widget_serialization)
error = Unicode(help='Error message').tag(sync=True)
value = TypedTuple(Dict(), help='The file upload value').tag(
sync=True, **_value_serialization)
sync=True, no_echo=True, **_value_serialization)

@default('description')
def _default_description(self):
Expand Down