Repository: holoviz/holoviews
Pull request #6389 (2024-09-26 14:47:46)

Ensure proper cleanup of stream subscribers
Stream subscribers were not being properly cleaned up correctly, because we were only cleaning up linked streams. This would result in keeping references to old plots being kept around preventing garbage collection and also mean that if you reused a Stream you would continue to pointlessly update discarded plots.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/holoviz/holoviews/pull/6389?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 88.49%. Comparing base [(`017361b`)](https://app.codecov.io/gh/holoviz/holoviews/commit/017361b88cf186ac67ff89df4f60bd37daa36f0b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) to head [(`de88ccd`)](https://app.codecov.io/gh/holoviz/holoviews/commit/de88ccde3ea9021a1f34d29148ced903f8f686c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #6389   +/-   ##
=======================================
  Coverage   88.48%   88.49%           
=======================================
  Files         323      323           
  Lines       68459    68479   +20     
=======================================
+ Hits        60579    60599   +20     
  Misses       7880     7880           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/holoviz/holoviews/pull/6389?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz).


Patch:
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index bef067ee9..261f73fe2 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -12,7 +12,7 @@ on:
 
 defaults:
   run:
-    shell: bash -l {0}
+    shell: bash -e {0}
 
 env:
   PYTHON_VERSION: "3.11"
@@ -59,6 +59,9 @@ jobs:
     runs-on: ubuntu-latest
     needs: [conda_build, waiting_room]
     if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')
+    defaults:
+      run:
+        shell: bash -el {0}
     steps:
       - uses: actions/download-artifact@v4
         with:
diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml
index 5dedd6b75..858db8fd3 100644
--- a/.github/workflows/docs.yaml
+++ b/.github/workflows/docs.yaml
@@ -22,7 +22,7 @@ on:
 
 defaults:
   run:
-    shell: bash -l {0}
+    shell: bash -e {0}
 
 jobs:
   pixi_lock:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 6a3131a02..37fa992a5 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,11 +1,8 @@
-# This is the configuration for pre-commit, a local framework for managing pre-commit hooks
-#   Check out the docs at: https://pre-commit.com/
-
 exclude: (\.min\.js$|\.svg$|\.html$)
-default_stages: [commit]
+default_stages: [pre-commit]
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: end-of-file-fixer
       - id: trailing-whitespace
@@ -20,7 +17,7 @@ repos:
       - id: check-json
       - id: detect-private-key
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.6.7
+    rev: v0.6.9
     hooks:
       - id: ruff
         files: holoviews/|scripts/
diff --git a/doc/releases.md b/doc/releases.md
index 8c59ab64f..7354f92d3 100644
--- a/doc/releases.md
+++ b/doc/releases.md
@@ -256,7 +256,7 @@ Enhancements:
 - Drop requirements for `OrderedDict` ([#5867](https://github.com/holoviz/holoviews/pull/5867), [#5890](https://github.com/holoviz/holoviews/pull/5890), [#5925](https://github.com/holoviz/holoviews/pull/5925))
 - Allow to link to an `Overlay` ([#5881](https://github.com/holoviz/holoviews/pull/5881))
 - Use contourpy for contour calculations ([#5910](https://github.com/holoviz/holoviews/pull/5910))
-- Use browser information to set `pixel_density` in `ResampleOperation2D` ([#5947](https://github.com/holoviz/holoviews/pull/5947))
+- Use browser information to set `pixel_ratio` in `ResampleOperation2D` ([#5947](https://github.com/holoviz/holoviews/pull/5947))
 - Avoid bounce back of events for `Range{X,Y,XY}` streams ([#5946](https://github.com/holoviz/holoviews/pull/5946))
 
 Bug fixes:
diff --git a/holoviews/core/data/dask.py b/holoviews/core/data/dask.py
index 9c0476b06..4d0034381 100644
--- a/holoviews/core/data/dask.py
+++ b/holoviews/core/data/dask.py
@@ -218,7 +218,7 @@ class DaskInterface(PandasInterface):
         for coord in indices:
             if any(isinstance(c, float) and np.isnan(c) for c in coord):
                 continue
-            if len(coord) == 1:
+            if len(coord) == 1 and not util.PANDAS_GE_220:
                 coord = coord[0]
             group = group_type(groupby.get_group(coord), **group_kwargs)
             data.append((coord, group))
diff --git a/holoviews/core/util.py b/holoviews/core/util.py
index 590b7ab6e..cd25fdd6a 100644
--- a/holoviews/core/util.py
+++ b/holoviews/core/util.py
@@ -44,6 +44,7 @@ pandas_version = Version(pd.__version__)
 
 NUMPY_GE_200 = numpy_version >= Version("2")
 PANDAS_GE_210 = pandas_version >= Version("2.1")
+PANDAS_GE_220 = pandas_version >= Version("2.2")
 
 # Types
 generator_types = (zip, range, types.GeneratorType)
diff --git a/holoviews/plotting/bokeh/element.py b/holoviews/plotting/bokeh/element.py
index 89e31d2d9..ea243584c 100644
--- a/holoviews/plotting/bokeh/element.py
+++ b/holoviews/plotting/bokeh/element.py
@@ -21,6 +21,7 @@ from bokeh.models import (
     GlyphRenderer,
     Legend,
     Renderer,
+    Span,
     Title,
     tools,
 )
@@ -577,10 +578,11 @@ class ElementPlot(BokehPlot, GenericElementPlot):
             tool_list.append(tool)
 
         copied_tools = []
+        skip_models = (Span,)
         for tool in tool_list:
             if isinstance(tool, tools.Tool):
                 properties = {
-                    p: v.clone() if isinstance(v, Model) else v
+                    p: v.clone() if isinstance(v, Model) and not isinstance(v, skip_models) else v
                     for p, v in tool.properties_with_values(include_defaults=False).items()
                 }
                 tool = type(tool)(**properties)
diff --git a/holoviews/plotting/bokeh/plot.py b/holoviews/plotting/bokeh/plot.py
index 91a72967b..109b7067e 100644
--- a/holoviews/plotting/bokeh/plot.py
+++ b/holoviews/plotting/bokeh/plot.py
@@ -249,10 +249,12 @@ class BokehPlot(DimensionedPlot, CallbackPlot):
             if plot.subplots:
                 plot.subplots.clear()
 
-            if isinstance(plot, GenericElementPlot):
-                for callback in plot.callbacks:
-                    streams += callback.streams
-                    callback.cleanup()
+            if not isinstance(plot, (GenericElementPlot, GenericOverlayPlot)):
+                continue
+
+            for callback in plot.callbacks:
+                streams += callback.streams
+                callback.cleanup()
 
             for stream in set(streams):
                 stream._subscribers = [
@@ -261,7 +263,6 @@ class BokehPlot(DimensionedPlot, CallbackPlot):
                     get_method_owner(subscriber) not in plots
                 ]
 
-
     def _fontsize(self, key, label='fontsize', common=True):
         """
         Converts integer fontsizes to a string specifying
diff --git a/holoviews/plotting/plot.py b/holoviews/plotting/plot.py
index eb963afea..444dd6812 100644
--- a/holoviews/plotting/plot.py
+++ b/holoviews/plotting/plot.py
@@ -195,7 +195,7 @@ class Plot(param.Parameterized):
         """
         plots = self.traverse(lambda x: x, [Plot])
         for plot in plots:
-            if not isinstance(plot, (GenericCompositePlot, GenericElementPlot, GenericOverlayPlot)):
+            if not isinstance(plot, (GenericElementPlot, GenericOverlayPlot)):
                 continue
             for stream in set(plot.streams):
                 stream._subscribers = [
diff --git a/pixi.toml b/pixi.toml
index 13dc11a73..c10edd0c5 100644
--- a/pixi.toml
+++ b/pixi.toml
@@ -1,6 +1,6 @@
 [project]
 name = "holoviews"
-channels = ["pyviz/label/dev", "bokeh", "conda-forge"]
+channels = ["pyviz/label/dev", "bokeh/label/rc", "conda-forge"]
 platforms = ["linux-64", "osx-arm64", "osx-64", "win-64"]
 
 [tasks]
@@ -19,7 +19,7 @@ test-311 = ["py311", "test-core", "test", "example", "test-example", "test-unit-
 test-312 = ["py312", "test-core", "test", "example", "test-example", "test-unit-task"]
 test-ui = ["py312", "test-core", "test", "test-ui"]
 test-core = ["py313", "test-core", "test-unit-task"]
-test-gpu = ["py311", "test-core", "test", "test-gpu"]
+test-gpu = ["py312", "test-core", "test", "test-gpu"]
 docs = ["py311", "example", "doc"]
 build = ["py311", "build"]
 lint = ["py311", "lint"]
@@ -53,9 +53,6 @@ bokeh_sampledata = "*"
 python = "3.12.*"
 bokeh_sampledata = "*"
 
-[feature.py313]
-channels = ["pyviz/label/dev", "conda-forge/label/python_rc", "conda-forge"]
-
 [feature.py313.dependencies]
 python = "3.13.*"
 bokeh_sampledata = "*"
@@ -124,7 +121,7 @@ test-example = { cmd = 'pytest -n logical --dist loadscope --nbval-lax examples'
 nbval = "*"
 
 [feature.test-ui]
-channels = ["pyviz/label/dev", "microsoft", "conda-forge"]
+channels = ["microsoft"]
 
 [feature.test-ui.dependencies]
 playwright = { version = "*", channel = "microsoft" }
@@ -138,7 +135,7 @@ cmd = 'pytest holoviews/tests/ui --ui --browser chromium'
 depends_on = ["_install-ui"]
 
 [feature.test-gpu]
-channels = ["pyviz/label/dev", "rapidsai", "conda-forge"]
+channels = ["rapidsai"]
 platforms = ["linux-64"]
 
 [feature.test-gpu.activation.env]
@@ -146,7 +143,7 @@ NUMBA_CUDA_LOW_OCCUPANCY_WARNINGS = "0"
 
 [feature.test-gpu.dependencies]
 cuda-version = "12.2.*"
-cudf = "24.08.*"
+cudf = "24.10.*"
 cupy = "*"
 cuspatial = "*"
 librmm = { version = "*", channel = "rapidsai" }
diff --git a/pyproject.toml b/pyproject.toml
index e0bd7eb04..914351eaa 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -110,7 +110,6 @@ filterwarnings = [
     "ignore:datetime.datetime.utcfromtimestamp():DeprecationWarning:bokeh", # https://github.com/bokeh/bokeh/issues/13125
     "ignore:datetime.datetime.utcnow():DeprecationWarning:bokeh", # https://github.com/bokeh/bokeh/issues/13125
     # 2024-01: Pandas 2.2
-    "ignore:When grouping with a length-1 list::dask.dataframe.groupby", # https://github.com/dask/dask/issues/10572
     "ignore:\\s*Pyarrow will become a required dependency of pandas:DeprecationWarning", # Will go away by itself in Pandas 3.0
     "ignore:Passing a (SingleBlockManager|BlockManager) to (Series|GeoSeries|DataFrame|GeoDataFrame) is deprecated:DeprecationWarning", # https://github.com/holoviz/spatialpandas/issues/137
     # 2024-04


Test patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 6f2db511b..9faaa2ac4 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -30,7 +30,7 @@ concurrency:
 
 defaults:
   run:
-    shell: bash -l {0}
+    shell: bash -e {0}
 
 env:
   DISPLAY: ":99.0"
@@ -115,29 +115,25 @@ jobs:
     name: unit:${{ matrix.environment }}:${{ matrix.os }}
     needs: [pre_commit, setup, pixi_lock]
     runs-on: ${{ matrix.os }}
+    if: needs.setup.outputs.code_change == 'true'
     strategy:
       fail-fast: false
       matrix: ${{ fromJson(needs.setup.outputs.matrix) }}
     timeout-minutes: 120
     steps:
       - uses: holoviz-dev/holoviz_tasks/pixi_install@v0
-        if: needs.setup.outputs.code_change == 'true'
         with:
           environments: ${{ matrix.environment }}
       - name: Check packages latest version
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           pixi run -e ${{ matrix.environment }} check-latest-packages bokeh panel param datashader
       - name: Test Unit
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           pixi run -e ${{ matrix.environment }} test-unit $COV
       - name: Test Examples
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           pixi run -e ${{ matrix.environment }} test-example
       - uses: codecov/codecov-action@v4
-        if: needs.setup.outputs.code_change == 'true'
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
 
@@ -145,6 +141,7 @@ jobs:
     name: ui:${{ matrix.environment }}:${{ matrix.os }}
     needs: [pre_commit, setup, pixi_lock]
     runs-on: ${{ matrix.os }}
+    if: needs.setup.outputs.code_change == 'true'
     strategy:
       fail-fast: false
       matrix:
@@ -155,11 +152,9 @@ jobs:
       PANEL_LOG_LEVEL: info
     steps:
       - uses: holoviz-dev/holoviz_tasks/pixi_install@v0
-        if: needs.setup.outputs.code_change == 'true'
         with:
           environments: ${{ matrix.environment }}
       - name: Test UI
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           # Create a .uicoveragerc file to set the concurrency library to greenlet
           # https://github.com/microsoft/playwright-python/issues/313
@@ -173,7 +168,6 @@ jobs:
           path: ./ui_screenshots
           if-no-files-found: ignore
       - uses: codecov/codecov-action@v4
-        if: needs.setup.outputs.code_change == 'true'
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
 
@@ -181,6 +175,7 @@ jobs:
     name: core:${{ matrix.environment }}:${{ matrix.os }}
     needs: [pre_commit, setup, pixi_lock]
     runs-on: ${{ matrix.os }}
+    if: needs.setup.outputs.code_change == 'true'
     strategy:
       fail-fast: false
       matrix:
@@ -189,14 +184,21 @@ jobs:
     timeout-minutes: 120
     steps:
       - uses: holoviz-dev/holoviz_tasks/pixi_install@v0
-        if: needs.setup.outputs.code_change == 'true'
         with:
           environments: ${{ matrix.environment }}
       - name: Check packages latest version
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           pixi run -e ${{ matrix.environment }} check-latest-packages numpy pandas bokeh panel param
       - name: Test Unit
-        if: needs.setup.outputs.code_change == 'true'
         run: |
           pixi run -e ${{ matrix.environment }} test-unit
+
+  result_test_suite:
+    name: result:test
+    needs: [unit_test_suite, ui_test_suite, core_test_suite]
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+      - name: check for failures
+        if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
+        run: echo job failed && exit 1
diff --git a/holoviews/tests/plotting/bokeh/test_plot.py b/holoviews/tests/plotting/bokeh/test_plot.py
index b5c1a5978..03c9761f2 100644
--- a/holoviews/tests/plotting/bokeh/test_plot.py
+++ b/holoviews/tests/plotting/bokeh/test_plot.py
@@ -2,19 +2,23 @@ import numpy as np
 import pyviz_comms as comms
 from bokeh.models import (
     ColumnDataSource,
+    CrosshairTool,
     CustomJS,
     HoverTool,
     LinearColorMapper,
     LogColorMapper,
+    Span,
 )
 from param import concrete_descendents
 
 from holoviews import Curve
 from holoviews.core.element import Element
 from holoviews.core.options import Store
+from holoviews.core.spaces import DynamicMap
 from holoviews.element.comparison import ComparisonTestCase
 from holoviews.plotting.bokeh.callbacks import Callback
 from holoviews.plotting.bokeh.element import ElementPlot
+from holoviews.streams import Pipe
 
 bokeh_renderer = Store.renderers['bokeh']
 
@@ -80,11 +84,60 @@ class TestBokehPlot(ComparisonTestCase):
                     self.assertIn(lookup[2:-1], cds.data)
 
         # Ensure all the glyph renderers have a hover tool
-        print(renderers, hover)
         for renderer in renderers:
             self.assertTrue(any(renderer in h.renderers for h in hover))
 
 
+def test_element_plot_stream_cleanup():
+    stream = Pipe()
+
+    dmap = DynamicMap(Curve, streams=[stream])
+
+    plot = bokeh_renderer.get_plot(dmap)
+
+    assert len(stream._subscribers) == 1
+
+    plot.cleanup()
+
+    assert not stream._subscribers
+
+
+def test_overlay_plot_stream_cleanup():
+    stream1 = Pipe()
+    stream2 = Pipe()
+
+    dmap1 = DynamicMap(Curve, streams=[stream1])
+    dmap2 = DynamicMap(Curve, streams=[stream2])
+
+    plot = bokeh_renderer.get_plot(dmap1 * dmap2)
+
+    assert len(stream1._subscribers) == 4
+    assert len(stream2._subscribers) == 4
+
+    plot.cleanup()
+
+    assert not stream1._subscribers
+    assert not stream2._subscribers
+
+
+def test_layout_plot_stream_cleanup():
+    stream1 = Pipe()
+    stream2 = Pipe()
+
+    dmap1 = DynamicMap(Curve, streams=[stream1])
+    dmap2 = DynamicMap(Curve, streams=[stream2])
+
+    plot = bokeh_renderer.get_plot(dmap1 + dmap2)
+
+    assert len(stream1._subscribers) == 2
+    assert len(stream2._subscribers) == 2
+
+    plot.cleanup()
+
+    assert not stream1._subscribers
+    assert not stream2._subscribers
+
+
 def test_sync_two_plots():
     curve = lambda i: Curve(np.arange(10) * i, label="ABC"[i])
     plot1 = curve(0) * curve(1)
@@ -124,3 +177,17 @@ def test_sync_three_plots():
                 assert v[0].code == "dst.muted = src.muted"
                 assert isinstance(v[1], CustomJS)
                 assert v[1].code == "dst.muted = src.muted"
+
+
+def test_span_not_cloned_crosshair():
+    # See https://github.com/holoviz/holoviews/issues/6386
+    height = Span(dimension="height")
+    cht = CrosshairTool(overlay=height)
+
+    layout = Curve([]).opts(tools=[cht]) + Curve([]).opts(tools=[cht])
+
+    (fig1, *_), (fig2, *_) = bokeh_renderer.get_plot(layout).handles["plot"].children
+    tool1 = next(t for t in fig1.tools if isinstance(t, CrosshairTool))
+    tool2 = next(t for t in fig2.tools if isinstance(t, CrosshairTool))
+
+    assert tool1.overlay is tool2.overlay
diff --git a/holoviews/tests/plotting/plotly/test_plot.py b/holoviews/tests/plotting/plotly/test_plot.py
index 3a033f884..0f71a9952 100644
--- a/holoviews/tests/plotting/plotly/test_plot.py
+++ b/holoviews/tests/plotting/plotly/test_plot.py
@@ -2,16 +2,68 @@ import plotly.graph_objs as go
 import pyviz_comms as comms
 from param import concrete_descendents
 
-from holoviews.core import Store
+from holoviews.core import DynamicMap, Store
+from holoviews.element import Curve
 from holoviews.element.comparison import ComparisonTestCase
 from holoviews.plotting.plotly.element import ElementPlot
 from holoviews.plotting.plotly.util import figure_grid
+from holoviews.streams import Pipe
 
 from .. import option_intersections
 
 plotly_renderer = Store.renderers['plotly']
 
 
+def test_element_plot_stream_cleanup():
+    stream = Pipe()
+
+    dmap = DynamicMap(Curve, streams=[stream])
+
+    plot = plotly_renderer.get_plot(dmap)
+
+    assert len(stream._subscribers) == 1
+
+    plot.cleanup()
+
+    assert not stream._subscribers
+
+
+def test_overlay_plot_stream_cleanup():
+    stream1 = Pipe()
+    stream2 = Pipe()
+
+    dmap1 = DynamicMap(Curve, streams=[stream1])
+    dmap2 = DynamicMap(Curve, streams=[stream2])
+
+    plot = plotly_renderer.get_plot(dmap1 * dmap2)
+
+    assert len(stream1._subscribers) == 4
+    assert len(stream2._subscribers) == 4
+
+    plot.cleanup()
+
+    assert not stream1._subscribers
+    assert not stream2._subscribers
+
+
+def test_layout_plot_stream_cleanup():
+    stream1 = Pipe()
+    stream2 = Pipe()
+
+    dmap1 = DynamicMap(Curve, streams=[stream1])
+    dmap2 = DynamicMap(Curve, streams=[stream2])
+
+    plot = plotly_renderer.get_plot(dmap1 + dmap2)
+
+    assert len(stream1._subscribers) == 2
+    assert len(stream2._subscribers) == 2
+
+    plot.cleanup()
+
+    assert not stream1._subscribers
+    assert not stream2._subscribers
+
+
 class TestPlotlyPlot(ComparisonTestCase):
 
     def setUp(self):