chaco
chaco copied to clipboard
DataRange2D set_bounds method fires multiple events
Problem Description
The goal of set_bounds
is to update all the bounds simultaneously under one event. Current implementation fires multiple events (depending on how many values have changed).
Reproduction Steps:
Checked by writing a unittest for DataRange2D
def test_set_bounds_events(self):
with self.subTest(msg="DataRange2D"):
r = DataRange2D()
with self.assertTraitChanges(r, 'updated', count=1):
r.set_bounds((-1, -2), (3, 4))
with self.subTest(msg="DataRange2D.x_range"):
r = DataRange2D()
with self.assertTraitChanges(r.x_range, 'updated', count=1):
r.set_bounds((-1, -2), (3, 4))
with self.subTest(msg="DataRange2D.y_range"):
r = DataRange2D()
with self.assertTraitChanges(r.y_range, 'updated', count=1):
r.set_bounds((-1, -2), (3, 4))
Output from each subtest:
FAIL: test_set_bounds_events (chaco.tests.test_datarange_2d.DataRange2DTestCase) [DataRange2D]
...
AssertionError: Change event for updated was fired 4 times instead of 1
FAIL: test_set_bounds_events (chaco.tests.test_datarange_2d.DataRange2DTestCase) [DataRange2D.x_range]
...
AssertionError: Change event for updated was fired 2 times instead of 1
FAIL: test_set_bounds_events (chaco.tests.test_datarange_2d.DataRange2DTestCase) [DataRange2D.y_range]
...
AssertionError: Change event for updated was fired 2 times instead of 1
Expected behavior:
The call to this method should fire only one event on DataRange2D and associated DataRange1D instances.
Hah, this is a fun one. For the first example, at least, the DataRange2D sets the bounds of x_range
and y_range
individually, and each of those generates an updated
event. The weird thing is that it claims to do the update of the lower bounds at least without firing an event (via the fire_event=False
parameter), but that parameter has no effect in the code. https://github.com/enthought/chaco/blob/192675d6cb579df56458b87988026d895e7a7a84/chaco/data_range_2d.py#L103
I think chaco could be more parsimonious with the number of events that it fires, but I don't know if changing this behaviour would have averse consequences elsewhere. I'd be interested in @corranwebster's take on this.
Nice work narrowing things down to a single (data) class!