Merge pull request #2 from coldfix/functools.partial

functools.partial is not usable

CHANGES.rst

@@ -1,6 +1,11 @@
 Changelog
 ~~~~~~~~~
 
+0.0.7
+-----
+
+- partial support for ``functools.partial``
+
 0.0.6
 -----
 

README.rst

@@ -91,8 +91,9 @@ If you want to get real crazy you can even use ast.expr_'s:
 .. _ast.expr: http://docs.python.org/3.3/library/ast.html?highlight=ast#abstract-grammar
 
 
-**WARNING**: Do not use ``wraps`` with ``functools.partial``! It won't
-work (if using any keyword bindings).
+**WARNING**: before using ``functools.partial`` with any of the functions in
+this module, make sure to read the warning below!
+
 
 .partial()
 ----------
@@ -133,6 +134,7 @@ There are some differences, though:
       >>> foo()
       0
 
+
 .decorator()
 ------------
 
@@ -187,6 +189,73 @@ A: No, it uses ugly `abstract syntax tree`_ code to do its dynamic code generati
 
 .. _abstract syntax tree: http://docs.python.org/3.3/library/ast.html?highlight=ast#ast
 
+
+WARNING: performance hits incoming
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Decorating a function with the tools in this module is a quite costy
+operation, so don't do it very often! Invoking the wrapper is no problem on
+the other hand.
+
+
+WARNING: functools.partial is evil
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Be careful when passing ``functools.partial`` objects into ``.wraps``, or
+any black magic functions more generally. ``functools.partial`` features
+very unsensible handling of arguments that are bound by keyword. These, and
+all subsequent arguments, become keyword-only parameters. Consider the
+following example:
+
+.. code:: python
+
+    >>> import functools
+    >>> def func(a, b, *args):
+    ...     return (a, b, c, args)
+    >>> part = functools.partial(func, a=0)
+    >>> part(1)
+    Traceback (most recent call last):
+        ...
+    TypeError: func() got multiple values for argument 'a'
+
+Furthermore, note that the ``*args`` parameter becomes completely
+inaccessible, forever!
+
+For compatibility between python versions and ease of use, I chose to handle
+``functools.partial`` objects as if you had actually used
+``black_magic.decorator.partial`` with the same arguments, i.e.:
+
+.. code:: python
+
+    >>> wrap = wraps(part)(part)
+    >>> wrap(1)
+    (0, 1, ())
+    >>> wrap(1, a=0)
+    Traceback (most recent call last):
+        ...
+    TypeError: <lambda>() got an unexpected keyword argument 'a'
+
+There are two exceptions:
+
+- unlike with ``black_magic.decorator.partial`` the ``*args`` argument stays
+  inaccessible
+
+- Furthermore, the parameter ``a`` can be overwritten if the original
+  function has a ``**kwargs`` argument:
+
+.. code:: python
+
+    >>> def kw_func(a, **kwargs):
+    ...     return (a, kwargs):
+    >>> kw_part = functools.partial(kw_func, a=0)
+    >>> kw_wrap = wraps(kw_part)(kw_part)
+    >>> kw_wrap(a=1)
+    (1, {})
+
+If you use ``functools.partial`` to bind only positional parameters, there
+should be no problem!
+
+
 Tests
 ~~~~~
 
@@ -195,6 +264,7 @@ PyPy1.9 using `Travis CI`_.
 
 .. _Travis CI: https://travis-ci.org/
 
+
 License
 ~~~~~~~
 

black_magic/decorator.py

@@ -22,6 +22,9 @@
 from . import compat
 from . import common
 
+def _is_partial_kw(param):
+    return getattr(param, '_partial_kwarg', False)
+
 
 class ASTorator(object):
     """
@@ -113,15 +116,25 @@ def attr(param, attr):
             starargs=None,
             kwargs=None)
 
+        _partial_kw = False
         for param in self.signature.parameters.values():
             context[param.name] = param
 
             # positional parameters
             if param.kind == param.POSITIONAL_OR_KEYWORD:
+                if _is_partial_kw(param):
+                    _partial_kw = True
+                    continue
+
                 sig.args.append(compat.ast_arg(
                     arg=param.name,
                     annotation=attr(param, 'annotation')))
-                call.args.append(ast.Name(id=param.name, ctx=ast.Load()))
+                if _partial_kw:
+                    call.keywords.append(ast.keyword(
+                        arg=param.name,
+                        value=ast.Name(id=param.name, ctx=ast.Load())))
+                else:
+                    call.args.append(ast.Name(id=param.name, ctx=ast.Load()))
                 if hasattr(param, 'default'):
                     sig.defaults.append(attr(param, 'default'))
 
@@ -343,18 +356,23 @@ def value(val):
 
 
 class _ParameterBinding(object):
+    # Okay, this code is a real mess now, I hate it! If anybody comes up
+    # with something that looks remotely clean, pleeeaaasee go ahead and
+    # send me a patch!
     def __init__(self,
                  parameters,
                  pos, kw,
                  var_pos, var_kw,
-                 bound_args, bound_kwargs):
+                 bound_args, bound_kwargs,
+                 partial_kw):
         self._parameters = parameters
         self._pos = pos
         self._kw = kw
         self._var_pos = var_pos
         self._var_kw = var_kw
         self.args = bound_args
         self.kwargs = bound_kwargs
+        self.partial_kw = partial_kw
 
     @classmethod
     def from_signature(cls, signature):
@@ -363,7 +381,11 @@ def from_signature(cls, signature):
         kw = {}
         var_pos = None
         var_kw = None
+        partial_kw = len(parameters)
         for i,par in enumerate(parameters):
+            if _is_partial_kw(par):
+                partial_kw = min(partial_kw, i)
+                continue
             if par.kind in (par.POSITIONAL_OR_KEYWORD, par.POSITIONAL_ONLY):
                 pos.append(i)
             if par.kind in (par.POSITIONAL_OR_KEYWORD, par.KEYWORD_ONLY):
@@ -373,15 +395,17 @@ def from_signature(cls, signature):
             if par.kind == par.VAR_KEYWORD:
                 var_kw = par
         return cls(parameters, pos, kw, var_pos, var_kw,
-                   [parameters[i].default for i in pos],
+                   [parameters[i].default for i in pos if i < partial_kw],
                    dict((p.name,p.default) for p in parameters
-                        if p.kind == p.KEYWORD_ONLY and p.default is not p.empty))
+                        if p.kind == p.KEYWORD_ONLY and p.default is not p.empty),
+                   partial_kw)
 
     def bind(self, *args, **kwargs):
         pos = list(self._pos)
         kw = dict(self._kw.items())
         bound_args = list(self.args)
         bound_kwargs = dict(self.kwargs.items())
+        partial_kw = self.partial_kw
 
         # bind **kwargs:
         for key,val in kwargs.items():
@@ -403,26 +427,34 @@ def bind(self, *args, **kwargs):
                 except ValueError:
                     bound_kwargs[key] = val
                 else:
-                    bound_args[index] = val
+                    if index > partial_kw:
+                        bound_kwargs[key] = val
+                    else:
+                        bound_args[index] = val
 
         # bind *args:
         for index,val in zip(pos, args):
-            bound_args[index] = val
-            kw.pop(self._parameters[index].name, None)
+            key = self._parameters[index].name
+            if index > partial_kw:
+                bound_kwargs[key] = val
+            else:
+                bound_args[index] = val
+            kw.pop(key, None)
         num_args = min(len(pos), len(args))
         pos = pos[num_args:]
         args = args[num_args:]
         if args:
             if self._var_pos is None:
                 raise TypeError(
-                    "%Got too many positional arguments.")
+                    "Got too many positional arguments.")
             bound_args += list(args)
 
         return _ParameterBinding(
             self._parameters,
             pos, kw,
             self._var_pos, self._var_kw,
-            bound_args, bound_kwargs)
+            bound_args, bound_kwargs,
+            partial_kw)
 
     def finalize(self):
         if self._kw:

test/test_decorator.py

@@ -7,6 +7,7 @@
 import unittest
 from black_magic.decorator import wraps, partial
 from black_magic.compat import signature
+import functools
 
 
 class Util(object):
@@ -153,7 +154,7 @@ def __call__(self, a, b=1, *args, **kwargs):
 
     def test_partial(self):
         def orig0(a, b, c, *args, **kwargs):
-            return hash((a, b, c, args))
+            return hash((a, b, c, args, hd(kwargs)))
         self.mutate(partial(orig0, 0, a=1))
         self.check_result(c=2, d=5)
         self.check_result(2, 3, 4)
@@ -178,4 +179,73 @@ def orig1(a, b, c):
         self.must_fail(2, b=2)
         self.must_fail(0, 2, d=3)
 
-
+    def test_functools_partial(self):
+        def orig0(a, b, c, **kwargs):
+            return hash((a, b, c, hd(kwargs)))
+        p0 = functools.partial(orig0, b=0, a=1)
+        w0 = wraps(p0)(p0)
+        self.assertEqual(orig0(1, 0, 2, d=5),
+                         w0(c=2, d=5))
+        self.assertEqual(orig0(1, 0, 2),
+                         w0(2))
+        self.assertRaises(TypeError, w0)
+
+        orig1 = orig0
+        p1 = functools.partial(orig1, 0, 1)
+        w1 = wraps(p1)(p1)
+        self.assertEqual(orig1(0, 1, 2),
+                         w1(2))
+        self.assertEqual(orig1(0, 1, c=2),
+                         w1(c=2))
+        self.assertEqual(orig1(0, 1, c=2, d=5),
+                         w1(2, d=5))
+        self.assertRaises(TypeError, w1)
+        self.assertRaises(TypeError, w1, a=1, c=2)
+
+        def orig2(a, b, c):
+            return hash((a, b, c))
+        p2 = functools.partial(orig2, b=1)
+        w2 = wraps(p2)(p2)
+        self.assertEqual(orig2(0, 1, 2),
+                         w2(0, 2))
+        self.assertEqual(orig2(0, 1, 2),
+                         w2(a=0, c=2))
+        self.assertEqual(orig2(0, 1, 2),
+                         w2(0, c=2))
+        self.assertRaises(TypeError, w2)
+        self.assertRaises(TypeError, w2, 2, b=2)
+        self.assertRaises(TypeError, w2, 2, d=3)
+
+        # behaviour that should be changed some time:
+        self.assertEqual(orig0(0, 0, 2),
+                         w0(a=0, c=2))
+        #self.assertRaises(TypeError, w0, a=0, c=2)
+        self.assertEqual(orig0(1, 1, 2),
+                         w0(b=1, c=2))
+        #self.assertRaises(TypeError, w0, b=1, c=2)
+
+    def test_partial_with_functools_partial(self):
+        def orig(a, b, c):
+            return (a, b, c)
+        part = functools.partial(orig, b=1)
+        wrap = partial(part)
+        self.assertEqual(orig(0, 1, 2),
+                         wrap(0, 2))
+        self.assertEqual(orig(0, 1, 2),
+                         wrap(a=0, c=2))
+        self.assertRaises(TypeError, wrap, 0, 2, b=1)
+
+        def orig_kw(a, b, c, **kwargs):
+            return (a, b, c, hd(kwargs))
+        part_kw = functools.partial(orig_kw, b=1)
+        wrap_kw = partial(part_kw)
+
+        self.assertEqual(orig_kw(0, 1, 2, d=1),
+                         wrap_kw(0, 2, d=1))
+        self.assertEqual(orig_kw(0, 1, 2),
+                         wrap_kw(a=0, c=2))
+
+        # behaviour that should be changed some time:
+        self.assertEqual(orig_kw(0, 3, 2),
+                         wrap_kw(0, 2, b=3))
+        # self.assertRaises(TypeError, wrap_kw, 0, 2, b=1)