From c807eb0ac3ecd00e493be29246501fbc985bfda3 Mon Sep 17 00:00:00 2001 From: Jonathan Foster Date: Tue, 5 Dec 2023 10:17:36 -0500 Subject: [PATCH] Provide stronger checking for region display. Fix x/y flip. --- glue/tests/visual/py311-test-visual.json | 3 +- glue/viewers/image/tests/test_viewer.py | 40 +++++++++++++++- glue/viewers/scatter/layer_artist.py | 60 ++++++++++++++++++++---- 3 files changed, 91 insertions(+), 12 deletions(-) diff --git a/glue/tests/visual/py311-test-visual.json b/glue/tests/visual/py311-test-visual.json index 0211b4d82..f0cff7ef1 100644 --- a/glue/tests/visual/py311-test-visual.json +++ b/glue/tests/visual/py311-test-visual.json @@ -1,7 +1,8 @@ { "glue.viewers.histogram.tests.test_viewer.test_simple_viewer": "cb08123fbad135ab614bb7ec13475fcc83321057d884fe80c3a32970b2d14762", "glue.viewers.image.tests.test_viewer.test_simple_viewer": "72abd60b484d14f721254f027bb0ab9b36245d5db77eb87693f4dd9998fd28be", - "glue.viewers.image.tests.test_viewer.test_region_layer": "b54088d1f31562d309d35c5e90c0a382c7ba4c16cc0db8134e9532f71f047076", + "glue.viewers.image.tests.test_viewer.test_region_layer": "0114922ab0a3980f56252656c69545927841aea0e6950250cdc2b1bafcd19d50", + "glue.viewers.image.tests.test_viewer.test_region_layer_flip": "a142142f34961aba7e98188ad43abafe0e6e5b82e13e8cdab5131d297ed5832c", "glue.viewers.profile.tests.test_viewer.test_simple_viewer": "f68a21be5080fec513388b2d2b220512e7b0df5498e2489da54e58708de435b3", "glue.viewers.scatter.tests.test_viewer.test_simple_viewer": "1020a7bd3abe40510b9e03047c3b423b75c3c64ac18e6dcd6257173cec1ed53f", "glue.viewers.scatter.tests.test_viewer.test_scatter_density_map": "3379d655262769a6ccbdbaf1970bffa9237adbec23a93d3ab75da51b9a3e7f8b" diff --git a/glue/viewers/image/tests/test_viewer.py b/glue/viewers/image/tests/test_viewer.py index bb6f4606f..ec8b38133 100644 --- a/glue/viewers/image/tests/test_viewer.py +++ b/glue/viewers/image/tests/test_viewer.py @@ -1,5 +1,5 @@ import numpy as np - +from echo import delay_callback from glue.tests.visual.helpers import visual_test from glue.viewers.image.viewer import SimpleImageViewer from glue.core.application_base import Application @@ -58,6 +58,42 @@ def test_region_layer(): viewer.add_data(image_data) viewer.add_data(region_data) - app.data_collection.new_subset_group(label='subset1', subset_state=region_data.id['values'] > 2) + return viewer.figure + + +@visual_test +def test_region_layer_flip(): + poly_1 = Polygon([(20, 20), (60, 20), (60, 40), (20, 40)]) + poly_2 = Polygon([(60, 50), (60, 70), (80, 70), (80, 50)]) + poly_3 = Polygon([(10, 10), (15, 10), (15, 15), (10, 15)]) + poly_4 = Polygon([(10, 20), (15, 20), (15, 30), (10, 30), (12, 25)]) + + polygons = MultiPolygon([poly_3, poly_4]) + + geoms = np.array([poly_1, poly_2, polygons]) + values = np.array([1, 2, 3]) + region_data = RegionData(regions=geoms, values=values) + + image_data = Data(x=np.arange(10000).reshape((100, 100)), label='data1') + app = Application() + app.data_collection.append(image_data) + app.data_collection.append(region_data) + + link1 = LinkSame(region_data.center_x_id, image_data.pixel_component_ids[0]) + link2 = LinkSame(region_data.center_y_id, image_data.pixel_component_ids[1]) + app.data_collection.add_link(link1) + app.data_collection.add_link(link2) + + viewer = app.new_data_viewer(SimpleImageViewer) + viewer.add_data(image_data) + viewer.add_data(region_data) + + # We need this delay callback here because, while this works in the QT GUI, + # we need to make sure not to try and redraw the regions while we are flipping + # the coordinates displayed. + + with delay_callback(viewer.state, 'x_att', 'y_att'): + viewer.state.x_att = image_data.pixel_component_ids[0] + viewer.state.y_att = image_data.pixel_component_ids[1] return viewer.figure diff --git a/glue/viewers/scatter/layer_artist.py b/glue/viewers/scatter/layer_artist.py index 9bab17dbe..c6277ad46 100644 --- a/glue/viewers/scatter/layer_artist.py +++ b/glue/viewers/scatter/layer_artist.py @@ -642,6 +642,7 @@ def _update_data(self): (not data.linked_to_center_comp(self._viewer_state.x_att_world))): raise IncompatibleAttribute x = ensure_numerical(self.layer[self._viewer_state.x_att].ravel()) + xx = ensure_numerical(data[data.center_x_id].ravel()) except (IncompatibleAttribute, IndexError): # The following includes a call to self.clear() self.disable_invalid_attributes(self._viewer_state.x_att) @@ -655,6 +656,7 @@ def _update_data(self): (not data.linked_to_center_comp(self._viewer_state.y_att_world))): raise IncompatibleAttribute y = ensure_numerical(self.layer[self._viewer_state.y_att].ravel()) + yy = ensure_numerical(data[data.center_y_id].ravel()) except (IncompatibleAttribute, IndexError): # The following includes a call to self.clear() self.disable_invalid_attributes(self._viewer_state.y_att) @@ -662,23 +664,55 @@ def _update_data(self): else: self.enable() + # We need to make sure that x and y viewer attributes are + # really the center_x and center_y attributes of the underlying + # data, so we compare the values on the centroids using the + # glue data access machinery. + regions = self.layer[region_att] + def flip_xy(g): + return transform(lambda x, y: (y, x), g) + + x_no_match = False + if np.array_equal(y, yy): + if np.array_equal(x, xx): + self.enable() + else: + x_no_match = True + else: + if np.array_equal(y, xx) and np.array_equal(x, yy): # This means x and y have been swapped + regions = [flip_xy(g) for g in regions] + self.enable() + else: + self.disable_invalid_attributes(self._viewer_state.y_att) + if x_no_match: + self.disable_invalid_attributes(self._viewer_state.x_att) + return + # If we are using world coordinates (i.e. the regions are specified in world coordinates) # we need to transform the geometries of the regions into pixel coordinates for display # Note that this calls a custom version of the transform function from shapely # to accomodate glue WCS objects if self._viewer_state._display_world: # First, convert to world coordinates - tfunc = data.get_transform_to_cids([self._viewer_state.x_att_world, self._viewer_state.y_att_world]) - regions = np.array([transform(tfunc, g) for g in regions]) + try: + tfunc = data.get_transform_to_cids([self._viewer_state.x_att_world, self._viewer_state.y_att_world]) + regions = np.array([transform(tfunc, g) for g in regions]) - # Then convert to pixels for display - world2pix = self._viewer_state.reference_data.coords.world_to_pixel_values - regions = np.array([transform_shapely(world2pix, g) for g in regions]) + # Then convert to pixels for display + world2pix = self._viewer_state.reference_data.coords.world_to_pixel_values + regions = np.array([transform_shapely(world2pix, g) for g in regions]) + except ValueError: + self.disable_invalid_attributes([self._viewer_state.x_att_world, self._viewer_state.y_att_world]) + return else: - tfunc = data.get_transform_to_cids([self._viewer_state.x_att, self._viewer_state.y_att]) - regions = np.array([transform(tfunc, g) for g in regions]) + try: + tfunc = data.get_transform_to_cids([self._viewer_state.x_att, self._viewer_state.y_att]) + regions = np.array([transform(tfunc, g) for g in regions]) + except ValueError: + self.disable_invalid_attributes([self._viewer_state.x_att, self._viewer_state.y_att]) + return # decompose GeometryCollections geoms, multiindex = _sanitize_geoms(regions, prefix="Geom") @@ -753,8 +787,16 @@ def _update_scatter_region(self, force=False, **kwargs): full_sphere = getattr(self._viewer_state, 'using_full_sphere', False) change_from_limits = full_sphere and len(changed & LIMIT_PROPERTIES) > 0 if force or change_from_limits or len(changed & DATA_PROPERTIES) > 0: - self._update_data() - force = True + # This is the signature of flipping the x and y axis of an image in the UI + # We must *not* run update_data right away, or it will return an error + # The delay_callback wrapper on state._on_xatt_world_change() and + # state._on_yatt_world_change() is not properly deferring the callback + # until after x and y have been fully swapped, so we need to do it manually. + if changed == {'x_att_world', 'x_att', 'y_att_world'} or changed == {'y_att_world', 'y_att', 'x_att_world'}: + pass + else: + self._update_data() + force = True if force or len(changed & VISUAL_PROPERTIES) > 0: self._update_visual_attributes(changed, force=force)