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

Default value for eval scope variables #660

Closed
agladysh opened this issue May 11, 2016 · 18 comments
Closed

Default value for eval scope variables #660

agladysh opened this issue May 11, 2016 · 18 comments
Labels

Comments

@agladysh
Copy link

I'd like to avoid Undefined symbol error being thrown when user tries to read from an undefined variable in an expression. Instead, I would like that variable to equal 0.

That is, for example, math.eval("x + y", { y: 42 }) should return 42.

Is there a way to do it?

@FSMaxB
Copy link
Collaborator

FSMaxB commented May 11, 2016

You have to change the code generation in SymbolNode's compile function to not throw an error but return 0 instead.

See lib/expression/node/SymbolNode.js.

return '(' +
          '"' + this.name + '" in scope ? scope["' + this.name + '"] : ' +
          (Unit.isValuelessUnit(this.name) ?
          'new Unit(null, "' + this.name + '")' :
          'undef("' + this.name + '")') +
          ')';

Change it to:

return '(' +
          '"' + this.name + '" in scope ? scope["' + this.name + '"] : ' +
          (Unit.isValuelessUnit(this.name) ?
          'new Unit(null, "' + this.name + '")' :
          '0') + // <-- change here
          ')';

This should work (although I haven't tried it).

I don't think there is a way without modifying mathjs itself. But I could be wrong.

@agladysh
Copy link
Author

Thank you for the hint!
A cleaner way would probably be to change the undef function to return 0.
The best solution (to my taste) would allow user to supply a callback accessor function along with the scope to be called for undefined values. Akin to Lua's __index metamethod.

@FSMaxB
Copy link
Collaborator

FSMaxB commented May 11, 2016

You're right, changing the undef function would be much cleaner. Although this still requires modifying the library.

I'm marking this as feature request for the ability to register hooks that are called when a symbol wasn't found in the scope.

@FSMaxB FSMaxB added the feature label May 11, 2016
@josdejong
Copy link
Owner

josdejong commented May 14, 2016

sounds good to create the ability to use hooks to adjust the default behavior of the parser. I think we should look a bit more generic than just this specific case where you want a different behavior for undef.

@eddieajau
Copy link

@josdejong happy to try to get this one moving. Could you give me some guidance on the acceptance criteria for the feature?

The use case I've got is that I'm merging several arrays from a query builder with, for example, a month column. But sometimes a symbol is not in the row because there was no data. For example:

[
    { month: 202001, target: '3400', cost: '918' },
    { month: 202002, target: '3400', cost: '837' },
    { month: 202003, target: '3100', cost: '665' },
    { month: 202004, target: '900', cost: '207' },
    { month: 202005, target: '6700', cost: '850' },
    { month: 202006, target: '8800', cost: '206', count: '205' },
    { month: 202007, target: '9500', cost: '496', count: '266' }
  ]

Any expression looking for "count" throws an error. All I personally need is a flag to say "don't throw" but I'm happy to look at a handler style thing if that is more useful.

Thanks in advance.

@josdejong
Copy link
Owner

Thanks for your offer @eddieajau 👍 .

First step would be to create a clear plan on what kind of "hooks" we want to introduce. So one thing we want to solve is have a hook which triggers in case of an undefined symbol. So we could think about an very specific onUndefinedSymbol hook. We could also think a bit broader though: a hook which is called before evaluation of every node, and one after evaluation. That would be way more flexible and powerful. On top of that we can then build the functionality we're looking for in this specific topic very easily. We would have to think though exactly what arguments we would have to pass to those hooks. What do you think?

@eddieajau
Copy link

Hi @josdejong ok, shooting from the hip, are we aiming for something like this:

import { create, all } from 'mathjs';

const mymath = create(all, {
  onUndefinedSymbol: (name) => 0
})

mymath.evaluate('number(unknown)') // 0

And then undef changes to something like:

  /**
   * Handles an undefined symbol.
   *
   * @param {string} name
   * @throws an error 'Undefined symbol {name}' if no handler defined.
   */
  function undef (name) {
    if ((config || {}).onUndefinedSymbol) {
      return config.onUndefinedSymbol(name);
    }
    throw new Error('Undefined symbol ' + name);
  }

Some sort of variation on that kind of theme?

@josdejong
Copy link
Owner

That is a very simple approach! I'l very careful with adding new global configuration, maybe it's an idea to pass this as an option to evaluate (and compile and all related functions) instead of introducing a global variable.

I would love to think this use case a bit more through though. It feels a bit like we're introducing a new configuration option for a very specific use case. Normally, when you try to evaluate something and there is a variable missing, there is something wrong and you would like to be informed. Silently "swallowing" an error is something I try to avoid generally in programming (though it would be a concious decision by the one who configures onUndefinedSymbol of course). I want to be careful introducing a new feature that may encourage usage patterns that we will regret in the future.

I do understand your use case. But in your case, wouldn't it make sense to write a little pre-processing to your data before evaluation? If I understand you correctly, you know all possible data fields, so you could simply add any missing count fields to your data beforehand. That would put the responsibility of making sure the input data is valid to this pre-processing step, instead of making the expression parser responsible to try "fix" issues in your data inputs.

Do we have more concrete use cases for this feature?

Just thinking aloud here...

@eddieajau
Copy link

Hi @josdejong and thanks for the reply.

I do understand your use case. But in your case, wouldn't it make sense to write a little pre-processing to your data before evaluation?

Heh, so the problem is that I am building a JSON schema for dynamic queries, ETL, etc. I'm using mathjs FOR the pre-processing step itself ;) Eg:

{
  // ...
  "map": {
    "computed-value": "cost/count"
  }
}

Yes, if I was actually writing the code I could just do a normal map over the result set. Now if I had to add another step I've got to give instructions to the technician (on top of writing the code support for it) to "make sure all expected variables are populated with something otherwise the math library I'm using will throw an error", and that's not ideal. But since I'm dealing with counts and costs of inventory items, if there is no symbol, zero is an appropriate default.

For now I am using CI to replace, after the npm install, the SymbolNode code where undef just returns 0 (I tried hacking the prototype but that didn't work) and that has unblocked me so far and giving me the results I need for the customer.

Do we have more concrete use cases for this feature?

The main use case I see are:

  1. As a user I can't have a hard error thrown if there is no symbol because ...
    a) it will lead to information disclosure, or
    b) it will stop execution in a place that is difficult to otherwise try-catch (because, reasons - legacy code, whatever), or
    c) I want to silently log the error but move on, or
    d) I want to log the error with additional information and then rethrow, or ... etc and so on
  2. As a user I sometimes need to define a custom default for a symbol if the symbol is not defined or does not satisfy some other criteria.

Just thinking aloud here...

Yeah, no worries. Same.

I don't mind if it's a parameter for evaluate I'd just need some guidance on how to execute it if that's something you wanted me to help with.

@josdejong
Copy link
Owner

Good to hear you have a workaround for the time being, that gives peace of mind :)

Thanks for thinking along with trying to get the use cases clear 👍 . I think it's definitely an issue that needs to be addressed. We should make it possible to be able to customize the behavior for undefined symbols. It's also a bit of an edge case, so it may not be necessary to create a full-fledged API for it, as long as it's easy to hook up something custom in this regard.

I tried hacking the prototype but that didn't work

I do like that idea! It doesn't work right now because the function undef is not exposed, but we could very easily change that. Then you could replace the method with your own logic. It would work similar to customizing isAlpha in the expression parser for example (see docs).

I've worked out this idea in #2035. What do you think?

@koosvanderkolk
Copy link

koosvanderkolk commented Dec 11, 2020

We have a slightly different use case: we try to evaluate expressions like:

foo <= 0

We have two cases:

  • "foo" exists and has a numeric value
  • "foo" does not exist.

In the second case, we tried to set "foo" to boolean false, but

false <= 0

evaluates as 'true' in JavaScript / Mathjs.

Our next trick was to give "foo" the value 'undefined' (which is, in JavaScript at least, something different than "foo" being not defined;

var a = undefined;
a === undefined; //true
b === undefined; // Uncaught ReferenceError: b is not defined

)

Our trick works in JavaScript, but not in Mathjs, as it throws an error.

@josdejong
Copy link
Owner

@koosvanderkolk thanks for sharing your usecase. So would the proposed solution in #2035 solve your issue? (the "foo" does not exist part of it, the has a numeric value needs a different solution as we're discussing in #2060).

ping @eddieajau what are your thoughts about #2035? Would it solve your case?

@koosvanderkolk
Copy link

@josdejong I am not sure, as I would do this:

math.SymbolNode.prototype.handleUndefinedSymbol = function (name) {
  return undefined;
}

but I cannot foresee whether this would work (in all cases).

@josdejong
Copy link
Owner

What would be a solution for your case(s) Koos?

@koosvanderkolk
Copy link

A setting which makes mathjs treat 'undefined' as native JS treats it would work I think.

For the time being, we do have a workaround in #2060, but you mentioned in #2060 that there are problems with our solution.

@josdejong
Copy link
Owner

So far mathjs only supports null, which is on purpose. Having to support both null and undefined makes things much more complicated. Having nullable variables is already complicated enough ("The Billion Dollar Mistake"), let alone two types of "nothing" 😉.

Just a thought: it may be possible to create an extra conversion function to automatically convert undefined into null. Similar to how a conversion function is added from Fraction to BigNumber in the following example: https://mathjs.org/examples/advanced/convert_fraction_to_bignumber.js.html

josdejong added a commit that referenced this issue Apr 10, 2021
…nstead when

  evaluating a non-existing function, and expose internal functions `FunctionNode.onUndefinedFunction(name)` and `SymbolNode.onUndefinedSymbol(name)`
@josdejong
Copy link
Owner

The just published v9.3.1 contains functions FunctionNode.onUndefinedFunction(name) and SymbolNode.onUndefinedSymbol(name) which can be overridden to change the behavior.

@koosvanderkolk
Copy link

Thanks!

joshhansen pushed a commit to LearnSomethingTeam/mathjs that referenced this issue Sep 16, 2021
…ed function ..." instead when

  evaluating a non-existing function, and expose internal functions `FunctionNode.onUndefinedFunction(name)` and `SymbolNode.onUndefinedSymbol(name)`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants