<< Back to previous view

[CLJ-1049] Make reducer/folder support reduce-kv Created: 22/Aug/12  Updated: 17/Aug/13  Resolved: 17/Aug/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Tom Jack Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None

Attachments: File 0001-reduce-kv-transformations.diff    
Patch: Code


Currently, although rfn makes an effort to support reduce-kv, it is wasted effort since the return values of reducer and folder don't satisfy IKVReduce.

I have a working patch for this, it's quite small. I have printed a CA but not sent it, so I won't reveal the patch yet...

This also applies to ClojureScript.

Comment by Tom Jack [ 15/Sep/12 12:47 AM ]

Hmm.. it's not quite as simple as I thought. My first patch simply had reducer/folder implement IKVReduce with `(clojure.core.protocols/kv-reduce coll (xf f1) init)`, but for map and mapcat, this results in strange behavior (reduce-kv reducefs being called with only 2 args).

Patch 0001 includes modifications to map and mapcat so that the reduce-kv reducef will always be called with 3 args. The previous implementation of mapcat also had the converse problem: during non-kv reduce, if the mapcat fn returned a map, the reducef would be called with 3 args since maps reduce with reduce-kv.

The patch gives behavior like:

user> (->> {1 {2 3 4 5} 6 {7 8 9 10}}
        (r/mapcat #(r/map + %2))
        (reduce-kv #(conj %1 [%2 %3]) []))
[[2 5] [4 9] [7 15] [9 19]]
user> (->> [{2 3 4 5} {7 8 9 10}]
        (r/mapcat identity)
        (r/reduce conj []))
[[2 3] [4 5] [7 8] [9 10]]
Comment by Tom Jack [ 23/Sep/12 8:34 PM ]

Would like to discuss these changes (and auto-kv for maps) on clojure-dev, but my membership is still pending. My CA has been received. Anyone know who to contact to get my membership approved?

Comment by Andy Fingerhut [ 14/Aug/13 7:47 PM ]

Tom, your patch 0001-reduce-kv-transformations.diff dated Sep 14 2012 no longer applies cleanly to the latest master, because patch lazy-rmapcat2.diff for ticket CLJ-1160 was committed to Clojure master on Aug 14 2013, and they both modify some of the same code. I haven't tried to determine whether their changes are harmonious with each other or not. I'd recommend taking a look.

Comment by Tom Jack [ 14/Aug/13 8:17 PM ]

I later realized that there are more issues with my goal (of supporting reduce-kv for the reducer combinators). I think I worked out the way I'd want things to be, but it would've involve backwards-incompatible changes to the way maps reduce. Short of that, the best thing I can think to do is probably just to not support reduce-kv at all (coincidentally, this is exactly what clojure.core.reducers does! . So as far as I'm concerned this issue is resolved. My patch certainly should not be applied, at least...

Comment by Alex Miller [ 17/Aug/13 3:16 PM ]

Closed per comment

Generated at Sun Jan 21 05:09:48 CST 2018 using JIRA 4.4#649-r158309.