Fixed #680: compare strings by their numerical value instead of alpha…

…betical order

HISTORY.md

@@ -2,6 +2,8 @@
 
 ## not yet released, version 4.0.0
 
+!!! BE CAREFUL: BREAKING CHANGES !!!
+
 Breaking changes:
 
 - Compiler of the expression parser is replaced with one that doesn't use
@@ -11,9 +13,14 @@ Breaking changes:
   - Internal code is easier to understand, maintain, and debug.
   Breaking change here: When using custom nodes in the expression parser, 
   the syntax of `_compile` has changed. This is an undocumented feature though.
-- Fixed #749: Changed `rad`, `deg`, and `grad` to have short prefixes,
+- Changed `rad`, `deg`, and `grad` to have short prefixes,
   and introduced `radian`, `degree`, and `gradian` and their plurals
-  having long prefixes.
+  having long prefixes. See #749.
+- Changed the behavior of relational functions (`compare`, `equal`,
+  `equalScalar`, `larger`, `largerEq`, `smaller`, `smallerEq`, `unequal`)
+  to compare strings by their numeric value they contain instead of
+  alphabetically. This also impacts functions `sort`, `min`, `max`,
+  `median`, and `partitionSelect`. See #680.
 
 ## 2018-01-17, version 3.20.1
 

lib/function/relational/compare.js

@@ -79,10 +79,6 @@ function factory (type, config, load, typed) {
       return compare(x.value, y.value);
     },
 
-    'string, string': function (x, y) {
-      return x === y ? 0 : (x > y ? 1 : -1);
-    },
-
     'Matrix, Matrix': function (x, y) {
       // result
       var c;

lib/function/relational/compareNatural.js

@@ -4,7 +4,6 @@ var naturalSort = require('javascript-natural-sort');
 
 function factory (type, config, load, typed) {
   var getTypeOf = load(require('../utils/typeof'));
-  var matrix = load(require('../../type/matrix/function/matrix'));
   var compare = load(require('./compare'));
 
   var compareBooleans = compare.signatures['boolean,boolean']

lib/function/relational/equalScalar.js

@@ -40,10 +40,6 @@ function factory (type, config, load, typed) {
         throw new Error('Cannot compare units with different base');
       }
       return equalScalar(x.value, y.value);
-    },
-
-    'string, string': function (x, y) {
-      return x === y;
     }
   });
 

lib/function/relational/larger.js

@@ -74,10 +74,6 @@ function factory (type, config, load, typed) {
       return larger(x.value, y.value);
     },
 
-    'string, string': function (x, y) {
-      return x > y;
-    },
-
     'Matrix, Matrix': function (x, y) {
       // result
       var c;

lib/function/relational/largerEq.js

@@ -70,10 +70,6 @@ function factory (type, config, load, typed) {
       return largerEq(x.value, y.value);
     },
 
-    'string, string': function (x, y) {
-      return x >= y;
-    },
-
     'Matrix, Matrix': function (x, y) {
       // result
       var c;

lib/function/relational/smaller.js

@@ -74,10 +74,6 @@ function factory (type, config, load, typed) {
       return smaller(x.value, y.value);
     },
 
-    'string, string': function (x, y) {
-      return x < y;
-    },
-
     'Matrix, Matrix': function (x, y) {
       // result
       var c;

lib/function/relational/smallerEq.js

@@ -69,10 +69,6 @@ function factory (type, config, load, typed) {
       return smallerEq(x.value, y.value);
     },
 
-    'string, string': function (x, y) {
-      return x <= y;
-    },
-
     'Matrix, Matrix': function (x, y) {
       // result
       var c;

lib/function/relational/unequal.js

@@ -186,10 +186,6 @@ function factory (type, config, load, typed) {
         throw new Error('Cannot compare units with different base');
       }
       return unequal(x.value, y.value);
-    },
-
-    'string, string': function (x, y) {
-      return x !== y;
     }
   });
 

lib/function/set/setDifference.js

@@ -5,7 +5,6 @@ var identify = require('../../utils/array').identify;
 var generalize = require('../../utils/array').generalize;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
   var index = load(require('../../type/matrix/MatrixIndex'));
   var matrix = load(require('../../type/matrix/DenseMatrix'));
   var size = load(require('../matrix/size'));
@@ -49,7 +48,7 @@ function factory (type, config, load, typed) {
         for (var i=0; i<b1.length; i++) {
           inb2 = false;
           for (var j=0; j<b2.length; j++) {
-            if (equal(b1[i].value, b2[j].value) && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
+            if (compareNatural(b1[i].value, b2[j].value) === 0 && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
               inb2 = true;
               break;
             }

lib/function/set/setDistinct.js

@@ -3,7 +3,6 @@
 var flatten = require('../../utils/array').flatten;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
   var index = load(require('../../type/matrix/MatrixIndex'));
   var matrix = load(require('../../type/matrix/DenseMatrix'));
   var size = load(require('../matrix/size'));
@@ -39,7 +38,7 @@ function factory (type, config, load, typed) {
         var result = [];
         result.push(b[0]);
         for (var i=1; i<b.length; i++) {
-          if (!equal(b[i], b[i-1])) {
+          if (compareNatural(b[i], b[i-1]) !== 0) {
             result.push(b[i]);
           }
         }

lib/function/set/setIntersect.js

@@ -5,7 +5,6 @@ var identify = require('../../utils/array').identify;
 var generalize = require('../../utils/array').generalize;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
   var index = load(require('../../type/matrix/MatrixIndex'));
   var matrix = load(require('../../type/matrix/DenseMatrix'));
   var size = load(require('../matrix/size'));
@@ -44,7 +43,7 @@ function factory (type, config, load, typed) {
         var result = [];
         for (var i=0; i<b1.length; i++) {
           for (var j=0; j<b2.length; j++) {
-              if (equal(b1[i].value, b2[j].value) && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
+              if (compareNatural(b1[i].value, b2[j].value) === 0 && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
                 result.push(b1[i]);
                 break;
               }

lib/function/set/setIsSubset.js

@@ -4,7 +4,6 @@ var flatten = require('../../utils/array').flatten;
 var identify = require('../../utils/array').identify;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
   var index = load(require('../../type/matrix/MatrixIndex'));
   var size = load(require('../matrix/size'));
   var subset = load(require('../matrix/subset'));
@@ -45,7 +44,7 @@ function factory (type, config, load, typed) {
       for (var i=0; i<b1.length; i++) {
         inb2 = false;
         for (var j=0; j<b2.length; j++) {
-          if (equal(b1[i].value, b2[j].value) && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
+          if (compareNatural(b1[i].value, b2[j].value) === 0 && b1[i].identifier === b2[j].identifier) { // the identifier is always a decimal int
             inb2 = true;
             break;
           }

lib/function/set/setMultiplicity.js

@@ -3,7 +3,7 @@
 var flatten = require('../../utils/array').flatten;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
+  var compareNatural = load(require('../relational/compareNatural'));
   var index = load(require('../../type/matrix/MatrixIndex'));
   var size = load(require('../matrix/size'));
   var subset = load(require('../matrix/subset'));
@@ -37,7 +37,7 @@ function factory (type, config, load, typed) {
       var b = flatten(Array.isArray(a) ? a : a.toArray());
       var count = 0;
       for (var i=0; i<b.length; i++) {
-        if (equal(b[i], e)) {
+        if (compareNatural(b[i], e) === 0) {
           count++;
         }
       }

lib/function/set/setSize.js

@@ -3,7 +3,6 @@
 var flatten = require('../../utils/array').flatten;
 
 function factory (type, config, load, typed) {
-  var equal = load(require('../relational/equal'));
   var compareNatural = load(require('../relational/compareNatural'));
 
   /**
@@ -39,7 +38,7 @@ function factory (type, config, load, typed) {
         var b = flatten(Array.isArray(a) ? a : a.toArray()).sort(compareNatural);
         var count = 1;
         for (var i=1; i<b.length; i++) {
-          if (!equal(b[i], b[i-1])) {
+          if (compareNatural(b[i], b[i-1]) !== 0) {
             count++;
           }
         }

lib/function/set/setSymDifference.js

@@ -6,7 +6,6 @@ function factory (type, config, load, typed) {
   var index = load(require('../../type/matrix/MatrixIndex'));
   var concat = load(require('../matrix/concat'));
   var size = load(require('../matrix/size'));
-  var sort = load(require('../matrix/sort'));
   var subset = load(require('../matrix/subset'));
   var setDifference = load(require('../set/setDifference'));
 

lib/function/statistics/median.js

@@ -1,7 +1,6 @@
 'use strict';
 
 var flatten = require('../../utils/array').flatten;
-var reduce = require('../../utils/collection/reduce');
 var containsCollections = require('../../utils/collection/containsCollections');
 
 function factory (type, config, load, typed) {

test/expression/security.test.js

@@ -229,7 +229,7 @@ describe('security', function () {
   it ('should not allow inserting fake nodes with bad code via node.map or node.transform', function () {
     assert.throws(function () {
       math.eval("badValue = {\"isNode\": true, \"_compile\": eval(\"f(a, b) = \\\"eval\\\"\")}; x = eval(\"f(child, path, parent) = path ==\\\"value\\\" ? newChild : child\", {\"newChild\": badValue}); parse(\"x = 1\").map(x).compile().eval()(\"console.log(\'hacked\')\")")
-    }, /TypeError: Callback function must return a Node/);
+    }, /Error: Cannot convert "object" to a number/);
 
     assert.throws(function () {
       math.eval("badValue = {\"isNode\": true, \"type\": \"ConstantNode\", \"valueType\": \"string\", \"_compile\": eval(\"f(a, b) = \\\"eval\\\"\")}; x = eval(\"f(child, path, parent) = path ==\\\"value\\\" ? newChild : child\", {\"newChild\": badValue}); parse(\"x = 1\").map(x).compile().eval()(\"console.log(\'hacked...\')\")")
@@ -275,7 +275,7 @@ describe('security', function () {
   it ('should not allow creating a bad ArrayNode', function () {
     assert.throws(function () {
       math.eval('g(x)="eval";f(x)=({join: g});fakeArrayNode={isNode: true, type: "ArrayNode", items: {map: f}};injectFakeArrayNode(child,path,parent)=path=="value"?fakeArrayNode:child;parse("a=3").map(injectFakeArrayNode).compile().eval()[1]("console.log(\'hacked...\')")')
-    }, /TypeError: Callback function must return a Node/);
+    }, /Error: Cannot convert "object" to a number/);
   })
 
   it ('should not allow unescaping escaped double quotes', function () {

test/function/matrix/partitionSelect.test.js

@@ -12,13 +12,6 @@ describe('partitionSelect', function() {
     assert.equal(partitionSelect([5,10,1], 2), 10);
   });
 
-  it('should sort an array with strings', function() {
-    assert.equal(partitionSelect(['C', 'B', 'A', 'D'], 0), 'A');
-    assert.equal(partitionSelect(['C', 'B', 'A', 'D'], 1), 'B');
-    assert.equal(partitionSelect(['C', 'B', 'A', 'D'], 2), 'C');
-    assert.equal(partitionSelect(['C', 'B', 'A', 'D'], 3), 'D');
-  });
-
   it('should sort a Matrix', function() {
     assert.equal(partitionSelect(matrix([5,10,1]), 0), 1);
     assert.equal(partitionSelect(matrix([5,10,1]), 1), 5);

test/function/matrix/sort.test.js

@@ -9,8 +9,9 @@ describe('sort', function() {
   });
 
   it('should sort an array with strings', function() {
-    assert.deepEqual(math.sort(['C', 'B', 'A', 'D']), ['A', 'B', 'C', 'D']);
-    assert.deepEqual(math.sort(['1', '2', '10'], 'asc'), ['1', '10', '2']);
+    assert.deepEqual(math.sort(['C', 'B', 'A', 'D'], 'natural'), ['A', 'B', 'C', 'D']);
+    assert.deepEqual(math.sort(['1', '2', '10'], 'asc'), ['1', '2', '10']);
+    assert.deepEqual(math.sort(['1', '2', '10'], 'natural'), ['1', '2', '10']);
   });
 
   it('should sort a Matrix', function() {

test/function/relational/compare.test.js

@@ -113,22 +113,16 @@ describe('compare', function() {
     assert.throws(function () {compare(unit('100cm'), bignumber(22));});
   });
 
-  it('should perform lexical comparison for two strings', function() {
+  it('should compare two strings', function() {
     assert.equal(compare('0', 0), 0);
-
-    assert.equal(compare('abd', 'abc'), 1);
-    assert.equal(compare('abc', 'abc'), 0);
-    assert.equal(compare('abc', 'abd'), -1);
-
-    assert.equal(compare('10', '2'), -1);
-
+    assert.equal(compare('10', '2'), 1);
   });
 
   describe('Array', function () {
 
     it('should compare array - scalar', function () {
-      assert.deepEqual(compare('B', ['A', 'B', 'C']), [1, 0, -1]);
-      assert.deepEqual(compare(['A', 'B', 'C'], 'B'), [-1, 0, 1]);
+      assert.deepEqual(compare(2, [1, 2, 3]), [1, 0, -1]);
+      assert.deepEqual(compare([1, 2, 3], 2), [-1, 0, 1]);
     });
 
     it('should compare array - array', function () {
@@ -147,8 +141,8 @@ describe('compare', function() {
   describe('DenseMatrix', function () {
 
     it('should compare dense matrix - scalar', function () {
-      assert.deepEqual(compare('B', matrix(['A', 'B', 'C'])), matrix([1, 0, -1]));
-      assert.deepEqual(compare(matrix(['A', 'B', 'C']), 'B'), matrix([-1, 0, 1]));
+      assert.deepEqual(compare(2, matrix([1, 2, 3])), matrix([1, 0, -1]));
+      assert.deepEqual(compare(matrix([1, 2, 3]), 2), matrix([-1, 0, 1]));
     });
 
     it('should compare dense matrix - array', function () {
@@ -167,8 +161,8 @@ describe('compare', function() {
   describe('SparseMatrix', function () {
 
     it('should compare sparse matrix - scalar', function () {
-      assert.deepEqual(compare('B', sparse([['A', 'B'], ['C', 'X']])), matrix([[1, 0], [-1, -1]]));
-      assert.deepEqual(compare(sparse([['A', 'B'], ['C', 'X']]), 'B'), matrix([[-1, 0], [1, 1]]));
+      assert.deepEqual(compare(2, sparse([[1, 2], [3, 4]])), matrix([[1, 0], [-1, -1]]));
+      assert.deepEqual(compare(sparse([[1, 2], [3, 4]]), 2), matrix([[-1, 0], [1, 1]]));
     });
 
     it('should compare sparse matrix - array', function () {

test/function/relational/equal.test.js

@@ -137,6 +137,15 @@ describe('equal', function() {
     assert.equal(equal(2, undefined), false);
   });
 
+  it('should compare strings by their numeric value', function() {
+    assert.equal(equal('2', 2), true);
+    assert.equal(equal(10, '10'), true);
+    assert.equal(equal('1e2', '100'), true);
+    assert.equal(equal(10, '8'), false);
+
+    assert.throws(function () {equal('A', 'B')}, /Cannot convert "A" to a number/);
+  });
+
   it('should apply configuration option epsilon', function() {
     var mymath = math.create();
     assert.equal(mymath.equal(1, 0.991), false);
@@ -159,17 +168,11 @@ describe('equal', function() {
     assert.throws(function () {equal(math.unit(5, 'km'), math.unit(100, 'gram'));});
   });
 
-  it('should compare two strings correctly', function() {
-    assert.equal(equal('0', 0), true);
-    assert.equal(equal('Hello', 'hello'), false);
-    assert.equal(equal('hello', 'hello'), true);
-  });
-
   describe('Array', function () {
 
     it('should compare array - scalar', function () {
-      assert.deepEqual(equal('B', ['A', 'B', 'C']), [false, true, false]);
-      assert.deepEqual(equal(['A', 'B', 'C'], 'B'), [false, true, false]);
+      assert.deepEqual(equal(2, [1, 2, 3]), [false, true, false]);
+      assert.deepEqual(equal([1, 2, 3], 2), [false, true, false]);
     });
 
     it('should compare array - array', function () {
@@ -192,8 +195,8 @@ describe('equal', function() {
   describe('DenseMatrix', function () {
 
     it('should compare dense matrix - scalar', function () {
-      assert.deepEqual(equal('B', matrix(['A', 'B', 'C'])), matrix([false, true, false]));
-      assert.deepEqual(equal(matrix(['A', 'B', 'C']), 'B'), matrix([false, true, false]));
+      assert.deepEqual(equal(2, matrix([1, 2, 3])), matrix([false, true, false]));
+      assert.deepEqual(equal(matrix([1, 2, 3]), 2), matrix([false, true, false]));
     });
 
     it('should compare dense matrix - array', function () {
@@ -212,8 +215,8 @@ describe('equal', function() {
   describe('SparseMatrix', function () {
 
     it('should compare sparse matrix - scalar', function () {
-      assert.deepEqual(equal('B', sparse([['A', 'B'], ['C', 'D']])), matrix([[false, true], [false, false]]));
-      assert.deepEqual(equal(sparse([['A', 'B'], ['C', 'D']]), 'B'), matrix([[false, true], [false, false]]));
+      assert.deepEqual(equal(2, sparse([[1, 2], [3, 4]])), matrix([[false, true], [false, false]]));
+      assert.deepEqual(equal(sparse([[1, 2], [3, 4]]), 2), matrix([[false, true], [false, false]]));
     });
 
     it('should compare sparse matrix - array', function () {