From ffac221c9c5b2ca6b16bae49443ee72318e4a49d Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 7 Dec 2016 15:03:22 -0600 Subject: [PATCH 1/5] Set `__module__` on curried objects. This can fix pickling global curried objects. Should fix dask/distributed#725 This was an oversight that should have been handled in #326. We only tested objects defined in `toolz.functoolz`, so pickling accidentally worked, because they were in the same module as `curry`. --- toolz/functoolz.py | 1 + toolz/tests/test_functoolz.py | 7 +++++++ toolz/tests/test_serialization.py | 7 +++++++ 3 files changed, 15 insertions(+) diff --git a/toolz/functoolz.py b/toolz/functoolz.py index ed37c0a1..8324c22d 100644 --- a/toolz/functoolz.py +++ b/toolz/functoolz.py @@ -200,6 +200,7 @@ def __init__(self, *args, **kwargs): self.__doc__ = getattr(func, '__doc__', None) self.__name__ = getattr(func, '__name__', '') + self.__module__ = getattr(func, '__module__', None) self._sigspec = None self._has_unknown_args = None diff --git a/toolz/tests/test_functoolz.py b/toolz/tests/test_functoolz.py index 6c7cb0ea..e37566b6 100644 --- a/toolz/tests/test_functoolz.py +++ b/toolz/tests/test_functoolz.py @@ -289,12 +289,19 @@ def foo(a, b, c=1): f = curry(foo, 1, c=2) f.__name__ = 'newname' f.__doc__ = 'newdoc' + f.__module__ = 'newmodule' assert f.__name__ == 'newname' assert f.__doc__ == 'newdoc' + assert f.__module__ == 'newmodule' if hasattr(f, 'func_name'): assert f.__name__ == f.func_name +def test_curry_module(): + from toolz.curried.exceptions import merge + assert merge.__module__ == 'toolz.curried.exceptions' + + def test_curry_comparable(): def foo(a, b, c=1): return a + b + c diff --git a/toolz/tests/test_serialization.py b/toolz/tests/test_serialization.py index a49f7a87..8bdb8938 100644 --- a/toolz/tests/test_serialization.py +++ b/toolz/tests/test_serialization.py @@ -1,5 +1,6 @@ from toolz import * import toolz +import toolz.curried.exceptions import pickle @@ -55,3 +56,9 @@ def test_flip(): g1 = flip(f)(1) g2 = pickle.loads(pickle.dumps(g1)) assert g1(2) == g2(2) == f(2, 1) + + +def test_curried_exceptions(): + # This tests a global curried object that isn't defined in toolz.functoolz + merge = pickle.loads(pickle.dumps(toolz.curried.exceptions.merge)) + assert merge is toolz.curried.exceptions.merge From 07359add1d2f5498696ec6b1a26cb431a08003a0 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Thu, 8 Dec 2016 01:28:17 -0600 Subject: [PATCH 2/5] Use `__qualname__` to help serialize curried objects. This isn't pretty, but hopefully tests provide adequate coverage. This includes a signficant change to `curry`: we defined `__getattr__` to get attributes from the wrapped func. --- toolz/functoolz.py | 44 ++++++++---- toolz/tests/test_serialization.py | 115 ++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 13 deletions(-) diff --git a/toolz/functoolz.py b/toolz/functoolz.py index 8324c22d..b4b856af 100644 --- a/toolz/functoolz.py +++ b/toolz/functoolz.py @@ -259,6 +259,9 @@ def keywords(self): def func_name(self): return self.__name__ + def __getattr__(self, attr): + return getattr(self.func, attr) + def __str__(self): return str(self.func) @@ -325,27 +328,42 @@ def __get__(self, instance, owner): def __reduce__(self): func = self.func modname = getattr(func, '__module__', None) - funcname = getattr(func, '__name__', None) - if modname and funcname: - module = import_module(modname) - obj = getattr(module, funcname, None) - if obj is self: - return funcname - elif isinstance(obj, curry) and obj.func is func: - func = '%s.%s' % (modname, funcname) + qualname = getattr(func, '__qualname__', None) + if qualname is None: + qualname = getattr(func, '__name__', None) + is_decorated = None + if modname and qualname: + attrs = [] + obj = import_module(modname) + for attr in qualname.split('.'): + if isinstance(obj, curry): + attrs.append('func') + obj = obj.func + obj = getattr(obj, attr, None) + if obj is None: + break + attrs.append(attr) + if isinstance(obj, curry) and obj.func is func: + is_decorated = obj is self + qualname = '.'.join(attrs) + func = '%s:%s' % (modname, qualname) # functools.partial objects can't be pickled userdict = tuple((k, v) for k, v in self.__dict__.items() if k != '_partial') - state = (type(self), func, self.args, self.keywords, userdict) + state = (type(self), func, self.args, self.keywords, userdict, is_decorated) return (_restore_curry, state) -def _restore_curry(cls, func, args, kwargs, userdict): +def _restore_curry(cls, func, args, kwargs, userdict, is_decorated): if isinstance(func, str): - modname, funcname = func.rsplit('.', 1) - module = import_module(modname) - func = getattr(module, funcname).func + modname, qualname = func.rsplit(':', 1) + obj = import_module(modname) + for attr in qualname.split('.'): + obj = getattr(obj, attr) + if is_decorated: + return obj + func = obj.func obj = cls(func, *args, **(kwargs or {})) obj.__dict__.update(userdict) return obj diff --git a/toolz/tests/test_serialization.py b/toolz/tests/test_serialization.py index 8bdb8938..5fc43faa 100644 --- a/toolz/tests/test_serialization.py +++ b/toolz/tests/test_serialization.py @@ -2,6 +2,7 @@ import toolz import toolz.curried.exceptions import pickle +from toolz.compatibility import PY3 def test_compose(): @@ -62,3 +63,117 @@ def test_curried_exceptions(): # This tests a global curried object that isn't defined in toolz.functoolz merge = pickle.loads(pickle.dumps(toolz.curried.exceptions.merge)) assert merge is toolz.curried.exceptions.merge + + +@toolz.curry +class GlobalCurried(object): + def __init__(self, x, y): + self.x = x + self.y = y + + @toolz.curry + def f1(self, a, b): + return self.x + self.y + a + b + + def g1(self): + pass + + def __reduce__(self): + """Allow us to serialize instances of GlobalCurried""" + return (GlobalCurried, (self.x, self.y)) + + @toolz.curry + class NestedCurried(object): + def __init__(self, x, y): + self.x = x + self.y = y + + @toolz.curry + def f2(self, a, b): + return self.x + self.y + a + b + + def g2(self): + pass + + def __reduce__(self): + """Allow us to serialize instances of NestedCurried""" + return (GlobalCurried.NestedCurried, (self.x, self.y)) + + class Nested(object): + def __init__(self, x, y): + self.x = x + self.y = y + + @toolz.curry + def f3(self, a, b): + return self.x + self.y + a + b + + def g3(self): + pass + + +def test_curried_qualname(): + if not PY3: + return + def preserves_identity(obj): + return pickle.loads(pickle.dumps(obj)) is obj + + assert preserves_identity(GlobalCurried) + assert preserves_identity(GlobalCurried.func.f1) + assert preserves_identity(GlobalCurried.func.g1) + assert preserves_identity(GlobalCurried.func.NestedCurried) + assert preserves_identity(GlobalCurried.func.NestedCurried.func.f2) + assert preserves_identity(GlobalCurried.func.NestedCurried.func.g2) + assert preserves_identity(GlobalCurried.func.Nested) + assert preserves_identity(GlobalCurried.func.Nested.f3) + assert preserves_identity(GlobalCurried.func.Nested.g3) + + # Rely on curry.__getattr__ + assert preserves_identity(GlobalCurried.f1) + assert preserves_identity(GlobalCurried.g1) + assert preserves_identity(GlobalCurried.NestedCurried) + assert preserves_identity(GlobalCurried.NestedCurried.f2) + assert preserves_identity(GlobalCurried.NestedCurried.g2) + assert preserves_identity(GlobalCurried.Nested) + assert preserves_identity(GlobalCurried.Nested.f3) + assert preserves_identity(GlobalCurried.Nested.g3) + + global_curried1 = GlobalCurried(1) + global_curried2 = pickle.loads(pickle.dumps(global_curried1)) + assert global_curried1 is not global_curried2 + assert global_curried1(2).f1(3, 4) == global_curried2(2).f1(3, 4) == 10 + + global_curried3 = global_curried1(2) + global_curried4 = pickle.loads(pickle.dumps(global_curried3)) + assert global_curried3 is not global_curried4 + assert global_curried3.f1(3, 4) == global_curried4.f1(3, 4) == 10 + + func1 = global_curried1(2).f1(3) + func2 = pickle.loads(pickle.dumps(func1)) + assert func1 is not func2 + assert func1(4) == func2(4) == 10 + + nested_curried1 = GlobalCurried.NestedCurried(1) + nested_curried2 = pickle.loads(pickle.dumps(nested_curried1)) + assert nested_curried1 is not nested_curried2 + assert nested_curried1(2).f2(3, 4) == nested_curried2(2).f2(3, 4) == 10 + + nested_curried3 = nested_curried1(2) + nested_curried4 = pickle.loads(pickle.dumps(nested_curried3)) + assert nested_curried3 is not nested_curried4 + assert nested_curried3.f2(3, 4) == nested_curried4.f2(3, 4) == 10 + + func1 = nested_curried1(2).f2(3) + func2 = pickle.loads(pickle.dumps(func1)) + assert func1 is not func2 + assert func1(4) == func2(4) == 10 + + nested3 = GlobalCurried.Nested(1, 2) + nested4 = pickle.loads(pickle.dumps(nested3)) + assert nested3 is not nested4 + assert nested3.f3(3, 4) == nested4.f3(3, 4) == 10 + + func1 = nested3.f3(3) + func2 = pickle.loads(pickle.dumps(func1)) + assert func1 is not func2 + assert func1(4) == func2(4) == 10 From 339086323206afb3a8fd2ab688d2e704e5a597d0 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Thu, 8 Dec 2016 01:39:55 -0600 Subject: [PATCH 3/5] Fix some coverage and pep8 issues. --- toolz/functoolz.py | 7 ++++--- toolz/tests/test_serialization.py | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/toolz/functoolz.py b/toolz/functoolz.py index b4b856af..204032c9 100644 --- a/toolz/functoolz.py +++ b/toolz/functoolz.py @@ -329,14 +329,14 @@ def __reduce__(self): func = self.func modname = getattr(func, '__module__', None) qualname = getattr(func, '__qualname__', None) - if qualname is None: + if qualname is None: # pragma: py3 no cover qualname = getattr(func, '__name__', None) is_decorated = None if modname and qualname: attrs = [] obj = import_module(modname) for attr in qualname.split('.'): - if isinstance(obj, curry): + if isinstance(obj, curry): # pragma: py2 no cover attrs.append('func') obj = obj.func obj = getattr(obj, attr, None) @@ -351,7 +351,8 @@ def __reduce__(self): # functools.partial objects can't be pickled userdict = tuple((k, v) for k, v in self.__dict__.items() if k != '_partial') - state = (type(self), func, self.args, self.keywords, userdict, is_decorated) + state = (type(self), func, self.args, self.keywords, userdict, + is_decorated) return (_restore_curry, state) diff --git a/toolz/tests/test_serialization.py b/toolz/tests/test_serialization.py index 5fc43faa..bc63bb9a 100644 --- a/toolz/tests/test_serialization.py +++ b/toolz/tests/test_serialization.py @@ -3,6 +3,7 @@ import toolz.curried.exceptions import pickle from toolz.compatibility import PY3 +from toolz.utils import raises def test_compose(): @@ -177,3 +178,11 @@ def preserves_identity(obj): func2 = pickle.loads(pickle.dumps(func1)) assert func1 is not func2 assert func1(4) == func2(4) == 10 + + +def test_curried_bad_qualname(): + @toolz.curry + class Bad(object): + __qualname__ = 'toolz.functoolz.not.a.valid.path' + + assert raises(pickle.PicklingError, lambda: pickle.dumps(Bad)) From 7d9ad0176eaf13989704968290ed236f002c1f3f Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Thu, 8 Dec 2016 01:59:25 -0600 Subject: [PATCH 4/5] Wow, Python 3.3 and 3.4 are terribly buggy with this. Skip tests. --- toolz/functoolz.py | 2 +- toolz/tests/test_serialization.py | 37 +++++++++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/toolz/functoolz.py b/toolz/functoolz.py index 204032c9..c3acc42e 100644 --- a/toolz/functoolz.py +++ b/toolz/functoolz.py @@ -350,7 +350,7 @@ def __reduce__(self): # functools.partial objects can't be pickled userdict = tuple((k, v) for k, v in self.__dict__.items() - if k != '_partial') + if k not in ('_partial', '_sigspec')) state = (type(self), func, self.args, self.keywords, userdict, is_decorated) return (_restore_curry, state) diff --git a/toolz/tests/test_serialization.py b/toolz/tests/test_serialization.py index bc63bb9a..e923caf7 100644 --- a/toolz/tests/test_serialization.py +++ b/toolz/tests/test_serialization.py @@ -2,7 +2,7 @@ import toolz import toolz.curried.exceptions import pickle -from toolz.compatibility import PY3 +from toolz.compatibility import PY3, PY33, PY34 from toolz.utils import raises @@ -121,23 +121,25 @@ def preserves_identity(obj): assert preserves_identity(GlobalCurried) assert preserves_identity(GlobalCurried.func.f1) - assert preserves_identity(GlobalCurried.func.g1) assert preserves_identity(GlobalCurried.func.NestedCurried) assert preserves_identity(GlobalCurried.func.NestedCurried.func.f2) - assert preserves_identity(GlobalCurried.func.NestedCurried.func.g2) - assert preserves_identity(GlobalCurried.func.Nested) assert preserves_identity(GlobalCurried.func.Nested.f3) - assert preserves_identity(GlobalCurried.func.Nested.g3) + if not PY33 and not PY34: + assert preserves_identity(GlobalCurried.func.g1) + assert preserves_identity(GlobalCurried.func.NestedCurried.func.g2) + assert preserves_identity(GlobalCurried.func.Nested) + assert preserves_identity(GlobalCurried.func.Nested.g3) # Rely on curry.__getattr__ assert preserves_identity(GlobalCurried.f1) - assert preserves_identity(GlobalCurried.g1) assert preserves_identity(GlobalCurried.NestedCurried) assert preserves_identity(GlobalCurried.NestedCurried.f2) - assert preserves_identity(GlobalCurried.NestedCurried.g2) - assert preserves_identity(GlobalCurried.Nested) assert preserves_identity(GlobalCurried.Nested.f3) - assert preserves_identity(GlobalCurried.Nested.g3) + if not PY33 and not PY34: + assert preserves_identity(GlobalCurried.g1) + assert preserves_identity(GlobalCurried.NestedCurried.g2) + assert preserves_identity(GlobalCurried.Nested) + assert preserves_identity(GlobalCurried.Nested.g3) global_curried1 = GlobalCurried(1) global_curried2 = pickle.loads(pickle.dumps(global_curried1)) @@ -169,15 +171,16 @@ def preserves_identity(obj): assert func1 is not func2 assert func1(4) == func2(4) == 10 - nested3 = GlobalCurried.Nested(1, 2) - nested4 = pickle.loads(pickle.dumps(nested3)) - assert nested3 is not nested4 - assert nested3.f3(3, 4) == nested4.f3(3, 4) == 10 + if not PY33 and not PY34: + nested3 = GlobalCurried.Nested(1, 2) + nested4 = pickle.loads(pickle.dumps(nested3)) + assert nested3 is not nested4 + assert nested3.f3(3, 4) == nested4.f3(3, 4) == 10 - func1 = nested3.f3(3) - func2 = pickle.loads(pickle.dumps(func1)) - assert func1 is not func2 - assert func1(4) == func2(4) == 10 + func1 = nested3.f3(3) + func2 = pickle.loads(pickle.dumps(func1)) + assert func1 is not func2 + assert func1(4) == func2(4) == 10 def test_curried_bad_qualname(): From 0ef082ca863cd5d017b2d4b99ce2bd204d0c344b Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Fri, 9 Dec 2016 14:43:06 -0600 Subject: [PATCH 5/5] Remove `curry.__getattr__` forwarding and comment out the tests this affects. IMHO, this is a failure of how Python handles `__qualname__` during pickling. Objects wrapped by decorators should only need to use `__wrapped__` for pickling to work as expected. It's unreasonable to expect wrapping objects such as `curry` to forward attributes to allow pickling to work. --- toolz/functoolz.py | 4 +- toolz/tests/test_functoolz.py | 5 +- toolz/tests/test_serialization.py | 78 ++++++++++++++++--------------- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/toolz/functoolz.py b/toolz/functoolz.py index c3acc42e..2bd80a4d 100644 --- a/toolz/functoolz.py +++ b/toolz/functoolz.py @@ -201,6 +201,7 @@ def __init__(self, *args, **kwargs): self.__doc__ = getattr(func, '__doc__', None) self.__name__ = getattr(func, '__name__', '') self.__module__ = getattr(func, '__module__', None) + self.__qualname__ = getattr(func, '__qualname__', None) self._sigspec = None self._has_unknown_args = None @@ -259,9 +260,6 @@ def keywords(self): def func_name(self): return self.__name__ - def __getattr__(self, attr): - return getattr(self.func, attr) - def __str__(self): return str(self.func) diff --git a/toolz/tests/test_functoolz.py b/toolz/tests/test_functoolz.py index e37566b6..deda6068 100644 --- a/toolz/tests/test_functoolz.py +++ b/toolz/tests/test_functoolz.py @@ -285,14 +285,17 @@ def foo(a, b, c=1): def test_curry_attributes_writable(): def foo(a, b, c=1): return a + b + c - + foo.__qualname__ = 'this.is.foo' f = curry(foo, 1, c=2) + assert f.__qualname__ == 'this.is.foo' f.__name__ = 'newname' f.__doc__ = 'newdoc' f.__module__ = 'newmodule' + f.__qualname__ = 'newqualname' assert f.__name__ == 'newname' assert f.__doc__ == 'newdoc' assert f.__module__ == 'newmodule' + assert f.__qualname__ == 'newqualname' if hasattr(f, 'func_name'): assert f.__name__ == f.func_name diff --git a/toolz/tests/test_serialization.py b/toolz/tests/test_serialization.py index e923caf7..afee159f 100644 --- a/toolz/tests/test_serialization.py +++ b/toolz/tests/test_serialization.py @@ -116,6 +116,7 @@ def g3(self): def test_curried_qualname(): if not PY3: return + def preserves_identity(obj): return pickle.loads(pickle.dumps(obj)) is obj @@ -124,22 +125,6 @@ def preserves_identity(obj): assert preserves_identity(GlobalCurried.func.NestedCurried) assert preserves_identity(GlobalCurried.func.NestedCurried.func.f2) assert preserves_identity(GlobalCurried.func.Nested.f3) - if not PY33 and not PY34: - assert preserves_identity(GlobalCurried.func.g1) - assert preserves_identity(GlobalCurried.func.NestedCurried.func.g2) - assert preserves_identity(GlobalCurried.func.Nested) - assert preserves_identity(GlobalCurried.func.Nested.g3) - - # Rely on curry.__getattr__ - assert preserves_identity(GlobalCurried.f1) - assert preserves_identity(GlobalCurried.NestedCurried) - assert preserves_identity(GlobalCurried.NestedCurried.f2) - assert preserves_identity(GlobalCurried.Nested.f3) - if not PY33 and not PY34: - assert preserves_identity(GlobalCurried.g1) - assert preserves_identity(GlobalCurried.NestedCurried.g2) - assert preserves_identity(GlobalCurried.Nested) - assert preserves_identity(GlobalCurried.Nested.g3) global_curried1 = GlobalCurried(1) global_curried2 = pickle.loads(pickle.dumps(global_curried1)) @@ -156,31 +141,50 @@ def preserves_identity(obj): assert func1 is not func2 assert func1(4) == func2(4) == 10 - nested_curried1 = GlobalCurried.NestedCurried(1) + nested_curried1 = GlobalCurried.func.NestedCurried(1) nested_curried2 = pickle.loads(pickle.dumps(nested_curried1)) assert nested_curried1 is not nested_curried2 assert nested_curried1(2).f2(3, 4) == nested_curried2(2).f2(3, 4) == 10 - nested_curried3 = nested_curried1(2) - nested_curried4 = pickle.loads(pickle.dumps(nested_curried3)) - assert nested_curried3 is not nested_curried4 - assert nested_curried3.f2(3, 4) == nested_curried4.f2(3, 4) == 10 - - func1 = nested_curried1(2).f2(3) - func2 = pickle.loads(pickle.dumps(func1)) - assert func1 is not func2 - assert func1(4) == func2(4) == 10 - - if not PY33 and not PY34: - nested3 = GlobalCurried.Nested(1, 2) - nested4 = pickle.loads(pickle.dumps(nested3)) - assert nested3 is not nested4 - assert nested3.f3(3, 4) == nested4.f3(3, 4) == 10 - - func1 = nested3.f3(3) - func2 = pickle.loads(pickle.dumps(func1)) - assert func1 is not func2 - assert func1(4) == func2(4) == 10 + # If we add `curry.__getattr__` forwarding, the following tests will pass + + # if not PY33 and not PY34: + # assert preserves_identity(GlobalCurried.func.g1) + # assert preserves_identity(GlobalCurried.func.NestedCurried.func.g2) + # assert preserves_identity(GlobalCurried.func.Nested) + # assert preserves_identity(GlobalCurried.func.Nested.g3) + # + # # Rely on curry.__getattr__ + # assert preserves_identity(GlobalCurried.f1) + # assert preserves_identity(GlobalCurried.NestedCurried) + # assert preserves_identity(GlobalCurried.NestedCurried.f2) + # assert preserves_identity(GlobalCurried.Nested.f3) + # if not PY33 and not PY34: + # assert preserves_identity(GlobalCurried.g1) + # assert preserves_identity(GlobalCurried.NestedCurried.g2) + # assert preserves_identity(GlobalCurried.Nested) + # assert preserves_identity(GlobalCurried.Nested.g3) + # + # nested_curried3 = nested_curried1(2) + # nested_curried4 = pickle.loads(pickle.dumps(nested_curried3)) + # assert nested_curried3 is not nested_curried4 + # assert nested_curried3.f2(3, 4) == nested_curried4.f2(3, 4) == 10 + # + # func1 = nested_curried1(2).f2(3) + # func2 = pickle.loads(pickle.dumps(func1)) + # assert func1 is not func2 + # assert func1(4) == func2(4) == 10 + # + # if not PY33 and not PY34: + # nested3 = GlobalCurried.func.Nested(1, 2) + # nested4 = pickle.loads(pickle.dumps(nested3)) + # assert nested3 is not nested4 + # assert nested3.f3(3, 4) == nested4.f3(3, 4) == 10 + # + # func1 = nested3.f3(3) + # func2 = pickle.loads(pickle.dumps(func1)) + # assert func1 is not func2 + # assert func1(4) == func2(4) == 10 def test_curried_bad_qualname():