math_eval() does not handle negative numbers

[rrb3942]

Attempting to use a negative number with math_eval in the mathops module results in a 'Mismatched tokens expression'

Example:
math_eval("10 + (-5)", "$var(test)");
Gives the following errors:
ERROR:mathops:pop_number: RPN Stack Empty
ERROR:mathops:pop_number: RPN Stack Empty
ERROR:mathops:evaluate_rpn_output: Parse expr error: stack has 0 elements
ERROR:mathops:evaluate_exp: Mismatched tokens in expression: <10 + (-5)>

Happens with or without the parentheses.

[liviuchircu]

From the module documentation:

Currently allowed syntax for specifying an expression:

• Nested parantheses
• binary + - / * operators

If you really need this feature, I would be more than happy to review a Pull Request. Otherwise, I will put it on the TODO list, but with low prio.

[rrb3942]

I understand what your saying, but coming at this from the stance of a user I think it is a problem since it can greatly complicate an algorithm if you cannot pass in a negative number.

It is also a problem when you can't feed the output of math_eval back into itself, especially since at the script level these are stored as strings, which can't be converted to an int to check their sign. This is a problem if you would like to keep an intermediate result around, and generally just unintuitive.

    math_eval("5-7","$var(temp)");
    //outputs 'Temp is : -2.000000'
    xlog("Temp is : $var(temp)");

    //Fails with 'cannot get left var value'
   //Fails without transformation as well but instead receives 'invalid LESS_THAN operation: left is          NONE/STRING_VAL, right is NUMBER/NO_VAL'
    if ($(var(temp){s.int}) < 0) {
            xlog("$var(temp) is negative");
    }

   //Will fail because $var(temp) is negative
    math_eval("$var(temp) + 9", "$var(temp2)");

I suppose you could do a substr to get the first character and check that way, but now we are doing string operations to deal with a math problem and we have to check the sign after every subtraction. Unless you are going to remove support for subtraction, negative numbers should really be handled.

What are you thoughts on using something like TinyExpr from https://github.com/codeplea/tinyexpr ?

It seems to handle the above cases well and even opens up for using standard C math functions in the expression as well. It also has test coverage, so it should handle potential corner cases better. It is zlib licensed, which I think is compatible? And it should be pretty easy to include.

If you think that looks like a reasonable solution I don't mind writing up a patch to convert the math_eval() function to use it.

[liviuchircu]

What are you thoughts on using something like TinyExpr from https://github.com/codeplea/tinyexpr ?

I think it's more than a reasonable solution, especially since it's easy to use (1 file + 1 header). I only wrote our own quick parser because I didn't want to impose too many dependencies, but the fact that TinyExpr is zlib licenced is a big reason to go with it! Overall, I'm all for the switch to TinyExpr in mathops.

//Fails with 'cannot get left var value'
if ($(var(temp){s.int}) < 0) {
...

This looks like a limitation - we definitely need to make {s.int} more permissive with such input, since it should have easily grabbed the -2 part, even if the input string were "-2.foobar"!

[rrb3942]

See pull request #812 (#812) for switching math_eval to use tinyexpr.