glue-jupyter
glue-jupyter copied to clipboard
POC: Refactor react
This is a proof of concept of applying https://github.com/widgetti/react-ipywidgets to glue-jupyter.
https://github.com/glue-viz/glue-jupyter/pull/273 was the inspiration to develop react-ipywidgets since it is another extreme example of error-prone simple bookkeeping (i.e. syncing state between 2 stateful libraries).
Although this PR is quite large, the second commit, or rather, this diff:
def create_scale(viewer_state, name):
+ is_log, set_x_log = use_echo_state(viewer_state, f"{name}_log")
v_min, set_v_min = use_echo_state(viewer_state, f"{name}_min")
v_max, set_v_max = use_echo_state(viewer_state, f"{name}_max")
- scale = bq.LinearScale(min=v_min, max=v_max, allow_padding=False)
+ if is_log:
+ scale = bq.LogScale(min=v_min, max=v_max, allow_padding=False)
+ else:
+ scale = bq.LinearScale(min=v_min, max=v_max, allow_padding=False)
return scale
is the complete replacement of #273, or rather:
- self.scale_x = bqplot.LinearScale(min=0, max=1, allow_padding=False)
- self.scale_y = bqplot.LinearScale(min=0, max=1)
+
+ # lazily create widgets
+ self.scale_y_log = None
+ self.scale_y_linear = None
+ self.scale_x_log = None
+ self.scale_x_linear = None
+
+ if self.state.y_log:
+ self.scale_y_log = bqplot.LogScale()
+ self.scale_y = self.scale_y_log
+ else:
+ self.scale_y_linear = bqplot.LinearScale(min=0, max=1)
+ self.scale_y = self.scale_y_linear
+
+ if self.state.x_log:
+ self.scale_x_log = bqplot.LogScale()
+ self.scale_x = self.scale_x_log
+ else:
+ self.scale_x_linear = bqplot.LinearScale(min=0, max=1)
+ self.scale_x = self.scale_x_linear
+
+ def update_scale_type_y(_ignore):
+ if self.state.y_log:
+ if self.scale_y_log is None:
+ self.scale_y_log = bqplot.LogScale()
+ scale = self.scale_y_log
+ else:
+ if self.scale_y_linear is None:
+ self.scale_y_linear = bqplot.LinearScale()
+ scale = self.scale_y_linear
+ self.scale_y = scale
+ self.state.add_callback('y_log', update_scale_type_y, priority=-1)
+
+ def update_scale_type_x(_ignore):
+ if self.state.x_log:
+ if self.scale_x_log is None:
+ self.scale_x_log = bqplot.LogScale()
+ scale = self.scale_x_log
+ else:
+ if self.scale_x_linear is None:
+ self.scale_x_linear = bqplot.LinearScale()
+ scale = self.scale_x_linear
+ self.scale_y = scale
+ self.state.add_callback('x_log', update_scale_type_x, priority=-1)
Which is significantly less code and therefore less likely to be buggy (I might have missed a _x or mistyped one).
Using react (for ipywidgets) is not without its own problem, mostly a rather steep learning curve.
I do like the replacement of the LayerArtist by simply 1 function:
@react.component
def Histogram(
scale_x,
scale_y,
viewer_state: HistogramViewerState,
layer_state: HistogramLayerState,
):
I'm not sure it's wise to adopt react-ipywidget in the near term, but I think this PR is useful to have as an example of what this would look like.
Thanks for the PR! Could you try if adding react_ipywidgets
to either install_requires
or test
in setup.cfg
will allow the CI tests to run?