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

Ramda 0.27.1 allPass working wrong #3114

Open
Orilow opened this issue Dec 3, 2020 · 10 comments
Open

Ramda 0.27.1 allPass working wrong #3114

Orilow opened this issue Dec 3, 2020 · 10 comments

Comments

@Orilow
Copy link

Orilow commented Dec 3, 2020

image
So I tried to test that modified example on screenshot, and that goes wrong: it must be false

@adispring
Copy link
Member

As the doc says: The function returned is a curried function whose arity matches that of the
highest-arity predicate.

isQueenOfSpades' length is 1, it only accept 1 argument, but you put in 3, neverthless it only process the first argument.

@adispring
Copy link
Member

adispring commented Dec 3, 2020

If you want to check all the items that satisfy a given predicate, you can use R.all, as follows:

const isQueen = R.propEq('rank', 'Q');
const isSpade = R.propEq('suit', '♠︎');

const isQueenOfSpades = R.allPass([isQueen, isSpade]);

const allAreQueenOfSpade = R.all(isQueenOfSpades)

allAreQueenOfSpade([{rank: 'Q', suit: '♠︎'}, {rank: 'Q', suit: '♣︎'}, {rank: 'Q', suit: '♠︎'}]) //=> false

@CrossEye
Copy link
Member

CrossEye commented Dec 3, 2020

I think you're combining the idea of all and allPass. The latter is described below. But if you think that we need better documentation for allPass, we'd love the hear your suggestions.

all takes a single predicate and an array of individual arguments and returns true if and only if that predicate is true for each argument.

allPass takes an array of predicates and returns a new function which takes some arguments and calls each function with them, returning true if and only if they all return true. 1

Thus if we have

const fn = allPass ([
  (x, y) => x + y > 10,
  (x, y) => x * y < 100,
  (x, y) => (x - y) % 2 == 1,
])

we get the following behavior:

fn (4, 5)   // false -- sum < 10
fn (14, 6)  // false -- even difference
fn (14, 15) // false -- product > 100
fn (14, 5)  // true  -- matches all predicates

Note that the arity of fn is 2 -- the largest arity of any function supplied. Generally, we would expect these all to have the same arity, but it's not a requirement.


1This is not precisely true: because of short-circuiting, some may not actually be called. But the idea is the same.

@adispring
Copy link
Member

When I'm not familiar with ramda, I often use allPass/anyPass in a wrong way: pass the list of predicate and the object being checked together in allPass/anyPass, as follows:

// Wrong way
R.allPass([isQueen, isSpade], {rank: 'Q', suit: '♠︎'})

// The correct way
R.allPass([isQueen, isSpade])({rank: 'Q', suit: '♠︎'})

which will return another function instead of the checked result.

@adispring
Copy link
Member

adispring commented Dec 3, 2020

There are 2 categories of curried function in ramda:

  1. most of the ramda functions can pass arguments partially;
  2. some of them must accept some arguments, return a new function, which accept other arguments;

I think this may also will confuse other ramda beginners?

We can give people some Notes in the document, or do some type checking in the code?

@CrossEye
Copy link
Member

CrossEye commented Dec 3, 2020

@adispring: This is an unusual case, and there are a few more like it. It mostly has to do with when we generate polyadic functions.

It's hard to curry this our normal way when it could be used as

allPass([x => x > 10, x => x < 100], 42) //=> true
allPass([x => x > 10, x => x < 100], 7)  //=> false

but also as

allPass([(x, y) => x + y > 10, (x, y) => x * y < 100], 14, 5)  //=> true
allPass([(x, y) => x + y > 10, (x, y) => x * y < 100], 14, 15) //=> false

or as

allPass([(x, y, z) => x + y >= z, (x, y, z) => x + z >= y, (x, y, z) => y + z >= x], 5, 12, 13) //=> true
allPass([(x, y, z) => x + y >= z, (x, y, z) => x + z >= y, (x, y, z) => y + z >= x], 5, 12, 18) //=> false

So we do this in two stages. I don't see any good alternative to that.

It would probably be a good idea to capture this more explicitly in the documentation comments, though. While the type signature is reasonable, [(*… → Boolean)] → (*… → Boolean), I think a note for these unusual functions would be useful.

@adispring
Copy link
Member

adispring commented Dec 4, 2020

If we begin to handle this issue -- Better error messaging and type-checking : #2998, we can check the type and the number of arguments people passed in, then give them some tips on development stage.

@Orilow
Copy link
Author

Orilow commented Dec 4, 2020

If you want to check all the items that satisfy a given predicate, you can use R.all, as follows:

const isQueen = R.propEq('rank', 'Q');
const isSpade = R.propEq('suit', '♠︎');

const isQueenOfSpades = R.allPass([isQueen, isSpade]);

const allAreQueenOfSpade = R.all(isQueenOfSpades)

allAreQueenOfSpade([{rank: 'Q', suit: '♠︎'}, {rank: 'Q', suit: '♣︎'}, {rank: 'Q', suit: '♠︎'}]) //=> false

That's I wanted to do, thanks!
So as we figured out, I misunderstood the description and used it wrong.
But is it possible to add R.all in "See Also" description? Or add a warning to beginners to show that they(allPass and all) are looking similar but you better read both descriptions to see differences?
Cos I, as a beginner in ramda, saw allPass first and thought it will be suitable.

And thank you for your detailed respones!

@adispring
Copy link
Member

But is it possible to add R.all in "See Also" description? Or add a warning to beginners to show that they(allPass and all) are looking similar but you better read both descriptions to see differences?

Yes, do you want to make a pr to add "see also", note or more descriptions to make that more clearly.

@CrossEye
Copy link
Member

CrossEye commented Dec 4, 2020

@adispring:

If we begin to handle this issue -- Better error messaging and type-checking : #2998, we can check the type and the number of arguments people passed in, then give them some tips on development stage.

This would still be significantly more work than other functions. I'm pretty sure that we would never be able to use our standard currying mechanism for functions like this. Our usual curry interrogates the function we pass in to determine how many arguments to expect. But here it's not just that allPass function which determines that. It's the arity of the longest function in the array of functions passed which determines that.

It's not that I don't think we could write such a thing, but then trying to keep it synchronized with our regularly curried functions when we add features (as, for instance, the way discussed in #2998) to the currying code could turn pretty ugly.

I suppose we could use this function as a test case and see how we might share code with the standard curry.

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

No branches or pull requests

4 participants