Merge pull request #694 from palantir/qscale-warn-nan-inf

QuantitiveScale warns and ignores attempts to set domain to include NaN ...
palantir · Jul 13, 2014 · ba31e6c · ba31e6c
2 parents 37e0a3a + 7f2ced1
commit ba31e6c
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 21 deletions.
diff --git a/plottable.js b/plottable.js
@@ -2786,9 +2786,6 @@ var Plottable;
             };
 
             Scale.prototype._setDomain = function (values) {
-                if (values[0] === Infinity || values[0] === -Infinity || values[1] === Infinity || values[1] === -Infinity) {
-                    throw new Error("data cannot contain Infinity or -Infinity");
-                }
                 this._d3Scale.domain(values);
                 this.broadcaster.broadcast();
             };
@@ -3542,6 +3539,17 @@ var Plottable;
                 return _super.prototype.domain.call(this, values);
             };
 
+            QuantitiveScale.prototype._setDomain = function (values) {
+                var isNaNOrInfinity = function (x) {
+                    return x !== x || x === Infinity || x === -Infinity;
+                };
+                if (isNaNOrInfinity(values[0]) || isNaNOrInfinity(values[1])) {
+                    console.log("Warning: QuantitiveScales cannot take NaN or Infinity as a domain value. Ignoring.");
+                    return;
+                }
+                _super.prototype._setDomain.call(this, values);
+            };
+
             QuantitiveScale.prototype.interpolate = function (factory) {
                 if (factory == null) {
                     return this._d3Scale.interpolate();

diff --git a/src/core/scale.ts b/src/core/scale.ts
@@ -82,10 +82,6 @@ export module Abstract {
     }
 
     public _setDomain(values: any[]) {
-      if (values[0] === Infinity || values[0] === -Infinity ||
-          values[1] === Infinity || values[1] === -Infinity) {
-        throw new Error("data cannot contain Infinity or -Infinity");
-      }
       this._d3Scale.domain(values);
       this.broadcaster.broadcast();
     }

diff --git a/src/scales/quantitiveScale.ts b/src/scales/quantitiveScale.ts
@@ -49,6 +49,15 @@ export module Abstract {
       return super.domain(values); // need to override type sig to enable method chaining :/
     }
 
+    public _setDomain(values: any[]) {
+        var isNaNOrInfinity = (x: any) => x !== x || x === Infinity || x === -Infinity;
+        if (isNaNOrInfinity(values[0]) || isNaNOrInfinity(values[1])) {
+            console.log("Warning: QuantitiveScales cannot take NaN or Infinity as a domain value. Ignoring.");
+            return;
+        }
+        super._setDomain(values);
+    }
+
     /**
      * Sets or gets the QuantitiveScale's output interpolator
      *

diff --git a/test/scales/scaleTests.ts b/test/scales/scaleTests.ts
@@ -107,11 +107,6 @@ describe("Scales", () => {
       assert.deepEqual(scale.domain(), [0, 5], "the bar accessor was overwritten");
     });
 
-    it("scales don't allow Infinity", () => {
-      assert.throws(() => scale._setDomain([5, Infinity]), Error);
-      assert.throws(() => scale._setDomain([-Infinity, 6]), Error);
-    });
-
     it("should resize when a plot is removed", () => {
       var svg = generateSVG(400, 400);
       var ds1 = [{x: 0, y: 0}, {x: 1, y: 1}];
@@ -142,6 +137,22 @@ describe("Scales", () => {
       assert.equal(d[0], 0);
       assert.equal(d[1], 1);
     });
+
+    it("domain can't include NaN or Infinity", () => {
+      var scale = new Plottable.Scale.Linear();
+      var log = console.log;
+      console.log = function() {}; // stop warnings from going to console
+      scale.domain([0, 1]);
+      scale.domain([5, Infinity]);
+      assert.deepEqual(scale.domain(), [0, 1], "Infinity containing domain was ignored");
+      scale.domain([5, -Infinity]);
+      assert.deepEqual(scale.domain(), [0, 1], "-Infinity containing domain was ignored");
+      scale.domain([NaN, 7]);
+      assert.deepEqual(scale.domain(), [0, 1], "NaN containing domain was ignored");
+      scale.domain([-1, 5]);
+      assert.deepEqual(scale.domain(), [-1, 5], "Regular domains still accepted");
+      console.log = log; // reset console.log
+    });
   });
 
   describe("Ordinal Scales", () => {

diff --git a/test/tests.js b/test/tests.js
@@ -3627,15 +3627,6 @@ describe("Scales", function () {
             assert.deepEqual(scale.domain(), [0, 5], "the bar accessor was overwritten");
         });
 
-        it("scales don't allow Infinity", function () {
-            assert.throws(function () {
-                return scale._setDomain([5, Infinity]);
-            }, Error);
-            assert.throws(function () {
-                return scale._setDomain([-Infinity, 6]);
-            }, Error);
-        });
-
         it("should resize when a plot is removed", function () {
             var svg = generateSVG(400, 400);
             var ds1 = [{ x: 0, y: 0 }, { x: 1, y: 1 }];
@@ -3666,6 +3657,23 @@ describe("Scales", function () {
             assert.equal(d[0], 0);
             assert.equal(d[1], 1);
         });
+
+        it("domain can't include NaN or Infinity", function () {
+            var scale = new Plottable.Scale.Linear();
+            var log = console.log;
+            console.log = function () {
+            }; // stop warnings from going to console
+            scale.domain([0, 1]);
+            scale.domain([5, Infinity]);
+            assert.deepEqual(scale.domain(), [0, 1], "Infinity containing domain was ignored");
+            scale.domain([5, -Infinity]);
+            assert.deepEqual(scale.domain(), [0, 1], "-Infinity containing domain was ignored");
+            scale.domain([NaN, 7]);
+            assert.deepEqual(scale.domain(), [0, 1], "NaN containing domain was ignored");
+            scale.domain([-1, 5]);
+            assert.deepEqual(scale.domain(), [-1, 5], "Regular domains still accepted");
+            console.log = log; // reset console.log
+        });
     });
 
     describe("Ordinal Scales", function () {