jupyter-widgets / ipywidgets

Interactive Widgets for the Jupyter Notebook
https://ipywidgets.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.15k stars 949 forks source link

Why not double-buffer `Output`? #3253

Open shaperilio opened 3 years ago

shaperilio commented 3 years ago

Problem

When using interact with several figures, output is typically cleared when the first change arrives, and so you can "see it drawing as it goes". Here's a simple snippet:

from matplotlib import pyplot
import ipywidgets
from IPython.display import display
import numpy

def long_plots(x, y, z):
    pts = numpy.arange(x)
    pyplot.figure()
    pyplot.plot(pts, pts * y)
    pyplot.figure()
    pyplot.plot(pts, pts * z)

display(ipywidgets.interact(long_plots, x=(2, 10), y=(-3, 3), z=(-5, 5)))

As you move the sliders, you'll see the two plots being created. It's snappy on Chrome, but a bit slower in VSCode. It results in some flashing which is not desirable.

Proposed Solution

Why not double-buffer with a second Output instance, so that the one that is being displayed only receives one change? i.e. in interaction.py, replace this code:

with self.out:
    if self.clear_output:
        clear_output(wait=True)
    for widget in self.kwargs_widgets:
        value = widget.get_interact_value()
        self.kwargs[widget._kwarg] = value
    self.result = self.f(**self.kwargs)
    show_inline_matplotlib_plots()
    if self.auto_display and self.result is not None:
        display(self.result)

with something like this:

buffer = Output()
buffer.clear_output()
with buffer:
    for widget in self.kwargs_widgets:
        value = widget.get_interact_value()
        self.kwargs[widget._kwarg] = value
    self.result = self.f(**self.kwargs)
    show_inline_matplotlib_plots()
    # Note: Not sure how to handle this
    # if self.auto_display and self.result is not None:
    #     display(self.result)
with self.out:
    if self.clear_output:
        clear_output(wait=True)
    display(buffer)

(I acknowledge I'm not proposing a complete solution here; just trying to show the idea. I've prototyped it by extracting the code from interact_output and modifying it and the concept seems sound.)

Is there a reason this isn't done?

blois commented 3 years ago

An alternative approach (which isn't quite working for me) would be to accumulate the output items on the kernel side then update the output widget with all of the resulting outputs at once.

An example of this would be to use display_pub.register_hook to re-route display messages to the output, based on https://github.com/jupyter-widgets/ipywidgets/blob/3558ce6a22b4a6b1badb69f0507a6ac5b245a17f/ipywidgets/widgets/interaction.py#L52:

from ipywidgets.widgets import interaction
from IPython.display import display, clear_output
import ipywidgets as widgets
import matplotlib.pyplot as plt

import numpy as np

def long_plots(x, y):
    pts = np.arange(100)
    plt.figure()
    plt.plot(pts, np.sin(pts * x))
    plt.figure()
    plt.plot(pts, y * np.sin(pts * x))

def interactive_output(f, controls):
    """Connect widget controls to a function.
    This function does not generate a user interface for the widgets (unlike `interact`).
    This enables customisation of the widget user interface layout.
    The user interface layout must be defined and displayed manually.
    """
    out = widgets.Output()
    def observer(change):
        kwargs = {k:v.value for k,v in controls.items()}
        interaction.show_inline_matplotlib_plots()

        outputs = []
        def output_hook(msg):
          if msg['msg_type'] == 'display_data':
            outputs.append(msg['content'])
            return None
          return msg

        get_ipython().display_pub.register_hook(output_hook)

        f(**kwargs)
        interaction.show_inline_matplotlib_plots()

        get_ipython().display_pub.unregister_hook(output_hook)
        out.outputs = outputs

    for k,w in controls.items():
        w.observe(observer, 'value')
    interaction.show_inline_matplotlib_plots()
    observer(None)
    return out

controls = {'x': widgets.IntSlider(min=1, max=10), 'y': widgets.IntSlider(min=-3, max=3)}
for k, w in controls.items():
  display(w)

interactive_output(long_plots, controls)

Unfortunately this doesn't seem to update the output contents... not sure why.

I really wish output widget did this by default as it would simplify a lot of output handling (and would avoid the flickering as the clear_outputs & accumulation of new outputs would be atomic)

maartenbreddels commented 1 year ago

@blois I totally agree this is the way to do it.

In https://github.com/widgetti/solara/pull/68/ (2nd commit https://github.com/widgetti/solara/pull/68/commits/f1d580a27e3a32d73a5c825caf665018728df1cb ) I implement this when running under solara-server. It is also a much simpler implementation than the current Output widget __enter__ and __exit__.

Note that your solution doesn't work because you mutate the list, while if you assign, it will probably work.

jasongrout commented 1 year ago

I really wish output widget did this by default

I gave a bit of context at https://github.com/jupyter-widgets/ipywidgets/pull/3759#issuecomment-1509963352, but for convenience, I'll copy it here too:

The fundamental decision we had to make when originally implementing output widgets and evaluating the experiments at https://github.com/ipython/ipykernel/pull/115 were: Do we make the kernel more complicated (with an output redirection mechanism) and the frontend simple (which arguably is a better architecture, like you point out), or do we make the frontend more complicated and need no changes to any kernel? We decided there were likely to be far more kernels than frontends, so the balance shifted to the somewhat awkward architecture we have today, which is kernel agnostic.

In practice, though, there have been fewer kernels implementing ipywidgets support and more frontends implementing ipywidgets support than (at least I) anticipated. So I'm glad that the tradeoff is being reevaluated.