Merge pull request #2465 from jfoster17/improve-region-axis-checking

Provide stronger checking for region display. Fix x/y flip.

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"

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

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,30 +656,63 @@ 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)
             return
         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)