Fix nice implementation for time scales.

The previous implementation didn't handle polylinear domains correctly.
d3 · Aug 1, 2012 · 7133162 · 7133162
1 parent 108d65d
commit 7133162
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 24 deletions.
diff --git a/d3.v2.js b/d3.v2.js
@@ -2104,15 +2104,10 @@
   function d3_scale_nice(domain, nice) {
     var i0 = 0, i1 = domain.length - 1, x0 = domain[i0], x1 = domain[i1], dx;
     if (x1 < x0) {
-      dx = i0;
-      i0 = i1;
-      i1 = dx;
-      dx = x0;
-      x0 = x1;
-      x1 = dx;
-    }
-    if (dx = x1 - x0) {
-      nice = nice(dx);
+      dx = i0, i0 = i1, i1 = dx;
+      dx = x0, x0 = x1, x1 = dx;
+    }
+    if (nice = nice(x1 - x0)) {
       domain[i0] = nice.floor(x0);
       domain[i1] = nice.ceil(x1);
     }
@@ -2181,7 +2176,7 @@
   }
   function d3_scale_linearNice(dx) {
     dx = Math.pow(10, Math.round(Math.log(dx) / Math.LN10) - 1);
-    return {
+    return dx && {
       floor: function(x) {
         return Math.floor(x / dx) * dx;
       },
@@ -6784,8 +6779,9 @@
       return scale;
     };
     scale.nice = function(m) {
-      var extent = d3_time_scaleExtent(scale.domain());
-      return scale.domain([ m.floor(extent[0]), m.ceil(extent[1]) ]);
+      return scale.domain(d3_scale_nice(scale.domain(), function() {
+        return m;
+      }));
     };
     scale.ticks = function(m, k) {
       var extent = d3_time_scaleExtent(scale.domain());

diff --git a/d3.v2.min.js b/d3.v2.min.js
diff --git a/src/scale/linear.js b/src/scale/linear.js
@@ -77,7 +77,7 @@ function d3_scale_linearRebind(scale, linear) {
 
 function d3_scale_linearNice(dx) {
   dx = Math.pow(10, Math.round(Math.log(dx) / Math.LN10) - 1);
-  return {
+  return dx && {
     floor: function(x) { return Math.floor(x / dx) * dx; },
     ceil: function(x) { return Math.ceil(x / dx) * dx; }
   };

diff --git a/src/scale/nice.js b/src/scale/nice.js
@@ -6,12 +6,11 @@ function d3_scale_nice(domain, nice) {
       dx;
 
   if (x1 < x0) {
-    dx = i0; i0 = i1; i1 = dx;
-    dx = x0; x0 = x1; x1 = dx;
+    dx = i0, i0 = i1, i1 = dx;
+    dx = x0, x0 = x1, x1 = dx;
   }
 
-  if (dx = x1 - x0) {
-    nice = nice(dx);
+  if (nice = nice(x1 - x0)) {
     domain[i0] = nice.floor(x0);
     domain[i1] = nice.ceil(x1);
   }

diff --git a/src/time/scale.js b/src/time/scale.js
@@ -15,8 +15,7 @@ function d3_time_scale(linear, methods, format) {
   };
 
   scale.nice = function(m) {
-    var extent = d3_time_scaleExtent(scale.domain());
-    return scale.domain([m.floor(extent[0]), m.ceil(extent[1])]);
+    return scale.domain(d3_scale_nice(scale.domain(), function() { return m; }));
   };
 
   scale.ticks = function(m, k) {

diff --git a/test/scale/linear-test.js b/test/scale/linear-test.js
@@ -211,6 +211,8 @@ suite.addBatch({
         assert.deepEqual(x.domain(), [130, 0]);
         var x = linear().domain([0, .49]).nice();
         assert.deepEqual(x.domain(), [0, .5]);
+      },
+      "has no effect on degenerate domains": function(linear) {
         var x = linear().domain([0, 0]).nice();
         assert.deepEqual(x.domain(), [0, 0]);
         var x = linear().domain([.5, .5]).nice();

diff --git a/test/scale/log-test.js b/test/scale/log-test.js
@@ -228,10 +228,12 @@ suite.addBatch({
         assert.deepEqual(x.domain(), [1000, 1]);
         var x = log().domain([.01, .49]).nice();
         assert.deepEqual(x.domain(), [.01, 1]);
+      },
+      "works on degenerate domains": function(log) {
         var x = log().domain([0, 0]).nice();
         assert.deepEqual(x.domain(), [0, 0]);
         var x = log().domain([.5, .5]).nice();
-        assert.inDelta(x.domain(), [.5, .5], 1e-6);
+        assert.inDelta(x.domain(), [.1, 1], 1e-6);
       },
       "nicing a polylog domain only affects the extent": function(log) {
         var x = log().domain([1.1, 1.5, 10.9]).nice();

diff --git a/test/time/scale-test.js b/test/time/scale-test.js
@@ -14,6 +14,24 @@ suite.addBatch({
       return d3.time.scale;
     },
 
+    "nice": {
+      "rounds using the specified time interval": function(scale) {
+        var x = scale().domain([local(2009, 0, 1, 0, 12), local(2009, 0, 1, 23, 48)]);
+        assert.deepEqual(x.nice(d3.time.day).domain(), [local(2009, 0, 1), local(2009, 0, 2)]);
+        assert.deepEqual(x.nice(d3.time.week).domain(), [local(2008, 11, 28), local(2009, 0, 4)]);
+        assert.deepEqual(x.nice(d3.time.month).domain(), [local(2008, 11, 1), local(2009, 1, 1)]);
+        assert.deepEqual(x.nice(d3.time.year).domain(), [local(2008, 0, 1), local(2010, 0, 1)]);
+      },
+      "works on degenerate domains": function(scale) {
+        var x = scale().domain([local(2009, 0, 1, 0, 12), local(2009, 0, 1, 0, 12)]);
+        assert.deepEqual(x.nice(d3.time.day).domain(), [local(2009, 0, 1), local(2009, 0, 2)]);
+      },
+      "nicing a polylinear domain only affects the extent": function(linear) {
+        var x = linear().domain([local(2009, 0, 1, 0, 12), local(2009, 0, 1, 23, 48), local(2009, 0, 2, 23, 48)]).nice(d3.time.day);
+        assert.deepEqual(x.domain(), [local(2009, 0, 1), local(2009, 0, 1, 23, 48), local(2009, 0, 3)]);
+      }
+    },
+
     "copy": {
       "changes to the domain are isolated": function(scale) {
         var x = scale().domain([local(2009, 0, 1), local(2010, 0, 1)]), y = x.copy();