From 2f1c5db81ffb351fa6b400d4a86b68ab16bb7acb Mon Sep 17 00:00:00 2001 From: Kernc Date: Mon, 13 Feb 2017 18:32:22 +0100 Subject: [PATCH 1/4] Highcharts: Fix freezing on Qt5 Before this change, the widget froze in _JSObjectChannel.send_object(). --- .../widgets/_highcharts/orange-selection.js | 6 ++-- Orange/widgets/highcharts.py | 34 +++++++++++++------ Orange/widgets/utils/webview.py | 11 ++++-- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/Orange/widgets/_highcharts/orange-selection.js b/Orange/widgets/_highcharts/orange-selection.js index 66d0aff6b16..0208470ac90 100644 --- a/Orange/widgets/_highcharts/orange-selection.js +++ b/Orange/widgets/_highcharts/orange-selection.js @@ -37,7 +37,7 @@ function unselectAllPoints(e) { e.target.parentElement.tagName.toLowerCase() == 'svg')) return true; this.deselectPointsIfNot(false); - _highcharts_bridge.on_selected_points([]); + pybridge._highcharts_on_selected_points([]); } function clickedPointSelect(e) { @@ -49,7 +49,7 @@ function clickedPointSelect(e) { selected.splice(selected.indexOf(this.index), 1); } else points[this.series.index].push(this.index); - _highcharts_bridge.on_selected_points(points); + pybridge._highcharts_on_selected_points(points); return true; } @@ -83,6 +83,6 @@ function rectSelectPoints(e) { } } - _highcharts_bridge.on_selected_points(this.getSelectedPointsForExport()); + pybridge._highcharts_on_selected_points(this.getSelectedPointsForExport()); return false; // Don't zoom } diff --git a/Orange/widgets/highcharts.py b/Orange/widgets/highcharts.py index 678ff02b100..a1a50b6cb0c 100644 --- a/Orange/widgets/highcharts.py +++ b/Orange/widgets/highcharts.py @@ -123,6 +123,29 @@ def __init__(self, if enable_select and not selection_callback: raise ValueError('enable_select requires selection_callback') + if enable_select: + # We need to make sure the _Bridge object below with the selection + # callback is exposed in JS via QWebChannel.registerObject() and + # not through WebviewWidget.exposeObject() as the latter mechanism + # doesn't transmit QObjects correctly. + class _Bridge(QObject): + @pyqtSlot('QVariantList') + def _highcharts_on_selected_points(self, points): + selection_callback([np.sort(selected).astype(int) + for selected in points]) + if bridge is None: + bridge = _Bridge() + else: + # Thus, we patch existing user-passed bridge with our + # selection callback method + attrs = bridge.__dict__.copy() + attrs['_highcharts_on_selected_points'] = _Bridge._highcharts_on_selected_points + assert isinstance(bridge, QObject), 'bridge needs to be a QObject' + _Bridge = type(bridge.__class__.__name__, + bridge.__class__.__mro__, + attrs) + bridge = _Bridge() + super().__init__(parent, bridge, debug=debug) self.highchart = highchart @@ -134,14 +157,6 @@ def __init__(self, mapNavigation_enableMouseWheelZoom=True, mapNavigation_enableButtons=False))) if enable_select: - - class _Bridge(QObject): - @pyqtSlot('QVariantList') - def on_selected_points(self, points): - selection_callback([np.sort(selected).astype(int) - for selected in points]) - - self.exposeObject('_highcharts_bridge', _Bridge()) _merge_dicts(options, _kwargs_options(dict( chart_events_click='/**/unselectAllPoints/**/'))) if enable_point_select: @@ -232,7 +247,7 @@ def svg(self): def main(): """ A simple test. """ - from AnyQt.QtGui import QApplication + from AnyQt.QtWidgets import QApplication app = QApplication([]) def _on_selected_points(points): @@ -251,4 +266,3 @@ def _on_selected_points(points): if __name__ == '__main__': main() - diff --git a/Orange/widgets/utils/webview.py b/Orange/widgets/utils/webview.py index dd3647e549c..690f125d67f 100644 --- a/Orange/widgets/utils/webview.py +++ b/Orange/widgets/utils/webview.py @@ -5,6 +5,7 @@ (extends QWebView), as available. """ import os +from os.path import join, dirname, abspath import warnings from random import random from collections.abc import Mapping, Set, Sequence, Iterable @@ -13,7 +14,6 @@ from urllib.parse import urljoin from urllib.request import pathname2url -from os.path import join, dirname, abspath import numpy as np @@ -92,7 +92,7 @@ def __init__(self, parent=None, bridge=None, *, debug=False, **kwargs): warnings.warn( 'To debug QWebEngineView, set environment variable ' 'QTWEBENGINE_REMOTE_DEBUGGING={port} and then visit ' - 'http://localhost:{port}/ in a Chromium-based browser. ' + 'http://127.0.0.1:{port}/ in a Chromium-based browser. ' 'See https://doc.qt.io/qt-5/qtwebengine-debugging.html ' 'This has also been done for you.'.format(port=port)) super().__init__(parent, @@ -113,7 +113,7 @@ def __init__(self, parent=None, bridge=None, *, debug=False, **kwargs): with open(_WEBENGINE_INIT_WEBCHANNEL, encoding="utf-8") as f: init_webchannel_src = f.read() self._onloadJS(source + init_webchannel_src % - dict(exposeObject_prefix=self._EXPOSED_OBJ_PREFIX), + dict(exposeObject_prefix=self._EXPOSED_OBJ_PREFIX), name='webchannel_init', injection_point=QWebEngineScript.DocumentCreation) else: @@ -456,6 +456,11 @@ def __init__(self, parent): self._objects = {} def send_object(self, name, obj): + if isinstance(obj, QObject): + raise ValueError( + "QWebChannel doesn't transmit QObject instances. If you " + "need a QObject available in JavaScript, pass it as a " + "bridge in WebviewWidget constructor.") id = next(self._id_gen) value = self._objects[id] = dict(id=id, name=name, obj=obj) # Wait till JS is connected to receive objects From ad4bd293eb63f4e009ff45d73ac13cce16684e16 Mon Sep 17 00:00:00 2001 From: Kernc Date: Mon, 13 Feb 2017 18:34:21 +0100 Subject: [PATCH 2/4] WebviewWidget: Handling Esc key, parent is sometimes overriden ... and no longer a callable Qt property. --- Orange/widgets/utils/webview.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Orange/widgets/utils/webview.py b/Orange/widgets/utils/webview.py index 690f125d67f..bffcf32de32 100644 --- a/Orange/widgets/utils/webview.py +++ b/Orange/widgets/utils/webview.py @@ -60,7 +60,7 @@ def hideWindow(self): while isinstance(w, QWidget): if w.windowFlags() & (Qt.Window | Qt.Dialog): return w.hide() - w = w.parent() + w = w.parent() if callable(w.parent) else w.parent if HAVE_WEBENGINE: From c64e964fd444cf2b528fe8cb06e051c7e21f8b2b Mon Sep 17 00:00:00 2001 From: Kernc Date: Mon, 13 Feb 2017 18:59:18 +0100 Subject: [PATCH 3/4] Highcharts: update selection test to cover more code --- Orange/widgets/tests/test_highcharts.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Orange/widgets/tests/test_highcharts.py b/Orange/widgets/tests/test_highcharts.py index 8f6eaf983dd..117535dad6f 100644 --- a/Orange/widgets/tests/test_highcharts.py +++ b/Orange/widgets/tests/test_highcharts.py @@ -1,8 +1,9 @@ import time import os +import sys import unittest -from AnyQt.QtCore import Qt, QPoint +from AnyQt.QtCore import Qt, QPoint, QObject from AnyQt.QtWidgets import qApp from AnyQt.QtTest import QTest @@ -12,15 +13,26 @@ class SelectionScatter(Highchart): - def __init__(self, selected_indices_callback): - super().__init__(enable_select='xy+', + def __init__(self, bridge, selected_indices_callback): + super().__init__(bridge=bridge, + enable_select='xy+', selection_callback=selected_indices_callback, options=dict(chart=dict(type='scatter'))) class HighchartTest(WidgetTest): @unittest.skipIf(os.environ.get('APPVEYOR'), 'test stalls on AppVeyor') + @unittest.skipIf(sys.version_info[:2] <= (3, 4), + 'the second iteration stalls on Travis / Py3.4') def test_selection(self): + + class NoopBridge(QObject): + pass + + for bridge in (NoopBridge(), None): + self._selection_test(bridge) + + def _selection_test(self, bridge): data = Table('iris') selected_indices = [] @@ -28,7 +40,7 @@ def selection_callback(indices): nonlocal selected_indices selected_indices = indices - scatter = SelectionScatter(selection_callback) + scatter = SelectionScatter(bridge, selection_callback) scatter.chart(options=dict(series=[dict(data=data.X[:, :2])])) scatter.show() @@ -65,5 +77,6 @@ def selection_callback(indices): self.assertFalse(len(selected_indices)) + # Test Esc hiding QTest.keyClick(scatter, Qt.Key_Escape) self.assertTrue(scatter.isHidden()) From 9ce670f58c21dc3b35ea0770224f9390d1e8b70d Mon Sep 17 00:00:00 2001 From: Kernc Date: Tue, 14 Feb 2017 13:54:47 +0100 Subject: [PATCH 4/4] Highcharts: refactor constructor --- Orange/widgets/highcharts.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/Orange/widgets/highcharts.py b/Orange/widgets/highcharts.py index a1a50b6cb0c..e4bbd12ff00 100644 --- a/Orange/widgets/highcharts.py +++ b/Orange/widgets/highcharts.py @@ -152,6 +152,18 @@ def _highcharts_on_selected_points(self, points): self.enable_zoom = enable_zoom enable_point_select = '+' in enable_select enable_rect_select = enable_select.replace('+', '') + + self._update_options_dict(options, enable_zoom, enable_select, + enable_point_select, enable_rect_select, + kwargs) + + with open(self._HIGHCHARTS_HTML) as html: + self.setHtml(html.read() % dict(javascript=javascript, + options=json(options)), + self.toFileURL(dirname(self._HIGHCHARTS_HTML)) + '/') + + def _update_options_dict(self, options, enable_zoom, enable_select, + enable_point_select, enable_rect_select, kwargs): if enable_zoom: _merge_dicts(options, _kwargs_options(dict( mapNavigation_enableMouseWheelZoom=True, @@ -170,11 +182,6 @@ def _highcharts_on_selected_points(self, points): if kwargs: _merge_dicts(options, _kwargs_options(kwargs)) - with open(self._HIGHCHARTS_HTML) as html: - self.setHtml(html.read() % dict(javascript=javascript, - options=json(options)), - self.toFileURL(dirname(self._HIGHCHARTS_HTML)) + '/') - def contextMenuEvent(self, event): """ Zoom out on right click. Also disable context menu.""" if self.enable_zoom: