Consistency and typing problems in isNumeric and hasNumericValue
- Dominant language
- JavaScript
- Stars
- 15.1k
- Forks
- 1.3k
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the bug**
* Complex numbers are surely numbers, just from a different system. Yet `isNumeric` and `hasNumericValue` return false on them. This directly contradicts (for example) the TypeScript type definition `MathNumericType = number | BigNumber | Fraction | Complex`
* `isNumeric` and `hasNumericValue` fail on e.g. `{a: 1, b: 2}`. Surely they should not fail on any inputs. In this particular case, I think the desired return value is 'false'. One could make an argument for `{a: true, b: true}` by analogy with how Arrays are handled, but I don't think any mathjs functions distribute over object structure in this way.
* Whether booleans are `isNumeric` is very debatable. And the fact it currently returns true contradicts the TypeScript `MathNumericType` definition, in the opposite direction from `Complex`. Certainly a boolean `hasNumericValue` in exactly the same way as a string does, in that they automatically convert to 0 and 1 in any numeric context. But the fact that `true * false` is `0` rather than false, the same way that `"1" * "0"` is just `0` not `"0"` suggests that booleans are not themselves numeric. I would strongly suggest leaving `hasNumericValue` true for booleans, and either:
* making `isNumeric` false for booleans, or
* making `*` non-short-circuiting `and`, and `+` non-short-circuiting `or`. Then the booleans are exactly a copy of the field of two elements -- definitely numeric. And in this case, add boolean to TypeScript MathNumericType, which should align with `isNumeric`.
* Typescript typing of `isNumeric` is currently: `isNumeric(x: any): x is number | BigNumber | Fraction | boolean`. This is wrong in three ways:
* isNumeric only accepts (currently a subset of) MathTypes, not truly any value. It could be extended to return false on anything else, in which case the input type is correct (but should be changed to `x: unknown` to comport with current usage and eliminate the disabling of the any rule); or the typing could be changed to reflect the types it actually takes.
* isNumeric is not (at least not currently) a type guard. It does not even always return a boolean value (e.g. on arrays).
* Even when isNumeric returns a truthy value, its argument is not necessarily one of the listed types. It could be a bigint, or an Array or a Matrix.
* Typescript typing of `hasNumericValue` should be similar to that for `isNumeric` and hence might need to be updated when the typing of `isNumeric` is fixed in some way or another.
* Since `isNumeric` returning a truthy value does not guarantee that its argument is a scalar, mathjs should have an `isScalar` function that _does_ serve as a type guard. Otherwise, it is difficult to use the result of `math.evaluate()` in some mathjs functions in typescript.
**To Reproduce**
```
math.isNumeric(Complex(1,2)) // returns false, expect true
math.isNumeric(false) // returns true, dubious
math.isNumeric({a: 1, b:2}) // throws TypeError, expect successful return of false
if (math.isNumeric(A)) return A[0] // could be perfectly OK code, but TypeScript reports: error TS7053:
// Element implicitly has an 'any' type because expression of type '0' can't be used to
// index type 'number | BigNumber | Fraction'.
if (math.isNumeric(n)) x = math.cube(n) // No TypeScript error, but is not actually OK; can
// throw a TypeError at runtime if n happens to be [3, 4, 5], say.
return math.isNumeric([3,4,5])[0] // Is perfectly OK, but TypeScript reports: error TS7053 for type Boolean.
```
Contributor guide
Assessment
This issue has not been assessed yet.