Merge pull request #3222 from plotly/joyplots-etpinard

Kully · web-flow · commit 0dea0e9a12d5 · 2018-11-07T08:56:08.000-05:00
Joyplots etpinard
diff --git a/src/traces/box/attributes.js b/src/traces/box/attributes.js
@@ -179,9 +179,9 @@ module.exports = {
         dflt: 0,
         editType: 'calc',
         description: [
-            'Sets the width of the box.',
+            'Sets the width of the box in data coordinate',
             'If *0* (default value) the width is automatically selected based on the positions',
-            'of other box traces in the same subplot.',
+            'of other box traces in the same subplot.'
         ].join(' ')
     },
 
diff --git a/src/traces/box/layout_attributes.js b/src/traces/box/layout_attributes.js
@@ -22,7 +22,8 @@ module.exports = {
             'If *group*, the boxes are plotted next to one another',
             'centered around the shared location.',
             'If *overlay*, the boxes are plotted over one another,',
-            'you might need to set *opacity* to see them multiple boxes.'
+            'you might need to set *opacity* to see them multiple boxes.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     },
     boxgap: {
@@ -34,7 +35,8 @@ module.exports = {
         editType: 'calc',
         description: [
             'Sets the gap (in plot fraction) between boxes of',
-            'adjacent location coordinates.'
+            'adjacent location coordinates.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     },
     boxgroupgap: {
@@ -46,7 +48,8 @@ module.exports = {
         editType: 'calc',
         description: [
             'Sets the gap (in plot fraction) between boxes of',
-            'the same location coordinate.'
+            'the same location coordinate.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     }
 };
diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js
@@ -22,19 +22,32 @@ function plot(gd, plotinfo, cdbox, boxLayer) {
     var xa = plotinfo.xaxis;
     var ya = plotinfo.yaxis;
     var numBoxes = fullLayout._numBoxes;
-    var groupFraction = (1 - fullLayout.boxgap);
     var group = (fullLayout.boxmode === 'group' && numBoxes > 1);
+    var groupFraction = (1 - fullLayout.boxgap);
+    var groupGapFraction = 1 - fullLayout.boxgroupgap;
 
     Lib.makeTraceGroups(boxLayer, cdbox, 'trace boxes').each(function(cd) {
         var plotGroup = d3.select(this);
         var cd0 = cd[0];
         var t = cd0.t;
         var trace = cd0.trace;
         if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
-        // box half width
-        var bdPos = t.dPos * groupFraction * (1 - fullLayout.boxgroupgap) / (group ? numBoxes : 1);
+
+        // position coordinate delta
+        var dPos = t.dPos;
+        // box half width;
+        var bdPos;
         // box center offset
-        var bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
+        var bPos;
+
+        if(trace.width) {
+            bdPos = dPos;
+            bPos = 0;
+        } else {
+            bdPos = dPos * groupFraction * groupGapFraction / (group ? numBoxes : 1);
+            bPos = group ? 2 * dPos * (-0.5 + (t.num + 0.5) / numBoxes) * groupFraction : 0;
+        }
+
         // whisker width
         var wdPos = bdPos * trace.whiskerwidth;
 
diff --git a/src/traces/violin/attributes.js b/src/traces/violin/attributes.js
@@ -136,18 +136,13 @@ module.exports = {
         ].join(' ')
     }),
 
-    width: {
-        valType: 'number',
-        min: 0,
-        role: 'info',
-        dflt: 0,
-        editType: 'calc',
+    width: extendFlat({}, boxAttrs.width, {
         description: [
-            'Sets the width of the violin.',
+            'Sets the width of the violin in data coordinates.',
             'If *0* (default value) the width is automatically selected based on the positions',
             'of other violin traces in the same subplot.',
         ].join(' ')
-    },
+    }),
 
     marker: boxAttrs.marker,
     text: boxAttrs.text,
diff --git a/src/traces/violin/calc.js b/src/traces/violin/calc.js
@@ -25,18 +25,10 @@ module.exports = function calc(gd, trace) {
         trace[trace.orientation === 'h' ? 'xaxis' : 'yaxis']
     );
 
-    var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
-    var scaleGroup = trace.scalegroup;
-    var groupStats = violinScaleGroupStats[scaleGroup];
-    if(!groupStats) {
-        groupStats = violinScaleGroupStats[scaleGroup] = {
-            maxWidth: 0,
-            maxCount: 0
-        };
-    }
-
     var spanMin = Infinity;
     var spanMax = -Infinity;
+    var maxKDE = 0;
+    var maxCount = 0;
 
     for(var i = 0; i < cd.length; i++) {
         var cdi = cd[i];
@@ -61,19 +53,36 @@ module.exports = function calc(gd, trace) {
 
         for(var k = 0, t = span[0]; t < (span[1] + step / 2); k++, t += step) {
             var v = kde(t);
-            groupStats.maxWidth = Math.max(groupStats.maxWidth, v);
             cdi.density[k] = {v: v, t: t};
+            maxKDE = Math.max(maxKDE, v);
         }
 
-        groupStats.maxCount = Math.max(groupStats.maxCount, vals.length);
-
+        maxCount = Math.max(maxCount, vals.length);
         spanMin = Math.min(spanMin, span[0]);
         spanMax = Math.max(spanMax, span[1]);
     }
 
     var extremes = Axes.findExtremes(valAxis, [spanMin, spanMax], {padded: true});
     trace._extremes[valAxis._id] = extremes;
 
+    if(trace.width) {
+        cd[0].t.maxKDE = maxKDE;
+    } else {
+        var violinScaleGroupStats = fullLayout._violinScaleGroupStats;
+        var scaleGroup = trace.scalegroup;
+        var groupStats = violinScaleGroupStats[scaleGroup];
+
+        if(groupStats) {
+            groupStats.maxKDE = Math.max(groupStats.maxKDE, maxKDE);
+            groupStats.maxCount = Math.max(groupStats.maxCount, maxCount);
+        } else {
+            violinScaleGroupStats[scaleGroup] = {
+                maxKDE: maxKDE,
+                maxCount: maxCount
+            };
+        }
+    }
+
     cd[0].t.labels.kde = Lib._(gd, 'kde:');
 
     return cd;
diff --git a/src/traces/violin/defaults.js b/src/traces/violin/defaults.js
@@ -27,13 +27,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
 
     coerce('bandwidth');
     coerce('side');
+
     var width = coerce('width');
     if(!width) {
         coerce('scalegroup', traceOut.name);
         coerce('scalemode');
-    } else {
-        traceOut.scalegroup = '';
-        traceOut.scalemode = 'width';
     }
 
     var span = coerce('span');
diff --git a/src/traces/violin/layout_attributes.js b/src/traces/violin/layout_attributes.js
@@ -19,19 +19,22 @@ module.exports = {
             'If *group*, the violins are plotted next to one another',
             'centered around the shared location.',
             'If *overlay*, the violins are plotted over one another,',
-            'you might need to set *opacity* to see them multiple violins.'
+            'you might need to set *opacity* to see them multiple violins.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     }),
     violingap: extendFlat({}, boxLayoutAttrs.boxgap, {
         description: [
             'Sets the gap (in plot fraction) between violins of',
-            'adjacent location coordinates.'
+            'adjacent location coordinates.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     }),
     violingroupgap: extendFlat({}, boxLayoutAttrs.boxgroupgap, {
         description: [
             'Sets the gap (in plot fraction) between violins of',
-            'the same location coordinate.'
+            'the same location coordinate.',
+            'Has no effect on traces that have *width* set.'
         ].join(' ')
     })
 };
diff --git a/src/traces/violin/plot.js b/src/traces/violin/plot.js
@@ -20,6 +20,10 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
     var fullLayout = gd._fullLayout;
     var xa = plotinfo.xaxis;
     var ya = plotinfo.yaxis;
+    var numViolins = fullLayout._numViolins;
+    var group = (fullLayout.violinmode === 'group' && numViolins > 1);
+    var groupFraction = 1 - fullLayout.violingap;
+    var groupGapFraction = 1 - fullLayout.violingroupgap;
 
     function makePath(pts) {
         var segments = linePoints(pts, {
@@ -39,16 +43,30 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
         var t = cd0.t;
         var trace = cd0.trace;
         if(!plotinfo.isRangePlot) cd0.node3 = plotGroup;
-        var numViolins = fullLayout._numViolins;
-        var group = (fullLayout.violinmode === 'group' && numViolins > 1);
-        var groupFraction = 1 - fullLayout.violingap;
+
+        // position coordinate delta
+        var dPos = t.dPos;
         // violin max half width
-        var bdPos = t.bdPos = t.dPos * groupFraction * (1 - fullLayout.violingroupgap) / (group ? numViolins : 1);
+        var bdPos;
         // violin center offset
-        var bPos = t.bPos = group ? 2 * t.dPos * (-0.5 + (t.num + 0.5) / numViolins) * groupFraction : 0;
+        var bPos;
         // half-width within which to accept hover for this violin
         // always split the distance to the closest violin
-        t.wHover = t.dPos * (group ? groupFraction / numViolins : 1);
+        var wHover;
+
+        if(trace.width) {
+            bdPos = dPos;
+            bPos = 0;
+            wHover = dPos;
+        } else {
+            bdPos = dPos * groupFraction * groupGapFraction / (group ? numViolins : 1);
+            bPos = group ? 2 * dPos * (-0.5 + (t.num + 0.5) / numViolins) * groupFraction : 0;
+            wHover = dPos * (group ? groupFraction / numViolins : 1);
+        }
+
+        t.bdPos = bdPos;
+        t.bPos = bPos;
+        t.wHover = wHover;
 
         if(trace.visible !== true || t.empty) {
             plotGroup.remove();
@@ -60,7 +78,6 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
         var hasBothSides = trace.side === 'both';
         var hasPositiveSide = hasBothSides || trace.side === 'positive';
         var hasNegativeSide = hasBothSides || trace.side === 'negative';
-        var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
 
         var violins = plotGroup.selectAll('path.violin').data(Lib.identity);
 
@@ -76,15 +93,15 @@ module.exports = function plot(gd, plotinfo, cdViolins, violinLayer) {
             var len = density.length;
             var posCenter = d.pos + bPos;
             var posCenterPx = posAxis.c2p(posCenter);
-            var scale;
 
-            switch(trace.scalemode) {
-                case 'width':
-                    scale = groupStats.maxWidth / bdPos;
-                    break;
-                case 'count':
-                    scale = (groupStats.maxWidth / bdPos) * (groupStats.maxCount / d.pts.length);
-                    break;
+            var scale;
+            if(trace.width) {
+                scale = t.maxKDE / bdPos;
+            } else {
+                var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup];
+                scale = trace.scalemode === 'count' ?
+                    (groupStats.maxKDE / bdPos) * (groupStats.maxCount / d.pts.length) :
+                    groupStats.maxKDE / bdPos;
             }
 
             var pathPos, pathNeg, path;
diff --git a/test/image/baselines/violin_box_multiple_widths.png b/test/image/baselines/violin_box_multiple_widths.png
diff --git a/test/image/mocks/violin_box_multiple_widths.json b/test/image/mocks/violin_box_multiple_widths.json
@@ -1,64 +1,44 @@
 {
     "data": [{
             "type": "violin",
-            "width": 0.315,
+            "width": 0.4,
+            "name": "width: 0.4",
             "x": [0, 5, 7, 8],
-            "points": "none",
             "side": "positive",
-            "box": {
-              "visible": false
-            },
-            "boxpoints": false,
             "line": {
                 "color": "black"
             },
             "fillcolor": "#8dd3c7",
             "opacity": 0.6,
-            "meanline": {
-                "visible": false
-            },
             "y0": 0.0
         }, {
             "type": "violin",
+            "name": "auto",
             "x": [0, 5, 7, 8],
-            "points": "none",
             "side": "positive",
-            "box": {
-              "visible": false
-            },
-            "boxpoints": false,
             "line": {
                 "color": "black"
             },
             "fillcolor": "#d3c78d",
             "opacity": 0.6,
-            "meanline": {
-                "visible": false
-            },
             "y0": 0.1
         }, {
             "type": "box",
-            "width": 0.5421,
+            "width": 0.6,
+            "name": "width: 0.6",
             "x": [0, 5, 7, 8],
-            "points": "none",
             "side": "positive",
-            "box": {
-              "visible": false
-            },
-            "boxpoints": false,
             "line": {
                 "color": "black"
             },
             "fillcolor": "#c78dd3",
             "opacity": 0.6,
-            "meanline": {
-                "visible": false
-            },
             "y0": 0.2
     }],
     "layout": {
         "title": "Joyplot - Violin with multiple widths",
+        "legend": {"x": 0},
         "xaxis": {"zeroline": false},
-        "violingap": 0
+        "yaxis": {"dtick": 0.1, "gridcolor": "black"}
     }
 }
diff --git a/test/jasmine/tests/violin_test.js b/test/jasmine/tests/violin_test.js
@@ -142,6 +142,23 @@ describe('Test violin defaults', function() {
         expect(traceOut.meanline.color).toBe('blue');
         expect(traceOut.meanline.width).toBe(10);
     });
+
+    it('should not coerce *scalegroup* and *scalemode* when *width* is set', function() {
+        _supply({
+            y: [1, 2, 1],
+            width: 1
+        });
+        expect(traceOut.scalemode).toBeUndefined();
+        expect(traceOut.scalegroup).toBeUndefined();
+
+        _supply({
+            y: [1, 2, 1],
+            // width=0 is ignored during calc
+            width: 0
+        });
+        expect(traceOut.scalemode).toBe('width');
+        expect(traceOut.scalegroup).toBe('');
+    });
 });
 
 describe('Test violin calc:', function() {
@@ -236,7 +253,7 @@ describe('Test violin calc:', function() {
             name: 'one',
             y: [0, 0, 0, 0, 10, 10, 10, 10]
         });
-        expect(fullLayout._violinScaleGroupStats.one.maxWidth).toBeCloseTo(0.055);
+        expect(fullLayout._violinScaleGroupStats.one.maxKDE).toBeCloseTo(0.055);
         expect(fullLayout._violinScaleGroupStats.one.maxCount).toBe(8);
     });
 
