Skip to content

Improvements to ScaleBar positioning and sizing#14005

Merged
mattpap merged 8 commits intobranch-3.6from
mattpap/13921_ScaleBar_positioning
Sep 23, 2024
Merged

Improvements to ScaleBar positioning and sizing#14005
mattpap merged 8 commits intobranch-3.6from
mattpap/13921_ScaleBar_positioning

Conversation

@mattpap
Copy link
Contributor

@mattpap mattpap commented Jul 30, 2024

Fixes #13921

This PR improves positioning of ScaleBar by allowing mixed screen and data coordinates.

Example:

Screencast.from.30.07.2024.16.07.10.webm
Code
import numpy as np
from bokeh.core.properties import field
from bokeh.io import show
from bokeh.layouts import column
from bokeh.models import (ColumnDataSource, HoverTool, Range1d, ScaleBar, FactorRange, Metric, WheelZoomTool)
from bokeh.palettes import Category10
from bokeh.plotting import figure

n_eeg_channels = 7
n_pos_channels = 3
n_channels = n_eeg_channels + n_pos_channels
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_eeg_channels)] + [f"POS {i}" for i in range(n_pos_channels)]

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

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, tools=["pan","reset",hover])

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

for i, channel in enumerate(channels):
    subp = p.subplot(
        x_source=p.x_range,
        #y_source=y_target_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 = subp.line(field("time"), field(channel), color=Category10[10][i], source=source, name=channel)
    renderers.append(line)

k_channel = 4
scale_bar = ScaleBar(
    range=renderers[k_channel].coordinates.y_source,
    unit="mm",
    dimensional=Metric(base_unit="mm"),
    orientation="vertical",
    location=("left", f"EEG {k_channel}"),
    label_location="right",
    background_fill_color=None,
    border_line_color=None,
    bar_length=1.0,
    bar_length_units="data",
    margin=10, # default margin and padding make positioning awkward in the data setup
    padding=0,
    length_sizing="exact",
)
p.add_layout(scale_bar)

ywheel_zoom = WheelZoomTool(renderers=renderers, level=1, dimensions="height")
p.add_tools(ywheel_zoom)
p.toolbar.active_scroll = ywheel_zoom

show(column(p))

@mattpap mattpap added this to the 3.6 milestone Jul 30, 2024
@mattpap
Copy link
Contributor Author

mattpap commented Aug 6, 2024

@droumis, @hoxbro, this PR is functionally ready, including data spacing scale bar sizing. There is still some technical fiddling needed to get this PR fully ready.

@mattpap mattpap force-pushed the mattpap/13921_ScaleBar_positioning branch from e713084 to f6a9d4a Compare August 6, 2024 10:55
@mattpap mattpap changed the title Improvements to ScaleBar positioning Improvements to ScaleBar positioning and sizing Aug 6, 2024
@droumis
Copy link
Member

droumis commented Aug 28, 2024

Hi @mattpap , what technical fiddling is needed for this?

@mattpap
Copy link
Contributor Author

mattpap commented Aug 29, 2024

what technical fiddling is needed for this?

Currently I'm primarily waiting for API feedback.

@droumis
Copy link
Member

droumis commented Aug 30, 2024

API seems good from a Bokeh user standpoint. @hoxbro, any preliminary feedback?

Code
from bokeh.layouts import column
from bokeh.plotting import figure, curdoc
import numpy as np

from bokeh.core.properties import field
from bokeh.layouts import column, row
from bokeh.models import (ColumnDataSource, CustomJS, Div, FactorRange, HoverTool, ScaleBar, Metric,
                          Range1d, Switch, WheelZoomTool, ZoomInTool, ZoomOutTool, GroupByModels)
from bokeh.palettes import Category10, GnBu, brewer
from bokeh.plotting import figure
from bokeh.models.annotations.legends import Val

n_eeg_channels = 7
n_pos_channels = 3
n_channels = n_eeg_channels + n_pos_channels
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_eeg_channels)] + [f"POS {i}" for i in range(n_pos_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, tools="pan,reset,xcrosshair")

source = ColumnDataSource(data=dict(time=time))
eeg_renderers = []
pos_renderers = []
pos_i, eeg_i = 0, 0
eeg_clrs = brewer['YlGnBu'][9]
pos_clrs = brewer['YlOrBr'][5]
for i, channel in enumerate(channels):
    is_eeg = channel.startswith('EEG')
    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]
    if is_eeg:
        line = xy.line(field("time"), field(channel), color=eeg_clrs[eeg_i], source=source, name=channel)
        eeg_i += 1
        eeg_renderers.append(line)
    else:
        line = xy.line(field("time"), field(channel), color=pos_clrs[pos_i], source=source, name=channel)
        pos_i += 1
        pos_renderers.append(line)
    
all_renderers = eeg_renderers + pos_renderers

level = 1
hit_test = True

behavior = GroupByModels(groups=[eeg_renderers, pos_renderers])

ywheel_zoom = WheelZoomTool(renderers=all_renderers, level=level, hit_test=hit_test, hit_test_mode="hline", hit_test_behavior=behavior, dimensions="height")
xwheel_zoom = WheelZoomTool(renderers=all_renderers, level=level, dimensions="width")
zoom_in = ZoomInTool(renderers=all_renderers, level=level, dimensions="height")
zoom_out = ZoomOutTool(renderers=all_renderers, level=level, dimensions="height")

p.add_tools(ywheel_zoom, xwheel_zoom, zoom_in, zoom_out, hover)
p.toolbar.active_scroll = ywheel_zoom

level_switch = Switch(active=level == 1)
level_switch.js_on_change("active", CustomJS(
    args=dict(tools=[ywheel_zoom, zoom_in, zoom_out]),
    code="""
export default ({tools}, obj) => {
    const level = obj.active ? 1 : 0
    for (const tool of tools) {
        tool.level = level
    }
}
"""))

hit_test_switch = Switch(active=hit_test)
hit_test_switch.js_on_change("active", CustomJS(
    args=dict(tool=ywheel_zoom),
    code="""
export default ({tool}, obj) => {
    tool.hit_test = obj.active
}
"""))

layout = column(
    row(Div(text="Zoom sub-coordinates:"), level_switch),
    row(Div(text="Zoom hit-tested:"), hit_test_switch),
    p,
)

eeg_channel_sb = 0
scale_bar = ScaleBar(
    range=eeg_renderers[eeg_channel_sb].coordinates.y_source,
    unit="µV",
    dimensional=Metric(base_unit="V"),
    orientation="vertical",
    location=("left", Val(f"EEG {eeg_channel_sb}")),
    anchor="center_left",
    label_location="right",
    background_fill_color=None,
    border_line_color=None,
    bar_length=0.5,
    bar_length_units="data",
    margin=10, # default margin and padding make positioning awkward in the data setup
    padding=0,
    #     length_sizing="exact",
)
p.add_layout(scale_bar)

pos_channel_sb = 0
scale_bar = ScaleBar(
    range=pos_renderers[pos_channel_sb].coordinates.y_source,
    unit="mm",
    dimensional=Metric(base_unit="m"),
    orientation="vertical",
    location=("left", Val(f"POS {eeg_channel_sb}")),
    anchor="center_left",
    label_location="right",
    background_fill_color=None,
    border_line_color=None,
    bar_length=0.5,
    bar_length_units="data",
    margin=10, # default margin and padding make positioning awkward in the data setup
    padding=0,
    #     length_sizing="exact",
)
p.add_layout(scale_bar)

curdoc().add_root(layout)
GMT20240830-234454_Clip_Scale.bar.mp4

Copy link
Contributor

@hoxbro hoxbro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this relates to changes made in this PR, but when you really zoom in on the subplots and then out again, some of the lines disappear. It's not a blocker, but I at least want to mention it.

Screencast.2024-09-13.15.47.51.mp4

@mattpap mattpap force-pushed the mattpap/13921_ScaleBar_positioning branch 3 times, most recently from c403861 to 4beb6ff Compare September 19, 2024 22:47
@mattpap
Copy link
Contributor Author

mattpap commented Sep 19, 2024

This is tentatively ready, but I'm still working on tests. I removed Val model and switched to more traditional API (see the updated example).

@mattpap
Copy link
Contributor Author

mattpap commented Sep 20, 2024

I don't think this relates to changes made in this PR, but when you really zoom in on the subplots and then out again, some of the lines disappear. It's not a blocker, but I at least want to mention it.

Can't reproduce that in any browser on Linux. Perhaps a platform specific problem. Please report an issue. /cc @hoxbro, in case you missed this response.

Copy link
Member

@bryevdv bryevdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. I think the PR also needs to add or update a Python example


location = Enum(Anchor, default="top_right", help="""
Location anchor for positioning scale bar.
# TODO rename to position and deprecate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in common usage "location" and "position" are more or less synonymous. What precise distinction do you want to draw between these two words, that we need to rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that they are practically synonymous, but in our API we have two quite different usages for location. One is relative placement based on Location enum, used in positioning toolbar in a plot or a grid plot, labels and titles in various models (including ScaleBar), etc. The other is placement within a container, used in ScaleBar, Legend, etc. Thus it would be nice to use different term for different applications, given there's a choice. Also, it's more a thought rather than something actionable in the near future.



HAnchor = Either(Enum(enums.Align), Enum(enums.HAlign), Percent)
VAnchor = Either(Enum(enums.Align), Enum(enums.VAlign), Percent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess practically it does not really matter, but it does seem odd to have an Either with two enums that overlap (both align enums have "center" unless I am mistaken)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after CSS and was introduced with the new layout. It's not that odd if Align can be used on its own, where {H,V}Align don't apply. Otherwise it provides start/left, start/top and end/right, end/bottom aliases.

@mattpap mattpap force-pushed the mattpap/13921_ScaleBar_positioning branch 3 times, most recently from 140c13d to ae76f97 Compare September 22, 2024 18:52
Copy link
Member

@bryevdv bryevdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see an example added or updated but otherwise LGTM

@mattpap mattpap force-pushed the mattpap/13921_ScaleBar_positioning branch 2 times, most recently from 01b625c to be045ad Compare September 23, 2024 09:00
n_pos_channels = 2
n_channels = n_eeg_channels + n_pos_channels
n_seconds = 15
total_samples = 512*n_seconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
total_samples = 512*n_seconds
total_samples = 512 * n_seconds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a more common style, in my opinion, to have spaces around additive operators and no spaces around multiplicative.

@mattpap mattpap force-pushed the mattpap/13921_ScaleBar_positioning branch from be045ad to 9956e8f Compare September 23, 2024 13:11
@mattpap mattpap merged commit ce94261 into branch-3.6 Sep 23, 2024
@mattpap mattpap deleted the mattpap/13921_ScaleBar_positioning branch September 23, 2024 13:44
@github-actions
Copy link

github-actions bot commented Jan 1, 2025

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Attach ScaleBar to subplot range

5 participants