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

Eval error with function and implicit multiplication #1035

Closed
ericman314 opened this Issue Feb 3, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@ericman314
Collaborator

ericman314 commented Feb 3, 2018

I came across a bug in the last example in the docs for implicit multiplication:

math.eval('sqrt(4)(1+2)');   //  TypeError: fn is not a function

The output of console.log(util.inspect(math.parse('sqrt(4)(1+2)'), {depth:100}));

Node {
  fn: 
   Node {
     fn: Node { name: 'sqrt' },
     args: [ Node { value: '4', valueType: 'number' } ] },
  args: 
   [ Node {
       implicit: false,
       op: '+',
       fn: 'add',
       args: 
        [ Node { value: '1', valueType: 'number' },
          Node { value: '2', valueType: 'number' } ] } ],
  comment: '' }

The error occurs in the current develop branch but not the master branch.

@josdejong josdejong added bug design decision and removed bug labels Feb 3, 2018

@josdejong

This comment has been minimized.

Owner

josdejong commented Feb 3, 2018

That's a good point. this behavior has been there for a long time though (I tried for example v3.10.0). Basically the output of sqrt(4) is expected to be a function, which is then invoked with argument (1+2). We can discuss whether this is desirable behavior or whether we want to change it in v4.

Here some example cases that we need to consider then I think:

//  function call or implicit multiplication?
(2)(3)           // current behavior: implicit multiplication
(fn)(2)          // current behavior: implicit multiplication
(obj.fn)(2)      // current behavior: implicit multiplication
fn(2)            // current behavior: function call
fn(2)(3)         // current behavior: function call          *** change this one? ***
obj.fn(2)        // current behavior: function call
obj['fn'](2)     // current behavior: function call

// get matrix subset or implicit multiplication?
fn(2)[3]         // current behavior: get matrix subset      *** change this one? ***
matrices[2][1:4] // current behavior: get matrix subset      *** change this one? ***
@josdejong

This comment has been minimized.

Owner

josdejong commented Feb 3, 2018

Thinking about it, it makes sense to make this change. If you want to invoke the output of a function or access a matrix subset, you can always first store the result in a variable, and then use that variable as function or matrix.

@josdejong josdejong referenced this issue Feb 3, 2018

Closed

Breaking changes for v4 #682

13 of 13 tasks complete

josdejong added a commit that referenced this issue Feb 3, 2018

@josdejong

This comment has been minimized.

Owner

josdejong commented Feb 3, 2018

@ericman314 I've implemented your idea in #1037.

I think usage as implicit multiplication is more common in maths than immediately invoking a returned function, so this will be an improvement.

What do you think?

@ericman314

This comment has been minimized.

Collaborator

ericman314 commented Feb 3, 2018

You're probably right.

@josdejong

This comment has been minimized.

Owner

josdejong commented Feb 4, 2018

I think it's the best choice, will merge #1037 now.

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