From 78bb3eda76b4b63bee8d85a1049887d0538fdd0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 5 Jun 2019 14:37:51 -0400 Subject: [PATCH 1/2] fix multiple single-valued overlaid autobinned traces case --- src/traces/histogram/calc.js | 10 ++++++++-- test/jasmine/tests/histogram_test.js | 8 ++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/traces/histogram/calc.js b/src/traces/histogram/calc.js index e3beebd12a9..d5d556d28fa 100644 --- a/src/traces/histogram/calc.js +++ b/src/traces/histogram/calc.js @@ -398,11 +398,12 @@ 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 @@ -456,11 +457,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 index bf8d46b9ea9..f0236f6c407 100644 --- a/test/jasmine/tests/histogram_test.js +++ b/test/jasmine/tests/histogram_test.js @@ -693,6 +693,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]}, From 9faab6c7cd10cbab6ec82f649bc5bed00e3343f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 5 Jun 2019 14:38:21 -0400 Subject: [PATCH 2/2] fix single-value overlay w/ bingroup edge cases --- src/traces/histogram/calc.js | 17 +++++---- test/jasmine/tests/histogram_test.js | 55 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/traces/histogram/calc.js b/src/traces/histogram/calc.js index d5d556d28fa..0338e2353da 100644 --- 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 && @@ -410,11 +407,15 @@ function handleSingleValueOverlays(gd, trace, pa, mainData, binAttr) { // - 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]; diff --git a/test/jasmine/tests/histogram_test.js b/test/jasmine/tests/histogram_test.js index f0236f6c407..97bd6446205 100644 --- 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'); @@ -760,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; }); }