fixes #1151 - auto-shift bins for date histograms similar to numeric

alexcjohnson · alexcjohnson · commit f6b902cdeb6d · 2016-11-26T00:03:46.000-05:00
to avoid all data lying ambiguously on bin edges
diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js
@@ -487,7 +487,11 @@ axes.expand = function(ax, data, options) {
 
 axes.autoBin = function(data, ax, nbins, is2d) {
     var datamin = Lib.aggNums(Math.min, null, data),
-        datamax = Lib.aggNums(Math.max, null, data);
+        datamax = Lib.aggNums(Math.max, null, data),
+        blankcount = 0,
+        datacount,
+        i;
+
     if(ax.type === 'category') {
         return {
             start: datamin - 0.5,
@@ -548,16 +552,16 @@ axes.autoBin = function(data, ax, nbins, is2d) {
     if(typeof dummyax.dtick === 'number') {
         var edgecount = 0,
             midcount = 0,
-            intcount = 0,
-            blankcount = 0;
-        for(var i = 0; i < data.length; i++) {
+            intcount = 0;
+
+        for(i = 0; i < data.length; i++) {
             if(data[i] % 1 === 0) intcount++;
             else if(!isNumeric(data[i])) blankcount++;
 
             if(nearEdge(data[i])) edgecount++;
             if(nearEdge(data[i] + dummyax.dtick / 2)) midcount++;
         }
-        var datacount = data.length - blankcount;
+        datacount = data.length - blankcount;
 
         if(intcount === datacount && ax.type !== 'date') {
             // all integers: if bin size is <1, it's because
@@ -586,6 +590,12 @@ axes.autoBin = function(data, ax, nbins, is2d) {
         binend = binstart + bincount * dummyax.dtick;
     }
     else {
+        // month ticks - should be the only nonlinear kind we have
+        // at this point.
+        if(dummyax.dtick.charAt(0) === 'M') {
+            binstart = autoShiftMonthBins(binstart, data, dummyax.dtick, datamin);
+        }
+
         // calculate the endpoint for nonlinear ticks - you have to
         // just increment until you're done
         binend = binstart;
@@ -602,6 +612,79 @@ axes.autoBin = function(data, ax, nbins, is2d) {
 };
 
 
+function autoShiftMonthBins(binStart, data, dtick, dataMin) {
+    var exactYears = 0,
+        exactMonths = 0,
+        exactDays = 0,
+        blankCount = 0,
+        dataCount,
+        di,
+        d,
+        year,
+        month;
+
+    for(var i = 0; i < data.length; i++) {
+        di = data[i];
+        if(!isNumeric(di)) {
+            blankCount ++;
+            continue;
+        }
+        d = new Date(di),
+        year = d.getUTCFullYear();
+        if(di === Date.UTC(year, 0, 1)) {
+            exactYears ++;
+        }
+        else {
+            month = d.getUTCMonth();
+            if(di === Date.UTC(year, month, 1)) {
+                exactMonths ++;
+            }
+            else if(di === Date.UTC(year, month, d.getUTCDate())) {
+                exactDays ++;
+            }
+        }
+    }
+
+    dataCount = data.length - blankCount;
+
+    // include bigger exact dates in the smaller ones
+    exactMonths += exactYears;
+    exactDays += exactMonths;
+
+    // unmber of data points that needs to be an exact value
+    // to shift that increment to (near) the bin center
+    var threshold = 0.8 * dataCount;
+
+    if(exactDays > threshold) {
+        var numMonths = Number(dtick.substr(1));
+
+        if((exactYears > threshold) && (numMonths % 12 === 0)) {
+            // The exact middle of a non-leap-year is 1.5 days into July
+            // so if we start the bins here, all but leap years will
+            // get hover-labeled as exact years.
+            binStart = axes.tickIncrement(binStart, 'M6', 'reverse') + ONEDAY * 1.5;
+        }
+        else if(exactMonths > threshold) {
+            // Months are not as clean, but if we shift half the *longest*
+            // month (31/2 days) then 31-day months will get labeled exactly
+            // and shorter months will get labeled with the correct month
+            // but shifted 12-36 hours into it.
+            binStart = axes.tickIncrement(binStart, 'M1', 'reverse') + ONEDAY * 15.5;
+        }
+        else {
+            // Shifting half a day is exact, but since these are month bins it
+            // will always give a somewhat odd-looking label, until we do something
+            // smarter like showing the bin boundaries (or the bounds of the actual
+            // data in each bin)
+            binStart -= ONEDAY / 2;
+        }
+        var nextBinStart = axes.tickIncrement(binStart, dtick);
+
+        if(nextBinStart <= dataMin) return nextBinStart;
+    }
+    return binStart;
+}
+
 // ----------------------------------------------------
 // Ticks and grids
 // ----------------------------------------------------
@@ -919,6 +1002,7 @@ function autoTickRound(ax) {
 // for pure powers of 10
 // numeric ticks always have constant differences, other datetime ticks
 // can all be calculated as constant number of milliseconds
+var THREEDAYS = 3 * ONEDAY;
 axes.tickIncrement = function(x, dtick, axrev) {
     var axSign = axrev ? -1 : 1;
 
@@ -930,10 +1014,23 @@ axes.tickIncrement = function(x, dtick, axrev) {
 
     // Dates: months (or years)
     if(tType === 'M') {
-        var y = new Date(x);
-        // is this browser consistent? setUTCMonth edits a date but
-        // returns that date's milliseconds
-        return y.setUTCMonth(y.getUTCMonth() + dtSigned);
+        /*
+         * set(UTC)Month does not (and CANNOT) always preserve day, since
+         * months have different lengths. The worst example of this is:
+         *   d = new Date(1970,0,31); d.setMonth(1) -> Feb 31 turns into Mar 3
+         *
+         * But we want to be able to iterate over the last day of each month,
+         * regardless of what its number is.
+         * So shift 3 days forward, THEN set the new month, then unshift:
+         *   1/31 -> 2/28 (or 29) -> 3/31 -> 4/30 -> ...
+         *
+         * Note that odd behavior still exists if you start from the 26th-28th:
+         *   1/28 -> 2/28 -> 3/31
+         * but at least you can't shift any dates into the wrong month,
+         * and ticks on these days incrementing by month would be very unusual
+         */
+        var y = new Date(x + THREEDAYS);
+        return y.setUTCMonth(y.getUTCMonth() + dtSigned) - THREEDAYS;
     }
 
     // Log scales: Linear, Digits
diff --git a/test/image/mocks/date_histogram.json b/test/image/mocks/date_histogram.json
@@ -9,66 +9,23 @@
                 "2012-03-01 00:00:00",
                 "2012-02-01 00:00:00"
             ],
-            "name": "trace 0",
-            "autobinx": false,
+            "autobinx": true,
             "nbinsx": 3,
-            "xbins": {
-                "start": "2011-12-16",
-                "end": "2012-03-16",
-                "size": "M1"
-            },
-            "autobiny": true,
-            "xaxis": "x",
-            "yaxis": "y",
             "showlegend": false,
             "type": "histogram"
         }
     ],
     "layout": {
-        "title": "Click to enter Plot title",
-        "font": {
-            "family": "\"Open sans\", verdana, arial, sans-serif",
-            "size": 12,
-            "color": "#444"
-        },
-        "showlegend": true,
         "width": 600,
         "height": 400,
         "xaxis": {
-            "title": "month",
-            "showgrid": false,
-            "zeroline": false,
-            "showline": false,
-            "ticks": "",
-            "showticklabels": true,
-            "tickcolor": "rgb(127,127,127)",
-            "gridcolor": "rgb(255,255,255)"
+            "title": "month"
         },
         "yaxis": {
             "title": "count",
-            "showgrid": true,
-            "zeroline": true,
-            "showline": false,
-            "ticks": "",
-            "tickcolor": "rgb(127,127,127)",
             "gridcolor": "rgb(255,255,255)"
         },
-        "legend": {
-            "x": 100,
-            "y": 0.5,
-            "traceorder": "reversed",
-            "font": {
-                "family": "",
-                "size": 0,
-                "color": ""
-            },
-            "bgcolor": "#fff",
-            "bordercolor": "transparent",
-            "borderwidth": 0
-        },
         "plot_bgcolor": "rgb(229,229,229)",
-        "barmode": "stack",
-        "bargap": 0.2,
-        "bargroupgap": 0
+        "bargap": 0.2
     }
 }
diff --git a/test/jasmine/tests/histogram2d_test.js b/test/jasmine/tests/histogram2d_test.js
@@ -92,14 +92,11 @@ describe('Test histogram2d', function() {
             // TODO: even though the binning is done on non-uniform bins,
             // the display makes them linear (using only y0 and dy)
             // when we sort out https://github.com/plotly/plotly.js/issues/1151
-            // lets also make it display the bins with nonuniform size,
-            // and ensure we don't generate an extra bin on the end (see
-            // first row of z below)
-            expect(out.y0).toBe('1969-07-02 14:24');
-            expect(out.dy).toBe(365.2 * oneDay);
+            // lets also make it display the bins with nonuniform size
+            expect(out.y0).toBe('1970-01-01 03:00');
+            expect(out.dy).toBe(365.25 * oneDay);
 
             expect(out.z).toEqual([
-                [0, 0, 0, 0],
                 [2, 0, 0, 0],
                 [0, 1, 0, 0],
                 [0, 0, 0, 0],
diff --git a/test/jasmine/tests/histogram_test.js b/test/jasmine/tests/histogram_test.js
@@ -149,32 +149,56 @@ describe('Test histogram', function() {
         var oneDay = 24 * 3600000;
 
         it('should handle auto dates with nonuniform (month) bins', function() {
+            // All data on exact years: shift so bin center is an
+            // exact year, except on leap years
             var out = _calc({
                 x: ['1970-01-01', '1970-01-01', '1971-01-01', '1973-01-01'],
                 nbinsx: 4
             });
 
-            // TODO: https://github.com/plotly/plotly.js/issues/1151
-            // these bins should shift when we implement that
-
-            // note that x1-x0 = 365 days, but the others are 365.5 days
-
-            // ALSO: this gives half-day gaps between all but the first two
+            // Note: this gives half-day gaps between all but the first two
             // bars. Now that we have explicit per-bar positioning, perhaps
             // we should fill the space, rather than insisting on equal-width
             // bars?
-            var x0 = 15768000000,
-                x1 = x0 + oneDay * 365,
-                x2 = x1 + oneDay * 365.5,
-                x3 = x2 + oneDay * 365.5;
-
             expect(out).toEqual([
                 // full calcdata has x and y too (and t in the first one),
                 // but those come later from setPositions.
-                {b: 0, p: x0, s: 2},
-                {b: 0, p: x1, s: 1},
-                {b: 0, p: x2, s: 0},
-                {b: 0, p: x3, s: 1}
+                {b: 0, p: Date.UTC(1970, 0, 1), s: 2},
+                {b: 0, p: Date.UTC(1971, 0, 1), s: 1},
+                {b: 0, p: Date.UTC(1972, 0, 1, 12), s: 0},
+                {b: 0, p: Date.UTC(1973, 0, 1), s: 1}
+            ]);
+
+            // All data on exact months: shift so bin center is on (31-day months)
+            // or in (shorter months) that month
+            out = _calc({
+                x: ['1970-01-01', '1970-01-01', '1970-02-01', '1970-04-01'],
+                nbinsx: 4
+            });
+
+            expect(out).toEqual([
+                {b: 0, p: Date.UTC(1970, 0, 1), s: 2},
+                {b: 0, p: Date.UTC(1970, 1, 1), s: 1},
+                {b: 0, p: Date.UTC(1970, 2, 2.5), s: 0},
+                {b: 0, p: Date.UTC(1970, 3, 1), s: 1}
+            ]);
+
+            // data on exact days: shift so each bin goes from noon to noon
+            // even though this gives kind of odd bin centers since the bins
+            // are months... but the important thing is it's unambiguous which
+            // bin any given day is in.
+            out = _calc({
+                x: ['1970-01-02', '1970-01-31', '1970-02-13', '1970-04-19'],
+                nbinsx: 4
+            });
+
+            expect(out).toEqual([
+                // dec 31 12:00 -> jan 31 12:00, middle is jan 16
+                {b: 0, p: Date.UTC(1970, 0, 16), s: 2},
+                // jan 31 12:00 -> feb 28 12:00, middle is feb 14 12:00
+                {b: 0, p: Date.UTC(1970, 1, 14, 12), s: 1},
+                {b: 0, p: Date.UTC(1970, 2, 16), s: 0},
+                {b: 0, p: Date.UTC(1970, 3, 15, 12), s: 1}
             ]);
         });
 
