consider alignment and offset group across matching axes

etpinard · etpinard · commit e746b088d6ff · 2019-02-18T17:34:08.000-05:00
... as well as across trace types (confirmed behavior)

to do so:
- add getAxisGroup axis_ids.js util
- mv alignmentgroup and offsetgroup coerce calls to crossTraceDefaults
diff --git a/src/plots/cartesian/axis_ids.js b/src/plots/cartesian/axis_ids.js
@@ -114,3 +114,13 @@ exports.idSort = function(id1, id2) {
     if(letter1 !== letter2) return letter1 > letter2 ? 1 : -1;
     return +(id1.substr(1) || 1) - +(id2.substr(1) || 1);
 };
+
+exports.getAxisGroup = function getAxisGroup(fullLayout, axId) {
+    var matchGroups = fullLayout._axisMatchGroups;
+
+    for(var i = 0; i < matchGroups.length; i++) {
+        var group = matchGroups[i];
+        if(group[axId]) return 'g' + i;
+    }
+    return axId;
+};
diff --git a/src/traces/bar/attributes.js b/src/traces/bar/attributes.js
@@ -180,7 +180,8 @@ module.exports = {
         dflt: '',
         editType: 'calc',
         description: [
-            'Set several traces linked to the same position axis to the same',
+            'Set several traces linked to the same position axis',
+            'or matching axes to the same',
             'offsetgroup where bars of the same position coordinate will line up.'
         ].join(' ')
     },
@@ -190,7 +191,8 @@ module.exports = {
         dflt: '',
         editType: 'calc',
         description: [
-            'Set several traces linked to the same position axis to the same',
+            'Set several traces linked to the same position axis',
+            'or matching axes to the same',
             'alignmentgroup. This controls whether bars compute their positional',
             'range dependently or independently.'
         ].join(' ')
diff --git a/src/traces/bar/cross_trace_calc.js b/src/traces/bar/cross_trace_calc.js
@@ -14,6 +14,7 @@ var BADNUM = require('../../constants/numerical').BADNUM;
 
 var Registry = require('../../registry');
 var Axes = require('../../plots/cartesian/axes');
+var getAxisGroup = require('../../plots/cartesian/axis_ids').getAxisGroup;
 var Sieve = require('./sieve.js');
 
 /*
@@ -286,7 +287,7 @@ function setOffsetAndWidthInGroupMode(gd, pa, sieve) {
     var overlap = (positions.length !== distinctPositions.length);
     var barGroupWidth = minDiff * (1 - bargap);
 
-    var groupId = pa._id + calcTraces[0][0].trace.orientation;
+    var groupId = getAxisGroup(fullLayout, pa._id) + calcTraces[0][0].trace.orientation;
     var alignmentGroups = fullLayout._alignmentOpts[groupId] || {};
 
     for(var i = 0; i < nTraces; i++) {
diff --git a/src/traces/bar/defaults.js b/src/traces/bar/defaults.js
@@ -14,6 +14,7 @@ var Registry = require('../../registry');
 
 var handleXYDefaults = require('../scatter/xy_defaults');
 var handleStyleDefaults = require('../bar/style_defaults');
+var getAxisGroup = require('../../plots/cartesian/axis_ids').getAxisGroup;
 var attributes = require('./attributes');
 
 function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
@@ -76,21 +77,16 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
     errorBarsSupplyDefaults(traceIn, traceOut, lineColor || Color.defaultLine, {axis: 'y'});
     errorBarsSupplyDefaults(traceIn, traceOut, lineColor || Color.defaultLine, {axis: 'x', inherit: 'y'});
 
-    handleGroupingDefaults(traceIn, traceOut, layout, coerce);
-
     Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
 }
 
-function handleGroupingDefaults(traceIn, traceOut, layout, coerce) {
+function handleGroupingDefaults(traceIn, traceOut, fullLayout, coerce) {
     var orientation = traceOut.orientation;
-    // TODO make this work across matching axes too?!?
-    // TODO should this work per trace-type?
-    //      one set for bar/histogram another for box/violin?
-    //      or just one set for all trace trace types?
+    // N.B. grouping is done across all trace trace types that support it
     var posAxId = traceOut[{v: 'x', h: 'y'}[orientation] + 'axis'];
-    var groupId = posAxId + orientation;
+    var groupId = getAxisGroup(fullLayout, posAxId) + orientation;
 
-    var alignmentOpts = layout._alignmentOpts || {};
+    var alignmentOpts = fullLayout._alignmentOpts || {};
     var alignmentgroup = coerce('alignmentgroup');
 
     var alignmentGroups = alignmentOpts[groupId];
@@ -123,7 +119,27 @@ function handleGroupingDefaults(traceIn, traceOut, layout, coerce) {
     }
 }
 
+function crossTraceDefaults(fullData, fullLayout) {
+    var traceIn, traceOut;
+
+    function coerce(attr) {
+        return Lib.coerce(traceOut._input, traceOut, attributes, attr);
+    }
+
+    for(var i = 0; i < fullData.length; i++) {
+        traceOut = fullData[i];
+
+        if(traceOut.type === 'bar') {
+            traceIn = traceOut._input;
+            if(fullLayout.barmode === 'group') {
+                handleGroupingDefaults(traceIn, traceOut, fullLayout, coerce);
+            }
+        }
+    }
+}
+
 module.exports = {
     supplyDefaults: supplyDefaults,
+    crossTraceDefaults: crossTraceDefaults,
     handleGroupingDefaults: handleGroupingDefaults
 };
diff --git a/src/traces/bar/index.js b/src/traces/bar/index.js
@@ -13,6 +13,7 @@ var Bar = {};
 Bar.attributes = require('./attributes');
 Bar.layoutAttributes = require('./layout_attributes');
 Bar.supplyDefaults = require('./defaults').supplyDefaults;
+Bar.crossTraceDefaults = require('./defaults').crossTraceDefaults;
 Bar.supplyLayoutDefaults = require('./layout_defaults');
 Bar.calc = require('./calc');
 Bar.crossTraceCalc = require('./cross_trace_calc').crossTraceCalc;
diff --git a/src/traces/bar/layout_defaults.js b/src/traces/bar/layout_defaults.js
@@ -15,8 +15,6 @@ var Lib = require('../../lib');
 var layoutAttributes = require('./layout_attributes');
 
 module.exports = function(layoutIn, layoutOut, fullData) {
-    var i, trace;
-
     function coerce(attr, dflt) {
         return Lib.coerce(layoutIn, layoutOut, layoutAttributes, attr, dflt);
     }
@@ -25,10 +23,9 @@ module.exports = function(layoutIn, layoutOut, fullData) {
     var shouldBeGapless = false;
     var gappedAnyway = false;
     var usedSubplots = {};
-    var tracesWithGroupAttrs = [];
 
-    for(i = 0; i < fullData.length; i++) {
-        trace = fullData[i];
+    for(var i = 0; i < fullData.length; i++) {
+        var trace = fullData[i];
         if(Registry.traceIs(trace, 'bar') && trace.visible) hasBars = true;
         else continue;
 
@@ -45,10 +42,6 @@ module.exports = function(layoutIn, layoutOut, fullData) {
                         trace[trace.orientation === 'v' ? 'xaxis' : 'yaxis']);
             if(pa.type !== 'category') shouldBeGapless = true;
         }
-
-        if(trace.alignmentgroup || trace.offsetgroup) {
-            tracesWithGroupAttrs.push(trace);
-        }
     }
 
     if(!hasBars) return;
@@ -58,12 +51,4 @@ module.exports = function(layoutIn, layoutOut, fullData) {
 
     coerce('bargap', (shouldBeGapless && !gappedAnyway) ? 0 : 0.2);
     coerce('bargroupgap');
-
-    if(mode !== 'group') {
-        for(i = 0; i < tracesWithGroupAttrs.length; i++) {
-            trace = tracesWithGroupAttrs[i];
-            delete trace.alignmentgroup;
-            delete trace.offsetgroup;
-        }
-    }
 };
diff --git a/src/traces/box/cross_trace_calc.js b/src/traces/box/cross_trace_calc.js
@@ -10,6 +10,7 @@
 
 var Axes = require('../../plots/cartesian/axes');
 var Lib = require('../../lib');
+var getAxisGroup = require('../../plots/cartesian/axis_ids').getAxisGroup;
 
 var orientations = ['v', 'h'];
 
@@ -104,7 +105,7 @@ function setPositionOffset(traceType, gd, boxList, posAxis) {
             dPos = dPos0;
 
             if(group) {
-                var groupId = posAxis._id + trace.orientation;
+                var groupId = getAxisGroup(fullLayout, posAxis._id) + trace.orientation;
                 var alignmentGroups = fullLayout._alignmentOpts[groupId] || {};
                 var alignmentGroupOpts = alignmentGroups[trace.alignmentgroup] || {};
                 var nOffsetGroups = Object.keys(alignmentGroupOpts.offsetGroups || {}).length;
diff --git a/src/traces/box/defaults.js b/src/traces/box/defaults.js
@@ -34,7 +34,6 @@ function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
     if(notched) coerce('notchwidth');
 
     handlePointsDefaults(traceIn, traceOut, coerce, {prefix: 'box'});
-    handleGroupingDefaults(traceIn, traceOut, layout, coerce);
 }
 
 function handleSampleDefaults(traceIn, traceOut, coerce, layout) {
@@ -110,8 +109,30 @@ function handlePointsDefaults(traceIn, traceOut, coerce, opts) {
     Lib.coerceSelectionMarkerOpacity(traceOut, coerce);
 }
 
+function crossTraceDefaults(fullData, fullLayout) {
+    var traceIn, traceOut;
+
+    function coerce(attr) {
+        return Lib.coerce(traceOut._input, traceOut, attributes, attr);
+    }
+
+    for(var i = 0; i < fullData.length; i++) {
+        traceOut = fullData[i];
+        var traceType = traceOut.type;
+
+        if(traceType === 'box' || traceType === 'violin') {
+            traceIn = traceOut._input;
+            if(fullLayout[traceType + 'mode'] === 'group') {
+                handleGroupingDefaults(traceIn, traceOut, fullLayout, coerce);
+            }
+        }
+    }
+}
+
 module.exports = {
     supplyDefaults: supplyDefaults,
+    crossTraceDefaults: crossTraceDefaults,
+
     handleSampleDefaults: handleSampleDefaults,
     handlePointsDefaults: handlePointsDefaults
 };
diff --git a/src/traces/box/index.js b/src/traces/box/index.js
@@ -13,6 +13,7 @@ var Box = {};
 Box.attributes = require('./attributes');
 Box.layoutAttributes = require('./layout_attributes');
 Box.supplyDefaults = require('./defaults').supplyDefaults;
+Box.crossTraceDefaults = require('./defaults').crossTraceDefaults;
 Box.supplyLayoutDefaults = require('./layout_defaults').supplyLayoutDefaults;
 Box.calc = require('./calc');
 Box.crossTraceCalc = require('./cross_trace_calc').crossTraceCalc;
diff --git a/src/traces/box/layout_defaults.js b/src/traces/box/layout_defaults.js
@@ -13,36 +13,22 @@ var Lib = require('../../lib');
 var layoutAttributes = require('./layout_attributes');
 
 function _supply(layoutIn, layoutOut, fullData, coerce, traceType) {
-    var i, trace;
-
     var category = traceType + 'Layout';
     var hasTraceType = false;
-    var tracesWithGroupAttrs = [];
 
-    for(i = 0; i < fullData.length; i++) {
-        trace = fullData[i];
+    for(var i = 0; i < fullData.length; i++) {
+        var trace = fullData[i];
 
         if(Registry.traceIs(trace, category)) {
             hasTraceType = true;
-
-            if(trace.alignmentgroup || trace.offsetgroup) {
-                tracesWithGroupAttrs.push(trace);
-            }
+            break;
         }
     }
     if(!hasTraceType) return;
 
-    var mode = coerce(traceType + 'mode');
+    coerce(traceType + 'mode');
     coerce(traceType + 'gap');
     coerce(traceType + 'groupgap');
-
-    if(mode !== 'group') {
-        for(i = 0; i < tracesWithGroupAttrs.length; i++) {
-            trace = tracesWithGroupAttrs[i];
-            delete trace.alignmentgroup;
-            delete trace.offsetgroup;
-        }
-    }
 }
 
 function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
diff --git a/src/traces/histogram/cross_trace_defaults.js b/src/traces/histogram/cross_trace_defaults.js
@@ -6,12 +6,13 @@
 * LICENSE file in the root directory of this source tree.
 */
 
-
 'use strict';
 
 var Lib = require('../../lib');
 var nestedProperty = Lib.nestedProperty;
 
+var handleGroupingDefaults = require('../bar/defaults').handleGroupingDefaults;
+var getAxisGroup = require('../../plots/cartesian/axis_ids').getAxisGroup;
 var attributes = require('./attributes');
 
 var BINATTRS = {
@@ -65,6 +66,8 @@ module.exports = function crossTraceDefaults(fullData, fullLayout) {
                 direction: binDirection
             };
         }
+
+        handleGroupingDefaults(traceOut._input, traceOut, fullLayout, coerce);
     }
 
     for(group in allBinOpts) {
@@ -111,13 +114,3 @@ module.exports = function crossTraceDefaults(fullData, fullLayout) {
         }
     }
 };
-
-function getAxisGroup(fullLayout, axId) {
-    var matchGroups = fullLayout._axisMatchGroups;
-
-    for(var i = 0; i < matchGroups.length; i++) {
-        var group = matchGroups[i];
-        if(group[axId]) return 'g' + i;
-    }
-    return axId;
-}
diff --git a/src/traces/histogram/defaults.js b/src/traces/histogram/defaults.js
@@ -13,7 +13,6 @@ var Lib = require('../../lib');
 var Color = require('../../components/color');
 
 var handleStyleDefaults = require('../bar/style_defaults');
-var handleGroupingDefaults = require('../bar/defaults').handleGroupingDefaults;
 var attributes = require('./attributes');
 
 module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
@@ -70,6 +69,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     var errorBarsSupplyDefaults = Registry.getComponentMethod('errorbars', 'supplyDefaults');
     errorBarsSupplyDefaults(traceIn, traceOut, lineColor || Color.defaultLine, {axis: 'y'});
     errorBarsSupplyDefaults(traceIn, traceOut, lineColor || Color.defaultLine, {axis: 'x', inherit: 'y'});
-
-    handleGroupingDefaults(traceIn, traceOut, layout, coerce);
 };
diff --git a/src/traces/violin/defaults.js b/src/traces/violin/defaults.js
@@ -12,7 +12,6 @@ var Lib = require('../../lib');
 var Color = require('../../components/color');
 
 var boxDefaults = require('../box/defaults');
-var handleGroupingDefaults = require('../bar/defaults').handleGroupingDefaults;
 var attributes = require('./attributes');
 
 module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
@@ -57,6 +56,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     var meanLineWidth = coerce2('meanline.width', lineWidth);
     var meanLineVisible = coerce('meanline.visible', Boolean(meanLineColor || meanLineWidth));
     if(!meanLineVisible) traceOut.meanline = {visible: false};
-
-    handleGroupingDefaults(traceIn, traceOut, layout, coerce);
 };
diff --git a/src/traces/violin/index.js b/src/traces/violin/index.js
@@ -12,6 +12,7 @@ module.exports = {
     attributes: require('./attributes'),
     layoutAttributes: require('./layout_attributes'),
     supplyDefaults: require('./defaults'),
+    crossTraceDefaults: require('../box/defaults').crossTraceDefaults,
     supplyLayoutDefaults: require('./layout_defaults'),
     calc: require('./calc'),
     crossTraceCalc: require('./cross_trace_calc'),
diff --git a/test/image/baselines/groups-over-matching-axes.png b/test/image/baselines/groups-over-matching-axes.png
diff --git a/test/image/mocks/groups-over-matching-axes.json b/test/image/mocks/groups-over-matching-axes.json
