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.

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

Transducers: map(prop(p)) != pluck(p) ? #1012

Closed
monfera opened this issue Apr 6, 2015 · 6 comments
Closed

Transducers: map(prop(p)) != pluck(p) ? #1012

monfera opened this issue Apr 6, 2015 · 6 comments
Assignees

Comments

@monfera
Copy link

monfera commented Apr 6, 2015

I must be overlooking something obvious:
With full application:

R.map(R.prop('a'))([{a: 1}, {a: 2}, {a: 3}, {a: 4}])
[1, 2, 3, 4]
R.pluck('a')([{a: 1}, {a: 2}, {a: 3}, {a: 4}])
[1, 2, 3, 4]

So far, so good. Now with transducers:

var numbers = [{a: 1}, {a: 2}, {a: 3}, {a: 4}];
var transducer = R.compose(R.map(R.prop('a')), R.map(R.add(1)), R.take(2));
R.transduce(transducer, R.flip(R.append), [], numbers); 
[2, 3]

It is still good, however this won't work:

var numbers = [{a: 1}, {a: 2}, {a: 3}, {a: 4}];
var transducer = R.compose(R.pluck('a'), R.map(R.add(1)), R.take(2));
R.transduce(transducer, R.flip(R.append), [], numbers); 
Uncaught TypeError: xf.step is not a function
    at _arrayReduce (http://localhost:63342/d6/ramda.js:4201:26)
    at _reduce (http://localhost:63342/d6/ramda.js:4230:24)
    at Object.<anonymous> (http://localhost:63342/d6/ramda.js:5927:16)
    at Object.<anonymous> (http://localhost:63342/d6/ramda.js:1358:27)
    at Object.transduce (http://localhost:63342/d6/ramda.js:1041:31)
    at <anonymous>:5:3
    at Object.InjectedScript._evaluateOn (<anonymous>:895:140)
    at Object.InjectedScript._evaluateAndWrap (<anonymous>:828:34)
    at Object.InjectedScript.evaluate (<anonymous>:694:21)
@monfera
Copy link
Author

monfera commented Apr 6, 2015

Doing a quick and dirty, and replacing R.pluck with tpluck in the above snippet, I also get the expected [2, 3].

function tpluck(prop) {
    return R.map(function(x) {return x[prop];});
}

@buzzdecafe buzzdecafe self-assigned this Apr 6, 2015
@CrossEye
Copy link
Member

CrossEye commented Apr 6, 2015

The problem, of course, is that R.pluck is not properly transduced.

If I'm not mistaken, and I can't tell for sure until I get in front of a computer, it's because pluck is built on an internal _map and not on the tranducerified public map. This, I suppose is more ammunition in my case for reducing the number of such internal functions we use. I'd rather be dogfooding our stuff as much as we can. It's not clear to me how easy an overall change this would be. But it should be simple to change pluck.

@monfera
Copy link
Author

monfera commented Apr 6, 2015

Probably it's not the underlying cause of the issue, but I noticed that ramda transducers don't follow the old or the new transducers protocol (https://github.com/cognitect-labs/transducers-js#the-transducer-protocol) which I ran into the hard way. Using Kefir.js for 'FRP', I failed to switch from transducers.js / transducers-js to ramda in a test. Is it due to some fundamental difference in the implementation approach?

@buzzdecafe
Copy link
Member

@monfera HEAD version should be compliant with new protocol. see: #1006

CrossEye added a commit that referenced this issue Apr 6, 2015
make pluck behave with transducers, fixes #1012
@monfera
Copy link
Author

monfera commented Apr 7, 2015

Interesting, I was already up to date and Kefir still balked at a point which I could solve by switching a R.map inside a R.compose to a t.map (transducers.js).

@buzzdecafe
Copy link
Member

please keep me posted -- i am interested to know what places we missed and need to bring into compliance. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants