Merge pull request #3935 from plotly/single-value-overlay-histogram

More single-value overlaid histogram fixes
plotly · Jun 5, 2019 · 0b7cde1 · 0b7cde1
2 parents 34af401 + 9faab6c
commit 0b7cde1
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 10 deletions.
diff --git a/src/traces/histogram/calc.js b/src/traces/histogram/calc.js
@@ -291,9 +291,6 @@ function calcAllAutoBins(gd, trace, pa, mainData, _overlayEdgeCase) {
             }
         }
 
-        // TODO how does work with bingroup ????
-        // - https://github.com/plotly/plotly.js/issues/3881
-        //
         // Edge case: single-valued histogram overlaying others
         // Use them all together to calculate the bin size for the single-valued one
         if(isOverlay && !Registry.traceIs(trace, '2dMap') && newBinSpec._dataSpan === 0 &&
@@ -398,22 +395,27 @@ function calcAllAutoBins(gd, trace, pa, mainData, _overlayEdgeCase) {
  * Returns the binSpec for the trace that sparked all this
  */
 function handleSingleValueOverlays(gd, trace, pa, mainData, binAttr) {
+    var fullLayout = gd._fullLayout;
     var overlaidTraceGroup = getConnectedHistograms(gd, trace);
     var pastThisTrace = false;
     var minSize = Infinity;
     var singleValuedTraces = [trace];
-    var i, tracei;
+    var i, tracei, binOpts;
 
     // first collect all the:
     // - min bin size from all multi-valued traces
     // - single-valued traces
     for(i = 0; i < overlaidTraceGroup.length; i++) {
         tracei = overlaidTraceGroup[i];
-        if(tracei === trace) pastThisTrace = true;
-        else if(!pastThisTrace) {
-            // This trace has already had its autobins calculated
-            // (so must not have been single-valued).
-            minSize = Math.min(minSize, tracei[binAttr].size);
+
+        if(tracei === trace) {
+            pastThisTrace = true;
+        } else if(!pastThisTrace) {
+            // This trace has already had its autobins calculated, so either:
+            // - it is part of a bingroup
+            // - it is NOT a single-valued trace
+            binOpts = fullLayout._histogramBinOpts[tracei['_' + mainData + 'bingroup']];
+            minSize = Math.min(minSize, binOpts.size || tracei[binAttr].size);
         } else {
             var resulti = calcAllAutoBins(gd, tracei, pa, mainData, true);
             var binSpeci = resulti[0];
@@ -456,11 +458,16 @@ function handleSingleValueOverlays(gd, trace, pa, mainData, binAttr) {
         tracei = singleValuedTraces[i];
         var calendar = tracei[mainData + 'calendar'];
 
-        tracei._input[binAttr] = tracei[binAttr] = {
+        var newBins = {
             start: pa.c2r(dataVals[i] - minSize / 2, 0, calendar),
             end: pa.c2r(dataVals[i] + minSize / 2, 0, calendar),
             size: minSize
         };
+
+        tracei._input[binAttr] = tracei[binAttr] = newBins;
+
+        binOpts = fullLayout._histogramBinOpts[tracei['_' + mainData + 'bingroup']];
+        if(binOpts) Lib.extendFlat(binOpts, newBins);
     }
 
     return trace[binAttr];

diff --git a/test/jasmine/tests/histogram_test.js b/test/jasmine/tests/histogram_test.js
@@ -1,4 +1,5 @@
 var Plotly = require('@lib/index');
+var Plots = require('@src/plots/plots');
 var Lib = require('@src/lib');
 var setConvert = require('@src/plots/cartesian/set_convert');
 
@@ -693,6 +694,14 @@ describe('Test histogram', function() {
             ]);
         });
 
+        it('handles multiple single-valued overlaid autobinned traces', function() {
+            var out = _calc({x: [1]}, [{x: [1]}], {barmode: 'overlay'}, true);
+
+            expect(out).toEqual([
+                {p: 1, s: 1, b: 0, pts: [0], ph1: 1, ph0: 1, width1: 1, i: 0}
+            ]);
+        });
+
         it('handles multiple single-valued overlaid autobinned traces with different values', function() {
             var out = _calc({x: [null, 13, '', 13]}, [
                 {x: [5]},
@@ -752,6 +761,60 @@ describe('Test histogram', function() {
             ]);
         });
 
+        // from issue #3881
+        it('handle single-value edge case 1', function() {
+            var gd = {
+                data: [
+                    {uid: 'a', type: 'histogram', y: [1]},
+                    {uid: 'b', type: 'histogram', y: [2]},
+
+                    {uid: 'c', type: 'histogram', y: [1], xaxis: 'x2'},
+                    {uid: 'd', type: 'histogram', y: [3], xaxis: 'x2'},
+
+                    {uid: 'e', type: 'histogram', y: [3]},
+                    {uid: 'f', type: 'histogram', y: [2], xaxis: 'x2'},
+
+                    {uid: 'g', type: 'histogram', x: [1]},
+                    {uid: 'h', type: 'histogram', x: [2], yaxis: 'y2'},
+                    {uid: 'i', type: 'histogram', x: [2]}
+                ],
+                layout: {barmode: 'overlay'}
+            };
+            supplyAllDefaults(gd);
+            Plots.doCalcdata(gd);
+
+            var allBinOpts = gd._fullLayout._histogramBinOpts;
+            var groups = Object.keys(allBinOpts);
+            expect(groups).toEqual(
+                ['a__y', 'b__y', 'c__y', 'd__y', 'e__y', 'f__y', 'g__x', 'h__x', 'i__x'],
+                'bin groups'
+            );
+        });
+
+        // from issue #3881
+        it('handle single-value edge case 2', function() {
+            var gd = {
+                data: [
+                    {bingroup: '1', type: 'histogram', y: [1]},
+                    {uid: 'b', type: 'histogram', y: [2]},
+                    {bingroup: '2', type: 'histogram', y: [1], xaxis: 'x2'},
+                    {bingroup: '1', type: 'histogram', y: [3], xaxis: 'x2'},
+                    {bingroup: '2', type: 'histogram', y: [3]},
+                    {uid: 'f', type: 'histogram', y: [2], xaxis: 'x2'},
+                    {bingroup: '3', type: 'histogram', x: [1]},
+                    {bingroup: '1', type: 'histogram', x: [2], yaxis: 'y2'},
+                    {bingroup: '3', type: 'histogram', x: [2]}
+                ],
+                layout: {barmode: 'overlay'}
+            };
+            supplyAllDefaults(gd);
+            Plots.doCalcdata(gd);
+
+            var allBinOpts = gd._fullLayout._histogramBinOpts;
+            var groups = Object.keys(allBinOpts);
+            expect(groups).toEqual(['1', '2', '3', 'b__y', 'f__y'], 'bin groups');
+        });
+
         function calcPositions(opts, extraTraces, prepend) {
             return _calc(opts, extraTraces, {}, prepend).map(function(v) { return v.p; });
         }