Skip to content

Commit

Permalink
fix: use utility isNaN for consistent max and min results
Browse files Browse the repository at this point in the history
  • Loading branch information
orelbn committed Feb 14, 2025
1 parent ad36350 commit 96cc771
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 18 deletions.
6 changes: 3 additions & 3 deletions src/expression/transform/max.transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { createMax } from '../../function/statistics/max.js'
import { lastDimToZeroBase } from './utils/lastDimToZeroBase.js'

const name = 'max'
const dependencies = ['typed', 'config', 'numeric', 'larger']
const dependencies = ['typed', 'config', 'numeric', 'larger', 'isNaN']

export const createMaxTransform = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, larger }) => {
const max = createMax({ typed, config, numeric, larger })
export const createMaxTransform = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, larger, isNaN }) => {
const max = createMax({ typed, config, numeric, larger, isNaN })

/**
* Attach a transform function to math.max
Expand Down
6 changes: 3 additions & 3 deletions src/expression/transform/min.transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { createMin } from '../../function/statistics/min.js'
import { lastDimToZeroBase } from './utils/lastDimToZeroBase.js'

const name = 'min'
const dependencies = ['typed', 'config', 'numeric', 'smaller']
const dependencies = ['typed', 'config', 'numeric', 'smaller', 'isNaN']

export const createMinTransform = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, smaller }) => {
const min = createMin({ typed, config, numeric, smaller })
export const createMinTransform = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, smaller, isNaN }) => {
const min = createMin({ typed, config, numeric, smaller, isNaN })

/**
* Attach a transform function to math.min
Expand Down
8 changes: 4 additions & 4 deletions src/function/statistics/max.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { safeNumberType } from '../../utils/number.js'
import { improveErrorMessage } from './utils/improveErrorMessage.js'

const name = 'max'
const dependencies = ['typed', 'config', 'numeric', 'larger']
const dependencies = ['typed', 'config', 'numeric', 'larger', 'isNaN']

export const createMax = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, larger }) => {
export const createMax = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, larger, isNaN }) => {
/**
* Compute the maximum value of a matrix or a list with values.
* In case of a multidimensional array, the maximum of the flattened array
Expand Down Expand Up @@ -83,8 +83,8 @@ export const createMax = /* #__PURE__ */ factory(name, dependencies, ({ typed, c

deepForEach(array, function (value) {
try {
if (typeof value === 'number' && isNaN(value)) {
res = NaN
if (isNaN(value)) {
res = value
} else if (res === undefined || larger(value, res)) {
res = value
}
Expand Down
8 changes: 4 additions & 4 deletions src/function/statistics/min.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { safeNumberType } from '../../utils/number.js'
import { improveErrorMessage } from './utils/improveErrorMessage.js'

const name = 'min'
const dependencies = ['typed', 'config', 'numeric', 'smaller']
const dependencies = ['typed', 'config', 'numeric', 'smaller', 'isNaN']

export const createMin = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, smaller }) => {
export const createMin = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, numeric, smaller, isNaN }) => {
/**
* Compute the minimum value of a matrix or a list of values.
* In case of a multidimensional array, the minimum of the flattened array
Expand Down Expand Up @@ -83,8 +83,8 @@ export const createMin = /* #__PURE__ */ factory(name, dependencies, ({ typed, c

deepForEach(array, function (value) {
try {
if (typeof value === 'number' && isNaN(value)) {
min = NaN
if (isNaN(value)) {
min = value
} else if (min === undefined || smaller(value, min)) {
min = value
}
Expand Down
9 changes: 7 additions & 2 deletions test/unit-tests/function/statistics/max.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const Complex = math.Complex
const DenseMatrix = math.DenseMatrix
const fraction = math.fraction
const max = math.max
const unit = math.unit

describe('max', function () {
it('should return the max of numbers', function () {
Expand Down Expand Up @@ -89,6 +90,10 @@ describe('max', function () {
assert(isNaN(max([1, 3, NaN])))
assert(isNaN(max([NaN, NaN, NaN])))
assert(isNaN(max(NaN, NaN)))
assert(isNaN(max(BigNumber(NaN), BigNumber(123))))
assert(isNaN((max(BigNumber(123), BigNumber(NaN), NaN))))
assert(isNaN(max(unit(NaN, 's'), unit(123, 's')).value))
assert(isNaN(max(unit(123, 's'), unit(NaN, 's')).value))
})

it('should return the largest of mixed types', function () {
Expand Down Expand Up @@ -124,8 +129,8 @@ describe('max', function () {
assert.throws(function () { max([[2, new Date(), 4]]) }, /TypeError: Cannot calculate max, unexpected type of argument/)
assert.throws(function () { max([2, null, 4]) }, /TypeError: Cannot calculate max, unexpected type of argument/)
assert.throws(function () { max([[2, 5], [4, null], [1, 7]], 0) }, /TypeError: Cannot calculate max, unexpected type of argument/)
assert.throws(function () { max('a', 'b') }, /Error: Cannot convert "b" to a number/)
assert.throws(function () { max('a') }, /SyntaxError: String "a" is not a valid number/)
assert.throws(function () { max('a', 'b') }, /Error: Cannot convert "a" to a number/)
assert.throws(function () { max('a') }, /Error: Cannot convert "a" to a number/)
})

it('should return undefined if called with an empty array', function () {
Expand Down
9 changes: 7 additions & 2 deletions test/unit-tests/function/statistics/min.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const Complex = math.Complex
const DenseMatrix = math.DenseMatrix
const fraction = math.fraction
const min = math.min
const unit = math.unit

describe('min', function () {
it('should return the min between several numbers', function () {
Expand Down Expand Up @@ -98,6 +99,10 @@ describe('min', function () {
assert(isNaN(min([1, 3, NaN])))
assert(isNaN(min([NaN, NaN, NaN])))
assert(isNaN(min(NaN, NaN, NaN)))
assert(isNaN(min(BigNumber(NaN), BigNumber(123))))
assert(isNaN((min(BigNumber(123), BigNumber(NaN), NaN))))
assert(isNaN(min(unit(NaN, 's'), unit(123, 's')).value))
assert(isNaN(min(unit(123, 's'), unit(NaN, 's')).value))
})

it('should return the smallest of mixed types', function () {
Expand Down Expand Up @@ -137,8 +142,8 @@ describe('min', function () {
assert.throws(function () { min([[2, new Date(), 4]]) }, /TypeError: Cannot calculate min, unexpected type of argument/)
assert.throws(function () { min([2, null, 4]) }, /TypeError: Cannot calculate min, unexpected type of argument/)
assert.throws(function () { min([[2, 5], [4, null], [1, 7]], 0) }, /TypeError: Cannot calculate min, unexpected type of argument/)
assert.throws(function () { min('a', 'b') }, /Error: Cannot convert "b" to a number/)
assert.throws(function () { min('a') }, /SyntaxError: String "a" is not a valid number/)
assert.throws(function () { min('a', 'b') }, /Error: Cannot convert "a" to a number/)
assert.throws(function () { min('a') }, /Error: Cannot convert "a" to a number/)
})

it('should LaTeX min', function () {
Expand Down

0 comments on commit 96cc771

Please sign in to comment.