clojure.core.reducers/fold broken

Description

clojure.core.reducers/fold currently does not work correctly if both a reducef and combinef are passed as arguments.

It is expected that (r/fold + + [1 2 3]) would return 6 (as do (r/fold + [1 2 3]) and (r/reduce + [1 2 3])). However, this is not the case because reducers.cljs currently defines fold as follows:

(def fold reduce)

As a result, the second + is used as the initial value for the reduce operation (instead of the value returned by ).

The attached patch fixes this bug. It also adds support for the protocol CollFold in CLJS.

(I will send in the CA tomorrow.)

Environment

None

Attachments

3
  • 29 Nov 2013, 09:58 PM
  • 29 Nov 2013, 07:45 PM
  • 27 Nov 2013, 06:53 AM

Activity

Show:

Jonas De VuystNovember 29, 2013 at 9:58 PM

Oh, sorry. I have attached a new patch.

David NolenNovember 29, 2013 at 8:45 PM

The patch has not been formatted so that it can be applied with git am. Please generate the patch following these instructions - http://github.com/clojure/clojurescript/wiki/Patches.

Jonas De VuystNovember 29, 2013 at 7:45 PM

I added a new patch, in which cljs.core/PersistentHashMap is substituted for clojure.lang.PersistentHashMap – although that part is actually commented out because CLJS maps do not have a method .fold.

I also added tests for reducing and folding of maps (which I also tested in Clojure).

David NolenNovember 28, 2013 at 2:05 PM

This mostly looks good but clojure.lang.PersistentHashMap should be cljs.core/PersistentHashMap and we should have a corresponding test.

Completed

Details

Assignee

Reporter

Patch

Code and Test

Priority

Created November 27, 2013 at 6:53 AM
Updated November 29, 2013 at 10:00 PM
Resolved November 29, 2013 at 10:00 PM