Fix the logic for region display when flipping x/y world coordinates

glue/viewers/image/tests/test_viewer.py

@@ -6,8 +6,10 @@
 from glue.core.data import Data
 from glue.core.link_helpers import LinkSame
 from glue.core.data_region import RegionData
+from astropy.wcs import WCS
 
-from shapely.geometry import Polygon, MultiPolygon
+from shapely.geometry import Polygon, MultiPolygon, Point
+import shapely
 
 
 @visual_test
@@ -97,3 +99,59 @@ def test_region_layer_flip():
         viewer.state.y_att = image_data.pixel_component_ids[1]
 
     return viewer.figure
+
+
+class TestWCSRegionDisplay(object):
+    def setup_method(self, method):
+
+        wcs1 = WCS(naxis=2)
+        wcs1.wcs.ctype = 'RA---TAN', 'DEC--TAN'
+        wcs1.wcs.crpix = -3, 5
+        wcs1.wcs.cd = [[2, -1], [1, 2]]
+
+        wcs1.wcs.set()
+
+        self.image1 = Data(label='image1', a=[[3, 3], [2, 2]], b=[[4, 4], [3, 2]],
+                            coords=wcs1)
+        SHAPELY_CIRCLE_ARRAY = np.array([Point(1.5, 2.5).buffer(1), Polygon([(1, 1), (2, 2), (2, 3), (1, 3)])])
+        self.region_data = RegionData(label='My Regions',
+                                      color=np.array(['red', 'blue']),
+                                      area=shapely.area(SHAPELY_CIRCLE_ARRAY),
+                                      boundary=SHAPELY_CIRCLE_ARRAY)
+        self.application = Application()
+
+        self.application.data_collection.append(self.image1)
+        self.application.data_collection.append(self.region_data)
+
+        self.viewer = self.application.new_data_viewer(SimpleImageViewer)
+
+    @visual_test
+    def test_flipped_viewer(self):
+        self.viewer.add_data(self.image1)
+
+        link1 = LinkSame(self.region_data.center_x_id, self.image1.world_component_ids[1])
+        link2 = LinkSame(self.region_data.center_y_id, self.image1.world_component_ids[0])
+
+        self.application.data_collection.add_link(link1)
+        self.application.data_collection.add_link(link2)
+
+        self.viewer.add_data(self.region_data)
+        original_path_patch = self.viewer.layers[1].region_collection.patches[1].get_path().vertices
+
+        # Flip x,y in the viewer
+        with delay_callback(self.viewer.state, 'x_att_world', 'y_att_world', 'x_att', 'y_att'):
+            self.viewer.state.x_att_world = self.image1.world_component_ids[0]
+            self.viewer.state.y_att_world = self.image1.world_component_ids[1]
+            self.viewer.state.x_att = self.image1.pixel_component_ids[0]
+            self.viewer.state.y_att = self.image1.pixel_component_ids[1]
+
+        assert self.viewer.state._display_world is True
+        assert len(self.viewer.state.layers) == 2
+        assert self.viewer.layers[0].enabled
+        assert self.viewer.layers[1].enabled
+        new_path_patch = self.viewer.layers[1].region_collection.patches[1].get_path().vertices
+
+        # Because we have flipped the viewer, the patches should have changed
+        assert np.array_equal(original_path_patch, np.flip(new_path_patch, axis=1))
+
+        return self.viewer.figure

glue/viewers/scatter/layer_artist.py

@@ -636,67 +636,64 @@ def _update_data(self):
                 data = self.layer.data
             region_att = data.extended_component_id
 
+        # In order to display a region we need to check that the representative
+        # points (center_x_id and center_y_id) are linked to the viewer x and y
+        # attributes. While we check this we also get the _values_ of the center
+        # attributes (x,y) and the _values_ for the viewer x and y attributes
+        # (xx,yy). We can compare these to figure out if the regions need to be
+        # flipped x,y <-> y,x. For now, if the image is displayed in world coordinates,
+        # the regions must be specified in world coordinates as well.
+
         try:
-            # These must be special attributes that are linked to a region_att
-            if ((not data.linked_to_center_comp(self._viewer_state.x_att)) and
-                     (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())
+            # These must be attributes that are linked to center_x_id
+            if self._viewer_state._display_world:
+                if not data.linked_to_center_comp(self._viewer_state.x_att_world):
+                    raise IncompatibleAttribute
+                else:
+                    xx = ensure_numerical(self.layer[self._viewer_state.x_att_world].ravel())
+            else:
+                if not data.linked_to_center_comp(self._viewer_state.x_att):
+                    raise IncompatibleAttribute
+                else:
+                    xx = ensure_numerical(self.layer[self._viewer_state.x_att].ravel())
+            x = ensure_numerical(self.layer[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)
             return
         else:
             self.enable()
 
         try:
-            # These must be special attributes that are linked to a region_att
-            if ((not data.linked_to_center_comp(self._viewer_state.y_att)) and
-                      (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())
+            # These must be attributes that are linked to center_y_id
+            if self._viewer_state._display_world:
+                if not data.linked_to_center_comp(self._viewer_state.y_att_world):
+                    raise IncompatibleAttribute
+                else:
+                    yy = ensure_numerical(self.layer[self._viewer_state.y_att_world].ravel())
+            else:
+                if not data.linked_to_center_comp(self._viewer_state.y_att):
+                    raise IncompatibleAttribute
+                else:
+                    yy = ensure_numerical(self.layer[self._viewer_state.y_att].ravel())
+
+            y = ensure_numerical(self.layer[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
+            # 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
+
             try:
+                # First, convert regions to world coordinates (relevant if there are multiple links or a
+                # transform is needed to get 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])
 
@@ -707,13 +704,35 @@ def flip_xy(g):
                 self.disable_invalid_attributes([self._viewer_state.x_att_world, self._viewer_state.y_att_world])
                 return
         else:
+            # If the image is just in pixels we just lookup how to transform the points in the region
             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
 
+        # Now we flip the x and y coordinates of each point in the regions if necessary.
+        # This has to happen after the transform into pixel coordinates.
+        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 = np.array([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
+
         # decompose GeometryCollections
         geoms, multiindex = _sanitize_geoms(regions, prefix="Geom")
         self.multiindex_geometry = multiindex