Make the behavior of mod for BigNumber and Fraction consistent with the behavior of number

[josdejong]

Discovered during the discussion in #2617: in mathjs, mod is implemented the mathematical way, which differs a bit from the typical programmatical way: math.mod(22, 0) returns 22. However, this behavior is not yet implemented for BigNumber and Fraction. We should make that behavior consistent:

math.mod(22, 0)                                  // 22 as expected
math.mod(math.bignumber(22), math.bignumber(0))  // NaN, should be BigNumber 22
math.mod(math.fraction(22), math.fraction(0))    // NaN, should be Fraction 22

The behavior of mod for numbers is implemented as follows, we should do the same for BigNumber and Fraction:

mathjs/src/plain/number/arithmetic.js

Lines 159 to 171 in 09a044f

	export function modNumber (x, y) {
	if (y > 0) {
	// We don't use JavaScript's % operator here as this doesn't work
	// correctly for x < 0 and x === 0
	// see https://en.wikipedia.org/wiki/Modulo_operation
	return x - y * Math.floor(x / y)
	} else if (y === 0) {
	return x
	} else { // y < 0
	// TODO: implement mod for a negative divisor
	throw new Error('Cannot calculate mod for a negative divisor')
	}
	}

Help would be welcome. Anyone interested in picking this up?

[gwhitney]

[Took the liberty of fixing the zeros in the issue statement to be 22s]

[josdejong]

Ah, yes, thanks 😅

[mdeshpande12]

@josdejong @gwhitney if this is still open I'd like to work on this issue.

[josdejong]

Thanks @mdeshpande12 , your help is very welcome!

A quick check shows that this behavior seems to be consistent for BigNumber now, but not yet for Fraction. It would be good though to have a more thorough look at both the behavior of BigNumber and Fraction.

[mdeshpande12]

@josdejong I did a little exploration of myself and it turns out, behaviour is consistent for both BigNumber, Fraction as that of the number in the latest version. Details are as follows:

Latest Version of mathjs: 12.2.1
Version used for comparison: 10.6.4

Please refer to the attached screenshots:

Result: I believe this issue is resolved with the latest version, and can be closed

[josdejong]

Ha, you're right! I did another quick test too, and the original issue doesn't occur anymore. That's good news :)