From 247b5475d03203ad20c1518dd629446148463d1b Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Tue, 11 Oct 2022 16:29:08 +0100 Subject: [PATCH 1/4] Avoid circular callbacks in layer options --- glue_jupyter/widgets/layer_options.py | 102 +++++++++++++------------- 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/glue_jupyter/widgets/layer_options.py b/glue_jupyter/widgets/layer_options.py index e5e87854..98b0865c 100644 --- a/glue_jupyter/widgets/layer_options.py +++ b/glue_jupyter/widgets/layer_options.py @@ -8,6 +8,8 @@ import ipywidgets as widgets from glue.core import message as msg from glue.core.hub import HubListener +from glue.utils import avoid_circular + from ..vuetify_helpers import WidgetCache @@ -26,60 +28,61 @@ def __init__(self, viewer): super().__init__() self.viewer = viewer - widgetCache = WidgetCache() + self.widgetCache = WidgetCache() self.observe(self._update_glue_state_from_layers, 'layers') self.current_layers_data = None - def layer_data(layer_artist): - if isinstance(layer_artist.state.layer, Subset): - label = layer_artist.layer.data.label + ':' + layer_artist.state.layer.label + def layer_data(self, layer_artist): + if isinstance(layer_artist.state.layer, Subset): + label = layer_artist.layer.data.label + ':' + layer_artist.state.layer.label + else: + label = layer_artist.state.layer.label + + return { + 'color': getattr(layer_artist.state, 'color', ''), + 'label': label, + 'visible': layer_artist.state.visible, + } + + def layer_to_dict(self, layer_artist, index): + def make_layer_panel(): + widget_cls = self.viewer._layer_style_widget_cls + if isinstance(widget_cls, dict): + return widget_cls[type(layer_artist)](layer_artist.state) else: - label = layer_artist.state.layer.label - - return { - 'color': getattr(layer_artist.state, 'color', ''), - 'label': label, - 'visible': layer_artist.state.visible, - } - - def layer_to_dict(layer_artist, index): - def make_layer_panel(): - widget_cls = viewer._layer_style_widget_cls - if isinstance(widget_cls, dict): - return widget_cls[type(layer_artist)](layer_artist.state) - else: - return widget_cls(layer_artist.state) - - data = layer_data(layer_artist) - data.update({ - 'index': index, - 'layer_panel': widgetCache.get_or_create(layer_artist, make_layer_panel) - }) - return data - - def _update_layers_from_glue_state(*args): - new_layers_data = [layer_data(layer) for layer in self.viewer.layers] - - # During the creation of the new widgets, layers can change again, causing a - # recursive loop. Short circuit this by checking if the content of the layers has - # actually changed. - if self.current_layers_data == new_layers_data: - return - - self.current_layers_data = new_layers_data - - self.layers = [layer_to_dict(layer_artist, i) for i, layer_artist in - enumerate(self.viewer.layers)] - - def _on_data_or_subset_update(msg): - if msg.attribute in ('label', 'style'): - _update_layers_from_glue_state() + return widget_cls(layer_artist.state) + + data = self.layer_data(layer_artist) + data.update({ + 'index': index, + 'layer_panel': self.widgetCache.get_or_create(layer_artist, make_layer_panel) + }) + return data + + @avoid_circular + def _update_layers_from_glue_state(self, *args): + new_layers_data = [self.layer_data(layer) for layer in self.viewer.layers] + + # During the creation of the new widgets, layers can change again, causing a + # recursive loop. Short circuit this by checking if the content of the layers has + # actually changed. + if self.current_layers_data == new_layers_data: + return + + self.current_layers_data = new_layers_data + + self.layers = [self.layer_to_dict(layer_artist, i) for i, layer_artist in + enumerate(self.viewer.layers)] + + def _on_data_or_subset_update(self, msg): + if msg.attribute in ('label', 'style'): + self._update_layers_from_glue_state() self.viewer.session.hub.subscribe(self, msg.DataUpdateMessage, - handler=_on_data_or_subset_update) + handler=self._on_data_or_subset_update) self.viewer.session.hub.subscribe(self, msg.SubsetUpdateMessage, - handler=_on_data_or_subset_update) + handler=self._on_data_or_subset_update) # The first call below looks for changes in self.viewer.layers and # the second call looks for changes in self.viewer.state.layers. The @@ -87,10 +90,11 @@ def _on_data_or_subset_update(msg): # the first one is needed because the callback function needs access # to the layer artist and in some cases the new layer artist hasn't been # created yet even if state.layers is up-to-date. - self.viewer._layer_artist_container.on_changed(_update_layers_from_glue_state) - self.viewer.state.add_callback('layers', _update_layers_from_glue_state) - _update_layers_from_glue_state() + self.viewer._layer_artist_container.on_changed(self._update_layers_from_glue_state) + self.viewer.state.add_callback('layers', self._update_layers_from_glue_state) + self._update_layers_from_glue_state() + @avoid_circular def _update_glue_state_from_layers(self, *args): for layer_data, layer in zip(self.layers, self.viewer.layers): layer.state.visible = layer_data['visible'] From 867586d52eb8ee0d2a32a9048a3221ac81cf0d3b Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Tue, 11 Oct 2022 16:38:28 +0100 Subject: [PATCH 2/4] Avoid sending state changes to the front-end if the serialized JSON hasn't actually changed --- glue_jupyter/state_traitlets_helpers.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/glue_jupyter/state_traitlets_helpers.py b/glue_jupyter/state_traitlets_helpers.py index 3e6594ca..755d595b 100644 --- a/glue_jupyter/state_traitlets_helpers.py +++ b/glue_jupyter/state_traitlets_helpers.py @@ -109,6 +109,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.tag(to_json=self.convert_state_to_json, from_json=self.update_state_from_json) + self._last_serialized = '' def validate(self, obj, value): @@ -126,12 +127,29 @@ def set(self, obj, state): state.add_global_callback(PartialCallback(self.on_state_change, obj=obj)) def on_state_change(self, *args, obj=None, **kwargs): + if self._block_on_state_change: return + + state = self.get(obj) + + # To avoid unecessary messages, we now check if the serialized version + # of the state has actually changed since the last time it was sent to + # front-end. In some cases it can happen that a glue state change doesn't + # result in any actual change to the JSON because some items are ignored + # in the serialization. + + serialized = self.convert_state_to_json(state, None) + + if serialized == self._last_serialized: + return + else: + self._last_serialized = serialized + obj.notify_change(Bunch({'name': self.name, 'type': 'change', - 'value': self.get(obj), - 'new': self.get(obj)})) + 'value': state, + 'new': state})) # NOTE: the following two methods are implemented as methods on the trait # because we need update_state_from_json to have an unambiguous reference From 422b7d1316b96dc1f741907c772a8432c97e30e8 Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Tue, 11 Oct 2022 16:43:46 +0100 Subject: [PATCH 3/4] Codestyle fixes --- glue_jupyter/widgets/layer_options.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/glue_jupyter/widgets/layer_options.py b/glue_jupyter/widgets/layer_options.py index 98b0865c..9a1793ce 100644 --- a/glue_jupyter/widgets/layer_options.py +++ b/glue_jupyter/widgets/layer_options.py @@ -6,7 +6,6 @@ import traitlets import ipywidgets as widgets -from glue.core import message as msg from glue.core.hub import HubListener from glue.utils import avoid_circular @@ -73,7 +72,7 @@ def _update_layers_from_glue_state(self, *args): self.current_layers_data = new_layers_data self.layers = [self.layer_to_dict(layer_artist, i) for i, layer_artist in - enumerate(self.viewer.layers)] + enumerate(self.viewer.layers)] def _on_data_or_subset_update(self, msg): if msg.attribute in ('label', 'style'): From 6b91f6d2ec402f818e96c035f7864a73a7c9ae1d Mon Sep 17 00:00:00 2001 From: Thomas Robitaille Date: Tue, 11 Oct 2022 16:56:58 +0100 Subject: [PATCH 4/4] Fix tests --- glue_jupyter/widgets/layer_options.py | 32 ++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/glue_jupyter/widgets/layer_options.py b/glue_jupyter/widgets/layer_options.py index 9a1793ce..a5cdbc46 100644 --- a/glue_jupyter/widgets/layer_options.py +++ b/glue_jupyter/widgets/layer_options.py @@ -6,6 +6,7 @@ import traitlets import ipywidgets as widgets +from glue.core import message as msg from glue.core.hub import HubListener from glue.utils import avoid_circular @@ -32,6 +33,21 @@ def __init__(self, viewer): self.current_layers_data = None + self.viewer.session.hub.subscribe(self, msg.DataUpdateMessage, + handler=self._on_data_or_subset_update) + self.viewer.session.hub.subscribe(self, msg.SubsetUpdateMessage, + handler=self._on_data_or_subset_update) + + # The first call below looks for changes in self.viewer.layers and + # the second call looks for changes in self.viewer.state.layers. The + # second one is needed to watch for changes in e.g. layer visibility, + # the first one is needed because the callback function needs access + # to the layer artist and in some cases the new layer artist hasn't been + # created yet even if state.layers is up-to-date. + self.viewer._layer_artist_container.on_changed(self._update_layers_from_glue_state) + self.viewer.state.add_callback('layers', self._update_layers_from_glue_state) + self._update_layers_from_glue_state() + def layer_data(self, layer_artist): if isinstance(layer_artist.state.layer, Subset): label = layer_artist.layer.data.label + ':' + layer_artist.state.layer.label @@ -61,6 +77,7 @@ def make_layer_panel(): @avoid_circular def _update_layers_from_glue_state(self, *args): + new_layers_data = [self.layer_data(layer) for layer in self.viewer.layers] # During the creation of the new widgets, layers can change again, causing a @@ -78,21 +95,6 @@ def _on_data_or_subset_update(self, msg): if msg.attribute in ('label', 'style'): self._update_layers_from_glue_state() - self.viewer.session.hub.subscribe(self, msg.DataUpdateMessage, - handler=self._on_data_or_subset_update) - self.viewer.session.hub.subscribe(self, msg.SubsetUpdateMessage, - handler=self._on_data_or_subset_update) - - # The first call below looks for changes in self.viewer.layers and - # the second call looks for changes in self.viewer.state.layers. The - # second one is needed to watch for changes in e.g. layer visibility, - # the first one is needed because the callback function needs access - # to the layer artist and in some cases the new layer artist hasn't been - # created yet even if state.layers is up-to-date. - self.viewer._layer_artist_container.on_changed(self._update_layers_from_glue_state) - self.viewer.state.add_callback('layers', self._update_layers_from_glue_state) - self._update_layers_from_glue_state() - @avoid_circular def _update_glue_state_from_layers(self, *args): for layer_data, layer in zip(self.layers, self.viewer.layers):