first cut at aggregated pies

alexcjohnson · alexcjohnson · commit ce8946c01327 · 2017-10-24T16:07:54.000-04:00
diff --git a/src/traces/pie/attributes.js b/src/traces/pie/attributes.js
@@ -24,7 +24,13 @@ module.exports = {
     labels: {
         valType: 'data_array',
         editType: 'calc',
-        description: 'Sets the sector labels.'
+        description: [
+            'Sets the sector labels.',
+            'If `labels` entries are duplicated, we sum associated `values`',
+            'or simply count occurrences if `values` is not provided.',
+            'For other array attributes (including color) we use the first',
+            'non-empty entry among all occurrences of the label.'
+        ].join(' ')
     },
     // equivalent of x0 and dx, if label is missing
     label0: {
@@ -50,7 +56,10 @@ module.exports = {
     values: {
         valType: 'data_array',
         editType: 'calc',
-        description: 'Sets the values of the sectors of this pie chart.'
+        description: [
+            'Sets the values of the sectors of this pie chart.',
+            'If omitted, we count occurrences of each label.'
+        ].join(' ')
     },
 
     marker: {
diff --git a/src/traces/pie/calc.js b/src/traces/pie/calc.js
@@ -16,18 +16,18 @@ var helpers = require('./helpers');
 
 module.exports = function calc(gd, trace) {
     var vals = trace.values,
+        hasVals = Array.isArray(vals) && vals.length,
         labels = trace.labels,
+        colors = trace.marker.colors,
         cd = [],
         fullLayout = gd._fullLayout,
         colorMap = fullLayout._piecolormap,
         allThisTraceLabels = {},
-        needDefaults = false,
         vTotal = 0,
         hiddenLabels = fullLayout.hiddenlabels || [],
         i,
         v,
         label,
-        color,
         hidden,
         pt;
 
@@ -38,46 +38,60 @@ module.exports = function calc(gd, trace) {
         }
     }
 
-    for(i = 0; i < vals.length; i++) {
-        v = vals[i];
-        if(!isNumeric(v)) continue;
-        v = +v;
-        if(v < 0) continue;
+    function pullColor(color, label) {
+        if(!color) return false;
+
+        color = tinycolor(color);
+        if(!color.isValid()) return false;
+
+        color = Color.addOpacity(color, color.getAlpha());
+        if(!colorMap[label]) colorMap[label] = color;
+
+        return color;
+    }
+
+    var seriesLen = (hasVals ? vals : labels).length;
+
+    for(i = 0; i < seriesLen; i++) {
+        if(hasVals) {
+            v = vals[i];
+            if(!isNumeric(v)) continue;
+            v = +v;
+            if(v < 0) continue;
+        }
+        else v = 1;
 
         label = labels[i];
         if(label === undefined || label === '') label = i;
         label = String(label);
-        // only take the first occurrence of any given label.
-        // TODO: perhaps (optionally?) sum values for a repeated label?
-        if(allThisTraceLabels[label] === undefined) allThisTraceLabels[label] = true;
-        else continue;
-
-        color = tinycolor(trace.marker.colors[i]);
-        if(color.isValid()) {
-            color = Color.addOpacity(color, color.getAlpha());
-            if(!colorMap[label]) {
-                colorMap[label] = color;
-            }
-        }
-        // have we seen this label and assigned a color to it in a previous trace?
-        else if(colorMap[label]) color = colorMap[label];
-        // color needs a default - mark it false, come back after sorting
-        else {
-            color = false;
-            needDefaults = true;
-        }
 
-        hidden = hiddenLabels.indexOf(label) !== -1;
+        var thisLabelIndex = allThisTraceLabels[label];
+        if(thisLabelIndex === undefined) {
+            allThisTraceLabels[label] = cd.length;
 
-        if(!hidden) vTotal += v;
+            hidden = hiddenLabels.indexOf(label) !== -1;
 
-        cd.push({
-            v: v,
-            label: label,
-            color: color,
-            i: i,
-            hidden: hidden
-        });
+            if(!hidden) vTotal += v;
+
+            cd.push({
+                v: v,
+                label: label,
+                color: pullColor(colors[i]),
+                i: i,
+                pts: [i],
+                hidden: hidden
+            });
+        }
+        else {
+            pt = cd[thisLabelIndex];
+            pt.v += v;
+            pt.pts.push(i);
+            if(!pt.hidden) vTotal += v;
+
+            if(pt.color === false && colors[i]) {
+                pt.color = pullColor(colors[i], label);
+            }
+        }
     }
 
     if(trace.sort) cd.sort(function(a, b) { return b.v - a.v; });
@@ -88,10 +102,14 @@ module.exports = function calc(gd, trace) {
      * in the order slices will be displayed
      */
 
-    if(needDefaults) {
-        for(i = 0; i < cd.length; i++) {
-            pt = cd[i];
-            if(pt.color === false) {
+    for(i = 0; i < cd.length; i++) {
+        pt = cd[i];
+        if(pt.color === false) {
+            // have we seen this label and assigned a color to it in a previous trace?
+            if(colorMap[pt.label]) {
+                pt.color = colorMap[pt.label];
+            }
+            else {
                 colorMap[pt.label] = pt.color = nextDefaultColor(fullLayout._piedefaultcolorcount);
                 fullLayout._piedefaultcolorcount++;
             }
@@ -113,7 +131,10 @@ module.exports = function calc(gd, trace) {
         for(i = 0; i < cd.length; i++) {
             pt = cd[i];
             thisText = hasLabel ? [pt.label] : [];
-            if(hasText && trace.text[pt.i]) thisText.push(trace.text[pt.i]);
+            if(hasText) {
+                var texti = helpers.getFirstFilled(trace.text, pt.pts);
+                if(texti) thisText.push(texti);
+            }
             if(hasValue) thisText.push(helpers.formatPieValue(pt.v, separators));
             if(hasPercent) thisText.push(helpers.formatPiePercent(pt.v / vTotal, separators));
             pt.text = thisText.join('<br>');
diff --git a/src/traces/pie/defaults.js b/src/traces/pie/defaults.js
@@ -19,13 +19,14 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     var coerceFont = Lib.coerceFont;
 
     var vals = coerce('values');
-    if(!Array.isArray(vals) || !vals.length) {
-        traceOut.visible = false;
-        return;
-    }
-
     var labels = coerce('labels');
     if(!Array.isArray(labels)) {
+        if(!Array.isArray(vals) || !vals.length) {
+            // must have at least one of vals or labels
+            traceOut.visible = false;
+            return;
+        }
+
         coerce('label0');
         coerce('dlabel');
     }
diff --git a/src/traces/pie/helpers.js b/src/traces/pie/helpers.js
@@ -25,3 +25,16 @@ exports.formatPieValue = function formatPieValue(v, separators) {
     }
     return Lib.numSeparate(vRounded, separators);
 };
+
+exports.getFirstFilled = function getFirstFilled(array, indices) {
+    if(!Array.isArray(array)) return;
+    for(var i = 0; i < indices.length; i++) {
+        var v = array[indices[i]];
+        if(v || v === 0) return v;
+    }
+};
+
+exports.castOption = function castOption(item, indices) {
+    if(Array.isArray(item)) return exports.getFirstFilled(item, indices);
+    else if(item) return item;
+};
diff --git a/src/traces/pie/plot.js b/src/traces/pie/plot.js
@@ -95,7 +95,19 @@ module.exports = function plot(gd, cdpie) {
                     // in case fullLayout or fullData has changed without a replot
                     var fullLayout2 = gd._fullLayout;
                     var trace2 = gd._fullData[trace.index];
-                    var hoverinfo = Fx.castHoverinfo(trace2, fullLayout2, pt.i);
+
+                    var hoverinfo = trace2.hoverinfo;
+                    if(Array.isArray(hoverinfo)) {
+                        // super hacky: we need to pull out the *first* hoverinfo from
+                        // pt.pts, then put it back into an array in a dummy trace
+                        // and call castHoverinfo on that.
+                        // TODO: do we want to have Fx.castHoverinfo somehow handle this?
+                        // it already takes an array for index, for 2D, so this seems tricky.
+                        hoverinfo = Fx.castHoverinfo({
+                            hoverinfo: [helpers.castOption(hoverinfo, pt.pts)],
+                            _module: trace._module
+                        }, fullLayout2, 0);
+                    }
 
                     if(hoverinfo === 'all') hoverinfo = 'label+text+value+percent+name';
 
@@ -115,31 +127,27 @@ module.exports = function plot(gd, cdpie) {
 
                     if(hoverinfo.indexOf('label') !== -1) thisText.push(pt.label);
                     if(hoverinfo.indexOf('text') !== -1) {
-                        if(trace2.hovertext) {
-                            thisText.push(
-                                Array.isArray(trace2.hovertext) ?
-                                    trace2.hovertext[pt.i] :
-                                    trace2.hovertext
-                            );
-                        } else if(trace2.text && trace2.text[pt.i]) {
-                            thisText.push(trace2.text[pt.i]);
-                        }
+                        var texti = helpers.castOption(trace2.hovertext || trace2.text, pt.pts);
+                        if(texti) thisText.push(texti);
                     }
                     if(hoverinfo.indexOf('value') !== -1) thisText.push(helpers.formatPieValue(pt.v, separators));
                     if(hoverinfo.indexOf('percent') !== -1) thisText.push(helpers.formatPiePercent(pt.v / cd0.vTotal, separators));
 
+                    var hoverLabel = trace.hoverlabel;
+                    var hoverFont = hoverLabel.font;
+
                     Fx.loneHover({
                         x0: hoverCenterX - rInscribed * cd0.r,
                         x1: hoverCenterX + rInscribed * cd0.r,
                         y: hoverCenterY,
                         text: thisText.join('<br>'),
                         name: hoverinfo.indexOf('name') !== -1 ? trace2.name : undefined,
                         idealAlign: pt.pxmid[0] < 0 ? 'left' : 'right',
-                        color: Fx.castHoverOption(trace, pt.i, 'bgcolor') || pt.color,
-                        borderColor: Fx.castHoverOption(trace, pt.i, 'bordercolor'),
-                        fontFamily: Fx.castHoverOption(trace, pt.i, 'font.family'),
-                        fontSize: Fx.castHoverOption(trace, pt.i, 'font.size'),
-                        fontColor: Fx.castHoverOption(trace, pt.i, 'font.color')
+                        color: helpers.castOption(hoverLabel.bgcolor, pt.pts) || pt.color,
+                        borderColor: helpers.castOption(hoverLabel.bordercolor, pt.pts),
+                        fontFamily: helpers.castOption(hoverFont.family, pt.pts),
+                        fontSize: helpers.castOption(hoverFont.size, pt.pts),
+                        fontColor: helpers.castOption(hoverFont.color, pt.pts)
                     }, {
                         container: fullLayout2._hoverlayer.node(),
                         outerContainer: fullLayout2._paper.node(),
@@ -182,7 +190,7 @@ module.exports = function plot(gd, cdpie) {
                     .on('click', handleClick);
 
                 if(trace.pull) {
-                    var pull = +(Array.isArray(trace.pull) ? trace.pull[pt.i] : trace.pull) || 0;
+                    var pull = +helpers.castOption(trace.pull, pt.pts) || 0;
                     if(pull > 0) {
                         cx += pull * pt.pxmid[0];
                         cy += pull * pt.pxmid[1];
@@ -233,8 +241,7 @@ module.exports = function plot(gd, cdpie) {
                 }
 
                 // add text
-                var textPosition = Array.isArray(trace.textposition) ?
-                        trace.textposition[pt.i] : trace.textposition,
+                var textPosition = helpers.castOption(trace.textposition, pt.pts),
                     sliceTextGroup = sliceTop.selectAll('g.slicetext')
                     .data(pt.text && (textPosition !== 'none') ? [0] : []);
 
@@ -492,7 +499,12 @@ function scootLabels(quadrants, trace) {
             otherPt = wholeSide[i];
 
             // overlap can only happen if the other point is pulled more than this one
-            if(otherPt === thisPt || ((trace.pull[thisPt.i] || 0) >= trace.pull[otherPt.i] || 0)) continue;
+            if(otherPt === thisPt || (
+                (helpers.castOption(trace.pull, thisPt.pts) || 0) >=
+                (helpers.castOption(trace.pull, otherPt.pts) || 0))
+            ) {
+                continue;
+            }
 
             if((thisPt.pxmid[1] - otherPt.pxmid[1]) * yDiffSign > 0) {
                 // closer to the equator - by construction all of these happen first
diff --git a/src/traces/pie/style_one.js b/src/traces/pie/style_one.js
@@ -9,15 +9,14 @@
 'use strict';
 
 var Color = require('../../components/color');
+var castOption = require('./helpers').castOption;
 
 module.exports = function styleOne(s, pt, trace) {
-    var lineColor = trace.marker.line.color;
-    if(Array.isArray(lineColor)) lineColor = lineColor[pt.i] || Color.defaultLine;
-
-    var lineWidth = trace.marker.line.width || 0;
-    if(Array.isArray(lineWidth)) lineWidth = lineWidth[pt.i] || 0;
+    var line = trace.marker.line;
+    var lineColor = castOption(line.color, pt.pts) || Color.defaultLine;
+    var lineWidth = castOption(line.width, pt.pts) || 0;
 
     s.style({'stroke-width': lineWidth})
-    .call(Color.fill, pt.color)
-    .call(Color.stroke, lineColor);
+        .call(Color.fill, pt.color)
+        .call(Color.stroke, lineColor);
 };
diff --git a/test/image/baselines/pie_aggregated.png b/test/image/baselines/pie_aggregated.png
diff --git a/test/image/mocks/pie_aggregated.json b/test/image/mocks/pie_aggregated.json
@@ -0,0 +1,39 @@
+{
+  "data": [
+    {
+        "labels": [
+            "Alice",
+            "Bob",
+            "Charlie",
+            "Charlie",
+            "Charlie",
+            "Alice"
+        ],
+        "type": "pie",
+        "domain": {"x": [0, 0.4]}
+    },
+    {
+        "labels": [
+            "Alice",
+            "Alice",
+            "Allison",
+            "Alys",
+            "Elise",
+            "Allison",
+            "Alys",
+            "Alys"
+        ],
+        "values": [
+            10, 20, 30, 40, 50, 60, 70, 80
+        ],
+        "marker": {"colors": ["", "", "#ccc"]},
+        "type": "pie",
+        "domain": {"x": [0.5, 1]},
+        "textinfo": "label+value+percent"
+    }
+  ],
+  "layout": {
+    "height": 300,
+    "width": 500
+  }
+}
diff --git a/test/jasmine/tests/pie_test.js b/test/jasmine/tests/pie_test.js
@@ -178,7 +178,7 @@ describe('pie hovering', function() {
             expect(unhoverData.points.length).toEqual(1);
 
             var fields = [
-                'v', 'label', 'color', 'i', 'hidden',
+                'v', 'label', 'color', 'i', 'pts', 'hidden',
                 'text', 'px1', 'pxmid', 'midangle',
                 'px0', 'largeArc',
                 'pointNumber', 'curveNumber',
@@ -405,12 +405,12 @@ describe('Test event property of interactions on a pie plot:', function() {
 
             var pt = futureData.points[0];
             expect(Object.keys(pt)).toEqual(jasmine.arrayContaining([
-                'v', 'label', 'color', 'i', 'hidden', 'vTotal', 'text', 't',
+                'v', 'label', 'color', 'i', 'pts', 'hidden', 'vTotal', 'text', 't',
                 'trace', 'r', 'cx', 'cy', 'px1', 'pxmid', 'midangle', 'px0',
                 'largeArc', 'cxFinal', 'cyFinal',
                 'pointNumber', 'curveNumber'
             ]));
-            expect(Object.keys(pt).length).toBe(21);
+            expect(Object.keys(pt).length).toBe(22);
 
             expect(typeof pt.color).toEqual(typeof '#1f77b4', 'points[0].color');
             expect(pt.cx).toEqual(200, 'points[0].cx');
