flesh out bin attribute descriptions and let autobin(x|y) pass validate

alexcjohnson · alexcjohnson · commit 76eee4b992ea · 2018-09-27T21:22:19.000-04:00
diff --git a/src/traces/histogram/attributes.js b/src/traces/histogram/attributes.js
@@ -9,6 +9,7 @@
 'use strict';
 
 var barAttrs = require('../bar/attributes');
+var makeBinAttrs = require('./bin_attributes');
 
 module.exports = {
     x: {
@@ -138,7 +139,7 @@ module.exports = {
             'Ignored if `xbins.size` is provided.'
         ].join(' ')
     },
-    xbins: makeBinsAttr('x'),
+    xbins: makeBinAttrs('x', true),
 
     nbinsy: {
         valType: 'integer',
@@ -153,77 +154,43 @@ module.exports = {
             'Ignored if `ybins.size` is provided.'
         ].join(' ')
     },
-    ybins: makeBinsAttr('y'),
+    ybins: makeBinAttrs('y', true),
+    autobinx: {
+        valType: 'boolean',
+        dflt: null,
+        role: 'style',
+        editType: 'calc',
+        description: [
+            'Obsolete: since v1.42 each bin attribute is auto-determined',
+            'separately and `autobinx` is not needed. However, we accept',
+            '`autobinx: true` or `false` and will update `xbins` accordingly',
+            'before deleting `autobinx` from the trace.'
+        ].join(' ')
+    },
+    autobiny: {
+        valType: 'boolean',
+        dflt: null,
+        role: 'style',
+        editType: 'calc',
+        impliedEdits: {
+            'ybins.start': undefined,
+            'ybins.end': undefined,
+            'ybins.size': undefined
+        },
+        description: [
+            'Obsolete: since v1.42 each bin attribute is auto-determined',
+            'separately and `autobiny` is not needed. However, we accept',
+            '`autobiny: true` or `false` and will update `ybins` accordingly',
+            'before deleting `autobiny` from the trace.'
+        ].join(' ')
+    },
 
     marker: barAttrs.marker,
 
     selected: barAttrs.selected,
     unselected: barAttrs.unselected,
 
     _deprecated: {
-        bardir: barAttrs._deprecated.bardir,
-        autobinx: {
-            valType: 'boolean',
-            dflt: null,
-            role: 'style',
-            editType: 'calc',
-            impliedEdits: {
-                'xbins.start': undefined,
-                'xbins.end': undefined,
-                'xbins.size': undefined
-            },
-            description: [
-                'Obsolete: since v1.42 each bin',
-                'attribute is auto-determined separately.'
-            ].join(' ')
-        },
-        autobiny: {
-            valType: 'boolean',
-            dflt: null,
-            role: 'style',
-            editType: 'calc',
-            impliedEdits: {
-                'ybins.start': undefined,
-                'ybins.end': undefined,
-                'ybins.size': undefined
-            },
-            description: [
-                'Obsolete: since v1.42 each bin',
-                'attribute is auto-determined separately.'
-            ].join(' ')
-        }
+        bardir: barAttrs._deprecated.bardir
     }
 };
-
-function makeBinsAttr(axLetter) {
-    return {
-        start: {
-            valType: 'any', // for date axes
-            role: 'style',
-            editType: 'calc',
-            description: [
-                'Sets the starting value for the', axLetter,
-                'axis bins.'
-            ].join(' ')
-        },
-        end: {
-            valType: 'any', // for date axes
-            role: 'style',
-            editType: 'calc',
-            description: [
-                'Sets the end value for the', axLetter,
-                'axis bins.'
-            ].join(' ')
-        },
-        size: {
-            valType: 'any', // for date axes
-            role: 'style',
-            editType: 'calc',
-            description: [
-                'Sets the step in-between value each', axLetter,
-                'axis bin.'
-            ].join(' ')
-        },
-        editType: 'calc'
-    };
-}
diff --git a/src/traces/histogram/bin_attributes.js b/src/traces/histogram/bin_attributes.js
@@ -0,0 +1,74 @@
+/**
+* Copyright 2012-2018, Plotly, Inc.
+* All rights reserved.
+*
+* This source code is licensed under the MIT license found in the
+* LICENSE file in the root directory of this source tree.
+*/
+
+'use strict';
+
+module.exports = function makeBinAttrs(axLetter, match) {
+    return {
+        start: {
+            valType: 'any', // for date axes
+            role: 'style',
+            editType: 'calc',
+            description: [
+                'Sets the starting value for the', axLetter,
+                'axis bins. Defaults to the minimum data value,',
+                'shifted down if necessary to make nice round values',
+                'and to remove ambiguous bin edges. For example, if most of the',
+                'data is integers we shift the bin edges 0.5 down, so a `size`',
+                'of 5 would have a default `start` of -0.5, so it is clear',
+                'that 0-4 are in the first bin, 5-9 in the second, but',
+                'continuous data gets a start of 0 and bins [0,5), [5,10) etc.',
+                'Dates behave similarly, and `start` should be a date string.',
+                'For category data, `start` is based on the category serial',
+                'numbers, and defaults to -0.5.',
+                (match ? (
+                    'If multiple non-overlaying histograms share a subplot, ' +
+                    'the first explicit `start` is used exactly and all others ' +
+                    'are shifted down (if necessary) to differ from that one ' +
+                    'by an integer number of bins.'
+                ) : '')
+            ].join(' ')
+        },
+        end: {
+            valType: 'any', // for date axes
+            role: 'style',
+            editType: 'calc',
+            description: [
+                'Sets the end value for the', axLetter,
+                'axis bins. The last bin may not end exactly at this value,',
+                'we increment the bin edge by `size` from `start` until we',
+                'reach or exceed `end`. Defaults to the maximum data value.',
+                'Like `start`, for dates use a date string, and for category',
+                'data `end` is based on the category serial numbers.'
+            ].join(' ')
+        },
+        size: {
+            valType: 'any', // for date axes
+            role: 'style',
+            editType: 'calc',
+            description: [
+                'Sets the size of each', axLetter, 'axis bin.',
+                'Default behavior: If `nbins' + axLetter + '` is 0 or omitted,',
+                'we choose a nice round bin size such that the number of bins',
+                'is about the same as the typical number of samples in each bin.',
+                'If `nbins' + axLetter + '` is provided, we choose a nice round',
+                'bin size giving no more than that many bins.',
+                'For date data, use milliseconds or *M<n>* for months, as in',
+                '`axis.dtick`. For category data, the number of categories to',
+                'bin together (always defaults to 1).',
+                (match ? (
+                    'If multiple non-overlaying histograms share a subplot, ' +
+                    'the first explicit `size` is used and all others discarded. ' +
+                    'If no `size` is provided,the sample data from all traces ' +
+                    'is combined to determine `size` as described above.'
+                ) : '')
+            ].join(' ')
+        },
+        editType: 'calc'
+    };
+};
diff --git a/src/traces/histogram/calc.js b/src/traces/histogram/calc.js
@@ -359,9 +359,11 @@ function calcAllAutoBins(gd, trace, pa, mainData, _overlayEdgeCase) {
     // Backward compatibility for one-time autobinning.
     // autobin: true is handled in cleanData, but autobin: false
     // needs to be here where we have determined the values.
-    if(trace._input['autobin' + mainData] === false) {
+    var autoBinAttr = 'autobin' + mainData;
+    if(trace._input[autoBinAttr] === false) {
         trace._input[binAttr] = Lib.extendFlat({}, trace[binAttr] || {});
-        delete trace._input['autobin' + mainData];
+        delete trace._input[autoBinAttr];
+        delete trace[autoBinAttr];
     }
 
     return [traceBinOptsCalc, pos0];
diff --git a/src/traces/histogram/defaults.js b/src/traces/histogram/defaults.js
@@ -53,6 +53,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     coerce('histnorm');
 
     // Note: bin defaults are now handled in Histogram.cleanData
+    // autobin(x|y) are only included here to appease Plotly.validate
+    coerce('autobin' + sampleLetter);
 
     handleStyleDefaults(traceIn, traceOut, coerce, defaultColor, layout);
 
diff --git a/src/traces/histogram2d/attributes.js b/src/traces/histogram2d/attributes.js
@@ -9,6 +9,7 @@
 'use strict';
 
 var histogramAttrs = require('../histogram/attributes');
+var makeBinAttrs = require('../histogram/bin_attributes');
 var heatmapAttrs = require('../heatmap/attributes');
 var colorscaleAttrs = require('../../components/colorscale/attributes');
 var colorbarAttrs = require('../../components/colorbar/attributes');
@@ -37,18 +38,16 @@ module.exports = extendFlat(
         histnorm: histogramAttrs.histnorm,
         histfunc: histogramAttrs.histfunc,
         nbinsx: histogramAttrs.nbinsx,
-        xbins: histogramAttrs.xbins,
+        xbins: makeBinAttrs('x'),
         nbinsy: histogramAttrs.nbinsy,
-        ybins: histogramAttrs.ybins,
+        ybins: makeBinAttrs('y'),
+        autobinx: histogramAttrs.autobinx,
+        autobiny: histogramAttrs.autobiny,
 
         xgap: heatmapAttrs.xgap,
         ygap: heatmapAttrs.ygap,
         zsmooth: heatmapAttrs.zsmooth,
-        zhoverformat: heatmapAttrs.zhoverformat,
-        _deprecated: {
-            autobinx: histogramAttrs._deprecated.autobinx,
-            autobiny: histogramAttrs._deprecated.autobiny
-        }
+        zhoverformat: heatmapAttrs.zhoverformat
     },
     colorscaleAttrs('', {
         cLetter: 'z',
diff --git a/src/traces/histogram2d/calc.js b/src/traces/histogram2d/calc.js
@@ -219,9 +219,11 @@ function doAutoBin(trace, axLetter, data, ax, r2c, c2r, calendar) {
     // Backward compatibility for one-time autobinning.
     // autobin: true is handled in cleanData, but autobin: false
     // needs to be here where we have determined the values.
-    if(trace._input['autobin' + axLetter] === false) {
+    var autoBinAttr = 'autobin' + axLetter;
+    if(trace._input[autoBinAttr] === false) {
         trace._input[binAttr] = Lib.extendFlat({}, binSpec);
-        delete trace._input['autobin' + axLetter];
+        delete trace._input[autoBinAttr];
+        delete trace[autoBinAttr];
     }
 }
 
diff --git a/src/traces/histogram2d/sample_defaults.js b/src/traces/histogram2d/sample_defaults.js
@@ -35,4 +35,7 @@ module.exports = function handleSampleDefaults(traceIn, traceOut, coerce, layout
     coerce('histnorm');
 
     // Note: bin defaults are now handled in Histogram2D.cleanData
+    // autobin(x|y) are only included here to appease Plotly.validate
+    coerce('autobinx');
+    coerce('autobiny');
 };
diff --git a/src/traces/histogram2dcontour/attributes.js b/src/traces/histogram2dcontour/attributes.js
@@ -27,16 +27,14 @@ module.exports = extendFlat({
     xbins: histogram2dAttrs.xbins,
     nbinsy: histogram2dAttrs.nbinsy,
     ybins: histogram2dAttrs.ybins,
+    autobinx: histogram2dAttrs.autobinx,
+    autobiny: histogram2dAttrs.autobiny,
 
     autocontour: contourAttrs.autocontour,
     ncontours: contourAttrs.ncontours,
     contours: contourAttrs.contours,
     line: contourAttrs.line,
-    zhoverformat: histogram2dAttrs.zhoverformat,
-    _deprecated: {
-        autobinx: histogram2dAttrs._deprecated.autobinx,
-        autobiny: histogram2dAttrs._deprecated.autobiny
-    }
+    zhoverformat: histogram2dAttrs.zhoverformat
 },
     colorscaleAttrs('', {
         cLetter: 'z',
diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js
@@ -568,4 +568,73 @@ describe('Plotly.validate', function() {
             'In layout, container polar3 did not get coerced'
         );
     });
+
+    it('understands histogram bin and autobin attributes', function() {
+        var out = Plotly.validate([{
+            type: 'histogram',
+            x: [1, 2, 3],
+            // allowed by Plotly.validate, even though we get rid of it
+            // in a real plot call
+            autobinx: true,
+            // valid attribute, but not coerced
+            autobiny: false
+        }]);
+        expect(out.length).toBe(1);
+        assertErrorContent(
+            out[0], 'unused', 'data', 0, ['autobiny'], 'autobiny',
+            'In data trace 0, key autobiny did not get coerced'
+        );
+
+        out = Plotly.validate([{
+            type: 'histogram',
+            x: [1, 2, 3],
+            xbins: {start: 1, end: 4, size: 0.5}
+        }]);
+        expect(out).toBeUndefined();
+
+        out = Plotly.validate([{
+            type: 'histogram',
+            x: [1, 2, 3],
+            xbins: {start: 0.8, end: 4, size: 0.5}
+        }, {
+            type: 'histogram',
+            x: [1, 2, 3],
+            // start and end still get coerced, even though start will get modified
+            // during calc. size will not be coerced because trace 0 already has it.
+            xbins: {start: 2, end: 3, size: 1}
+        }]);
+
+        expect(out.length).toBe(1);
+        assertErrorContent(
+            out[0], 'unused', 'data', 1, ['xbins', 'size'], 'xbins.size',
+            'In data trace 1, key xbins.size did not get coerced'
+        );
+    });
+
+    it('understands histogram2d(contour) bin and autobin attributes', function() {
+        var out = Plotly.validate([{
+            type: 'histogram2d',
+            x: [1, 2, 3],
+            y: [1, 2, 3],
+            autobinx: true,
+            autobiny: false,
+            xbins: {start: 5, end: 10},
+            ybins: {size: 2}
+        }, {
+            type: 'histogram2d',
+            x: [1, 2, 3],
+            y: [1, 2, 3],
+            xbins: {start: 0, end: 7, size: 1},
+            ybins: {size: 3}
+        }, {
+            type: 'histogram2dcontour',
+            x: [1, 2, 3],
+            y: [1, 2, 3],
+            autobinx: false,
+            autobiny: false,
+            xbins: {start: 1, end: 5, size: 2},
+            ybins: {size: 4}
+        }]);
+        expect(out).toBeUndefined();
+    });
 });

Original file line number	Diff line number	Diff line change
`@@ -219,9 +219,11 @@ function doAutoBin(trace, axLetter, data, ax, r2c, c2r, calendar) {`
`219`	`219`	`// Backward compatibility for one-time autobinning.`
`220`	`220`	`// autobin: true is handled in cleanData, but autobin: false`
`221`	`221`	`// needs to be here where we have determined the values.`
`222`		`- if(trace._input['autobin' + axLetter] === false) {`
	`222`	`+ var autoBinAttr = 'autobin' + axLetter;`
	`223`	`+ if(trace._input[autoBinAttr] === false) {`
`223`	`224`	`trace._input[binAttr] = Lib.extendFlat({}, binSpec);`
`224`		`- delete trace._input['autobin' + axLetter];`
	`225`	`+ delete trace._input[autoBinAttr];`
	`226`	`+ delete trace[autoBinAttr];`
`225`	`227`	`}`
`226`	`228`	`}`
`227`	`229`