Implement linking support for all BaseCartesianData subclasses and fix bugs that caused hanging in image viewer

doc/developer_guide/data.rst

@@ -65,7 +65,11 @@ The methods you need to define are:
   (``view=[(1, 2, 3), (4, 3, 4)]``), and so on. If a view is specified, only that
   subset of values should be returned. For example if the data has an overall
   shape of ``(10,)`` and ``view=slice(1, 6, 2)``, ``get_data`` should
-  return an array with shape ``(3,)``.
+  return an array with shape ``(3,)``. By default, :meth:`BaseCartesianData.get_data <glue.core.data.BaseCartesianData.get_data>`
+  will return values for pixel and world :class:`~glue.core.component_id.ComponentID`
+  objects as well as any linked :class:`~glue.core.component_id.ComponentID`, so we
+  recommend that your implementation calls :meth:`BaseCartesianData.get_data <glue.core.data.BaseCartesianData.get_data>`
+  for any :class:`~glue.core.component_id.ComponentID` you do not expose yourself.
 * :meth:`~glue.core.data.BaseCartesianData.get_mask`: given a
   :class:`~glue.core.subset.SubsetState` object (described in `Subset states`_)
   and optionally a ``view``, return a boolean array describing which values are
@@ -138,6 +142,17 @@ While developing your data class, one way to make sure that glue doesn't crash
 if you haven't yet implemented support for a specific subset state is to
 interpret any unimplemented subset state as simply indicating an empty subset.
 
+Linking
+-------
+
+You should be able to link data objects that inherit from
+:class:`~glue.core.data.BaseCartesianData` with other datasets - however
+for this to work properly you should make sure that your implementation of
+:meth:`~glue.core.data.BaseCartesianData.get_data` calls
+:meth:`BaseCartesianData.get_data <glue.core.data.BaseCartesianData.get_data>`
+for any unrecognized :class:`~glue.core.component_id.ComponentID`, as the base
+implementation will handle returning linked values.
+
 Using your data object
 ----------------------
 

doc/developer_guide/random_data.py

@@ -27,10 +27,10 @@ def get_kind(self, cid):
         return 'numerical'
 
     def get_data(self, cid, view=None):
-        if cid in self.pixel_component_ids:
-            return super(RandomData, self).get_data(cid, view=view)
-        else:
+        if cid is self.data_cid:
             return np.random.random(view_shape(self.shape, view))
+        else:
+            return super(RandomData, self).get_data(cid, view=view)
 
     def get_mask(self, subset_state, view=None):
         return subset_state.to_mask(self, view=view)

glue/core/data.py

@@ -398,6 +398,8 @@ class BaseCartesianData(BaseData, metaclass=abc.ABCMeta):
     def __init__(self, coords=None):
         super(BaseCartesianData, self).__init__()
         self._coords = coords
+        self._externally_derivable_components = OrderedDict()
+        self._pixel_aligned_data = OrderedDict()
 
     @property
     def coords(self):
@@ -448,13 +450,21 @@ def get_data(self, cid, view=None):
                 return broadcast_to(pix, self.shape)[view]
         elif cid in self.world_component_ids:
             comp = self._world_components[cid]
-            if view is None:
-                return comp.data
-            else:
-                return comp[view]
+        elif cid in self._externally_derivable_components:
+            comp = self._externally_derivable_components[cid]
         else:
             raise IncompatibleAttribute(cid)
 
+        # Note that above we have extracted Component objects from internal
+        # properties - we don't actually expose Component objects in this class,
+        # only in the Data class, but we use these components internally for
+        # convenience.
+
+        if view is None:
+            return comp.data
+        else:
+            return comp[view]
+
     @abc.abstractmethod
     def get_mask(self, subset_state, view=None):
         """
@@ -596,13 +606,106 @@ def world_component_ids(self):
             self._world_component_ids = []
             self._world_components = {}
             for i in range(self.ndim):
+                # Note: we use a Component object here for convenience but we
+                # don't actually expose it via the BaseCartesianData API - in
+                # get_data we extract the data from the component.
                 comp = CoordinateComponent(self, i, world=True)
                 label = axis_label(self.coords, i)
                 cid = ComponentID(label, parent=self)
                 self._world_component_ids.append(cid)
                 self._world_components[cid] = comp
         return self._world_component_ids
 
+    def _set_externally_derivable_components(self, derivable_components):
+        """
+        Externally deriable components are components identified by component
+        IDs from other datasets.
+
+        This method is meant for internal use only and is called by the link
+        manager. The ``derivable_components`` argument should be set to a
+        dictionary where the keys are the derivable component IDs, and the
+        values are DerivedComponent instances which can be used to get the
+        data.
+        """
+
+        # Note that even though Component objects are not normally exposed as
+        # part of the BaseCartesianData API, we use these internally here as
+        # a convenience, and extract the data from them in get_data. The actual
+        # derived components are however used in the Data class.
+
+        if len(self._externally_derivable_components) == 0 and len(derivable_components) == 0:
+
+            return
+
+        elif len(self._externally_derivable_components) == len(derivable_components):
+
+            for key in derivable_components:
+                if key in self._externally_derivable_components:
+                    if self._externally_derivable_components[key].link is not derivable_components[key].link:
+                        break
+                else:
+                    break
+            else:
+                return  # Unchanged!
+
+        self._externally_derivable_components = derivable_components
+
+        if self.hub:
+            msg = ExternallyDerivableComponentsChangedMessage(self)
+            self.hub.broadcast(msg)
+
+    def _get_external_link(self, cid):
+        if cid in self._externally_derivable_components:
+            return self._externally_derivable_components[cid].link
+        else:
+            return None
+
+    def _get_coordinate_transform(self, world_cid):
+        if world_cid in self._world_components:
+            def transform(values):
+                return self._world_components._calculate(view=values)
+            return transform
+        else:
+            return None
+
+    def _set_pixel_aligned_data(self, pixel_aligned_data):
+        """
+        Pixel-aligned data are datasets that contain pixel component IDs
+        that are equivalent (identically, not transformed) with all pixel
+        component IDs in the present dataset.
+
+        Note that the other datasets may have more but not fewer dimensions, so
+        this information may not be symmetric between datasets with differing
+        numbers of dimensions.
+        """
+
+        # First check if anything has changed, as if not then we should just
+        # leave things as-is and avoid emitting a message.
+        if len(self._pixel_aligned_data) == len(pixel_aligned_data):
+            for data in self._pixel_aligned_data:
+                if data not in pixel_aligned_data or pixel_aligned_data[data] != self._pixel_aligned_data[data]:
+                    break
+            else:
+                return
+
+        self._pixel_aligned_data = pixel_aligned_data
+        if self.hub:
+            msg = PixelAlignedDataChangedMessage(self)
+            self.hub.broadcast(msg)
+
+    @property
+    def pixel_aligned_data(self):
+        """
+        Information about other datasets in the same data collection that have
+        matching or a subset of pixel component IDs.
+
+        This is returned as a dictionary where each key is a dataset with
+        matching pixel component IDs, and the value is the order in which the
+        pixel component IDs of the other dataset can be found in the current
+        one.
+        """
+        return self._pixel_aligned_data
+
 
 class Data(BaseCartesianData):
     """
@@ -650,8 +753,6 @@ def __init__(self, label="", coords=None, **kwargs):
 
         # Components
         self._components = OrderedDict()
-        self._externally_derivable_components = OrderedDict()
-        self._pixel_aligned_data = OrderedDict()
         self._pixel_component_ids = ComponentIDList()
         self._world_component_ids = ComponentIDList()
 
@@ -1007,77 +1108,6 @@ def add_component(self, component, label):
 
         return component_id
 
-    def _set_externally_derivable_components(self, derivable_components):
-        """
-        Externally deriable components are components identified by component
-        IDs from other datasets.
-
-        This method is meant for internal use only and is called by the link
-        manager. The ``derivable_components`` argument should be set to a
-        dictionary where the keys are the derivable component IDs, and the
-        values are DerivedComponent instances which can be used to get the
-        data.
-        """
-
-        if len(self._externally_derivable_components) == 0 and len(derivable_components) == 0:
-
-            return
-
-        elif len(self._externally_derivable_components) == len(derivable_components):
-
-            for key in derivable_components:
-                if key in self._externally_derivable_components:
-                    if self._externally_derivable_components[key].link is not derivable_components[key].link:
-                        break
-                else:
-                    break
-            else:
-                return  # Unchanged!
-
-        self._externally_derivable_components = derivable_components
-
-        if self.hub:
-            msg = ExternallyDerivableComponentsChangedMessage(self)
-            self.hub.broadcast(msg)
-
-    def _set_pixel_aligned_data(self, pixel_aligned_data):
-        """
-        Pixel-aligned data are datasets that contain pixel component IDs
-        that are equivalent (identically, not transformed) with all pixel
-        component IDs in the present dataset.
-
-        Note that the other datasets may have more but not fewer dimensions, so
-        this information may not be symmetric between datasets with differing
-        numbers of dimensions.
-        """
-
-        # First check if anything has changed, as if not then we should just
-        # leave things as-is and avoid emitting a message.
-        if len(self._pixel_aligned_data) == len(pixel_aligned_data):
-            for data in self._pixel_aligned_data:
-                if data not in pixel_aligned_data or pixel_aligned_data[data] != self._pixel_aligned_data[data]:
-                    break
-            else:
-                return
-
-        self._pixel_aligned_data = pixel_aligned_data
-        if self.hub:
-            msg = PixelAlignedDataChangedMessage(self)
-            self.hub.broadcast(msg)
-
-    @property
-    def pixel_aligned_data(self):
-        """
-        Information about other datasets in the same data collection that have
-        matching or a subset of pixel component IDs.
-
-        This is returned as a dictionary where each key is a dataset with
-        matching pixel component IDs, and the value is the order in which the
-        pixel component IDs of the other dataset can be found in the current
-        one.
-        """
-        return self._pixel_aligned_data
-
     @contract(link=ComponentLink,
               label='cid_like|None',
               returns=DerivedComponent)

glue/core/fixed_resolution_buffer.py

@@ -1,7 +1,7 @@
 import numpy as np
 from glue.core import Data
-from glue.core.exceptions import IncompatibleDataException
-from glue.core.component import CoordinateComponent, DaskComponent
+from glue.core.exceptions import IncompatibleAttribute, IncompatibleDataException
+from glue.core.component import DaskComponent
 from glue.core.coordinate_helpers import dependent_axes
 from glue.utils import unbroadcast, broadcast_to, broadcast_arrays_minimal
 
@@ -48,30 +48,35 @@ def translate_pixel(data, pixel_coords, target_cid):
     # TODO: check that things are efficient if the PixelComponentID is in a
     # pixel-aligned dataset.
 
-    component = data.get_component(target_cid)
-
-    if hasattr(component, 'link'):
-        link = component.link
-        values_all = []
-        dimensions_all = []
-        for cid in link._from:
-            values, dimensions = translate_pixel(data, pixel_coords, cid)
-            values_all.append(values)
-            dimensions_all.extend(dimensions)
-        # Unbroadcast arrays to smallest common shape for performance
-        if len(values_all) > 0:
-            shape = values_all[0].shape
-            values_all = broadcast_arrays_minimal(*values_all)
-            results = link._using(*values_all)
-            result = broadcast_to(results, shape)
+    link = data._get_external_link(target_cid)
+
+    if link is None:
+        if target_cid in data.main_components or target_cid in data.derived_components:
+            raise Exception("Dependency on non-pixel component", target_cid)
+        elif target_cid in data.coordinate_components:
+            if isinstance(data, Data):
+                comp = data.get_component(target_cid)
+            else:
+                comp = data._world_components[target_cid]
+            return comp._calculate(view=pixel_coords), dependent_axes(data.coords, comp.axis)
         else:
-            result = None
-        return result, sorted(set(dimensions_all))
-    elif isinstance(component, CoordinateComponent):
-        # FIXME: Hack for now - if we pass arrays in the view, it's interpreted
-        return component._calculate(view=pixel_coords), dependent_axes(data.coords, component.axis)
+            raise IncompatibleAttribute(target_cid)
+
+    values_all = []
+    dimensions_all = []
+    for cid in link._from:
+        values, dimensions = translate_pixel(data, pixel_coords, cid)
+        values_all.append(values)
+        dimensions_all.extend(dimensions)
+    # Unbroadcast arrays to smallest common shape for performance
+    if len(values_all) > 0:
+        shape = values_all[0].shape
+        values_all = broadcast_arrays_minimal(*values_all)
+        results = link._using(*values_all)
+        result = broadcast_to(results, shape)
     else:
-        raise Exception("Dependency on non-pixel component", target_cid)
+        result = None
+    return result, sorted(set(dimensions_all))
 
 
 class AnyScalar(object):

glue/core/layer_artist.py

@@ -124,6 +124,10 @@ def disable(self, reason):
            Used by the UI
         """
         self._disabled_reason = reason
+        # If layer is already disabled, avoid continuing to not repeatadly
+        # disable layer and emit messages which might force a redraw
+        if not self._enabled:
+            return
         self._enabled = False
         self.clear()
         if self._layer is not None and self._layer.hub is not None: