Merge pull request #1139 from astrofrog/fix-serialization-coordinate-…

…link

Fail to save session after using advanced link function

CHANGES.md

@@ -2,7 +2,15 @@ Full changelog
 ==============
 
 v0.9.0 (unreleased)
------------------
+-------------------
+
+* Fix serialization of celestial coordinate link functions. Classes 
+  inheriting from MultiLink should now call MultiLink.__init__ with 
+  individual components (not grouped into left/right) then the create_links 
+  method with the components separated into left/right and the methods for 
+  forward/backward transformation. The original behavior can be retained
+  by using the ``multi_link`` function instead of the ``MultiLink`` class.
+  [#1139]
 
 * Improve support for spectral cubes. [#1075]
 
@@ -55,7 +63,7 @@ v0.9.0 (unreleased)
 
 * Added a command-line option, ``--no-maximized``, that prevents glue
   from opening up with the application window maximized. [#1093, #1126]
-  
+
 * When opening multiple files in one go, if one of the files fails to
   read, the error will now indicate which file failed. [#1104]
 
@@ -64,10 +72,10 @@ v0.9.0 (unreleased)
 
 * Fixed a bug that caused the functionality to execute scripts (glue -x) to not
   work in Python 3. [#1101, #1114]
-  
+
 * The minimum supported version of Astropy is now 1.0, and the minimum
   supported version of IPython is now 1.0. [#1076]
-  
+
 * Show world coordinates and units in the cube slicer. [#1059, #1068]
 
 * Fix errors that occurred when selecting categorical data. [#1069]

glue/core/component_link.py

@@ -12,7 +12,7 @@
 from glue.core.util import join_component_view
 
 
-__all__ = ['ComponentLink', 'BinaryComponentLink']
+__all__ = ['ComponentLink', 'BinaryComponentLink', 'CoordinateComponentLink']
 
 
 def identity(x):

glue/core/link_helpers.py

@@ -122,9 +122,15 @@ class MultiLink(LinkCollection):
               objects.
     """
 
-    def __init__(self, cids_left, cids_right, forwards=None, backwards=None):
-        cids_left = list(map(_toid, cids_left))
-        cids_right = list(map(_toid, cids_right))
+    cids = None
+
+    def __init__(self, *args):
+        self.cids = args
+
+    def create_links(self, cids_left, cids_right, forwards=None, backwards=None):
+
+        if self.cids is None:
+            raise Exception("MultiLink.__init__ was not called before creating links")
 
         if forwards is None and backwards is None:
             raise TypeError("Must supply either forwards or backwards")
@@ -139,6 +145,19 @@ def __init__(self, cids_left, cids_right, forwards=None, backwards=None):
                 func = PartialResult(backwards, i, name_prefix=self.__class__.__name__ + ".")
                 self.append(ComponentLink(cids_right, l, func))
 
+    def __gluestate__(self, context):
+        return {'cids': [context.id(cid) for cid in self.cids]}
+
+    @classmethod
+    def __setgluestate__(cls, rec, context):
+        return cls(*[context.object(cid) for cid in rec['cids']])
+
+
+def multi_link(cids_left, cids_right, forwards=None, backwards=None):
+    ml = MultiLink(cids_left + cids_right)
+    ml.create_links(cids_left, cids_right, forwards=forwards, backwards=backwards)
+    return ml
+
 
 class LinkAligned(LinkCollection):
 

glue/core/state.py

@@ -88,7 +88,11 @@ def load(rec, context)
 
 if six.PY2:
     literals += (long,)
-literals += (np.ScalarType,)
+
+
+literals += np.ScalarType
+
+
 
 # We need to make sure that we don't break backward-compatibility when we move
 # classes/functions around in Glue, so we have a file that maps the old paths to
@@ -253,7 +257,7 @@ def id(self, obj):
         if isinstance(obj, six.string_types):
             return 'st__%s' % obj
 
-        if isinstance(obj, literals):
+        if type(obj) in literals:
             return obj
 
         oid = id(obj)
@@ -291,7 +295,7 @@ def do(self, obj):
         if isinstance(obj, six.string_types):
             return 'st__' + obj
 
-        if isinstance(obj, literals):
+        if type(obj) in literals:
             return obj
 
         oid = id(obj)
@@ -305,6 +309,8 @@ def do(self, obj):
 
         if isinstance(obj, types.FunctionType):
             result['_type'] = 'types.FunctionType'
+        elif isinstance(obj, types.MethodType):
+            result['_type'] = 'types.MethodType'
         else:
             result['_type'] = "%s.%s" % (type(obj).__module__,
                                          type(obj).__name__)
@@ -852,6 +858,18 @@ def _load_function(rec, context):
     return lookup_class_with_patches(rec['function'])
 
 
+@saver(types.MethodType)
+def _save_method(method, context):
+    # Note: this only works for methods for which the class can be serialized
+    return {'instance': context.id(method.__self__), 'method': method.__name__}
+
+
+@loader(types.MethodType)
+def _load_method(rec, context):
+    instance = context.object(rec['instance'])
+    return getattr(instance, rec['method'])
+
+
 @saver(core.Session)
 def _save_session(session, context):
     # we will rely on GlueApplication to re-populate

glue/core/tests/test_link_helpers.py

@@ -8,7 +8,7 @@
 from glue.core import ComponentID, Data, Component, DataCollection
 
 from .. import link_helpers as lh
-from ..link_helpers import (LinkTwoWay, MultiLink,
+from ..link_helpers import (LinkTwoWay, multi_link,
                             LinkSame, LinkAligned)
 
 
@@ -44,7 +44,7 @@ def test_LinkTwoWay():
 
 
 def test_multilink_forwards():
-    result = MultiLink([R, D], [L, B], forwards)
+    result = multi_link([R, D], [L, B], forwards)
     assert len(result) == 2
     check_link(result[0], [R, D], L)
     check_link(result[1], [R, D], B)
@@ -53,7 +53,7 @@ def test_multilink_forwards():
 
 
 def test_multilink_backwards():
-    result = MultiLink([R, D], [L, B], backwards=backwards)
+    result = multi_link([R, D], [L, B], backwards=backwards)
     assert len(result) == 2
     check_link(result[0], [L, B], R)
     check_link(result[1], [L, B], D)
@@ -62,7 +62,7 @@ def test_multilink_backwards():
 
 
 def test_multilink_forwards_backwards():
-    result = MultiLink([R, D], [L, B], forwards, backwards)
+    result = multi_link([R, D], [L, B], forwards, backwards)
     assert len(result) == 4
     check_link(result[0], [R, D], L)
     check_link(result[1], [R, D], B)
@@ -76,7 +76,7 @@ def test_multilink_forwards_backwards():
 
 def test_multilink_nofunc():
     with pytest.raises(TypeError) as exc:
-        MultiLink([R, D], [L, B])
+        multi_link([R, D], [L, B])
     assert exc.value.args[0] == "Must supply either forwards or backwards"
 
 

glue/dialogs/link_editor/qt/link_equation.py

@@ -8,7 +8,7 @@
 from qtpy import PYSIDE
 from glue import core
 from glue.utils import nonpartial
-from glue.utils.qt import load_ui
+from glue.utils.qt import load_ui, messagebox_on_error
 
 __all__ = ['LinkEquation']
 
@@ -210,6 +210,7 @@ def function(self, val):
             raise KeyError("No function or helper found %s" % [val])
         self._ui.function.setCurrentIndex(pos)
 
+    @messagebox_on_error("Failed to create links")
     def links(self):
         """ Create ComponentLinks from the state of the widget
 

glue/plugins/coordinate_helpers/link_helpers.py

@@ -24,9 +24,9 @@ class BaseCelestialMultiLink(MultiLink):
     frame_out = None
 
     def __init__(self, in_lon, in_lat, out_lon, out_lat):
-        MultiLink.__init__(self, [in_lon, in_lat],
-                           [out_lon, out_lat],
-                           self.forward, self.backward)
+        super(BaseCelestialMultiLink, self).__init__(in_lon, in_lat, out_lon, out_lat)
+        self.create_links([in_lon, in_lat], [out_lon, out_lat],
+                          forwards=self.forward, backwards=self.backward)
 
     def forward(self, in_lon, in_lat):
         c = self.frame_in(in_lon * u.deg, in_lat * u.deg)

glue/plugins/coordinate_helpers/tests/test_link_helpers.py

@@ -7,6 +7,7 @@
 
 from glue.core import ComponentID
 from glue.core.tests.test_link_helpers import check_link, check_using
+from glue.core.tests.test_state import clone
 
 from ..link_helpers import (Galactic_to_FK5, FK4_to_FK5, ICRS_to_FK5,
                             Galactic_to_FK4, ICRS_to_FK4, ICRS_to_Galactic)
@@ -56,3 +57,10 @@ def test_conversion(conv_class, expected):
     check_using(result[1], (x, y), expected[0][1])
     check_using(result[2], (x, y), expected[1][0])
     check_using(result[3], (x, y), expected[1][1])
+
+    # Check that state saving works
+
+    check_using(clone(result[0]), (x, y), expected[0][0])
+    check_using(clone(result[1]), (x, y), expected[0][1])
+    check_using(clone(result[2]), (x, y), expected[1][0])
+    check_using(clone(result[3]), (x, y), expected[1][1])

glue/qglue.py

@@ -101,7 +101,7 @@ def parse_data(data, label):
 
 
 def parse_links(dc, links):
-    from glue.core.link_helpers import MultiLink
+    from glue.core.link_helpers import multi_link
     from glue.core import ComponentLink
 
     data = dict((d.label, d) for d in dc)
@@ -134,7 +134,7 @@ def find_cid(s):
             result.append(ComponentLink(f, t, u))
         else:
             t = [find_cid(item) for item in t]
-            result += MultiLink(f, t, u, u2)
+            result += multi_link(f, t, u, u2)
 
     return result