Repository: holoviz/holoviews
Pull request #5664 (2023-03-15 17:19:37)

Rectangles use Bokeh quad not rect glyph
This fixes a bug using `hv.Rectangles` on logarithmic axes when using the Bokeh WebGL backend. Related Bokeh issue is https://github.com/bokeh/bokeh/issues/12734. Here is an example of the problem:

<img width="728" alt="Screenshot 2023-03-15 at 17 07 51" src="https://user-images.githubusercontent.com/580326/225387124-4c2df6a5-7307-4a3e-a935-72eda010b563.png">

The cause of the problem is that `hv.Rectangles`, which are defined by min and max `x` and `y` values, are implemented using bokeh `rect` glyphs which use centre coordinates, `width` and `height`. The center coordinates are calculated as the means of the `x` and `y` coordinates, which makes sense for linear axes but not necessarily for logarithic. The solution is to use the Bokeh `quad` glyph instead which is characterised by min and max `x` and `y` so no transform of these parameters is required. Here is the output using this PR:

<img width="731" alt="Screenshot 2023-03-15 at 17 07 16" src="https://user-images.githubusercontent.com/580326/225387138-6c36fea0-8dad-4f50-8ed0-180e14bab898.png">

`quad` glyph has been in Bokeh since at least version 1.4.0 (https://docs.bokeh.org/en/1.4.0/docs/reference/models/glyphs/quad.html?highlight=quad#bokeh.models.glyphs.Quad) in November 2019.

I haven't added a new test as I don't think there are any specific webgl tests implemented.

codecov-commenter: ## [Codecov](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) Report
> Merging [#5664](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) (15de709) into [main](https://codecov.io/gh/holoviz/holoviews/commit/df03b3dd0aea226f9d77c79c348240098d03020c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) (df03b3d) will **decrease** coverage by `4.15%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #5664      +/-   ##
==========================================
- Coverage   88.30%   84.15%   -4.15%     
==========================================
  Files         302      302              
  Lines       62539    62519      -20     
==========================================
- Hits        55223    52615    -2608     
- Misses       7316     9904    +2588     
```


| [Impacted Files](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz) | Coverage Δ | |
|---|---|---|
| [holoviews/plotting/mpl/geometry.py](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz#diff-aG9sb3ZpZXdzL3Bsb3R0aW5nL21wbC9nZW9tZXRyeS5weQ==) | `35.29% <ø> (ø)` | |
| [holoviews/plotting/bokeh/geometry.py](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz#diff-aG9sb3ZpZXdzL3Bsb3R0aW5nL2Jva2VoL2dlb21ldHJ5LnB5) | `100.00% <100.00%> (ø)` | |
| [holoviews/tests/plotting/bokeh/test\_callbacks.py](https://codecov.io/gh/holoviz/holoviews/pull/5664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz#diff-aG9sb3ZpZXdzL3Rlc3RzL3Bsb3R0aW5nL2Jva2VoL3Rlc3RfY2FsbGJhY2tzLnB5) | `100.00% <100.00%> (+1.34%)` | :arrow_up: |

... and [50 files with indirect coverage changes](https://codecov.io/gh/holoviz/holoviews/pull/5664/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=holoviz)

Hoxbro: It also looks good to me. 

Is this ready to be merged?   
ianthomas23: > Is this ready to be merged?

Yes.

Patch:
diff --git a/holoviews/plotting/bokeh/geometry.py b/holoviews/plotting/bokeh/geometry.py
index c5fa92301..5b56c8c2e 100644
--- a/holoviews/plotting/bokeh/geometry.py
+++ b/holoviews/plotting/bokeh/geometry.py
@@ -50,7 +50,7 @@ class RectanglesPlot(GeomMixin, LegendPlot, ColorbarPlot):
 
     _allow_implicit_categories = False
     _nonvectorized_styles = base_properties + ['cmap']
-    _plot_methods = dict(single='rect')
+    _plot_methods = dict(single='quad')
     _batched_style_opts = line_properties + fill_properties
     _color_style = 'fill_color'
 
@@ -60,11 +60,11 @@ class RectanglesPlot(GeomMixin, LegendPlot, ColorbarPlot):
         x0, x1 = np.min([x0, x1], axis=0), np.max([x0, x1], axis=0)
         y0, y1 = np.min([y0, y1], axis=0), np.max([y0, y1], axis=0)
         data = {
-            'x': x0+(x1-x0)/2.,
-            'y': y0+(y1-y0)/2.,
-            'width': x1-x0,
-            'height': y1-y0
+            'left': x0,
+            'right': x1,
+            'bottom': y0,
+            'top': y1
         }
-        mapping = {'x': 'x', 'y': 'y', 'width': 'width', 'height': 'height'}
+        mapping = {'left': 'left', 'right': 'right', 'bottom': 'bottom', 'top': 'top'}
         self._get_hover_data(data, element)
         return data, mapping, style
diff --git a/holoviews/plotting/mpl/geometry.py b/holoviews/plotting/mpl/geometry.py
index dd227cf45..a54a424e6 100644
--- a/holoviews/plotting/mpl/geometry.py
+++ b/holoviews/plotting/mpl/geometry.py
@@ -44,7 +44,7 @@ class SegmentPlot(GeomMixin, ColorbarPlot):
 
 class RectanglesPlot(GeomMixin, ColorbarPlot):
     """
-    Rectanlges are polygons in 2D space where the key dimensions represent
+    Rectangles are polygons in 2D space where the key dimensions represent
     the bottom-left and top-right corner of the rectangle.
     """
 


Test patch:
diff --git a/holoviews/tests/plotting/bokeh/test_callbacks.py b/holoviews/tests/plotting/bokeh/test_callbacks.py
index 5b4c8df64..cfba3e8f6 100644
--- a/holoviews/tests/plotting/bokeh/test_callbacks.py
+++ b/holoviews/tests/plotting/bokeh/test_callbacks.py
@@ -256,10 +256,10 @@ class TestEditToolCallbacks(CallbackTestCase):
         self.assertIsInstance(plot.callbacks[0], BoxEditCallback)
         callback = plot.callbacks[0]
         source = plot.handles['cds']
-        self.assertEqual(source.data['x'], [0])
-        self.assertEqual(source.data['y'], [0])
-        self.assertEqual(source.data['width'], [1])
-        self.assertEqual(source.data['height'], [1])
+        self.assertEqual(source.data['left'], [-0.5])
+        self.assertEqual(source.data['bottom'], [-0.5])
+        self.assertEqual(source.data['right'], [0.5])
+        self.assertEqual(source.data['top'], [0.5])
         data = {'x': [0, 1], 'y': [0, 1], 'width': [0.5, 2], 'height': [2, 0.5]}
         callback.on_msg({'data': data})
         element = Rectangles([(-0.25, -1, 0.25, 1), (0, 0.75, 2, 1.25)])