Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many improvements to curry based on #147 #150 #155 #159 #170

Merged
merged 6 commits into from
May 6, 2014
Merged

Many improvements to curry based on #147 #150 #155 #159 #170

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c655b9c
Many improvements to `curry` based on #147 #150 #155 #159
eriknw May 6, 2014
b2f9c02
Add tests to get 100% coverage
eriknw May 6, 2014
34f14ac
oops.
eriknw May 6, 2014
cc1a609
Remove unnecessary tests.
eriknw May 6, 2014
b44da40
Calculate hash in `__hash__` for curry, not in `__init__`.
eriknw May 6, 2014
027ea6e
Pickle `curried_func.__dict__` too.
eriknw May 6, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 63 additions & 29 deletions toolz/functoolz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,50 +141,84 @@ def __init__(self, func, *args, **kwargs):
if not callable(func):
raise TypeError("Input must be callable")

self.func = func
self.args = args
self.keywords = kwargs if kwargs else None
self.__doc__ = self.func.__doc__
# curry- or functools.partial-like object? Unpack and merge arguments
if (hasattr(func, 'func') and hasattr(func, 'args')
and hasattr(func, 'keywords')):
_kwargs = {}
if func.keywords:
_kwargs.update(func.keywords)
_kwargs.update(kwargs)
kwargs = _kwargs
args = func.args + args
func = func.func

if kwargs:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this check? Why not just always throw in the kwargs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a performance tweak. partial is faster when keywords is None instead of {}.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that I don't care much about performance tweaks during function creation as I assume that these will happen only a few times. In these cases I think that code cleanliness is more important.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but this performance tweak isn't for initialization, it is for usage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. This is only going to happen whenever I add new args onto the curry, right? I guess that in my use of curried functions this happens only a few times. Can you supply a motivating example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The performance increase is when the curried function gets called, not upon creating the curried object. Maybe this will clarify:

In [1]: from functools import partial

In [2]: def f():
   ...:     pass
   ...: 

In [3]: kwargs = {}

In [4]: f1 = partial(f, **kwargs)

In [5]: f2 = partial(f)

In [6]: f1.keywords
Out[6]: {}

In [7]: f2.keywords

In [8]: f2.keywords is None
Out[8]: True

In [9]: %timeit f1()
1000000 loops, best of 3: 379 ns per loop

In [10]: %timeit f2()
1000000 loops, best of 3: 291 ns per loop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I understand. Alright, I guess that this makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's subtle. One would expect partial to behave the same for f1 and f2 from above, but it doesn't.

self._partial = partial(func, *args, **kwargs)
else:
self._partial = partial(func, *args)

try:
self.func_name = self.func.func_name
except AttributeError:
pass
self._hashvalue = hash((self.func, self.args,
frozenset(self.keywords.items())
if self.keywords else None))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on having this in the constructor rather than having it inside __hash__?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is faster to have this in the constructor. args, keywords, and func are readonly, and if anything is unhashable, the hash reverts to None.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again I think that hashing curry objects isn't likely to be a bottleneck and so my preference here is to keep the code clean. Mostly I'm just anal about keeping __call__ as fast as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. You're the one who requested this feature, so I'll go with your preference :)

except TypeError:
self._hashvalue = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might this cause problems? How do systems that call for hashes respond to a null value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one of the preferred way to remove hashability (for instance, if a new class inherits from a hashable class, but is no longer hashable). In other words, this behaves as expected. It is also included in the test suites.

self.__doc__ = getattr(func, '__doc__', None)
self.__name__ = getattr(func, '__name__', '<curry>')

@property
def func(self):
return self._partial.func

@property
def args(self):
return self._partial.args

@property
def keywords(self):
return self._partial.keywords

@property
def func_name(self):
return self.__name__

def __str__(self):
return str(self.func)

def __repr__(self):
return repr(self.func)

def __call__(self, *args, **_kwargs):
args = self.args + args
if _kwargs:
kwargs = {}
if self.keywords:
kwargs.update(self.keywords)
kwargs.update(_kwargs)
elif self.keywords:
kwargs = self.keywords
else:
kwargs = {}
def __hash__(self):
return self._hashvalue

def __eq__(self, other):
return (isinstance(other, curry) and self.func == other.func and
self.args == other.args and self.keywords == other.keywords)

def __ne__(self, other):
return not self.__eq__(other)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? I thought that this happened by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed necessary. Without it, assert not (f1 != f2) test fails even though assert f1 == f2 passes.


def __call__(self, *args, **kwargs):
try:
return self.func(*args, **kwargs)
return self._partial(*args, **kwargs)
except TypeError:
required_args = _num_required_args(self.func)

# If there was a genuine TypeError
if required_args is not None and len(args) >= required_args:
required_args = _num_required_args(self.func)
if (required_args is not None and
len(args) + len(self.args) >= required_args):
raise

# If we only need one more argument
if (required_args is not None and required_args - len(args) == 1):
if kwargs:
return partial(self.func, *args, **kwargs)
else:
return partial(self.func, *args)
if args or kwargs:
return curry(self._partial, *args, **kwargs)
return self
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just return curry(self._partial, *args, **kwargs) in all cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Shall I change it?


# pickle protocol because functools.partial objects can't be pickled
def __getstate__(self):
return self.func, self.args, self.keywords
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we pickle __doc__ and __name__? What about other items that the user added to __dict__?


return curry(self.func, *args, **kwargs)
def __setstate__(self, state):
func, args, kwargs = state
self.__init__(func, *args, **(kwargs or {}))


@curry
Expand Down
81 changes: 81 additions & 0 deletions toolz/tests/test_functoolz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ def f(x, y=0):
assert fm2(3) == f2(3)


def test_memoize_partial():
def f(x, y=0):
return x + y

f2 = partial(f, y=1)
fm2 = memoize(f2)

assert fm2(3) == f2(3)
assert fm2(3) == f2(3)


def test_memoize_key_signature():
# Single argument should not be tupled as a key. No keywords.
mf = memoize(lambda x: False, cache={1: True})
Expand Down Expand Up @@ -221,6 +232,76 @@ def foo(a, b, c=1):
assert p(1, 2) == c(1, 2)


def test_curry_is_idempotent():
def foo(a, b, c=1):
return a + b + c

f = curry(foo, 1, c=2)
g = curry(f)
assert isinstance(f, curry)
assert isinstance(g, curry)
assert not isinstance(g.func, curry)
assert not hasattr(g.func, 'func')
assert f.func == g.func
assert f.args == g.args
assert f.keywords == g.keywords


def test_curry_attributes_readonly():
def foo(a, b, c=1):
return a + b + c

f = curry(foo, 1, c=2)
assert raises(AttributeError, lambda: setattr(f, 'args', (2,)))
assert raises(AttributeError, lambda: setattr(f, 'keywords', {'c': 3}))
assert raises(AttributeError, lambda: setattr(f, 'func', f))


def test_curry_attributes_writable():
def foo(a, b, c=1):
return a + b + c

f = curry(foo, 1, c=2)
f.__name__ = 'newname'
f.__doc__ = 'newdoc'
assert f.__name__ == 'newname'
assert f.__doc__ == 'newdoc'
if hasattr(f, 'func_name'):
assert f.__name__ == f.func_name


def test_curry_comparable():
def foo(a, b, c=1):
return a + b + c
f1 = curry(foo, 1, c=2)
f2 = curry(foo, 1, c=2)
g1 = curry(foo, 1, c=3)
h1 = curry(foo, c=2)
h2 = h1(c=2)
h3 = h1()
assert f1 == f2
assert not (f1 != f2)
assert f1 != g1
assert not (f1 == g1)
assert f1 != h1
assert h1 == h2
assert h1 == h3

# test function comparison works
def bar(a, b, c=1):
return a + b + c
b1 = curry(bar, 1, c=2)
assert b1 != f1

assert set([f1, f2, g1, h1, h2, h3, b1, b1()]) == set([f1, g1, h1, b1])

# test unhashable input
unhash1 = curry(foo, [])
assert raises(TypeError, lambda: hash(unhash1))
unhash2 = curry(foo, c=[])
assert raises(TypeError, lambda: hash(unhash2))


def test__num_required_args():
assert _num_required_args(map) is None
assert _num_required_args(lambda x: x) == 1
Expand Down