From 5d1a553a6ff8a573c83700430ead4b0c27406c6f Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Tue, 28 Oct 2025 14:16:15 +0100 Subject: [PATCH 01/15] add specified tools directly on overlay --- holoviews/plotting/bokeh/element.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py index f5ef295e91..d2746038dd 100644 --- a/holoviews/plotting/bokeh/element.py +++ b/holoviews/plotting/bokeh/element.py @@ -3271,6 +3271,20 @@ def _init_tools(self, element, callbacks=None): else: tool_types.append(tool_type) init_tools.append(tool) + + # Add tools specified directly on the overlay + overlay_tools = self.default_tools + self.tools + for tool in overlay_tools: + if isinstance(tool, str): + tool_type = TOOL_TYPES.get(tool) + else: + tool_type = type(tool) + # Only add tools that haven't been added by subplots + if tool_type not in tool_types: + tool_types.append(tool_type) + init_tools.append(tool) + + self.handles['hover_tools'] = hover_tools return init_tools From 769c83828ff25d9b762c1fc7fd8d81e234ba3eae Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Wed, 29 Oct 2025 15:07:31 +0100 Subject: [PATCH 02/15] add tool_id helper fxn and address sub_cordinate_y plots --- holoviews/plotting/bokeh/element.py | 48 ++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py index d2746038dd..b9bed404aa 100644 --- a/holoviews/plotting/bokeh/element.py +++ b/holoviews/plotting/bokeh/element.py @@ -3238,7 +3238,29 @@ def _init_tools(self, element, callbacks=None): hover_tools = {} zooms_subcoordy = {} _zoom_types = (tools.WheelZoomTool, tools.ZoomInTool, tools.ZoomOutTool) - init_tools, tool_types = [], [] + init_tools = [] + tool_ids = set() + + def get_tool_id(tool, tool_type): + """Generate a unique identifier for a tool.""" + if isinstance(tool, str): + directional_tools = ('wheel_zoom', 'pan', 'zoom_in', 'zoom_out', 'box_zoom') + if tool in directional_tools: + return (tool_type, 'both') + elif tool.startswith(('x', 'y')): + if tool[1:] in directional_tools: + dimension = 'width' if tool.startswith('x') else 'height' + return (tool_type, dimension) + elif tool == 'auto_box_zoom': + return (tool_type, 'auto') + return tool + elif hasattr(tool, 'dimensions') and tool.dimensions: + return (tool_type, tool.dimensions) + elif hasattr(tool, 'description') and tool.description: + return (tool_type, tool.description) + else: + return tool_type + for key, subplot in self.subplots.items(): el = element.get(key) if el is not None: @@ -3248,6 +3270,8 @@ def _init_tools(self, element, callbacks=None): tool_type = TOOL_TYPES.get(tool) else: tool_type = type(tool) + + tool_id = get_tool_id(tool, tool_type) if isinstance(tool, tools.HoverTool): if isinstance(tool.tooltips, bokeh.models.dom.Div): tooltips = tool.tooltips @@ -3266,10 +3290,10 @@ def _init_tools(self, element, callbacks=None): else: zooms_subcoordy[tool.tags[1]] = tool self.handles['zooms_subcoordy'] = zooms_subcoordy - elif tool_type in tool_types: + elif tool_id in tool_ids: continue - else: - tool_types.append(tool_type) + + tool_ids.add(tool_id) init_tools.append(tool) # Add tools specified directly on the overlay @@ -3279,11 +3303,19 @@ def _init_tools(self, element, callbacks=None): tool_type = TOOL_TYPES.get(tool) else: tool_type = type(tool) - # Only add tools that haven't been added by subplots - if tool_type not in tool_types: - tool_types.append(tool_type) - init_tools.append(tool) + tool_id = get_tool_id(tool, tool_type) + + # Skip subcoordinate_y zoom tools that were already handled + if (self.subcoordinate_y and tool_type in _zoom_types and + isinstance(tool, str) and not tool.startswith('x')): + tool_name = tool.replace('_tool', '') + if tool_name in zooms_subcoordy: + continue + + if tool_id not in tool_ids: + tool_ids.add(tool_id) + init_tools.append(tool) self.handles['hover_tools'] = hover_tools return init_tools From 025d4fd512f96918e657e31135771b3469238ffa Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Wed, 29 Oct 2025 15:15:01 +0100 Subject: [PATCH 03/15] add tests --- .../tests/plotting/bokeh/test_overlayplot.py | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index 838d0dc1a8..68fb59ff7a 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -422,6 +422,145 @@ def test_ndoverlay_categorical_y_ranges(order): expected = sorted(map(str, df.values.ravel())) assert output == expected +def test_overlay_opts_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in']) + plot = bokeh_renderer.get_plot(overlay) + + tool_types = [type(tool).__name__ for tool in plot.state.tools] + assert 'ZoomInTool' in tool_types + +def test_overlay_opts_tools_multiple(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in', 'zoom_out']) + plot = bokeh_renderer.get_plot(overlay) + + tool_types = [type(tool).__name__ for tool in plot.state.tools] + assert 'ZoomInTool' in tool_types + assert 'ZoomOutTool' in tool_types + +def test_overlay_opts_tools_with_element_tools(): + overlay = (Curve([1, 2, 3]).opts(tools=['zoom_out']) * Curve([2, 3, 4])).opts(tools=['zoom_in']) + plot = bokeh_renderer.get_plot(overlay) + + tool_types = [type(tool).__name__ for tool in plot.state.tools] + assert 'ZoomInTool' in tool_types + assert 'ZoomOutTool' in tool_types + +def test_overlay_default_tools_preserved(): + overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) + plot = bokeh_renderer.get_plot(overlay) + + tool_types = [type(tool).__name__ for tool in plot.state.tools] + expected_defaults = ['PanTool', 'WheelZoomTool', 'SaveTool', 'BoxZoomTool', 'ResetTool'] + + for expected in expected_defaults: + assert expected in tool_types + +def test_overlay_default_tools_not_duplicated(): + overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) + plot = bokeh_renderer.get_plot(overlay) + + # Count each tool type + tool_type_counts = {} + for tool in plot.state.tools: + tool_type = type(tool).__name__ + tool_type_counts[tool_type] = tool_type_counts.get(tool_type, 0) + 1 + + # Each default tool should appear exactly once + assert tool_type_counts.get('PanTool', 0) == 1 + assert tool_type_counts.get('WheelZoomTool', 0) == 1 + assert tool_type_counts.get('SaveTool', 0) == 1 + assert tool_type_counts.get('BoxZoomTool', 0) == 1 + assert tool_type_counts.get('ResetTool', 0) == 1 + +def test_overlay_opts_directional_pan_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xpan', 'ypan']) + plot = bokeh_renderer.get_plot(overlay) + + pan_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'PanTool'] + # Should have 3 PanTools: default 'pan' (both) + xpan (width) + ypan (height) + assert len(pan_tools) == 3 + + dimensions = [tool.dimensions for tool in pan_tools] + assert 'both' in dimensions + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_generic_and_directional_pan_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['pan', 'xpan', 'ypan']) + plot = bokeh_renderer.get_plot(overlay) + + # Default 'pan' tool should not be duplicated + pan_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'PanTool'] + assert len(pan_tools) == 3 + + dimensions = [tool.dimensions for tool in pan_tools] + assert 'both' in dimensions + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_directional_zoom_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xzoom_in', 'yzoom_in']) + plot = bokeh_renderer.get_plot(overlay) + + zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'ZoomInTool'] + assert len(zoom_tools) == 2 + + dimensions = [tool.dimensions for tool in zoom_tools] + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_generic_and_directional_zoom_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in', 'xzoom_in', 'yzoom_in']) + plot = bokeh_renderer.get_plot(overlay) + + zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'ZoomInTool'] + assert len(zoom_tools) == 3 + + dimensions = [tool.dimensions for tool in zoom_tools] + assert 'both' in dimensions + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_directional_box_zoom_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xbox_zoom', 'ybox_zoom']) + plot = bokeh_renderer.get_plot(overlay) + + box_zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'BoxZoomTool'] + # Should have 3 BoxZoomTools: default 'auto_box_zoom' (auto) + xbox_zoom (width) + ybox_zoom (height) + assert len(box_zoom_tools) == 3 + + # Check that we have auto_box_zoom (auto), xbox_zoom (width), and ybox_zoom (height) + dimensions = [tool.dimensions for tool in box_zoom_tools] + assert 'auto' in dimensions + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_generic_and_directional_box_zoom_tools(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['box_zoom', 'xbox_zoom', 'ybox_zoom']) + plot = bokeh_renderer.get_plot(overlay) + + box_zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'BoxZoomTool'] + # Should have 4 BoxZoomTools: default 'auto_box_zoom' + 3 added + assert len(box_zoom_tools) == 4 + + dimensions = [tool.dimensions for tool in box_zoom_tools] + assert 'auto' in dimensions + assert 'both' in dimensions + assert 'width' in dimensions + assert 'height' in dimensions + +def test_overlay_opts_mixed_tools_no_duplicates(): + overlay = ( + Curve([1, 2, 3]).opts(tools=['xpan']) * + Curve([2, 3, 4]).opts(tools=['xpan']) + ).opts(tools=['xpan']) + plot = bokeh_renderer.get_plot(overlay) + + pan_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'PanTool'] + # Should only have one xpan tool despite being specified multiple times + xpan_tools = [tool for tool in pan_tools if tool.dimensions == 'width'] + assert len(xpan_tools) == 1 + class TestLegends(TestBokehPlot): From 7cdb7364b026002fc6007cff421d03b9854f846e Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Thu, 27 Nov 2025 19:23:45 +0100 Subject: [PATCH 04/15] refactor init_tools --- holoviews/plotting/bokeh/element.py | 99 ++++++++++++++--------------- 1 file changed, 49 insertions(+), 50 deletions(-) diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py index 4dca224803..d11a813400 100644 --- a/holoviews/plotting/bokeh/element.py +++ b/holoviews/plotting/bokeh/element.py @@ -3254,10 +3254,9 @@ def get_tool_id(tool, tool_type): directional_tools = ('wheel_zoom', 'pan', 'zoom_in', 'zoom_out', 'box_zoom') if tool in directional_tools: return (tool_type, 'both') - elif tool.startswith(('x', 'y')): - if tool[1:] in directional_tools: - dimension = 'width' if tool.startswith('x') else 'height' - return (tool_type, dimension) + elif tool.startswith(('x', 'y')) and tool[1:] in directional_tools: + dimension = 'width' if tool.startswith('x') else 'height' + return (tool_type, dimension) elif tool == 'auto_box_zoom': return (tool_type, 'auto') return tool @@ -3265,63 +3264,63 @@ def get_tool_id(tool, tool_type): return (tool_type, tool.dimensions) elif hasattr(tool, 'description') and tool.description: return (tool_type, tool.description) + return tool_type + + def is_subcoordy_zoom(tool, tool_type): + """Check if tool is a subcoordinate_y zoom tool.""" + return (self.subcoordinate_y and isinstance(tool, _zoom_types) and + 'hv_created' in tool.tags and len(tool.tags) == 2) + + def process_tool(tool, skip_subcoordy_overlay_check=False): + """Process a single tool and return whether to add it.""" + if isinstance(tool, str): + tool_type = TOOL_TYPES.get(tool) else: - return tool_type + tool_type = type(tool) + + # Skip subcoordinate_y zoom tools already handled by subplots (overlay tools only) + if (skip_subcoordy_overlay_check and self.subcoordinate_y and + tool_type in _zoom_types and isinstance(tool, str) and + not tool.startswith('x') and tool.replace('_tool', '') in zooms_subcoordy): + return False + tool_id = get_tool_id(tool, tool_type) + + # Handle HoverTool deduplication by tooltips + if isinstance(tool, tools.HoverTool): + if isinstance(tool.tooltips, bokeh.models.dom.Div): + tooltips = tool.tooltips + else: + tooltips = tuple(tool.tooltips) if tool.tooltips else () + if tooltips in hover_tools: + return False + hover_tools[tooltips] = tool + # Handle subcoordinate_y zoom tools + elif is_subcoordy_zoom(tool, tool_type): + if tool.tags[1] in zooms_subcoordy: + return False + zooms_subcoordy[tool.tags[1]] = tool + self.handles['zooms_subcoordy'] = zooms_subcoordy + # Skip duplicate tools + elif tool_id in tool_ids: + return False + + tool_ids.add(tool_id) + return True + + # Collect tools from subplots for key, subplot in self.subplots.items(): el = element.get(key) if el is not None: el_tools = subplot._init_tools(el, self.callbacks) for tool in el_tools: - if isinstance(tool, str): - tool_type = TOOL_TYPES.get(tool) - else: - tool_type = type(tool) - - tool_id = get_tool_id(tool, tool_type) - if isinstance(tool, tools.HoverTool): - if isinstance(tool.tooltips, bokeh.models.dom.Div): - tooltips = tool.tooltips - else: - tooltips = tuple(tool.tooltips) if tool.tooltips else () - if tooltips in hover_tools: - continue - else: - hover_tools[tooltips] = tool - elif ( - self.subcoordinate_y and isinstance(tool, _zoom_types) - and 'hv_created' in tool.tags and len(tool.tags) == 2 - ): - if tool.tags[1] in zooms_subcoordy: - continue - else: - zooms_subcoordy[tool.tags[1]] = tool - self.handles['zooms_subcoordy'] = zooms_subcoordy - elif tool_id in tool_ids: - continue - - tool_ids.add(tool_id) - init_tools.append(tool) + if process_tool(tool): + init_tools.append(tool) # Add tools specified directly on the overlay overlay_tools = self.default_tools + self.tools for tool in overlay_tools: - if isinstance(tool, str): - tool_type = TOOL_TYPES.get(tool) - else: - tool_type = type(tool) - - tool_id = get_tool_id(tool, tool_type) - - # Skip subcoordinate_y zoom tools that were already handled - if (self.subcoordinate_y and tool_type in _zoom_types and - isinstance(tool, str) and not tool.startswith('x')): - tool_name = tool.replace('_tool', '') - if tool_name in zooms_subcoordy: - continue - - if tool_id not in tool_ids: - tool_ids.add(tool_id) + if process_tool(tool, skip_subcoordy_overlay_check=True): init_tools.append(tool) self.handles['hover_tools'] = hover_tools From 8dfb34384b7623cb9c44bf47361cb202a2e04eaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8xbro=20Hansen?= Date: Fri, 28 Nov 2025 12:42:49 +0100 Subject: [PATCH 05/15] small refactoring --- holoviews/plotting/bokeh/element.py | 47 +++------------ holoviews/plotting/bokeh/util.py | 22 +++++++ .../tests/plotting/bokeh/test_overlayplot.py | 57 +++++++++---------- 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py index d11a813400..eca3512c8b 100644 --- a/holoviews/plotting/bokeh/element.py +++ b/holoviews/plotting/bokeh/element.py @@ -88,6 +88,7 @@ get_scale, get_tab_title, get_ticker_axis_props, + get_tool_id, glyph_order, hold_policy, hold_render, @@ -3244,48 +3245,17 @@ def _init_tools(self, element, callbacks=None): callbacks = [] hover_tools = {} zooms_subcoordy = {} - _zoom_types = (tools.WheelZoomTool, tools.ZoomInTool, tools.ZoomOutTool) - init_tools = [] - tool_ids = set() - - def get_tool_id(tool, tool_type): - """Generate a unique identifier for a tool.""" - if isinstance(tool, str): - directional_tools = ('wheel_zoom', 'pan', 'zoom_in', 'zoom_out', 'box_zoom') - if tool in directional_tools: - return (tool_type, 'both') - elif tool.startswith(('x', 'y')) and tool[1:] in directional_tools: - dimension = 'width' if tool.startswith('x') else 'height' - return (tool_type, dimension) - elif tool == 'auto_box_zoom': - return (tool_type, 'auto') - return tool - elif hasattr(tool, 'dimensions') and tool.dimensions: - return (tool_type, tool.dimensions) - elif hasattr(tool, 'description') and tool.description: - return (tool_type, tool.description) - return tool_type - - def is_subcoordy_zoom(tool, tool_type): - """Check if tool is a subcoordinate_y zoom tool.""" - return (self.subcoordinate_y and isinstance(tool, _zoom_types) and - 'hv_created' in tool.tags and len(tool.tags) == 2) + zoom_types = (tools.WheelZoomTool, tools.ZoomInTool, tools.ZoomOutTool) + init_tools, tool_ids = [], set() def process_tool(tool, skip_subcoordy_overlay_check=False): - """Process a single tool and return whether to add it.""" - if isinstance(tool, str): - tool_type = TOOL_TYPES.get(tool) - else: - tool_type = type(tool) + tool_id = get_tool_id(tool) - # Skip subcoordinate_y zoom tools already handled by subplots (overlay tools only) if (skip_subcoordy_overlay_check and self.subcoordinate_y and - tool_type in _zoom_types and isinstance(tool, str) and + tool_id[0] in zoom_types and isinstance(tool, str) and not tool.startswith('x') and tool.replace('_tool', '') in zooms_subcoordy): return False - tool_id = get_tool_id(tool, tool_type) - # Handle HoverTool deduplication by tooltips if isinstance(tool, tools.HoverTool): if isinstance(tool.tooltips, bokeh.models.dom.Div): @@ -3295,13 +3265,14 @@ def process_tool(tool, skip_subcoordy_overlay_check=False): if tooltips in hover_tools: return False hover_tools[tooltips] = tool - # Handle subcoordinate_y zoom tools - elif is_subcoordy_zoom(tool, tool_type): + elif ( + self.subcoordinate_y and isinstance(tool, zoom_types) + and 'hv_created' in tool.tags and len(tool.tags) == 2 + ): if tool.tags[1] in zooms_subcoordy: return False zooms_subcoordy[tool.tags[1]] = tool self.handles['zooms_subcoordy'] = zooms_subcoordy - # Skip duplicate tools elif tool_id in tool_ids: return False diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index d01cc8d698..11056b0b11 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1262,3 +1262,25 @@ def get_ticker_axis_props(ticker): if labels is not None: axis_props['major_label_overrides'] = dict(zip(ticks, labels, strict=None)) return axis_props + + +def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | None]: + """Returns the tool type and an identifier for a given tool.""" + is_str = isinstance(tool, str) + tool_type = TOOL_TYPES.get(tool) if is_str else type(tool) + + if is_str: + directional_tools = ('wheel_zoom', 'pan', 'zoom_in', 'zoom_out', 'box_zoom') + if tool in directional_tools: + return tool_type, 'both' + elif tool.startswith(('x', 'y')) and tool[1:] in directional_tools: + dimension = 'width' if tool.startswith('x') else 'height' + return tool_type, dimension + elif tool == 'auto_box_zoom': + return tool_type, 'auto' + else: + # TODO(Azaya): More way to identify? Take a look at merge_tools + for name in ("dimensions", "description"): + if identifier := getattr(tool, name, None): + return tool_type, identifier + return tool_type, None diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index 68fb59ff7a..355c681ac0 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -1,3 +1,5 @@ +from collections import defaultdict + import numpy as np import pandas as pd import panel as pn @@ -422,56 +424,49 @@ def test_ndoverlay_categorical_y_ranges(order): expected = sorted(map(str, df.values.ravel())) assert output == expected -def test_overlay_opts_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in']) +@pytest.mark.parametrize(("tools", "new_tools"), + ( + [None, []], + [["zoom_in"], ["ZoomInTool"]], + [["zoom_in", "zoom_out"], ["ZoomInTool", "ZoomOutTool"]] + ), ids=["0", "1", "2"]) +def test_overlay_opts_tools(tools, new_tools): + overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) + if tools: + overlay.opts(tools=tools) plot = bokeh_renderer.get_plot(overlay) tool_types = [type(tool).__name__ for tool in plot.state.tools] - assert 'ZoomInTool' in tool_types - -def test_overlay_opts_tools_multiple(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in', 'zoom_out']) - plot = bokeh_renderer.get_plot(overlay) + defaults = ['WheelZoomTool', 'SaveTool', 'PanTool', 'BoxZoomTool', 'ResetTool'] + assert tool_types == [*defaults, *new_tools] - tool_types = [type(tool).__name__ for tool in plot.state.tools] - assert 'ZoomInTool' in tool_types - assert 'ZoomOutTool' in tool_types def test_overlay_opts_tools_with_element_tools(): - overlay = (Curve([1, 2, 3]).opts(tools=['zoom_out']) * Curve([2, 3, 4])).opts(tools=['zoom_in']) + overlay = Curve([1, 2, 3]).opts(tools=['zoom_out']) * Curve([2, 3, 4]).opts(tools=['zoom_in']) plot = bokeh_renderer.get_plot(overlay) tool_types = [type(tool).__name__ for tool in plot.state.tools] - assert 'ZoomInTool' in tool_types - assert 'ZoomOutTool' in tool_types - -def test_overlay_default_tools_preserved(): - overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) - plot = bokeh_renderer.get_plot(overlay) - - tool_types = [type(tool).__name__ for tool in plot.state.tools] - expected_defaults = ['PanTool', 'WheelZoomTool', 'SaveTool', 'BoxZoomTool', 'ResetTool'] - - for expected in expected_defaults: - assert expected in tool_types + defaults = ['WheelZoomTool', 'SaveTool', 'PanTool', 'BoxZoomTool', 'ResetTool'] + # INFO(Azaya): Can this really be right? + assert tool_types == [defaults[0], "ZoomOutTool", "ZoomInTool", *defaults[1:]] def test_overlay_default_tools_not_duplicated(): overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) plot = bokeh_renderer.get_plot(overlay) # Count each tool type - tool_type_counts = {} + tool_type_counts = defaultdict(int) for tool in plot.state.tools: - tool_type = type(tool).__name__ - tool_type_counts[tool_type] = tool_type_counts.get(tool_type, 0) + 1 + tool_type_counts[type(tool).__name__] += 1 # Each default tool should appear exactly once - assert tool_type_counts.get('PanTool', 0) == 1 - assert tool_type_counts.get('WheelZoomTool', 0) == 1 - assert tool_type_counts.get('SaveTool', 0) == 1 - assert tool_type_counts.get('BoxZoomTool', 0) == 1 - assert tool_type_counts.get('ResetTool', 0) == 1 + assert tool_type_counts['PanTool'] == 1 + assert tool_type_counts['WheelZoomTool'] == 1 + assert tool_type_counts['SaveTool'] == 1 + assert tool_type_counts['BoxZoomTool'] == 1 + assert tool_type_counts['ResetTool'] == 1 +#TODO(Azaya): Make these test more parameterize def test_overlay_opts_directional_pan_tools(): overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xpan', 'ypan']) plot = bokeh_renderer.get_plot(overlay) From 5d579a3fa9a988d01948129a313597eccd06700c Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Fri, 28 Nov 2025 14:41:59 +0100 Subject: [PATCH 06/15] parameterize the tests --- .../tests/plotting/bokeh/test_overlayplot.py | 109 ++++++++---------- 1 file changed, 45 insertions(+), 64 deletions(-) diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index 355c681ac0..104d06e635 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -447,7 +447,6 @@ def test_overlay_opts_tools_with_element_tools(): tool_types = [type(tool).__name__ for tool in plot.state.tools] defaults = ['WheelZoomTool', 'SaveTool', 'PanTool', 'BoxZoomTool', 'ResetTool'] - # INFO(Azaya): Can this really be right? assert tool_types == [defaults[0], "ZoomOutTool", "ZoomInTool", *defaults[1:]] def test_overlay_default_tools_not_duplicated(): @@ -466,83 +465,65 @@ def test_overlay_default_tools_not_duplicated(): assert tool_type_counts['BoxZoomTool'] == 1 assert tool_type_counts['ResetTool'] == 1 -#TODO(Azaya): Make these test more parameterize -def test_overlay_opts_directional_pan_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xpan', 'ypan']) - plot = bokeh_renderer.get_plot(overlay) - - pan_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'PanTool'] - # Should have 3 PanTools: default 'pan' (both) + xpan (width) + ypan (height) - assert len(pan_tools) == 3 - - dimensions = [tool.dimensions for tool in pan_tools] - assert 'both' in dimensions - assert 'width' in dimensions - assert 'height' in dimensions -def test_overlay_opts_generic_and_directional_pan_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['pan', 'xpan', 'ypan']) +@pytest.mark.parametrize( + ("tool_strings", "expected_dimensions", "expected_count"), + [ + (["xpan", "ypan"], ["both", "width", "height"], 3), + (["pan", "xpan", "ypan"], ["both", "width", "height"], 3), + ], + ids=["directional_only", "generic_and_directional"], +) +def test_overlay_opts_directional_pan_tools(tool_strings, expected_dimensions, expected_count): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) - # Default 'pan' tool should not be duplicated - pan_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'PanTool'] - assert len(pan_tools) == 3 - - dimensions = [tool.dimensions for tool in pan_tools] - assert 'both' in dimensions - assert 'width' in dimensions - assert 'height' in dimensions + tools = [tool for tool in plot.state.tools if type(tool).__name__ == "PanTool"] + assert len(tools) == expected_count -def test_overlay_opts_directional_zoom_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xzoom_in', 'yzoom_in']) - plot = bokeh_renderer.get_plot(overlay) - - zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'ZoomInTool'] - assert len(zoom_tools) == 2 + dimensions = [tool.dimensions for tool in tools] + for dimension in expected_dimensions: + assert dimension in dimensions - dimensions = [tool.dimensions for tool in zoom_tools] - assert 'width' in dimensions - assert 'height' in dimensions -def test_overlay_opts_generic_and_directional_zoom_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['zoom_in', 'xzoom_in', 'yzoom_in']) +@pytest.mark.parametrize( + ("tool_strings", "expected_dimensions", "expected_count"), + [ + (["xzoom_in", "yzoom_in"], ["width", "height"], 2), + (["zoom_in", "xzoom_in", "yzoom_in"], ["both", "width", "height"], 3), + ], + ids=["directional_only", "generic_and_directional"], +) +def test_overlay_opts_directional_zoom_in_tools(tool_strings, expected_dimensions, expected_count): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) - zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'ZoomInTool'] - assert len(zoom_tools) == 3 + tools = [tool for tool in plot.state.tools if type(tool).__name__ == "ZoomInTool"] + assert len(tools) == expected_count - dimensions = [tool.dimensions for tool in zoom_tools] - assert 'both' in dimensions - assert 'width' in dimensions - assert 'height' in dimensions + dimensions = [tool.dimensions for tool in tools] + for dimension in expected_dimensions: + assert dimension in dimensions -def test_overlay_opts_directional_box_zoom_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xbox_zoom', 'ybox_zoom']) - plot = bokeh_renderer.get_plot(overlay) - box_zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'BoxZoomTool'] - # Should have 3 BoxZoomTools: default 'auto_box_zoom' (auto) + xbox_zoom (width) + ybox_zoom (height) - assert len(box_zoom_tools) == 3 - - # Check that we have auto_box_zoom (auto), xbox_zoom (width), and ybox_zoom (height) - dimensions = [tool.dimensions for tool in box_zoom_tools] - assert 'auto' in dimensions - assert 'width' in dimensions - assert 'height' in dimensions - -def test_overlay_opts_generic_and_directional_box_zoom_tools(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['box_zoom', 'xbox_zoom', 'ybox_zoom']) +@pytest.mark.parametrize( + ("tool_strings", "expected_dimensions", "expected_count"), + [ + (["xbox_zoom", "ybox_zoom"], ["auto", "width", "height"], 3), + (["box_zoom", "xbox_zoom", "ybox_zoom"], ["auto", "both", "width", "height"], 4), + ], + ids=["directional_only", "generic_and_directional"], +) +def test_overlay_opts_directional_box_zoom_tools(tool_strings, expected_dimensions, expected_count): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) - box_zoom_tools = [tool for tool in plot.state.tools if type(tool).__name__ == 'BoxZoomTool'] - # Should have 4 BoxZoomTools: default 'auto_box_zoom' + 3 added - assert len(box_zoom_tools) == 4 + tools = [tool for tool in plot.state.tools if type(tool).__name__ == "BoxZoomTool"] + assert len(tools) == expected_count - dimensions = [tool.dimensions for tool in box_zoom_tools] - assert 'auto' in dimensions - assert 'both' in dimensions - assert 'width' in dimensions - assert 'height' in dimensions + dimensions = [tool.dimensions for tool in tools] + for dimension in expected_dimensions: + assert dimension in dimensions def test_overlay_opts_mixed_tools_no_duplicates(): overlay = ( From 93bd708c1de786acf2e4ea3c5250922ce5436d75 Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Fri, 28 Nov 2025 14:55:22 +0100 Subject: [PATCH 07/15] add more identifiers --- holoviews/plotting/bokeh/util.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index 11056b0b11..f2864c9caa 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1279,8 +1279,10 @@ def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | None]: elif tool == 'auto_box_zoom': return tool_type, 'auto' else: - # TODO(Azaya): More way to identify? Take a look at merge_tools - for name in ("dimensions", "description"): + for name in ("dimensions", "tags", "name", "description", "icon"): if identifier := getattr(tool, name, None): + # Convert lists/tuples to tuples (hashable) + if isinstance(identifier, list | tuple): + identifier = tuple(identifier) return tool_type, identifier return tool_type, None From 42761e609344a290f67b550e7dcc7289fed1b8ad Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Fri, 28 Nov 2025 16:48:48 +0100 Subject: [PATCH 08/15] no need to check for tuples --- holoviews/plotting/bokeh/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index f2864c9caa..a23c0eacbd 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1281,8 +1281,8 @@ def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | None]: else: for name in ("dimensions", "tags", "name", "description", "icon"): if identifier := getattr(tool, name, None): - # Convert lists/tuples to tuples (hashable) - if isinstance(identifier, list | tuple): + # Convert lists to tuples (hashable) + if isinstance(identifier, list): identifier = tuple(identifier) return tool_type, identifier return tool_type, None From a725c8d9d7989f5f3fe74a2fba5d96642273e8de Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Thu, 4 Dec 2025 10:27:09 +0100 Subject: [PATCH 09/15] deduplicate by multiple properties --- holoviews/plotting/bokeh/util.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index a23c0eacbd..a77a12c028 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1279,10 +1279,22 @@ def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | None]: elif tool == 'auto_box_zoom': return tool_type, 'auto' else: + identifiers = [] for name in ("dimensions", "tags", "name", "description", "icon"): if identifier := getattr(tool, name, None): + # Skip tags that are only for internal bookkeeping + if name == "tags" and identifier == ['hv_created']: + continue # Convert lists to tuples (hashable) if isinstance(identifier, list): identifier = tuple(identifier) - return tool_type, identifier + identifiers.append((name, identifier)) + + # Return composite identifier if multiple properties exist + if len(identifiers) == 0: + return tool_type, None + elif len(identifiers) == 1: + return tool_type, identifiers[0][1] + else: + return tool_type, tuple(identifiers) return tool_type, None From 9d9d7561aed03f8464fc0950b13c492f6f51071b Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Thu, 4 Dec 2025 10:27:27 +0100 Subject: [PATCH 10/15] add docstring --- holoviews/plotting/bokeh/util.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index a77a12c028..6a4b83c806 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1264,8 +1264,28 @@ def get_ticker_axis_props(ticker): return axis_props -def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | None]: - """Returns the tool type and an identifier for a given tool.""" +def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | tuple | None]: + """ + Returns the tool type and an identifier for a given tool. + + The identifier allows distinguishing tools of the same type but with + different properties. This function checks all disambiguation properties + (dimensions, tags, name, description, icon) and returns a composite + identifier if multiple properties are present. + + Parameters + ---------- + tool : str or Bokeh Tool class + Tool specification as string name or Tool class instance + + Returns + ------- + tuple[type[tools.Tool], str | tuple | None] + Tuple of (tool_type, identifier). The identifier can be: + - str: Single property value (e.g., 'both', 'width') + - tuple: Multiple property values as tuple of tuples + - None: No distinguishing properties + """ is_str = isinstance(tool, str) tool_type = TOOL_TYPES.get(tool) if is_str else type(tool) From 5fb2bb29d0783d4599b564b317cf3c2f496b2802 Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Wed, 10 Dec 2025 16:04:55 +0100 Subject: [PATCH 11/15] refactor get_tool_id --- holoviews/plotting/bokeh/util.py | 57 ++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index c6cbff5756..8ffc164b01 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1265,19 +1265,27 @@ def get_ticker_axis_props(ticker): return axis_props -def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | tuple | None]: +def get_tool_id( + tool: str | tools.Tool, + *, + properties: tuple[str, ...] = ("dimensions", "tags", "name", "description", "icon"), + skip_tags: set[str] | None = None +) -> tuple[type[tools.Tool], str | tuple | None]: """ Returns the tool type and an identifier for a given tool. The identifier allows distinguishing tools of the same type but with different properties. This function checks all disambiguation properties - (dimensions, tags, name, description, icon) and returns a composite - identifier if multiple properties are present. + and returns a composite identifier if multiple properties are present. Parameters ---------- tool : str or Bokeh Tool class Tool specification as string name or Tool class instance + properties : tuple of str, optional + Properties to check for disambiguation (default: dimensions, tags, name, description, icon) + skip_tags : set of str or None, optional + Tag values to ignore during identification (default: {'hv_created'}) Returns ------- @@ -1287,6 +1295,9 @@ def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | tuple | - tuple: Multiple property values as tuple of tuples - None: No distinguishing properties """ + if skip_tags is None: + skip_tags = {'hv_created'} + is_str = isinstance(tool, str) tool_type = TOOL_TYPES.get(tool) if is_str else type(tool) @@ -1299,23 +1310,25 @@ def get_tool_id(tool: str | tools.Tool) -> tuple[type[tools.Tool], str | tuple | return tool_type, dimension elif tool == 'auto_box_zoom': return tool_type, 'auto' + return tool_type, None + + identifiers = {} + for prop_name in properties: + value = getattr(tool, prop_name, None) + if not value: + continue + # Skip internal tags + if prop_name == "tags" and (value == list(skip_tags) or set(value) == skip_tags): + continue + # Convert lists to tuples for hashability + if isinstance(value, list): + value = tuple(value) + + identifiers[prop_name] = value + + if not identifiers: + return tool_type, None + elif len(identifiers) == 1: + return tool_type, next(iter(identifiers.values())) else: - identifiers = [] - for name in ("dimensions", "tags", "name", "description", "icon"): - if identifier := getattr(tool, name, None): - # Skip tags that are only for internal bookkeeping - if name == "tags" and identifier == ['hv_created']: - continue - # Convert lists to tuples (hashable) - if isinstance(identifier, list): - identifier = tuple(identifier) - identifiers.append((name, identifier)) - - # Return composite identifier if multiple properties exist - if len(identifiers) == 0: - return tool_type, None - elif len(identifiers) == 1: - return tool_type, identifiers[0][1] - else: - return tool_type, tuple(identifiers) - return tool_type, None + return tool_type, tuple(identifiers.items()) From 21d63288fdf3c4f43cb183f6e24d58618da7ef2f Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Mon, 15 Dec 2025 15:19:37 +0100 Subject: [PATCH 12/15] small test improvement --- holoviews/tests/plotting/bokeh/test_overlayplot.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index 9b8068eded..a1c2264320 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -425,11 +425,11 @@ def test_ndoverlay_categorical_y_ranges(order): assert output == expected @pytest.mark.parametrize(("tools", "new_tools"), - ( - [None, []], - [["zoom_in"], ["ZoomInTool"]], - [["zoom_in", "zoom_out"], ["ZoomInTool", "ZoomOutTool"]] - ), ids=["0", "1", "2"]) + [ + (None, []), + (["zoom_in"], ["ZoomInTool"]), + (["zoom_in", "zoom_out"], ["ZoomInTool", "ZoomOutTool"]) + ], ids=["none", "zoom_in", "zoom_in_and_zoom_out"]) def test_overlay_opts_tools(tools, new_tools): overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) if tools: From 1f796b6598df4622c21d323234f33ee8a2771517 Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Wed, 4 Mar 2026 12:43:15 +0000 Subject: [PATCH 13/15] refactor get_tool_id and init_tools --- holoviews/plotting/bokeh/element.py | 22 +++++--- holoviews/plotting/bokeh/util.py | 83 ++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 32 deletions(-) diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py index fa398c3ad8..8cbc4ea2f2 100644 --- a/holoviews/plotting/bokeh/element.py +++ b/holoviews/plotting/bokeh/element.py @@ -3091,6 +3091,8 @@ class OverlayPlot(GenericOverlayPlot, LegendPlot): multiple_legends = param.Boolean(default=False, doc=""" Whether to split the legend for subplots into multiple legends.""") + _zoom_tool_types = (tools.WheelZoomTool, tools.ZoomInTool, tools.ZoomOutTool) + _propagate_options = ['width', 'height', 'xaxis', 'yaxis', 'labelled', 'bgcolor', 'fontsize', 'invert_axes', 'show_frame', 'show_grid', 'logx', 'logy', 'xticks', 'toolbar', @@ -3251,14 +3253,13 @@ def _init_tools(self, element, callbacks=None): callbacks = [] hover_tools = {} zooms_subcoordy = {} - zoom_types = (tools.WheelZoomTool, tools.ZoomInTool, tools.ZoomOutTool) init_tools, tool_ids = [], set() - def process_tool(tool, skip_subcoordy_overlay_check=False): + def process_tool(tool, is_overlay_tool=False): tool_id = get_tool_id(tool) - if (skip_subcoordy_overlay_check and self.subcoordinate_y and - tool_id[0] in zoom_types and isinstance(tool, str) and + if (is_overlay_tool and self.subcoordinate_y and + tool_id[0] in self._zoom_tool_types and isinstance(tool, str) and not tool.startswith('x') and tool.replace('_tool', '') in zooms_subcoordy): return False @@ -3271,15 +3272,20 @@ def process_tool(tool, skip_subcoordy_overlay_check=False): if tooltips in hover_tools: return False hover_tools[tooltips] = tool - elif ( - self.subcoordinate_y and isinstance(tool, zoom_types) + return True + + if ( + self.subcoordinate_y and isinstance(tool, self._zoom_tool_types) and 'hv_created' in tool.tags and len(tool.tags) == 2 ): if tool.tags[1] in zooms_subcoordy: return False zooms_subcoordy[tool.tags[1]] = tool self.handles['zooms_subcoordy'] = zooms_subcoordy - elif tool_id in tool_ids: + tool_ids.add(tool_id) + return True + + if tool_id in tool_ids: return False tool_ids.add(tool_id) @@ -3297,7 +3303,7 @@ def process_tool(tool, skip_subcoordy_overlay_check=False): # Add tools specified directly on the overlay overlay_tools = self.default_tools + self.tools for tool in overlay_tools: - if process_tool(tool, skip_subcoordy_overlay_check=True): + if process_tool(tool, is_overlay_tool=True): init_tools.append(tool) self.handles['hover_tools'] = hover_tools diff --git a/holoviews/plotting/bokeh/util.py b/holoviews/plotting/bokeh/util.py index 8ffc164b01..a730997137 100644 --- a/holoviews/plotting/bokeh/util.py +++ b/holoviews/plotting/bokeh/util.py @@ -1265,11 +1265,52 @@ def get_ticker_axis_props(ticker): return axis_props +_DIRECTIONAL_TOOL_BASES = { + 'wheel_zoom': 'both', + 'pan': 'both', + 'zoom_in': 'both', + 'zoom_out': 'both', + 'box_zoom': 'both', + 'wheel_pan': 'both', + 'box_select': 'both', + 'crosshair': 'both', +} + +_TOOL_ALIAS_IDS = { + 'tap': 'tap', + 'click': 'inspect', + 'doubletap': 'doubletap', + 'auto_box_zoom': 'auto', +} + + +def _get_tool_id_from_str( + tool: str, + tool_type: type[tools.Tool], +) -> tuple[type[tools.Tool], str | None]: + """Return (tool_type, identifier) for a string tool name.""" + if tool in _DIRECTIONAL_TOOL_BASES: + return tool_type, _DIRECTIONAL_TOOL_BASES[tool] + if tool.startswith(('x', 'y')) and tool[1:] in _DIRECTIONAL_TOOL_BASES: + dimension = 'width' if tool.startswith('x') else 'height' + return tool_type, dimension + if tool in _TOOL_ALIAS_IDS: + return tool_type, _TOOL_ALIAS_IDS[tool] + return tool_type, None + + def get_tool_id( tool: str | tools.Tool, *, - properties: tuple[str, ...] = ("dimensions", "tags", "name", "description", "icon"), - skip_tags: set[str] | None = None + properties: tuple[str, ...] = ( + 'dimensions', + 'dimension', + 'tags', + 'name', + 'description', + 'icon', + ), + skip_tags: set[str] | None = None, ) -> tuple[type[tools.Tool], str | tuple | None]: """ Returns the tool type and an identifier for a given tool. @@ -1283,7 +1324,8 @@ def get_tool_id( tool : str or Bokeh Tool class Tool specification as string name or Tool class instance properties : tuple of str, optional - Properties to check for disambiguation (default: dimensions, tags, name, description, icon) + Properties to check for disambiguation (default: dimensions, dimension, + tags, name, description, icon) skip_tags : set of str or None, optional Tag values to ignore during identification (default: {'hv_created'}) @@ -1298,37 +1340,30 @@ def get_tool_id( if skip_tags is None: skip_tags = {'hv_created'} - is_str = isinstance(tool, str) - tool_type = TOOL_TYPES.get(tool) if is_str else type(tool) - - if is_str: - directional_tools = ('wheel_zoom', 'pan', 'zoom_in', 'zoom_out', 'box_zoom') - if tool in directional_tools: - return tool_type, 'both' - elif tool.startswith(('x', 'y')) and tool[1:] in directional_tools: - dimension = 'width' if tool.startswith('x') else 'height' - return tool_type, dimension - elif tool == 'auto_box_zoom': - return tool_type, 'auto' - return tool_type, None + if isinstance(tool, str): + tool_type = TOOL_TYPES.get(tool) + return _get_tool_id_from_str(tool, tool_type) + tool_type = type(tool) identifiers = {} for prop_name in properties: value = getattr(tool, prop_name, None) - if not value: - continue - # Skip internal tags - if prop_name == "tags" and (value == list(skip_tags) or set(value) == skip_tags): + if value is None: continue + # Filter out internal tags; skip if nothing user-visible remains + if prop_name == 'tags': + visible = [t for t in value if t not in skip_tags] + if not visible: + continue + value = tuple(visible) # Convert lists to tuples for hashability - if isinstance(value, list): + elif isinstance(value, list): value = tuple(value) identifiers[prop_name] = value if not identifiers: return tool_type, None - elif len(identifiers) == 1: + if len(identifiers) == 1: return tool_type, next(iter(identifiers.values())) - else: - return tool_type, tuple(identifiers.items()) + return tool_type, tuple(identifiers.items()) From 436bfaff6e4d82207020a7e521746a99e3b0a42a Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Wed, 4 Mar 2026 12:44:28 +0000 Subject: [PATCH 14/15] add more tests --- .../tests/plotting/bokeh/test_overlayplot.py | 39 ++++++- holoviews/tests/plotting/bokeh/test_utils.py | 100 ++++++++++++++++-- 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index a1c2264320..2f8b4fc237 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -4,7 +4,14 @@ import pandas as pd import panel as pn import pytest -from bokeh.models import FactorRange, FixedTicker, HoverTool, Range1d, Span +from bokeh.models import ( + FactorRange, + FixedTicker, + HoverTool, + Range1d, + Span, + tools as bk_tools, +) from holoviews.core import DynamicMap, HoloMap, NdOverlay, Overlay from holoviews.element import ( @@ -432,7 +439,7 @@ def test_ndoverlay_categorical_y_ranges(order): ], ids=["none", "zoom_in", "zoom_in_and_zoom_out"]) def test_overlay_opts_tools(tools, new_tools): overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) - if tools: + if tools is not None: overlay.opts(tools=tools) plot = bokeh_renderer.get_plot(overlay) @@ -538,6 +545,34 @@ def test_overlay_opts_mixed_tools_no_duplicates(): assert len(xpan_tools) == 1 +def test_overlay_opts_xwheel_pan_ywheel_pan_distinct(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xwheel_pan', 'ywheel_pan']) + plot = bokeh_renderer.get_plot(overlay) + + wheel_pan_tools = [t for t in plot.state.tools if isinstance(t, bk_tools.WheelPanTool)] + assert len(wheel_pan_tools) == 2 + dimensions = {t.dimension for t in wheel_pan_tools} + assert dimensions == {'width', 'height'} + + +def test_overlay_opts_tap_doubletap_distinct(): + overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['tap', 'doubletap']) + plot = bokeh_renderer.get_plot(overlay) + + tap_tools = [t for t in plot.state.tools if isinstance(t, bk_tools.TapTool)] + assert len(tap_tools) == 2 + + +def test_overlay_hover_string_not_blocked_by_subplot_hover(): + curve1 = Curve([1, 2, 3]).opts(tools=[HoverTool(tooltips=[('x', '@x')])]) + curve2 = Curve([2, 3, 4]).opts(tools=[HoverTool(tooltips=[('y', '@y')])]) + overlay = curve1 * curve2 + plot = bokeh_renderer.get_plot(overlay) + + hover_tools = [t for t in plot.state.tools if isinstance(t, bk_tools.HoverTool)] + assert len(hover_tools) == 2 + + class TestLegends(TestBokehPlot): def test_overlay_legend(self): diff --git a/holoviews/tests/plotting/bokeh/test_utils.py b/holoviews/tests/plotting/bokeh/test_utils.py index 46fdc01f43..d6da816bb5 100644 --- a/holoviews/tests/plotting/bokeh/test_utils.py +++ b/holoviews/tests/plotting/bokeh/test_utils.py @@ -1,5 +1,5 @@ import pytest -from bokeh.models import Tool +from bokeh.models import Tool, tools as bk_tools import holoviews as hv from holoviews.core import Store @@ -7,6 +7,7 @@ from holoviews.plotting.bokeh.util import ( TOOL_TYPES, filter_batched_data, + get_tool_id, glyph_order, select_legends, ) @@ -110,9 +111,94 @@ def test_select_legends_figure_index(figure_index, expected): def test_bokeh_tools_types(): - bk_tools = Tool._known_aliases - assert len(bk_tools) == len(TOOL_TYPES) - assert sorted(bk_tools) == sorted(TOOL_TYPES) - - for key in bk_tools: - assert isinstance(bk_tools[key](), TOOL_TYPES[key]) + bk_tools_aliases = Tool._known_aliases + assert len(bk_tools_aliases) == len(TOOL_TYPES) + assert sorted(bk_tools_aliases) == sorted(TOOL_TYPES) + + for key in bk_tools_aliases: + assert isinstance(bk_tools_aliases[key](), TOOL_TYPES[key]) + +class TestGetToolId: + + def test_string_wheel_zoom(self): + tool_type, ident = get_tool_id('wheel_zoom') + assert tool_type is bk_tools.WheelZoomTool + assert ident == 'both' + + def test_string_xwheel_zoom(self): + tool_type, ident = get_tool_id('xwheel_zoom') + assert tool_type is bk_tools.WheelZoomTool + assert ident == 'width' + + def test_string_ywheel_zoom(self): + tool_type, ident = get_tool_id('ywheel_zoom') + assert tool_type is bk_tools.WheelZoomTool + assert ident == 'height' + + def test_string_xwheel_pan(self): + tool_type, ident = get_tool_id('xwheel_pan') + assert tool_type is bk_tools.WheelPanTool + assert ident == 'width' + + def test_string_ywheel_pan(self): + tool_type, ident = get_tool_id('ywheel_pan') + assert tool_type is bk_tools.WheelPanTool + assert ident == 'height' + + def test_string_tap(self): + tool_type, ident = get_tool_id('tap') + assert tool_type is bk_tools.TapTool + assert ident == 'tap' + + def test_string_click(self): + tool_type, ident = get_tool_id('click') + assert tool_type is bk_tools.TapTool + assert ident == 'inspect' + + def test_string_doubletap(self): + tool_type, ident = get_tool_id('doubletap') + assert tool_type is bk_tools.TapTool + assert ident == 'doubletap' + + def test_instance_wheel_zoom_default(self): + tool = bk_tools.WheelZoomTool() + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.WheelZoomTool + assert ident == 'both' + + def test_instance_wheel_zoom_width(self): + tool = bk_tools.WheelZoomTool(dimensions='width') + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.WheelZoomTool + assert ident == 'width' + + def test_instance_wheel_pan_width(self): + tool = bk_tools.WheelPanTool(dimension='width') + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.WheelPanTool + assert ident == 'width' + + def test_instance_save_tool_no_tags(self): + tool = bk_tools.SaveTool(tags=['hv_created']) + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.SaveTool + assert ident is None + + def test_instance_save_tool_user_tag(self): + tool = bk_tools.SaveTool(tags=['my_tag']) + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.SaveTool + assert ident == ('my_tag',) + + def test_instance_save_tool_hv_and_user_tag(self): + tool = bk_tools.SaveTool(tags=['hv_created', 'my_tag']) + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.SaveTool + assert ident == ('my_tag',) + + def test_instance_wheel_zoom_hv_created_tag(self): + tool = bk_tools.WheelZoomTool(tags=['hv_created']) + tool_type, ident = get_tool_id(tool) + assert tool_type is bk_tools.WheelZoomTool + # dimensions takes precedence over tags + assert ident == 'both' From b2f65ad7ed0479e1887eceb652e1297ea96656b8 Mon Sep 17 00:00:00 2001 From: Isaiah Akorita Date: Thu, 5 Mar 2026 11:43:34 +0000 Subject: [PATCH 15/15] use hv. imports --- .../tests/plotting/bokeh/test_overlayplot.py | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/holoviews/tests/plotting/bokeh/test_overlayplot.py b/holoviews/tests/plotting/bokeh/test_overlayplot.py index 66ca1e37a2..c9f6090a7b 100644 --- a/holoviews/tests/plotting/bokeh/test_overlayplot.py +++ b/holoviews/tests/plotting/bokeh/test_overlayplot.py @@ -84,7 +84,7 @@ def test_hover_tool_instance_renderer_association(self): assert plot.handles['hover'].tooltips == tooltips # def test_hover_tool_overlay_renderers(self): - # overlay = Curve(range(2)).opts(tools=['hover']) * ErrorBars([]).opts(tools=['hover']) + # overlay = hv.Curve(range(2)).opts(tools=['hover']) * ErrorBars([]).opts(tools=['hover']) # plot = bokeh_renderer.get_plot(overlay) # assert len(plot.handles['hover'].renderers) == 1 # assert plot.handles['hover'].tooltips == [('x', '@{x}'), ('y', '@{y}')] @@ -427,7 +427,7 @@ def test_ndoverlay_categorical_y_ranges(order): (["zoom_in", "zoom_out"], ["ZoomInTool", "ZoomOutTool"]) ], ids=["none", "zoom_in", "zoom_in_and_zoom_out"]) def test_overlay_opts_tools(tools, new_tools): - overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) + overlay = hv.Curve([1, 2, 3]) *hv.Curve([2, 3, 4]) if tools is not None: overlay.opts(tools=tools) plot = bokeh_renderer.get_plot(overlay) @@ -438,7 +438,7 @@ def test_overlay_opts_tools(tools, new_tools): def test_overlay_opts_tools_with_element_tools(): - overlay = Curve([1, 2, 3]).opts(tools=['zoom_out']) * Curve([2, 3, 4]).opts(tools=['zoom_in']) + overlay = hv.Curve([1, 2, 3]).opts(tools=['zoom_out']) *hv.Curve([2, 3, 4]).opts(tools=['zoom_in']) plot = bokeh_renderer.get_plot(overlay) tool_types = [type(tool).__name__ for tool in plot.state.tools] @@ -446,7 +446,7 @@ def test_overlay_opts_tools_with_element_tools(): assert tool_types == [defaults[0], "ZoomOutTool", "ZoomInTool", *defaults[1:]] def test_overlay_default_tools_not_duplicated(): - overlay = Curve([1, 2, 3]) * Curve([2, 3, 4]) + overlay = hv.Curve([1, 2, 3]) *hv.Curve([2, 3, 4]) plot = bokeh_renderer.get_plot(overlay) # Count each tool type @@ -471,7 +471,7 @@ def test_overlay_default_tools_not_duplicated(): ids=["directional_only", "generic_and_directional"], ) def test_overlay_opts_directional_pan_tools(tool_strings, expected_dimensions, expected_count): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) + overlay = (hv.Curve([1, 2, 3]) * hv.Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) tools = [tool for tool in plot.state.tools if type(tool).__name__ == "PanTool"] @@ -491,7 +491,7 @@ def test_overlay_opts_directional_pan_tools(tool_strings, expected_dimensions, e ids=["directional_only", "generic_and_directional"], ) def test_overlay_opts_directional_zoom_in_tools(tool_strings, expected_dimensions, expected_count): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) + overlay = (hv.Curve([1, 2, 3]) * hv.Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) tools = [tool for tool in plot.state.tools if type(tool).__name__ == "ZoomInTool"] @@ -511,7 +511,7 @@ def test_overlay_opts_directional_zoom_in_tools(tool_strings, expected_dimension ids=["directional_only", "generic_and_directional"], ) def test_overlay_opts_directional_box_zoom_tools(tool_strings, expected_dimensions, expected_count): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=tool_strings) + overlay = (hv.Curve([1, 2, 3]) * hv.Curve([2, 3, 4])).opts(tools=tool_strings) plot = bokeh_renderer.get_plot(overlay) tools = [tool for tool in plot.state.tools if type(tool).__name__ == "BoxZoomTool"] @@ -523,8 +523,8 @@ def test_overlay_opts_directional_box_zoom_tools(tool_strings, expected_dimensio def test_overlay_opts_mixed_tools_no_duplicates(): overlay = ( - Curve([1, 2, 3]).opts(tools=['xpan']) * - Curve([2, 3, 4]).opts(tools=['xpan']) + hv.Curve([1, 2, 3]).opts(tools=['xpan']) * + hv.Curve([2, 3, 4]).opts(tools=['xpan']) ).opts(tools=['xpan']) plot = bokeh_renderer.get_plot(overlay) @@ -535,7 +535,7 @@ def test_overlay_opts_mixed_tools_no_duplicates(): def test_overlay_opts_xwheel_pan_ywheel_pan_distinct(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['xwheel_pan', 'ywheel_pan']) + overlay = (hv.Curve([1, 2, 3]) * hv.Curve([2, 3, 4])).opts(tools=['xwheel_pan', 'ywheel_pan']) plot = bokeh_renderer.get_plot(overlay) wheel_pan_tools = [t for t in plot.state.tools if isinstance(t, bk_tools.WheelPanTool)] @@ -545,7 +545,7 @@ def test_overlay_opts_xwheel_pan_ywheel_pan_distinct(): def test_overlay_opts_tap_doubletap_distinct(): - overlay = (Curve([1, 2, 3]) * Curve([2, 3, 4])).opts(tools=['tap', 'doubletap']) + overlay = (hv.Curve([1, 2, 3]) * hv.Curve([2, 3, 4])).opts(tools=['tap', 'doubletap']) plot = bokeh_renderer.get_plot(overlay) tap_tools = [t for t in plot.state.tools if isinstance(t, bk_tools.TapTool)] @@ -553,8 +553,8 @@ def test_overlay_opts_tap_doubletap_distinct(): def test_overlay_hover_string_not_blocked_by_subplot_hover(): - curve1 = Curve([1, 2, 3]).opts(tools=[HoverTool(tooltips=[('x', '@x')])]) - curve2 = Curve([2, 3, 4]).opts(tools=[HoverTool(tooltips=[('y', '@y')])]) + curve1 = hv.Curve([1, 2, 3]).opts(tools=[HoverTool(tooltips=[('x', '@x')])]) + curve2 = hv.Curve([2, 3, 4]).opts(tools=[HoverTool(tooltips=[('y', '@y')])]) overlay = curve1 * curve2 plot = bokeh_renderer.get_plot(overlay)