Merge pull request #2873 from ales-erjavec/feature-constructor-serialize

[FIX] Feature Constructor: Make FeatureFunc picklable

Author: lanzagar

Orange/__init__.py

@@ -1,13 +1,3 @@
-import pickle
-from unittest.mock import patch
-# Needed because the pure-Python Unpickler that dill uses can also fail
-# with struct.error Exception. This seems to work, side effects unknown.
-with patch('pickle._Unpickler', pickle.Unpickler):
-    import dill
-dill.settings['protocol'] = pickle.HIGHEST_PROTOCOL
-dill.settings['recurse'] = True
-dill.settings['byref'] = True
-
 from .misc.lazy_module import _LazyModule
 from .misc.datasets import _DatasetInfo
 from .version import \

Orange/widgets/data/owfeatureconstructor.py

@@ -14,10 +14,12 @@
 import random
 import logging
 import ast
+import types
 
 from traceback import format_exception_only
 from collections import namedtuple, OrderedDict
 from itertools import chain, count
+from typing import List, Dict, Any  # pylint: disable=unused-import
 
 import numpy as np
 
@@ -632,9 +634,6 @@ def send_report(self):
             report.plural("Constructed feature{s}", len(items)), items)
 
 
-
-
-
 def freevars(exp, env):
     """
     Return names of all free variables in a parsed (expression) AST.
@@ -838,46 +837,76 @@ def bind_variable(descriptor, env):
         (descriptor, (instance -> value) | (table -> value list))
     """
     if not descriptor.expression.strip():
-        return (descriptor, lambda _: float("nan"))
+        return descriptor, FeatureFunc("nan", [], {"nan": float("nan")})
 
     exp_ast = ast.parse(descriptor.expression, mode="eval")
     freev = unique(freevars(exp_ast, []))
     variables = {sanitized_name(v.name): v for v in env}
     source_vars = [(name, variables[name]) for name in freev
                    if name in variables]
 
-    values = []
+    values = {}
     if isinstance(descriptor, DiscreteDescriptor):
         values = [sanitized_name(v) for v in descriptor.values]
-    return descriptor, FeatureFunc(exp_ast, source_vars, values)
+        values = {name: i for i, name in enumerate(values)}
+    return descriptor, FeatureFunc(descriptor.expression, source_vars, values)
 
 
-def make_lambda(expression, args, values):
-    def make_arg(name):
-        if sys.version_info >= (3, 0):
-            return ast.arg(arg=name, annotation=None)
-        else:
-            return ast.Name(id=name, ctx=ast.Param(), lineno=1, col_offset=0)
+def make_lambda(expression, args, env={}):
+    # type: (ast.Expression, List[str], Dict[str, Any]) -> types.FunctionType
+    """
+    Create an lambda function from a expression AST.
 
+    Parameters
+    ----------
+    expression : ast.Expression
+        The body of the lambda.
+    args : List[str]
+        A list of positional argument names
+    env : Dict[str, Any]
+        Extra environment to capture in the lambda's closure.
+
+    Returns
+    -------
+    func : types.FunctionType
+    """
+    # lambda *{args}* : EXPRESSION
     lambda_ = ast.Lambda(
         args=ast.arguments(
-            args=[make_arg(arg) for arg in args + values],
+            args=[ast.arg(arg=arg, annotation=None) for arg in args],
             varargs=None,
             varargannotation=None,
             kwonlyargs=[],
             kwarg=None,
             kwargannotation=None,
-            defaults=[ast.Num(i) for i in range(len(values))],
+            defaults=[],
             kw_defaults=[]),
         body=expression.body,
     )
     lambda_ = ast.copy_location(lambda_, expression.body)
-    exp = ast.Expression(body=lambda_, lineno=1, col_offset=0)
-    ast.dump(exp)
+    # lambda **{env}** : lambda *{args}*: EXPRESSION
+    outer = ast.Lambda(
+        args=ast.arguments(
+            args=[ast.arg(arg=name, annotation=None) for name in env],
+            varargs=None,
+            varargannotation=None,
+            kwonlyargs=[],
+            kwarg=None,
+            kwargannotation=None,
+            defaults=[],
+            kw_defaults=[],
+        ),
+        body=lambda_,
+    )
+    exp = ast.Expression(body=outer, lineno=1, col_offset=0)
     ast.fix_missing_locations(exp)
     GLOBALS = __GLOBALS.copy()
     GLOBALS["__builtins__"] = {}
-    return eval(compile(exp, "<lambda>", "eval"), GLOBALS)
+    fouter = eval(compile(exp, "<lambda>", "eval"), GLOBALS)
+    assert isinstance(fouter, types.FunctionType)
+    finner = fouter(**env)
+    assert isinstance(finner, types.FunctionType)
+    return finner
 
 
 __ALLOWED = [
@@ -934,11 +963,26 @@ def make_arg(name):
 
 
 class FeatureFunc:
-    def __init__(self, expression, args, values):
+    """
+    Parameters
+    ----------
+    expression : str
+        An expression string
+    args : List[Tuple[str, Orange.data.Variable]]
+        A list of (`name`, `variable`) tuples where `name` is the name of
+        a variable as used in `expression`, and `variable` is the variable
+        instance used to extract the corresponding column/value from a
+        Table/Instance.
+    extra_env : Dict[str, Any]
+        Extra environment specifying constant values to be made available
+        in expression. It must not shadow names in `args`
+    """
+    def __init__(self, expression, args, extra_env={}):
         self.expression = expression
         self.args = args
-        self.values = values
-        self.func = make_lambda(expression, [name for name, _ in args], values)
+        self.extra_env = dict(extra_env)
+        self.func = make_lambda(ast.parse(expression, mode="eval"),
+                                [name for name, _ in args], self.extra_env)
 
     def __call__(self, instance, *_):
         if isinstance(instance, Orange.data.Table):
@@ -947,6 +991,12 @@ def __call__(self, instance, *_):
             args = [instance[var] for _, var in self.args]
             return self.func(*args)
 
+    def __reduce__(self):
+        return type(self), (self.expression, self.args, self.extra_env)
+
+    def __repr__(self):
+        return "{0.__name__}{1!r}".format(*self.__reduce__())
+
 
 def unique(seq):
     seen = set()
@@ -958,7 +1008,7 @@ def unique(seq):
     return unique_el
 
 
-def main(argv=None):
+def main(argv=None):  # pragma: no cover
     from AnyQt.QtWidgets import QApplication
     if argv is None:
         argv = sys.argv
@@ -981,5 +1031,6 @@ def main(argv=None):
     w.saveSettings()
     return 0
 
+
 if __name__ == "__main__":
     sys.exit(main())

Orange/widgets/data/tests/test_owfeatureconstructor.py

@@ -2,6 +2,8 @@
 import ast
 import sys
 import math
+import pickle
+import copy
 
 import numpy as np
 
@@ -14,9 +16,9 @@
     construct_variables, OWFeatureConstructor,
     FeatureEditor, DiscreteFeatureEditor)
 
-from Orange.widgets.data.owfeatureconstructor import freevars, validate_exp
-
-import dill as pickle  # Import dill after Orange because patched
+from Orange.widgets.data.owfeatureconstructor import (
+    freevars, validate_exp, FeatureFunc
+)
 
 
 class FeatureConstructorTest(unittest.TestCase):
@@ -89,27 +91,6 @@ def test_construct_numeric_names(self):
         ContinuousVariable._clear_all_caches()
 
 
-GLOBAL_CONST = 2
-
-
-class PicklingTest(unittest.TestCase):
-    CLASS_CONST = 3
-
-    def test_lambdas_pickle(self):
-        NONLOCAL_CONST = 5
-
-        lambda_func = lambda x, local_const=7: \
-            x * local_const * NONLOCAL_CONST * self.CLASS_CONST * GLOBAL_CONST
-
-        def nested_func(x, local_const=7):
-            return x * local_const * NONLOCAL_CONST * self.CLASS_CONST * GLOBAL_CONST
-
-        self.assertEqual(lambda_func(11),
-                         pickle.loads(pickle.dumps(lambda_func))(11))
-        self.assertEqual(nested_func(11),
-                         pickle.loads(pickle.dumps(nested_func))(11))
-
-
 class TestTools(unittest.TestCase):
     def test_free_vars(self):
         stmt = ast.parse("foo", "", "single")
@@ -218,6 +199,30 @@ def validate_(source):
             validate_("{a:1 for a in s}")
 
 
+class FeatureFuncTest(unittest.TestCase):
+    def test_reconstruct(self):
+        f = FeatureFunc("a * x + c", [("x", "x")], {"a": 2, "c": 10})
+        self.assertEqual(f({"x": 2}), 14)
+        f1 = pickle.loads(pickle.dumps(f))
+        self.assertEqual(f1({"x": 2}), 14)
+        fc = copy.copy(f)
+        self.assertEqual(fc({"x": 3}), 16)
+
+    def test_repr(self):
+        self.assertEqual(repr(FeatureFunc("a + 1", [("a", 2)])),
+                         "FeatureFunc('a + 1', [('a', 2)], {})")
+
+    def test_call(self):
+        f = FeatureFunc("a + 1", [("a", "a")])
+        self.assertEqual(f({"a": 2}), 3)
+
+        iris = Table("iris")
+        f = FeatureFunc("sepal_width + 10",
+                        [("sepal_width", iris.domain["sepal width"])])
+        r = f(iris)
+        np.testing.assert_array_equal(r, iris.X[:, 1] + 10)
+
+
 class OWFeatureConstructorTests(WidgetTest):
     def setUp(self):
         self.widget = OWFeatureConstructor()

Orange/widgets/model/owloadmodel.py

@@ -1,6 +1,6 @@
 import os
+import pickle
 
-import dill as pickle
 from AnyQt.QtCore import QTimer
 from AnyQt.QtWidgets import (
     QSizePolicy, QHBoxLayout, QComboBox, QStyle, QFileDialog

Orange/widgets/model/owsavemodel.py

@@ -1,6 +1,7 @@
 import os
 
-import dill as pickle
+import pickle
+
 from AnyQt.QtWidgets import (
     QComboBox, QStyle, QSizePolicy, QFileDialog, QApplication
 )

Orange/widgets/model/tests/test_owloadmodel.py

@@ -4,8 +4,6 @@
 import pickle
 from tempfile import mkstemp
 
-import dill    # Import dill after Orange because patched
-
 from Orange.classification.majority import ConstantModel
 from Orange.widgets.model.owloadmodel import OWLoadModel
 from Orange.widgets.tests.base import WidgetTest
@@ -23,11 +21,10 @@ def test_show_error(self):
         fd, fname = mkstemp(suffix='.pkcls')
         os.close(fd)
         try:
-            for pickle_impl in (pickle, dill):
-                with open(fname, 'wb') as f:
-                    pickle_impl.dump(clsf, f)
-                self.widget.load(fname)
-                self.assertFalse(self.widget.Error.load_error.is_shown())
+            with open(fname, 'wb') as f:
+                pickle.dump(clsf, f)
+            self.widget.load(fname)
+            self.assertFalse(self.widget.Error.load_error.is_shown())
 
             with open(fname, "w") as f:
                 f.write("X")

conda-recipe/meta.yaml

@@ -48,7 +48,6 @@ requirements:
     - anyqt         >=0.0.6
     - joblib
     - python.app    # [osx]
-    - dill          # pickle anything
     - commonmark
     - serverfiles
 

requirements-core.txt

@@ -11,6 +11,5 @@ chardet>=3.0.2
 joblib>=0.9.4
 keyring
 keyrings.alt    # for alternative keyring implementations
-dill
 setuptools>=36.3
 serverfiles		# for Data Sets synchronization

scripts/macos/requirements.txt

@@ -7,7 +7,6 @@
 AnyQt==0.0.8
 Bottleneck==1.2.0
 chardet==3.0.4
-dill==0.2.6
 docutils==0.13.1
 joblib==0.11
 keyring==10.3.1

scripts/windows/specs/PY34-win32.txt

@@ -18,7 +18,6 @@ AnyQt==0.0.8
 PyQt5==5.5.1
 docutils==0.13.1
 pip==9.0.1
-dill==0.2.6
 pyqtgraph==0.10.0
 six==1.10.0
 xlrd==1.0.0