mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

null/undefined treated as valid only for left side of comparison operators

Open rcrogers opened this issue 6 years ago • 7 comments

Using [email protected]

When the null value is on the right side, the correct error is thrown (actual: null):

> require('mathjs').eval('1 > null')
TypeError: Unexpected type of argument in function larger (expected: number or Array or DenseMatrix or SparseMatrix or string or boolean or Matrix or BigNumber or Complex or Fraction, actual: null, index: 1)

However, when the null value is on the left side, the error mistakenly identifies the right side as the source of the problem (actual: number):

> require('mathjs').eval('null > 1')
TypeError: Unexpected type of argument in function larger (expected: Array or DenseMatrix or SparseMatrix or Matrix, actual: number, index: 1)

rcrogers avatar Jan 31 '19 17:01 rcrogers

Thanks for reporting, the second error isn't very helpful indeed.

It's not a very easy fix, has to do with the way the types of a function are matched against the different signatures of the function.

josdejong avatar Feb 03 '19 19:02 josdejong

Makes sense. How feasible would it be to change the function def resolution so that a match only occurs if both operator arguments are compatible with the parameters of the function signature?

rcrogers avatar Feb 11 '19 15:02 rcrogers

The reason in this case is because there are signatures like larger(any, DenseMatrix) to allow multiplying a scalar and a matrix:

https://github.com/josdejong/mathjs/blob/develop/src/function/relational/larger.js#L108-L132

So, thinking about it, the solution may be quite easy (though it requires some labor): replace the any wildcard with explicit types, ie. replace

  const larger = typed('larger', {
    // ...
    'any, DenseMatrix': function (x, y) {
      return algorithm14(y, x, larger, true)
    },
    // ...
  })

with

  const larger = typed('larger', {
    // ...
    'boolean | number | BigNumber | Fraction | Complex | Unit, DenseMatrix': function (x, y) {
      return algorithm14(y, x, larger, true)
    },
    // ...
  })

for all six occurrences, and do this for all operators (not only larger).

Downside is that when you add support for a new data type, you have to add the data type at many places. This can be partly resolved by defining one string on top and use that everywhere like:

  const any = 'boolean | number | BigNumber | Fraction | Complex | Unit'

  const larger = typed('larger', {
    // ...
    `${any}, DenseMatrix`: function (x, y) {
      return algorithm14(y, x, larger, true)
    },
    // ...
  })

Anyone interested in thinking such a solution through and implementing the fix?

josdejong avatar Feb 11 '19 19:02 josdejong

I too facing the same issue when I passed the null into the expression..

Please find the error message below ERROR TypeError: Unexpected type of argument in function addScalar (expected: number or string or boolean or BigNumber or Complex or Fraction, actual: null, index: 1)

Do you have any workaround or temporary solution for this case ?

nelsongnanaraj avatar Apr 28 '21 17:04 nelsongnanaraj

Hey Nelson, The obvious workaround is not passing null into expressions 😅️ Is there a specific reason for why you do it?

cshaa avatar Apr 29 '21 01:04 cshaa

I am working for dynamic expression parser using mathjs. So i have to allow the null values in mathjs. Here i shared the simple case of expression.

ifCondition(isNull(dateField1),today(),addDate(dateField1,add(numberField1,numberField2),'DAY'))

ifCondition, isNull, today, addDate are my own defined functions.

IsNull function is highly required for check most of the business cases. Without allowing null in data, How we can identify and return whether the specific expression having null or not.

nelsongnanaraj avatar May 04 '21 07:05 nelsongnanaraj

This is definitely a bug, and it definitely still exists. In fact, it's worse, in that if M is a matrix, null > M also reports TypeError: Unexpected type of argument in function larger (expected: Array or DenseMatrix or SparseMatrix or Matrix, actual: number, index: 1) when there isn't even any number in that expression... Hopefully this issue will go away in v13, but if it doesn't, it should be explicitly addressed.

gwhitney avatar Oct 06 '23 04:10 gwhitney