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

Filter does not properly handle FunctionAssignmentNode #846

Closed
30percent opened this Issue May 8, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@30percent

30percent commented May 8, 2017

When calling filter through eval, if the callback parameter is a FunctionAssignmentNode, it is not properly called. Simple test here:

var context = {collection: [1,2,3,4]};
console.log(math.eval("filter(collection, f(x) = x > 2)", context)); //prints [1,2,3,4]; expected: [3,4]
console.log(math.eval("filter(collection, x > 2)", context)); //prints [3,4]

This is not in line with map, which has behavior in reverse...sort of

var context = {collection: [1,2,3,4]};
console.log(math.eval("map(collection, f(x) = x + 2)", context)); //prints [3,4,5,6];
console.log(math.eval("map(collection, x + 2)", context)); //Error: Undefined symbol x

Source of issue:
Filter has a special transform factory to check for OperatorNode and create a function with parameters based on undefined symbols. (This allows for: math.eval("filter(collection, x > 2)", {collection: [1,2,3,4]}) //[3,4]). However, the transform checks for:

if (args[1] && args[1].isSymbolNode) {
...
} else {
// an equation like filter([3, -2, 5], x > 0)
...

The trouble is that this creates the assumption that anything not a SymbolNode is a OperatorNode.

Solution:
Either:

  1. swapping the conditionals such that we only do special behavior on specific target case (if(args[1].isOperatorNode), otherwise do what amounts to a passthrough.
  2. change ln22: if (args[1].isSymbolNode || args[1].isFunctionAssignmentNode) { //we don't need to check for args[1] existence again, it's above

@josdejong josdejong added the bug label May 8, 2017

@josdejong

This comment has been minimized.

Owner

josdejong commented May 8, 2017

Thanks for your clear report!

I see both filter and map aren't behaving correctly for input like filter([3, -2, 5], boolean(x)), will fix that as well.

@josdejong josdejong closed this in 9666c16 Jul 29, 2017

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