Zoom tools automatically vertically scaled on subcoordinate_y overlays

[maximlt]

resolves #5902

An overlay set with wheel_zoom (default, no need to provide it), zoom_in,zoom_out and their y-<zoom> counterpart is set to automatically vertically scale the subplots. This is achieved by setting dimensions to height and level to 1 on the Bokeh tools. The Bokeh zoom instance must know the list of all the renderers. To achieve that we:

• save a handle of the scaled zoom tool on each element plot (exactly a dict of : to deal with the different types of zoom supported)
• save a handle of the first subplot on the overlay plot
• after each subplot renderer is created, set it on the subplot zoom instance renderers attribute (the default being 'auto')
• aggregate all the renderers on the overlay plot

import numpy as np
import holoviews as hv
import panel as pn
hv.extension('bokeh')

n_channels = 10
n_seconds = 15

total_samples = 512*n_seconds
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = [f"EEG {i}" for i in range(n_channels)]

channel_curves = []
for channel, channel_data in zip(channels, data):
    ds = hv.Dataset((time, channel_data, channel), ["Time", "Amplitude", "channel"])
    curve = hv.Curve(ds, "Time", ["Amplitude", "channel"], label=channel)
    curve.opts(
        subcoordinate_y=True, line_width=1, tools=["zoom_in", "zoom_out"],
    )
    channel_curves.append(curve)

hv.Overlay(channel_curves, kdims="Channel").opts(
    xlabel="Time (s)", ylabel="Channel", show_legend=False, aspect=3, frame_height=400,
)

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (4672b04) 88.65% compared to head (7cf0a94) 88.66%.

Files	Patch %	Lines
holoviews/tests/plotting/bokeh/test_subcoordy.py	94.73%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6051      +/-   ##
==========================================
+ Coverage   88.65%   88.66%   +0.01%     
==========================================
  Files         314      314              
  Lines       65688    65764      +76     
==========================================
+ Hits        58234    58309      +75     
- Misses       7454     7455       +1     

Flag	Coverage Δ
ui-tests	23.65% <6.32%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maximlt]

Interestingly I've found the HoloViews version slower to zoom in/out when scrolling fast compared to the pure Bokeh one. @droumis could you try out both and see if you observe the same difference on your system?

Pure Bokeh code to serve

import numpy as np

from bokeh.core.properties import field
from bokeh.io import curdoc, show
from bokeh.models import (ColumnDataSource, DataRange1d, FactorRange,
                          HoverTool, Range1d, WheelZoomTool, ZoomInTool, ZoomOutTool)
from bokeh.plotting import figure
from bokeh.palettes import Category10

np.random.seed(0)

n_channels = 10
n_seconds = 15
fs = 512
max_ch_disp = 5  # max channels to initially display
max_t_disp = 3 # max time in seconds to initially display

total_samples = n_seconds * fs
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = [f'EEG {i}' for i in range(n_channels)]

hover = HoverTool(tooltips=[
    ("Channel", "$name"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV"),
])

x_range = Range1d(start=time.min(), end=time.max())
y_range = FactorRange(factors=channels)

p = figure(x_range=x_range, y_range=y_range, lod_threshold=None)

source = ColumnDataSource(data=dict(time=time))
renderers = []

for i, channel in enumerate(channels):
    xy = p.subplot(
        x_source=p.x_range,
        y_source=Range1d(start=data[i].min(), end=data[i].max()),
        x_target=p.x_range,
        y_target=Range1d(start=i, end=i + 1),
    )

    source.data[channel] = data[i]
    line = xy.line(field("time"), field(channel), color=Category10[10][i], source=source, name=channel)
    renderers.append(line)

wheel_zoom = WheelZoomTool(renderers=renderers, level=1, dimensions='height')

p.add_tools(wheel_zoom, hover)
p.toolbar.active_scroll = wheel_zoom

curdoc().add_root(p)

[droumis]

It's a bit harder to compare the latency with the wheel zoom, but I don't detect a big difference when running both in a notebook. They are both very responsive with 10 channels and 15 seconds.

Screen.Recording.2023-12-26.at.3.53.08.PM.mov

If you scale it to 10 channels and 150 seconds, both become equally terrible.

Sidenote, I'm not sure how you got the zoom_in and zoom_out tools to appear for you for the HoloViews plot.. they don't appear for me even with your code. I'm using bokeh 3.3.2.

[maximlt]

Sidenote, I'm not sure how you got the zoom_in and zoom_out tools to appear for you for the HoloViews plot.. they don't appear for me even with your code. I'm using bokeh 3.3.2.

My mistake! I made a wrong simplification in e1d688e after opening the PR, it's now reverted and you should be able to see the zoom_in/zoom_out tools with the code you used.

Happy to hear you're not seeing any performance differences between the pure Bokeh and HoloViews versions. I'm also no longer seeing that, there must have been something temporarily wrong with my system.

@droumis how do you want to proceed with the review of this PR? Addressing #5901 will have to build on this PR so the faster it's merged the less likely both work will conflict.

[droumis]

Yes, I think we should proceed with review. I anticipate that there may be some discussion about this new default behavior, but let's address any concerns when they arise.

To summarize the rationale for others:

First, we need to have the zoom tools apply to the appropriate level for the context. When the context is using subplots via subcoordinate_y, having the zoom tools apply to each subcoordinate axis is done by specifying level=1 to Bokeh. This is the first thing that this PR achieves.

Second, for this subplot context, there doesn't seem to be a use case in which zooming each subcoordinate axis in both the x and y simultaneously is the desired behavior. Therefore, a more reasonable default would be to have a zoom tool apply only to the y ('height') axis by default. This is the second thing that this PR achieves. If a user wants a zoom tool for just the x axis, they can add, e.g. 'xwheel_zoom' to the list of tools in opts.

The upside is that these changes would capture IMO the vast majority of use cases for subcoordinate_y without having to specify anything about the zoom tools.

The downside is that this changes (contextualizes) the behavior of a default zoom tool. It also makes it hard for a user to do the thing that probably no one would want to do - zooming each subcoordinate axis in both the x and y simultaneously). However, it also makes it harder to apply a zoom tool to the base axis (level=0). This last point could be mitigated with the use of a 'box_zoom' tools or a minimap (RangeToolLink) widget.

What's the alternative? If there was a clean way to specify options (level, dimension) for a given tool, then that would be an alternative, but that seems to be more of a departure from the HoloViews way of doing things compared to the contextualized approach in this PR. That said, there are other benefits to exposing options for zoom tools (like being able to set 'maintain focus') but this is beyond scope, so maybe we adapt later on.

[maximlt]

Thanks @droumis for this summary! Feel free to approve this PR if you're satisfied with the functionality and implementation. I guess we'll need @hoxbro's feedback to get this merged, either directly from him or delegated to somebody else.

[droumis]

A way to override the default behavior here is to pass in a Bokeh zoom model

[droumis]

Another alternative raised for consideration by @jlstevens was to create a new subplot zoom string

[droumis]

Yet another possibility is to have the zoom tools be further specified directly through planned UI elements/interactivity with the Bokeh toolbar

[droumis]

Update. In response to a question by @hoxbro, I checked what happens when wheel-zooming while the cursor is over the x-axis. Unfortunately with this PR, nothing happens. I guess the ideal behavior is that it would switch to xwheel-zoom when over the x axis

[jlstevens]

I support the intent of this PR and picking the right behavior given the plot makes sense.

My only hesitation is that this updated tool only applies to y and you lose the ability to zoom just on x. One suggestion I think is worth considering is that this kind of subcoordinate plot could add an additional zoom tool to supplement to usual default.

This would be another wheel zoom tool so ideally we would have a new icon to distinguish it (otherwise visually it will look like a duplication and you can't tell at a glance which is which).

[jlstevens]

Actually, what we really want would be to pick from a submenu (like we can for box zoom):

I don't know if we can easily achieve this at the moment though.

[maximlt]

Thanks @jlstevens for your feedback. We discussed that again with @droumis who pointed at that in the current state of this PR users can already add the "xwheel_zoom" tool to their plot to be able to scroll on the x-axis. They will get two identical icons with different tooltips. This isn't great UX, but I believe that there isn't much we can do at this stage to make the UX great anyway. We agreed in our discussion that the PR was fine as is in terms of functionality. @jlstevens the code is ready to be reviewed.

[jlstevens]

... users can already add the "xwheel_zoom" tool

Of course! That is a reasonable workaround for now: I was trying to think of ways to make the disruption to the normal UX as small as possible. None of my suggestions were going to block this PR but I did want to consider some options for potential future PRs.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.