From a629e0648216a4faed68add97dc08a1b3d9cc63b Mon Sep 17 00:00:00 2001 From: janezd Date: Sat, 7 Mar 2020 13:20:21 +0100 Subject: [PATCH 1/3] ScatterplotGraph: Plot selected, subset and rare colors above the others --- .../widgets/visualize/owscatterplotgraph.py | 104 ++++++++- .../visualize/tests/test_owscatterplotbase.py | 208 ++++++++++++++++++ 2 files changed, 303 insertions(+), 9 deletions(-) diff --git a/Orange/widgets/visualize/owscatterplotgraph.py b/Orange/widgets/visualize/owscatterplotgraph.py index c4007004e24..3b5a56b1532 100644 --- a/Orange/widgets/visualize/owscatterplotgraph.py +++ b/Orange/widgets/visualize/owscatterplotgraph.py @@ -244,22 +244,60 @@ def __init__(self, scatter_widget, parent=None, _="None", view_box=InteractiveVi class ScatterPlotItem(pg.ScatterPlotItem): - """PyQtGraph's ScatterPlotItem calls updateSpots at any change of sizes/colors/symbols, - which then rebuilds the stored pixmaps for each symbol. Because Orange calls - set* function in succession, we postpone updateSpots() to paint().""" + """ + Modifies the behaviour of ScatterPlotItem as follows: + + - Add z-index. ScatterPlotItem paints points in order of appearance in + self.data. Plotting by z-index is achieved by sorting before calling + super().paint() and re-sorting afterwards. Re-sorting (instead of + storing the original data) is needed because the inherited paint + may modify the data. + + - Prevent multiple calls to updateSpots. ScatterPlotItem calls updateSpots + at any change of sizes/colors/symbols, which then rebuilds the stored + pixmaps for each symbol. Orange calls set* functions in succession, + so we postpone updateSpots() to paint().""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._update_spots_in_paint = False + self._z_mapping = None + self._inv_mapping = None + + def setZ(self, z): + """ + Set z values for all points. - _update_spots_in_paint = False + Points with higher values are plotted on top of those with lower. + + Args: + z (np.ndarray or None): a vector of z values + """ + if z is None: + self._z_mapping = self._inv_mapping = None + else: + assert len(z) == len(self.data) + self._z_mapping = np.argsort(z) + self._inv_mapping = np.argsort(self._z_mapping) def updateSpots(self, dataSet=None): # pylint: disable=unused-argument self._update_spots_in_paint = True self.update() + # pylint: disable=arguments-differ def paint(self, painter, option, widget=None): - if self._update_spots_in_paint: - self._update_spots_in_paint = False - super().updateSpots() - painter.setRenderHint(QPainter.SmoothPixmapTransform, True) - super().paint(painter, option, widget) + try: + if self._z_mapping is not None: + assert len(self._z_mapping) == len(self.data) + self.data = self.data[self._z_mapping] + if self._update_spots_in_paint: + self._update_spots_in_paint = False + super().updateSpots() + painter.setRenderHint(QPainter.SmoothPixmapTransform, True) + super().paint(painter, option, widget) + finally: + if self._inv_mapping is not None: + self.data = self.data[self._inv_mapping] def _define_symbols(): @@ -1084,6 +1122,7 @@ def update_colors(self): pen_data, brush_data = self.get_colors() self.scatterplot_item.setPen(pen_data, update=False, mask=None) self.scatterplot_item.setBrush(brush_data, mask=None) + self.update_z_values() self.update_legends() self.update_density() @@ -1128,6 +1167,7 @@ def update_selection_colors(self): pen, brush = self.get_colors_sel() self.scatterplot_item_sel.setPen(pen, update=False, mask=None) self.scatterplot_item_sel.setBrush(brush, mask=None) + self.update_z_values() def get_colors_sel(self): """ @@ -1292,6 +1332,52 @@ def update_shapes(self): self.scatterplot_item.setSymbol(shape_data) self.update_legends() + def update_z_values(self): + """ + Set z-values for point in the plot + + The order is as follows: + - selected points that are also in the subset on top, + - followed by selected points, + - followed by points from the subset, + - followed by the rest. + Within each of these four groups, points are ordered by their colors. + + Points with less frequent colors are above those with more frequent. + The points for which the value for the color is missing are at the + bottom of their respective group. + """ + if not self.scatterplot_item: + return + + subset = self.master.get_subset_mask() + c_data = self.master.get_color_data() + if subset is None and self.selection is None and c_data is None: + self.scatterplot_item.setZ(None) + return + + z = np.zeros(self.n_shown) + + if subset is not None: + subset = self._filter_visible(subset) + z[subset] += 1000 + + if self.selection is not None: + z[self._filter_visible(self.selection) != 0] += 2000 + + if c_data is not None: + c_nan = np.isnan(c_data) + vis_data = self._filter_visible(c_data) + vis_nan = np.isnan(vis_data) + z[vis_nan] -= 999 + if not self.master.is_continuous_color(): + dist = np.bincount(c_data[~c_nan].astype(int)) + vis_knowns = vis_data[~vis_nan].astype(int) + argdist = np.argsort(dist) + z[~vis_nan] -= argdist[vis_knowns] + + self.scatterplot_item.setZ(z) + def update_grid_visibility(self): """Show or hide the grid""" self.plot_widget.showGrid(x=self.show_grid, y=self.show_grid) diff --git a/Orange/widgets/visualize/tests/test_owscatterplotbase.py b/Orange/widgets/visualize/tests/test_owscatterplotbase.py index 8b9391a9b7b..01024c08ca2 100644 --- a/Orange/widgets/visualize/tests/test_owscatterplotbase.py +++ b/Orange/widgets/visualize/tests/test_owscatterplotbase.py @@ -1,5 +1,6 @@ # pylint: disable=missing-docstring,too-many-lines,too-many-public-methods # pylint: disable=protected-access +from itertools import count from unittest.mock import patch, Mock import numpy as np @@ -9,6 +10,7 @@ from pyqtgraph import mkPen +from orangewidget.tests.base import GuiTest from Orange.widgets.settings import SettingProvider from Orange.widgets.tests.base import WidgetTest from Orange.widgets.utils import colorpalettes @@ -700,6 +702,127 @@ def test_selection_colors(self): else: self.assertEqual(pen.style(), Qt.NoPen) + def test_z_values(self): + def check_ranks(exp_ranks): + z = set_z.call_args[0][0] + self.assertEqual(len(z), len(exp_ranks)) + for i, exp1, z1 in zip(count(), exp_ranks, z): + for j, exp2, z2 in zip(range(i), exp_ranks, z): + if exp1 != exp2: + self.assertEqual(exp1 < exp2, z1 < z2, + f"error at pair ({j}, {i})") + + colors = np.array([0, 1, 1, 0, np.nan, 2, 2, 2, 1, 1]) + self.master.get_color_data = lambda: colors + self.master.is_continuous_color = lambda: False + + graph = self.graph + with patch.object(ScatterPlotItem, "setZ") as set_z: + # Just colors + graph.reset_graph() + check_ranks([3, 1, 1, 3, 0, 2, 2, 2, 1, 1]) + + # Colors and selection + graph.selection_select([1, 5]) + check_ranks([3, 11, 1, 3, 0, 12, 2, 2, 1, 1]) + + # Colors and selection, and nan is selected + graph.selection_append([4]) + check_ranks([3, 11, 1, 3, 10, 12, 2, 2, 1, 1]) + + # Just colors again, no selection + graph.selection_select([]) + check_ranks([3, 1, 1, 3, 0, 2, 2, 2, 1, 1]) + + # Colors and subset + self.master.get_subset_mask = \ + lambda: np.array([True, True, False, False, True] * 2) + graph.update_colors() # selecting subset triggers update_colors + check_ranks([23, 21, 1, 3, 20, 22, 22, 2, 1, 21]) + + # Colors, subset and selection + graph.selection_select([1, 5]) + check_ranks([23, 31, 1, 3, 20, 32, 22, 2, 1, 21]) + + # Continuous colors + self.master.is_continuous_color = lambda: True + graph.update_colors() + check_ranks([20, 30, 0, 0, 20, 30, 20, 0, 0, 20]) + + # No colors => just subset and selection + # pylint: disable=attribute-defined-outside-init + self.master.get_colors = lambda: None + graph.update_colors() + check_ranks([20, 30, 0, 0, 20, 30, 20, 0, 0, 20]) + + # No selection or subset, but continuous colors with nan + graph.selection_select([1, 5]) + self.master.get_subset_mask = lambda: None + self.master.get_color_data = lambda: colors + graph.update_colors() + check_ranks(np.isfinite(colors)) + + def test_z_values_with_sample(self): + def check_ranks(exp_ranks): + z = set_z.call_args[0][0] + self.assertEqual(len(z), len(exp_ranks)) + for i, exp1, z1 in zip(count(), exp_ranks, z): + for j, exp2, z2 in zip(range(i), exp_ranks, z): + if exp1 != exp2: + self.assertEqual(exp1 < exp2, z1 < z2, + f"error at pair ({j}, {i})") + + def create_sample(): + graph.sample_indices = np.array([0, 1, 3, 4, 5, 6, 7, 8, 9]) + graph.n_shown = 9 + + graph = self.graph + graph.sample_size = 9 + graph._create_sample = create_sample + + self.master.is_continuous_color = lambda: False + self.master.get_color_data = \ + lambda: np.array([0, 1, 1, 0, np.nan, 2, 2, 2, 1, 1]) + + with patch.object(ScatterPlotItem, "setZ") as set_z: + # Just colors + graph.reset_graph() + check_ranks([3, 1, 3, 0, 2, 2, 2, 1, 1]) + + # Colors and selection + graph.selection_select([1, 5]) + check_ranks([3, 11, 3, 0, 12, 2, 2, 1, 1]) + + # Colors and selection, and nan is selected + graph.selection_append([4]) + check_ranks([3, 11, 3, 10, 12, 2, 2, 1, 1]) + + # Just colors again, no selection + graph.selection_select([]) + check_ranks([3, 1, 3, 0, 2, 2, 2, 1, 1]) + + # Colors and subset + self.master.get_subset_mask = \ + lambda: np.array([True, True, False, False, True] * 2) + graph.update_colors() # selecting subset triggers update_colors + check_ranks([23, 21, 3, 20, 22, 22, 2, 1, 21]) + + # Colors, subset and selection + graph.selection_select([1, 5]) + check_ranks([23, 31, 3, 20, 32, 22, 2, 1, 21]) + + # Continuous colors => just subset and selection + self.master.is_continuous_color = lambda: False + graph.update_colors() + check_ranks([20, 30, 0, 20, 30, 20, 0, 0, 20]) + + # No colors => just subset and selection + self.master.is_continuous_color = lambda: True + # pylint: disable=attribute-defined-outside-init + self.master.get_colors = lambda: None + graph.update_colors() + check_ranks([20, 30, 0, 20, 30, 20, 0, 0, 20]) + def test_density(self): graph = self.graph density = object() @@ -1294,6 +1417,91 @@ def test_no_needless_buildatlas(self): self.assertIsNone(graph.scatterplot_item.fragmentAtlas.atlas) +class TestScatterPlotItem(GuiTest): + def test_setZ(self): + """setZ sets the appropriate mapping and inverse mapping""" + scp = ScatterPlotItem(x=np.arange(5), y=np.arange(5)) + scp.setZ(np.array([3.12, 5.2, 1.2, 0, 2.15])) + np.testing.assert_equal(scp._z_mapping, [3, 2, 4, 0, 1]) + np.testing.assert_equal(scp._inv_mapping, [3, 4, 1, 0, 2]) + + scp.setZ(None) + self.assertIsNone(scp._z_mapping) + self.assertIsNone(scp._inv_mapping) + + self.assertRaises(AssertionError, scp.setZ, np.arange(4)) + + @staticmethod + def test_paint_mapping(): + """paint permutes the points and reverses the permutation afterwards""" + def test_self_data(this, *_, **_1): + x, y = this.getData() + np.testing.assert_equal(x, exp_x) + np.testing.assert_equal(y, exp_y) + + orig_x = np.arange(10, 15) + orig_y = np.arange(20, 25) + scp = ScatterPlotItem(x=orig_x[:], y=orig_y[:]) + with patch("pyqtgraph.ScatterPlotItem.paint", new=test_self_data): + exp_x = orig_x + exp_y = orig_y + scp.paint(Mock(), Mock()) + + scp._z_mapping = np.array([3, 2, 4, 0, 1]) + scp._inv_mapping = np.array([3, 4, 1, 0, 2]) + exp_x = [13, 12, 14, 10, 11] + exp_y = [23, 22, 24, 20, 21] + scp.paint(Mock(), Mock()) + x, y = scp.getData() + np.testing.assert_equal(x, np.arange(10, 15)) + np.testing.assert_equal(y, np.arange(20, 25)) + + def test_paint_mapping_exception(self): + """exception in paint does not leave the points permuted""" + orig_x = np.arange(10, 15) + orig_y = np.arange(20, 25) + scp = ScatterPlotItem(x=orig_x[:], y=orig_y[:]) + scp._z_mapping = np.array([3, 2, 4, 0, 1]) + scp._inv_mapping = np.array([3, 4, 1, 0, 2]) + with patch("pyqtgraph.ScatterPlotItem.paint", side_effect=ValueError): + self.assertRaises(ValueError, scp.paint, Mock(), Mock()) + x, y = scp.getData() + np.testing.assert_equal(x, np.arange(10, 15)) + np.testing.assert_equal(y, np.arange(20, 25)) + + @staticmethod + def test_paint_mapping_integration(): + """setZ causes rendering in the appropriate order""" + def test_self_data(this, *_, **_1): + x, y = this.getData() + np.testing.assert_equal(x, exp_x) + np.testing.assert_equal(y, exp_y) + + orig_x = np.arange(10, 15) + orig_y = np.arange(20, 25) + scp = ScatterPlotItem(x=orig_x[:], y=orig_y[:]) + with patch("pyqtgraph.ScatterPlotItem.paint", new=test_self_data): + exp_x = orig_x + exp_y = orig_y + scp.paint(Mock(), Mock()) + + scp.setZ(np.array([3.12, 5.2, 1.2, 0, 2.15])) + exp_x = [13, 12, 14, 10, 11] + exp_y = [23, 22, 24, 20, 21] + scp.paint(Mock(), Mock()) + x, y = scp.getData() + np.testing.assert_equal(x, np.arange(10, 15)) + np.testing.assert_equal(y, np.arange(20, 25)) + + scp.setZ(None) + exp_x = orig_x + exp_y = orig_y + scp.paint(Mock(), Mock()) + x, y = scp.getData() + np.testing.assert_equal(x, np.arange(10, 15)) + np.testing.assert_equal(y, np.arange(20, 25)) + + if __name__ == "__main__": import unittest unittest.main() From e141cebe65fd7d2854e82930fc3ea3b7e658582a Mon Sep 17 00:00:00 2001 From: janezd Date: Sat, 7 Mar 2020 13:21:46 +0100 Subject: [PATCH 2/3] OWScatterPlotBase._update_plot_coordinates: Assert correct vector lengths --- Orange/widgets/visualize/owscatterplotgraph.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Orange/widgets/visualize/owscatterplotgraph.py b/Orange/widgets/visualize/owscatterplotgraph.py index 3b5a56b1532..97bceb615f2 100644 --- a/Orange/widgets/visualize/owscatterplotgraph.py +++ b/Orange/widgets/visualize/owscatterplotgraph.py @@ -804,7 +804,9 @@ def _jitter_data(self, x, y, span_x=None, span_y=None): def _update_plot_coordinates(self, plot, x, y): """ - Change the coordinates of points while keeping other properites + Change the coordinates of points while keeping other properites. + + Asserts that the number of points stays the same. Note. Pyqtgraph does not offer a method for this: setting coordinates invalidates other data. We therefore retrieve the data to set it @@ -814,6 +816,7 @@ def _update_plot_coordinates(self, plot, x, y): update for every property would essentially reset the graph, which can be time consuming. """ + assert self.n_shown == len(x) == len(y) data = dict(x=x, y=y) for prop in ('pen', 'brush', 'size', 'symbol', 'data', 'sourceRect', 'targetRect'): From 995220ddfc7644dd853475ec3559dfe44df56d78 Mon Sep 17 00:00:00 2001 From: janezd Date: Sat, 7 Mar 2020 13:27:02 +0100 Subject: [PATCH 3/3] OWScatterPlotBase: Lint --- Orange/widgets/visualize/owscatterplotgraph.py | 13 +++++++------ .../visualize/tests/test_owscatterplotbase.py | 6 +++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Orange/widgets/visualize/owscatterplotgraph.py b/Orange/widgets/visualize/owscatterplotgraph.py index 97bceb615f2..6c2ee9f7333 100644 --- a/Orange/widgets/visualize/owscatterplotgraph.py +++ b/Orange/widgets/visualize/owscatterplotgraph.py @@ -128,7 +128,8 @@ def restoreAnchor(self, anchors): anchor, parentanchor = anchors self.anchor(*bound_anchor_pos(anchor, parentanchor)) - def paint(self, painter, option, widget=None): + # pylint: disable=arguments-differ + def paint(self, painter, _option, _widget=None): painter.setPen(self.__pen) painter.setBrush(self.__brush) rect = self.contentsRect() @@ -619,7 +620,7 @@ def _get_jittering_tooltip(self): def update_jittering(self): self.update_tooltip() x, y = self.get_coordinates() - if x is None or not len(x) or self.scatterplot_item is None: + if x is None or len(x) == 0 or self.scatterplot_item is None: return self._update_plot_coordinates(self.scatterplot_item, x, y) self._update_plot_coordinates(self.scatterplot_item_sel, x, y) @@ -834,7 +835,7 @@ def update_coordinates(self): the complete update by calling `reset_graph` instead of this method. """ x, y = self.get_coordinates() - if x is None or not len(x): + if x is None or len(x) == 0: return if self.scatterplot_item is None: if self.sample_indices is None: @@ -1053,9 +1054,9 @@ def _get_continuous_colors(self, c_data, subset): # Reuse pens and brushes with the same colors because PyQtGraph then # builds smaller pixmap atlas, which makes the drawing faster - def reuse(cache, fn, *args): + def reuse(cache, fun, *args): if args not in cache: - cache[args] = fn(args) + cache[args] = fun(args) return cache[args] def create_pen(col): @@ -1113,7 +1114,7 @@ def _get_discrete_colors(self, c_data, subset): def update_colors(self): """ - Trigger an update of point sizes + Trigger an update of point colors The method calls `self.get_colors`, which in turn calls the widget's `get_color_data` to get the indices in the pallette. `get_colors` diff --git a/Orange/widgets/visualize/tests/test_owscatterplotbase.py b/Orange/widgets/visualize/tests/test_owscatterplotbase.py index 01024c08ca2..7cfbb9844d0 100644 --- a/Orange/widgets/visualize/tests/test_owscatterplotbase.py +++ b/Orange/widgets/visualize/tests/test_owscatterplotbase.py @@ -58,6 +58,7 @@ def setUp(self): # pylint: disable=keyword-arg-before-vararg def setRange(self, rect=None, *_, **__): if isinstance(rect, QRectF): + # pylint: disable=attribute-defined-outside-init self.last_setRange = [[rect.left(), rect.right()], [rect.top(), rect.bottom()]] @@ -308,12 +309,13 @@ def test_sampling_keeps_selection(self): base = "Orange.widgets.visualize.owscatterplotgraph.OWScatterPlotBase." + @staticmethod @patch(base + "update_sizes") @patch(base + "update_colors") @patch(base + "update_selection_colors") @patch(base + "update_shapes") @patch(base + "update_labels") - def test_reset_calls_all_updates_and_update_doesnt(self, *mocks): + def test_reset_calls_all_updates_and_update_doesnt(*mocks): master = MockWidget() graph = OWScatterPlotBase(master) for mock in mocks: @@ -443,6 +445,7 @@ def impute_max(size_data): graph = self.graph + # pylint: disable=attribute-defined-outside-init self.master.get_size_data = lambda: d self.master.impute_sizes = impute_max d = np.arange(10, dtype=float) @@ -1168,6 +1171,7 @@ def test_shapes_nan(self): def impute0(data, _): data[np.isnan(data)] = 0 + # pylint: disable=attribute-defined-outside-init self.master.impute_shapes = impute0 d = np.arange(10, dtype=float) % 3 d[2] = np.nan