From 357263d6e28b078eb85515423551cb8127222a6a Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Fri, 2 Apr 2021 13:52:47 -0500 Subject: [PATCH 01/10] first stab at resolving the NotifierNotFound error --- chaco/tools/range_selection.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index 113ad8d8d..240f0cb93 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -663,6 +663,14 @@ def _determine_axis(self): def __mapper_changed(self, event): self.deselect() + def _component_changed(self, old, new): + if old is not None: + self.plot.observe( + self.__mapper_changed, self.axis + "_mapper", remove=True + ) + if new is not None: + self.plot.observe(self.__mapper_changed, self.axis + "_mapper") + def _axis_changed(self, old, new): if old is not None: self.plot.observe( From 1f174d9811f656da578619056e26b034ee18337b Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 12:33:12 -0500 Subject: [PATCH 02/10] add a test to ensure handler is hooked up / called (this previously failed) --- chaco/tools/tests/test_range_selection.py | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/chaco/tools/tests/test_range_selection.py b/chaco/tools/tests/test_range_selection.py index 44357af50..1b9f7a861 100644 --- a/chaco/tools/tests/test_range_selection.py +++ b/chaco/tools/tests/test_range_selection.py @@ -3,6 +3,7 @@ import numpy as np +from chaco.api import LinearMapper from chaco.array_plot_data import ArrayPlotData from chaco.plot import Plot from chaco.tools.range_selection import RangeSelection @@ -75,3 +76,27 @@ def test_selection_no_warning(self): tool.selection = (1.5, 3.5) tool.selection = [1.0, 2.0] tool.selection = None + + @unittest.mock.patch('chaco.tools.range_selection.RangeSelection.deselect') + def test_notifiers_connected(self, mocked_deselect): + plot_data = ArrayPlotData() + arr = np.arange(4.0) + plot_data.set_data("x", arr) + plot_data.set_data("y", arr) + + plot = Plot(plot_data) + + renderer = plot.plot(("x", "y"))[0] + renderer.bounds = [10, 20] + tool = RangeSelection(renderer) + renderer.tools.append(tool) + + # attempt to trigger change handler for the index_mapper trait on the + # RangeSelection tool's plot + # assign a new mapper with same attrs + renderer.index_mapper = LinearMapper( + range=renderer.index_mapper.range, + stretch_data=renderer.index_mapper.stretch_data + ) + + mocked_deselect.assert_called_once() From 9f64a747ac2e8636024d8c7b64728cd2a9e53108 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 12:34:29 -0500 Subject: [PATCH 03/10] add comment with issue number --- chaco/tools/tests/test_range_selection.py | 1 + 1 file changed, 1 insertion(+) diff --git a/chaco/tools/tests/test_range_selection.py b/chaco/tools/tests/test_range_selection.py index 1b9f7a861..3fe989aff 100644 --- a/chaco/tools/tests/test_range_selection.py +++ b/chaco/tools/tests/test_range_selection.py @@ -77,6 +77,7 @@ def test_selection_no_warning(self): tool.selection = [1.0, 2.0] tool.selection = None + # regression test for enthought/chaco#597 @unittest.mock.patch('chaco.tools.range_selection.RangeSelection.deselect') def test_notifiers_connected(self, mocked_deselect): plot_data = ArrayPlotData() From c299e5a32910354163bb69bd51504d4d90af9b48 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 12:43:05 -0500 Subject: [PATCH 04/10] flake8 --- chaco/tools/tests/test_range_selection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chaco/tools/tests/test_range_selection.py b/chaco/tools/tests/test_range_selection.py index 3fe989aff..454b0af5b 100644 --- a/chaco/tools/tests/test_range_selection.py +++ b/chaco/tools/tests/test_range_selection.py @@ -93,7 +93,7 @@ def test_notifiers_connected(self, mocked_deselect): renderer.tools.append(tool) # attempt to trigger change handler for the index_mapper trait on the - # RangeSelection tool's plot + # RangeSelection tool's plot # assign a new mapper with same attrs renderer.index_mapper = LinearMapper( range=renderer.index_mapper.range, From 4067035393e9790bda5a6ec2295c62b6a0f2156f Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 15:20:40 -0500 Subject: [PATCH 05/10] decorarte the method we are observing directly, remove magically named methods --- chaco/tools/range_selection.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index 17c8d048d..e7711ef81 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -15,6 +15,7 @@ Instance, Int, List, + observe, Property, Str, Trait, @@ -676,21 +677,7 @@ def _determine_axis(self): else: return 0 + @observe("component.index_mapper,component.value_mapper") def __mapper_changed(self, event): - self.deselect() - - def _component_changed(self, old, new): - if old is not None: - self.plot.observe( - self.__mapper_changed, self.axis + "_mapper", remove=True - ) - if new is not None: - self.plot.observe(self.__mapper_changed, self.axis + "_mapper") - - def _axis_changed(self, old, new): - if old is not None: - self.plot.observe( - self.__mapper_changed, old + "_mapper", remove=True - ) - if new is not None: - self.plot.observe(self.__mapper_changed, new + "_mapper") + if event.name == self.axis + "_mapper": + self.deselect() From 51fcdce9dbdd6bc969935f0bb21c9e62cdb71f7c Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Tue, 13 Apr 2021 15:22:47 -0500 Subject: [PATCH 06/10] rename method to not look like a magic named method --- chaco/tools/range_selection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index e7711ef81..e843e20f8 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -678,6 +678,6 @@ def _determine_axis(self): return 0 @observe("component.index_mapper,component.value_mapper") - def __mapper_changed(self, event): + def _axis_mapper_updated(self, event): if event.name == self.axis + "_mapper": self.deselect() From 0138600efb920d1c7c2fce367743b8cab8eb4219 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Thu, 15 Apr 2021 13:14:39 -0700 Subject: [PATCH 07/10] handle _plot changing case --- chaco/tools/range_selection.py | 7 +++++- chaco/tools/tests/test_range_selection.py | 30 +++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index e843e20f8..82475bbcd 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -677,7 +677,12 @@ def _determine_axis(self): else: return 0 - @observe("component.index_mapper,component.value_mapper") + @observe([ + "component.index_mapper", + "component.value_mapper", + "_plot.index_mapper", + "_plot.value_mapper" + ]) def _axis_mapper_updated(self, event): if event.name == self.axis + "_mapper": self.deselect() diff --git a/chaco/tools/tests/test_range_selection.py b/chaco/tools/tests/test_range_selection.py index 454b0af5b..ebf8e6088 100644 --- a/chaco/tools/tests/test_range_selection.py +++ b/chaco/tools/tests/test_range_selection.py @@ -3,7 +3,7 @@ import numpy as np -from chaco.api import LinearMapper +from chaco.api import LinearMapper, PlotComponent from chaco.array_plot_data import ArrayPlotData from chaco.plot import Plot from chaco.tools.range_selection import RangeSelection @@ -88,7 +88,6 @@ def test_notifiers_connected(self, mocked_deselect): plot = Plot(plot_data) renderer = plot.plot(("x", "y"))[0] - renderer.bounds = [10, 20] tool = RangeSelection(renderer) renderer.tools.append(tool) @@ -101,3 +100,30 @@ def test_notifiers_connected(self, mocked_deselect): ) mocked_deselect.assert_called_once() + + # regression test for enthought/chaco#597 + @unittest.mock.patch('chaco.tools.range_selection.RangeSelection.deselect') + def test_notifiers_connected_specify_plot(self, mocked_deselect): + plot_data = ArrayPlotData() + arr = np.arange(4.0) + plot_data.set_data("x", arr) + plot_data.set_data("y", arr) + + plot = Plot(plot_data) + + renderer = plot.plot(("x", "y"))[0] + tool = RangeSelection(renderer) + renderer.tools.append(tool) + + new_renderer = plot.plot(("x", "y"), type='scatter')[0] + tool.plot = new_renderer + + # attempt to trigger change handler for the index_mapper trait on the + # RangeSelection tool's plot + # assign a new mapper with same attrs + new_renderer.index_mapper = LinearMapper( + range=new_renderer.index_mapper.range, + stretch_data=new_renderer.index_mapper.stretch_data + ) + + mocked_deselect.assert_called_once() From 306e8d99473db314a5abffee7bb76876d586cf78 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Thu, 15 Apr 2021 13:20:49 -0700 Subject: [PATCH 08/10] be more explicit about traits being optional in observer expression --- chaco/tools/range_selection.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index 82475bbcd..4d068568d 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -20,6 +20,7 @@ Str, Trait, ) +from traits.observation.api import trait from enable.api import KeySpec # Chaco imports @@ -678,10 +679,11 @@ def _determine_axis(self): return 0 @observe([ - "component.index_mapper", - "component.value_mapper", - "_plot.index_mapper", - "_plot.value_mapper" + trait("component").then(trait("index_mapper") | trait("value_mapper")), + trait("_plot").then( + trait("index_mapper", optional=True) \ + | trait("value_mapper", optional=True) + ), ]) def _axis_mapper_updated(self, event): if event.name == self.axis + "_mapper": From 9dc4ba10f5e3788efc4e3d12829032ad4bb64de4 Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Thu, 15 Apr 2021 13:21:26 -0700 Subject: [PATCH 09/10] flake8 --- chaco/tools/tests/test_range_selection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chaco/tools/tests/test_range_selection.py b/chaco/tools/tests/test_range_selection.py index ebf8e6088..225eeee5d 100644 --- a/chaco/tools/tests/test_range_selection.py +++ b/chaco/tools/tests/test_range_selection.py @@ -3,7 +3,7 @@ import numpy as np -from chaco.api import LinearMapper, PlotComponent +from chaco.api import LinearMapper from chaco.array_plot_data import ArrayPlotData from chaco.plot import Plot from chaco.tools.range_selection import RangeSelection From 7b54518e3a14ef49bbfe1a8c234b124b76ac8c1f Mon Sep 17 00:00:00 2001 From: Aaron Ayres Date: Thu, 15 Apr 2021 13:22:14 -0700 Subject: [PATCH 10/10] Revert "be more explicit about traits being optional in observer expression" This reverts commit 306e8d99473db314a5abffee7bb76876d586cf78. --- chaco/tools/range_selection.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/chaco/tools/range_selection.py b/chaco/tools/range_selection.py index 4d068568d..82475bbcd 100644 --- a/chaco/tools/range_selection.py +++ b/chaco/tools/range_selection.py @@ -20,7 +20,6 @@ Str, Trait, ) -from traits.observation.api import trait from enable.api import KeySpec # Chaco imports @@ -679,11 +678,10 @@ def _determine_axis(self): return 0 @observe([ - trait("component").then(trait("index_mapper") | trait("value_mapper")), - trait("_plot").then( - trait("index_mapper", optional=True) \ - | trait("value_mapper", optional=True) - ), + "component.index_mapper", + "component.value_mapper", + "_plot.index_mapper", + "_plot.value_mapper" ]) def _axis_mapper_updated(self, event): if event.name == self.axis + "_mapper":