From 65ab0cc009c77d2811ffa4988bf8f68a9d856f35 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Thu, 8 Apr 2021 11:39:16 -0500 Subject: [PATCH 1/9] catch nans before they cause crash --- chaco/ticks.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/chaco/ticks.py b/chaco/ticks.py index 7c1405263..852adab96 100644 --- a/chaco/ticks.py +++ b/chaco/ticks.py @@ -24,6 +24,7 @@ finfo, float64, floor, + isnan, linspace, log10, minimum, @@ -242,6 +243,8 @@ def auto_ticks( An array of tick mark locations. The first and last tick entries are the axis end points. """ + if isnan([data_low, data_high, bound_low, bound_high]).any(): + return [] is_auto_low = bound_low == "auto" is_auto_high = bound_high == "auto" From 3a0314e3888ec7a86047e91e77ea5391c5212f1f Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 12:32:59 -0500 Subject: [PATCH 2/9] if there no data or screen range, we shouldn't even try to pan --- chaco/tools/pan_tool.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/chaco/tools/pan_tool.py b/chaco/tools/pan_tool.py index db055009a..bc2c38348 100644 --- a/chaco/tools/pan_tool.py +++ b/chaco/tools/pan_tool.py @@ -170,6 +170,11 @@ def panning_mouse_move(self, event): for direction, bound_name, index in direction_info: if not self.constrain or self.constrain_direction == direction: mapper = getattr(plot, direction + "_mapper") + + # there is nowhere one could pan! + if mapper._null_screen_range or mapper._null_data_range: + continue + domain_min, domain_max = mapper.domain_limits eventpos = getattr(event, direction) origpos = self._original_xy[index] From 299099e0bc639977054ac626b3eaba033114373c Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 13:25:37 -0500 Subject: [PATCH 3/9] undo previous change and dont return an array from map_data --- chaco/linear_mapper.py | 4 ++-- chaco/tools/pan_tool.py | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/chaco/linear_mapper.py b/chaco/linear_mapper.py index 16dd61e46..670aee09d 100644 --- a/chaco/linear_mapper.py +++ b/chaco/linear_mapper.py @@ -58,9 +58,9 @@ def map_data(self, screen_val): """ self._compute_scale() if self._null_screen_range: - return array([self.range.low]) + return self.range.low elif self._null_data_range: - return array([self.range.low]) + return self.range.low else: return (screen_val - self.low_pos) / self._scale + self.range.low diff --git a/chaco/tools/pan_tool.py b/chaco/tools/pan_tool.py index bc2c38348..db055009a 100644 --- a/chaco/tools/pan_tool.py +++ b/chaco/tools/pan_tool.py @@ -170,11 +170,6 @@ def panning_mouse_move(self, event): for direction, bound_name, index in direction_info: if not self.constrain or self.constrain_direction == direction: mapper = getattr(plot, direction + "_mapper") - - # there is nowhere one could pan! - if mapper._null_screen_range or mapper._null_data_range: - continue - domain_min, domain_max = mapper.domain_limits eventpos = getattr(event, direction) origpos = self._original_xy[index] From a6b755a2cf583153ceb6a8a789ffc750d287e15d Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 13:47:40 -0500 Subject: [PATCH 4/9] undo changes since the original fix --- chaco/linear_mapper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chaco/linear_mapper.py b/chaco/linear_mapper.py index 670aee09d..59ee98d6b 100644 --- a/chaco/linear_mapper.py +++ b/chaco/linear_mapper.py @@ -58,9 +58,9 @@ def map_data(self, screen_val): """ self._compute_scale() if self._null_screen_range: - return self.range.low + return array([self.range.low]) elif self._null_data_range: - return self.range.low + return arrray([self.range.low]) else: return (screen_val - self.low_pos) / self._scale + self.range.low From bb271de6bd4c8ee671572ae3c32d83847a6dc785 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 13:55:53 -0500 Subject: [PATCH 5/9] add a comment for the fix --- chaco/ticks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chaco/ticks.py b/chaco/ticks.py index 852adab96..4e10b083b 100644 --- a/chaco/ticks.py +++ b/chaco/ticks.py @@ -243,6 +243,8 @@ def auto_ticks( An array of tick mark locations. The first and last tick entries are the axis end points. """ + # if a NaN makes it here, we cant determine tick mark locations. Return + # empty list to prevent crash. Motivated by enthought/chaco#636 if isnan([data_low, data_high, bound_low, bound_high]).any(): return [] From e7de578ee5d25146fe02380d2e9eedefc3c25132 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 14:46:26 -0500 Subject: [PATCH 6/9] fix typo and add regression test --- chaco/linear_mapper.py | 2 +- chaco/tests/test_plot.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/chaco/linear_mapper.py b/chaco/linear_mapper.py index 59ee98d6b..16dd61e46 100644 --- a/chaco/linear_mapper.py +++ b/chaco/linear_mapper.py @@ -60,7 +60,7 @@ def map_data(self, screen_val): if self._null_screen_range: return array([self.range.low]) elif self._null_data_range: - return arrray([self.range.low]) + return array([self.range.low]) else: return (screen_val - self.low_pos) / self._scale + self.range.low diff --git a/chaco/tests/test_plot.py b/chaco/tests/test_plot.py index 704441065..6db58f0b4 100644 --- a/chaco/tests/test_plot.py +++ b/chaco/tests/test_plot.py @@ -2,9 +2,15 @@ from numpy import alltrue, arange, array +from enable.api import ComponentEditor +from enable.testing import EnableTestAssistant +from traits.api import HasTraits, Instance +from traitsui.api import Item, View + # Chaco imports from chaco.api import ArrayPlotData, Plot, DataRange1D, PlotGraphicsContext from chaco.default_colormaps import viridis +from chaco.tools.api import PanTool, ZoomTool class PlotTestCase(unittest.TestCase): @@ -89,3 +95,35 @@ def test_text_plot(self): gc.render_component(plot) actual = gc.bmp_array[:, :, :] self.assertFalse(alltrue(actual == 255)) + + +class EmptyLinePlot(HasTraits): + plot = Instance(Plot) + x = [] + y = [] + traits_view = View( + Item('plot', editor=ComponentEditor(), show_label=False), + width=500, + height=500, + resizable=True + ) + def _plot_default(self): + plot = Plot(ArrayPlotData(x=self.x, y=self.y)) + plot.plot(("x", "y"), type="line", color="blue") + plot.tools.append(PanTool(plot)) + plot.overlays.append(ZoomTool(plot, zoom_factor=1.1)) + return plot + +# regression test for enthought/chaco#636 +class TestEmptyPlot(unittest.TestCase, EnableTestAssistant): + + def test_dont_crash_on_click(self): + from traitsui.testing.api import UITester + tester = UITester() + empty_plot = EmptyLinePlot() + + with tester.create_ui(empty_plot) as ui: + self.press_move_release( + empty_plot.plot, + [(1, 1), (25, 25), (50, 50), (100, 100)], + ) From 80aa0648ed7ea8cb3bdb6df39e3500310ac42483 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 9 Apr 2021 14:57:49 -0500 Subject: [PATCH 7/9] skip test on null backend --- chaco/tests/test_plot.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chaco/tests/test_plot.py b/chaco/tests/test_plot.py index 6db58f0b4..59f5efd92 100644 --- a/chaco/tests/test_plot.py +++ b/chaco/tests/test_plot.py @@ -5,6 +5,7 @@ from enable.api import ComponentEditor from enable.testing import EnableTestAssistant from traits.api import HasTraits, Instance +from traits.etsconfig.api import ETSConfig from traitsui.api import Item, View # Chaco imports @@ -115,6 +116,7 @@ def _plot_default(self): return plot # regression test for enthought/chaco#636 +@unittest.skipIf(ETSConfig.toolkit == "null", "Skip on 'null' toolkit") class TestEmptyPlot(unittest.TestCase, EnableTestAssistant): def test_dont_crash_on_click(self): From d75d2de68e6430ca38b06123d28715e71cd9e794 Mon Sep 17 00:00:00 2001 From: aaronayres35 <36972686+aaronayres35@users.noreply.github.com> Date: Tue, 13 Apr 2021 04:49:18 -0700 Subject: [PATCH 8/9] Apply suggestions from code review Co-authored-by: Poruri Sai Rahul --- chaco/tests/test_plot.py | 2 +- chaco/ticks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chaco/tests/test_plot.py b/chaco/tests/test_plot.py index 59f5efd92..87599691f 100644 --- a/chaco/tests/test_plot.py +++ b/chaco/tests/test_plot.py @@ -115,7 +115,7 @@ def _plot_default(self): plot.overlays.append(ZoomTool(plot, zoom_factor=1.1)) return plot -# regression test for enthought/chaco#636 +# regression test for enthought/chaco#529 @unittest.skipIf(ETSConfig.toolkit == "null", "Skip on 'null' toolkit") class TestEmptyPlot(unittest.TestCase, EnableTestAssistant): diff --git a/chaco/ticks.py b/chaco/ticks.py index 4e10b083b..99320a7b7 100644 --- a/chaco/ticks.py +++ b/chaco/ticks.py @@ -244,7 +244,7 @@ def auto_ticks( axis end points. """ # if a NaN makes it here, we cant determine tick mark locations. Return - # empty list to prevent crash. Motivated by enthought/chaco#636 + # empty list to prevent crash. Motivated by enthought/chaco#529 if isnan([data_low, data_high, bound_low, bound_high]).any(): return [] From 89a1a5e757ca367977c3b8ade349b3201329b590 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 08:36:51 -0500 Subject: [PATCH 9/9] flake8 --- chaco/tests/test_plot.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chaco/tests/test_plot.py b/chaco/tests/test_plot.py index e60163874..fc7b74ccc 100644 --- a/chaco/tests/test_plot.py +++ b/chaco/tests/test_plot.py @@ -110,6 +110,7 @@ class EmptyLinePlot(HasTraits): height=500, resizable=True ) + def _plot_default(self): plot = Plot(ArrayPlotData(x=self.x, y=self.y)) plot.plot(("x", "y"), type="line", color="blue") @@ -117,6 +118,7 @@ def _plot_default(self): plot.overlays.append(ZoomTool(plot, zoom_factor=1.1)) return plot + # regression test for enthought/chaco#529 @unittest.skipIf(ETSConfig.toolkit == "null", "Skip on 'null' toolkit") class TestEmptyPlot(unittest.TestCase, EnableTestAssistant): @@ -126,7 +128,7 @@ def test_dont_crash_on_click(self): tester = UITester() empty_plot = EmptyLinePlot() - with tester.create_ui(empty_plot) as ui: + with tester.create_ui(empty_plot): self.press_move_release( empty_plot.plot, [(1, 1), (25, 25), (50, 50), (100, 100)],