Ensure draw/edit tools sync data on bokeh server (#8137)

* Ensure draw/edit tools sync data on bokeh server

* Refactored events in edit tools

* Add initial PointDrawTool integration tests

* Improvements to server selenium testing

* Added server selenium tests for PointDrawTool

* Improvements for testing drag and doubletap events

* Added BoxEditTool selenium tests

* Added BoxEditTool server selenium tests

* Added approximate comparisons for CDS data

* remove unused import

bokeh/_testing/plugins/bokeh.py

@@ -22,6 +22,7 @@
 #-----------------------------------------------------------------------------
 
 # Standard library imports
+import time
 from threading import Thread
 
 # External imports
@@ -152,6 +153,13 @@ def click_element_at_position(self, element, x, y):
         actions.click()
         actions.perform()
 
+    def double_click_element_at_position(self, element, x, y):
+        actions = ActionChains(self._driver)
+        actions.move_to_element_with_offset(element, x, y)
+        actions.click()
+        actions.click()
+        actions.perform()
+
     def drag_element_at_position(self, element, x, y, dx, dy, mod=None):
         actions = ActionChains(self._driver)
         if mod:
@@ -164,16 +172,42 @@ def drag_element_at_position(self, element, x, y, dx, dy, mod=None):
             actions.key_up(mod)
         actions.perform()
 
+    def send_keys(self, *keys):
+        actions = ActionChains(self._driver)
+        actions.send_keys(*keys)
+        actions.perform()
+
     def has_no_console_errors(self):
         return self._has_no_console_errors(self._driver)
 
+
+class _CanvasMixin(object):
+
+    def click_canvas_at_position(self, x, y):
+        self.click_element_at_position(self.canvas, x, y)
+
+    def double_click_canvas_at_position(self, x, y):
+        self.double_click_element_at_position(self.canvas, x, y)
+
+    def click_custom_action(self):
+        button = self._driver.find_element_by_class_name("bk-toolbar-button-custom-action")
+        button.click()
+
+    def drag_canvas_at_position(self, x, y, dx, dy, mod=None):
+        self.drag_element_at_position(self.canvas, x, y, dx, dy, mod)
+
+    def get_toolbar_button(self, name):
+        return self.driver.find_element_by_class_name('bk-tool-icon-' + name)
+
+
 @pytest.fixture()
 def bokeh_model_page(driver, output_file_url, has_no_console_errors):
     def func(model):
         return _BokehModelPage(model, driver, output_file_url, has_no_console_errors)
     return func
 
-class _SinglePlotPage(_BokehModelPage):
+
+class _SinglePlotPage(_BokehModelPage, _CanvasMixin):
 
     # model may be a layout, but should only contain a single plot
     def __init__(self, model, driver, output_file_url, has_no_console_errors):
@@ -182,19 +216,6 @@ def __init__(self, model, driver, output_file_url, has_no_console_errors):
         self.canvas = self._driver.find_element_by_tag_name('canvas')
         wait_for_canvas_resize(self.canvas, self._driver)
 
-    def click_canvas_at_position(self, x, y):
-        self.click_element_at_position(self.canvas, x, y)
-
-    def click_custom_action(self):
-        button = self._driver.find_element_by_class_name("bk-toolbar-button-custom-action")
-        button.click()
-
-    def drag_canvas_at_position(self, x, y, dx, dy):
-        self.drag_element_at_position(self.canvas, x, y, dx, dy)
-
-    def get_toolbar_button(self, name):
-        return self.driver.find_element_by_class_name('bk-tool-icon-' + name)
-
 
 @pytest.fixture()
 def single_plot_page(driver, output_file_url, has_no_console_errors):
@@ -203,16 +224,21 @@ def func(model):
     return func
 
 
-class _BokehServerPage(_BokehModelPage):
+class _BokehServerPage(_SinglePlotPage, _CanvasMixin):
+
     def __init__(self, modify_doc, driver, bokeh_app_url, has_no_console_errors):
         self._driver = driver
         self._has_no_console_errors = has_no_console_errors
 
         self._app_url = bokeh_app_url(modify_doc)
+        time.sleep(0.1)
         self._driver.get(self._app_url)
 
         self.init_results()
 
+        self.canvas = self._driver.find_element_by_tag_name('canvas')
+        wait_for_canvas_resize(self.canvas, self._driver)
+
 
 @pytest.fixture()
 def bokeh_server_page(driver, bokeh_app_url, has_no_console_errors):

bokeh/_testing/util/compare.py

@@ -0,0 +1,77 @@
+#-----------------------------------------------------------------------------
+# Copyright (c) 2012 - 2017, Anaconda, Inc. All rights reserved.
+#
+# Powered by the Bokeh Development Team.
+#
+# The full license is in the file LICENSE.txt, distributed with this software.
+#-----------------------------------------------------------------------------
+''' Provide tools for executing Selenium tests.
+
+'''
+
+#-----------------------------------------------------------------------------
+# Boilerplate
+#-----------------------------------------------------------------------------
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import logging
+log = logging.getLogger(__name__)
+
+#-----------------------------------------------------------------------------
+# Imports
+#-----------------------------------------------------------------------------
+
+# Standard library imports
+
+# External imports
+import numpy as np
+
+# Bokeh imports
+
+#-----------------------------------------------------------------------------
+# Globals and constants
+#-----------------------------------------------------------------------------
+
+#-----------------------------------------------------------------------------
+# General API
+#-----------------------------------------------------------------------------
+
+def cds_data_almost_equal(data1, data2, rtol=1e-09, atol=0.0):
+    '''Compares data dictionaries containing floats, lists and arrays
+    Also supports nested lists and arrays
+    '''
+    if sorted(data1.keys()) != sorted(data2.keys()):
+        return False
+    for c in data1.keys():
+        cd1 = data1[c]
+        cd2 = data2[c]
+        if len(cd1) != len(cd2):
+            return False
+        print(c)
+        for v1, v2 in zip(cd1, cd2):
+            print(v1, v2)
+            if isinstance(v1, (float, int)) and isinstance(v2, (float, int)):
+                if not np.isclose(v1, v2, rtol, atol):
+                    return False
+            elif isinstance(v1, (list, np.ndarray)) and isinstance(v2, (list, np.ndarray)):
+                v1, v2 = np.asarray(v1), np.asarray(v2)
+                if v1.dtype.kind in 'iufcmM' and v2.kind in 'iufcmM':
+                    if (~np.isclose(v1, v2, rtol, atol)).any():
+                        return False
+                elif (v1 != v2).any():
+                    return False
+            elif v1 != v2:
+                return False
+    return True
+
+#-----------------------------------------------------------------------------
+# Dev API
+#-----------------------------------------------------------------------------
+
+#-----------------------------------------------------------------------------
+# Private API
+#-----------------------------------------------------------------------------
+
+#-----------------------------------------------------------------------------
+# Code
+#-----------------------------------------------------------------------------

bokehjs/src/lib/models/tools/edit/box_edit_tool.ts

@@ -41,7 +41,7 @@ export class BoxEditToolView extends EditToolView {
     const frame = this.plot_model.frame;
     // Type once dataspecs are typed
     const glyph: any = renderer.glyph;
-    const ds = renderer.data_source;
+    const cds = renderer.data_source;
     const xscale = frame.xscales[renderer.x_range_name];
     const yscale = frame.yscales[renderer.y_range_name];
     const [x0, x1] = xscale.r_invert(sx0, sx1);
@@ -51,23 +51,20 @@ export class BoxEditToolView extends EditToolView {
     const [xkey, ykey] = [glyph.x.field, glyph.y.field];
     const [wkey, hkey] = [glyph.width.field, glyph.height.field];
     if (append) {
-      this._pop_glyphs(ds, this.model.num_objects)
-      if (xkey) ds.get_array(xkey).push(x)
-      if (ykey) ds.get_array(ykey).push(y)
-      if (wkey) ds.get_array(wkey).push(w)
-      if (hkey) ds.get_array(hkey).push(h)
-      this._pad_empty_columns(ds, [xkey, ykey, wkey, hkey])
+      this._pop_glyphs(cds, this.model.num_objects)
+      if (xkey) cds.get_array(xkey).push(x)
+      if (ykey) cds.get_array(ykey).push(y)
+      if (wkey) cds.get_array(wkey).push(w)
+      if (hkey) cds.get_array(hkey).push(h)
+      this._pad_empty_columns(cds, [xkey, ykey, wkey, hkey])
     } else {
-      const index = ds.data[xkey].length - 1
-      if (xkey) ds.data[xkey][index] = x
-      if (ykey) ds.data[ykey][index] = y
-      if (wkey) ds.data[wkey][index] = w
-      if (hkey) ds.data[hkey][index] = h
-    }
-    ds.change.emit();
-    if (emit) {
-      ds.properties.data.change.emit();
+      const index = cds.data[xkey].length - 1
+      if (xkey) cds.data[xkey][index] = x
+      if (ykey) cds.data[ykey][index] = y
+      if (wkey) cds.data[wkey][index] = w
+      if (hkey) cds.data[hkey][index] = h
     }
+    this._emit_cds_changes(cds, true, false, emit)
   }
 
   _update_box(ev: UIEvent, append: boolean = false, emit: boolean = false): void {
@@ -87,10 +84,6 @@ export class BoxEditToolView extends EditToolView {
     if (this._draw_basepoint != null) {
       this._update_box(ev, false, true)
       this._draw_basepoint = null;
-      for (const renderer of this.model.renderers) {
-        renderer.data_source.selected.indices = [];
-        renderer.data_source.properties.data.change.emit();
-      }
     } else {
       this._draw_basepoint = [ev.sx, ev.sy];
       this._select_event(ev, true, this.model.renderers);
@@ -130,10 +123,8 @@ export class BoxEditToolView extends EditToolView {
       this._draw_basepoint = null;
     } else {
       this._basepoint = null;
-    }
-    for (const renderer of this.model.renderers) {
-      renderer.data_source.selected.indices = [];
-      renderer.data_source.properties.data.change.emit();
+      for (const renderer of this.model.renderers)
+        this._emit_cds_changes(renderer.data_source, false, true, true)
     }
   }
 }

bokehjs/src/lib/models/tools/edit/edit_tool.ts

@@ -49,9 +49,7 @@ export abstract class EditToolView extends GestureToolView {
         values.splice(ind-index, 1);
       }
     }
-    cds.change.emit();
-    cds.properties.data.change.emit();
-    cds.selection_manager.clear();
+    this._emit_cds_changes(cds)
   }
 
   _pop_glyphs(cds: ColumnarDataSource, num_objects: number) {
@@ -72,6 +70,17 @@ export abstract class EditToolView extends GestureToolView {
     }
   }
 
+  _emit_cds_changes(cds: ColumnarDataSource, redraw: boolean = true, clear: boolean = true, emit: boolean = true) {
+    if (clear)
+      cds.selection_manager.clear();
+    if (redraw)
+      cds.change.emit();
+    if (emit) {
+      cds.data = cds.data;
+      cds.properties.data.change.emit();
+    }
+  }
+
   _drag_points(ev: UIEvent, renderers: (GlyphRenderer & HasXYGlyph)[]): void {
     if (this._basepoint == null) { return; };
     const [bx, by] = this._basepoint;
@@ -86,16 +95,13 @@ export abstract class EditToolView extends GestureToolView {
       const [dx, dy] = [x-px, y-py];
       // Type once dataspecs are typed
       const glyph: any = renderer.glyph;
-      const ds = renderer.data_source;
+      const cds = renderer.data_source;
       const [xkey, ykey] = [glyph.x.field, glyph.y.field];
-      for (const index of ds.selected.indices) {
-        if (xkey) ds.data[xkey][index] += dx
-        if (ykey) ds.data[ykey][index] += dy
+      for (const index of cds.selected.indices) {
+        if (xkey) cds.data[xkey][index] += dx
+        if (ykey) cds.data[ykey][index] += dy
       }
-    }
-    for (const renderer of renderers) {
-      renderer.data_source.change.emit()
-      renderer.data_source.properties.data.change.emit()
+      cds.change.emit()
     }
     this._basepoint = [ev.sx, ev.sy];
   }

bokehjs/src/lib/models/tools/edit/point_draw_tool.ts

@@ -21,17 +21,18 @@ export class PointDrawToolView extends EditToolView {
     }
     // Type once dataspecs are typed
     const glyph: any = renderer.glyph
-    const ds = renderer.data_source;
+    const cds = renderer.data_source;
     const [xkey, ykey] = [glyph.x.field, glyph.y.field];
     const [x, y] = point;
 
-    this._pop_glyphs(ds, this.model.num_objects)
-    if (xkey) ds.get_array(xkey).push(x)
-    if (ykey) ds.get_array(ykey).push(y)
-    this._pad_empty_columns(ds, [xkey, ykey]);
+    this._pop_glyphs(cds, this.model.num_objects)
+    if (xkey) cds.get_array(xkey).push(x)
+    if (ykey) cds.get_array(ykey).push(y)
+    this._pad_empty_columns(cds, [xkey, ykey]);
 
-    ds.change.emit();
-    ds.properties.data.change.emit();
+    cds.change.emit();
+    cds.data = cds.data;
+    cds.properties.data.change.emit();
   }
 
   _keyup(ev: KeyEvent): void {
@@ -40,9 +41,7 @@ export class PointDrawToolView extends EditToolView {
       if (ev.keyCode === Keys.Backspace) {
         this._delete_selected(renderer);
       } else if (ev.keyCode == Keys.Esc) {
-        // Type once selection_manager is typed
-        const cds = renderer.data_source;
-        cds.selection_manager.clear();
+        renderer.data_source.selection_manager.clear();
       }
     }
   }
@@ -63,14 +62,8 @@ export class PointDrawToolView extends EditToolView {
   _pan_end(ev: GestureEvent): void {
     if (!this.model.drag) { return; }
     this._pan(ev);
-    for (const renderer of this.model.renderers) {
-      renderer.data_source.selected.indices = []
-
-      // This is only needed to call @_tell_document_about_change()
-      renderer.data_source.data = renderer.data_source.data
-
-      renderer.data_source.properties.data.change.emit()
-    }
+    for (const renderer of this.model.renderers)
+      this._emit_cds_changes(renderer.data_source, false, true, true)
     this._basepoint = null;
   }
 }