merge_with raises err on no inputs #73
Conversation
| @@ -36,6 +36,9 @@ def merge_with(fn, *dicts): | |||
| See Also: | |||
| merge | |||
| """ | |||
| if len(dicts) == 0: | |||
There was a problem hiding this comment.
Why isn't this a NOP if dicts is empty? That is what Clojure's merge-with does (returns nil).
There was a problem hiding this comment.
Yup. That's the controversy. Normally this would return {}. This screws up curry though because merge_with(sum) actually works and so we don't close sum into the function. There are competing objectives. I'm not sure which way is best.
There was a problem hiding this comment.
Well, you can probably predict where my prejudice falls :-) [clojure-like semantics]
Let’s see if @eriknw has a (tiebreaker?) opinion.
On Nov 7, 2013, at 3:21 PM, Matthew Rocklin notifications@github.com wrote:
In toolz/dicttoolz/core.py:
@@ -36,6 +36,9 @@ def merge_with(fn, *dicts):
See Also:
merge
"""
- if len(dicts) == 0:
Yup. That's the controversy. Normally this would return {}. This screws up curry though because merge_with(sum) actually works and so we don't close sum into the function. There are competing objectives. I'm not sure which way is best.—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
The right thing here is probably for me to be forced to make a special curried implementation of merge_with within the curried.py file.
There was a problem hiding this comment.
Perhaps it would be useful to have a HOF that changes variadic arguments to a single iterable argument.
There was a problem hiding this comment.
That would be useful. concat and concatv is a good use case.
On Thu, Nov 7, 2013 at 1:44 PM, Erik Welch notifications@github.com wrote:
In toolz/dicttoolz/core.py:
@@ -36,6 +36,9 @@ def merge_with(fn, *dicts):
See Also:
merge
"""
- if len(dicts) == 0:
Perhaps it would be useful to have a HOF that changes variadic arguments
to a single iterable argument.—
Reply to this email directly or view it on GitHubhttps://github.com//pull/73/files#r7511342
.
|
Since I'm on the spot, I would say keep the old behavior (at least for now). This raises two things I've been contemplating:
|
|
See new commit. Standard Again, old behavior is preserved in standard namespace. New behavior is isolated to |
| @@ -37,7 +37,7 @@ def nargs(f): | |||
|
|
|||
|
|
|||
| def should_curry(f): | |||
| do_curry = set((toolz.map, toolz.filter)) | |||
| do_curry = set((toolz.map, toolz.filter, toolz.merge_with)) | |||
There was a problem hiding this comment.
This change is now unnecessary since merge_with is defined below.
|
I'm planning to merge this early in the morning if no comments. |
Conflicts: toolz/tests/test_curried.py
|
I lied. I'm merging this now. We can revert the behavior in the future if necessary. |
merge_with raises err on no inputs
This allows
merge_withto interoperate withcurrybut fails on the possible base casemerge_with(func) == {}