drop *scaleanchor* constraints for axes under *matches* constraint

etpinard · etpinard · commit 91431ec42505 · 2019-02-12T12:43:10.000-05:00
... for now, until we found a way to apply domain scaleanchor constraints
    on matching axes (which is theoretically possible).
diff --git a/src/plots/cartesian/constraints.js b/src/plots/cartesian/constraints.js
@@ -29,7 +29,6 @@ exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, a
     // coerce the constraint mechanics even if this axis has no scaleanchor
     // because it may be the anchor of another axis.
     var constrain = coerce('constrain');
-
     Lib.coerce(containerIn, containerOut, {
         constraintoward: {
             valType: 'enumerated',
@@ -38,38 +37,41 @@ exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, a
         }
     }, 'constraintoward');
 
-    var scaleOpts = containerIn.scaleanchor && !(containerOut.fixedrange && constrain !== 'domain') ?
-        getConstraintOpts(constraintGroups, thisID, allAxisIds, layoutOut, constrain) :
-        {};
+    var matches, matchOpts;
 
-    var matchOpts = (containerIn.matches || splomStash.matches) && !containerOut.fixedrange ?
-        getConstraintOpts(matchGroups, thisID, allAxisIds, layoutOut) :
-        {};
+    if((containerIn.matches || splomStash.matches) && !containerOut.fixedrange) {
+        matchOpts = getConstraintOpts(matchGroups, thisID, allAxisIds, layoutOut);
+        matches = Lib.coerce(containerIn, containerOut, {
+            matches: {
+                valType: 'enumerated',
+                values: matchOpts.linkableAxes || [],
+                dflt: splomStash.matches
+            }
+        }, 'matches');
+    }
 
-    var scaleanchor = Lib.coerce(containerIn, containerOut, {
-        scaleanchor: {
-            valType: 'enumerated',
-            values: scaleOpts.linkableAxes || []
-        }
-    }, 'scaleanchor');
+    // 'matches' wins over 'scaleanchor' (for now)
+    var scaleanchor, scaleOpts;
 
-    var matches = Lib.coerce(containerIn, containerOut, {
-        matches: {
-            valType: 'enumerated',
-            values: matchOpts.linkableAxes || [],
-            dflt: splomStash.matches
-        }
-    }, 'matches');
+    if(!matches && containerIn.scaleanchor && !(containerOut.fixedrange && constrain !== 'domain')) {
+        scaleOpts = getConstraintOpts(constraintGroups, thisID, allAxisIds, layoutOut, constrain);
+        scaleanchor = Lib.coerce(containerIn, containerOut, {
+            scaleanchor: {
+                valType: 'enumerated',
+                values: scaleOpts.linkableAxes || []
+            }
+        }, 'scaleanchor');
+    }
 
-    // disallow constraining AND matching range
-    if(constrain === 'range' && scaleanchor && matches && scaleanchor === matches) {
-        delete containerOut.scaleanchor;
+    if(matches) {
         delete containerOut.constrain;
-        scaleanchor = null;
+        updateConstraintGroups(matchGroups, matchOpts.thisGroup, thisID, matches, 1);
+    } else if(allAxisIds.indexOf(containerIn.matches) !== -1) {
+        Lib.warn('ignored ' + containerOut._name + '.matches: "' +
+            containerIn.matches + '" to avoid either an infinite loop ' +
+            'or because the target axis has fixed range.');
     }
 
-    var found = false;
-
     if(scaleanchor) {
         var scaleratio = coerce('scaleratio');
 
@@ -81,19 +83,11 @@ exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, a
         if(!scaleratio) scaleratio = containerOut.scaleratio = 1;
 
         updateConstraintGroups(constraintGroups, scaleOpts.thisGroup, thisID, scaleanchor, scaleratio);
-        found = true;
-    }
-
-    if(matches) {
-        updateConstraintGroups(matchGroups, matchOpts.thisGroup, thisID, matches, 1);
-        found = true;
-    }
-
-    if(!found && allAxisIds.indexOf(containerIn.scaleanchor) !== -1) {
+    } else if(allAxisIds.indexOf(containerIn.scaleanchor) !== -1) {
         Lib.warn('ignored ' + containerOut._name + '.scaleanchor: "' +
             containerIn.scaleanchor + '" to avoid either an infinite loop ' +
-            'and possibly inconsistent scaleratios, or because the target' +
-            'axis has fixed range.');
+            'and possibly inconsistent scaleratios, or because the target ' +
+            'axis has fixed range or this axis declares a *matches* constraint.');
     }
 };
 
diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js
@@ -169,7 +169,9 @@ module.exports = {
             'or the same letter (to match scales across subplots).',
             'Loops (`yaxis: {scaleanchor: *x*}, xaxis: {scaleanchor: *y*}` or longer) are redundant',
             'and the last constraint encountered will be ignored to avoid possible',
-            'inconsistent constraints via `scaleratio`.'
+            'inconsistent constraints via `scaleratio`.',
+            'Note that setting axes simultaneously in both a `scaleanchor` and a `matches` constraint',
+            'is currently forbidden.'
         ].join(' ')
     },
     scaleratio: {
@@ -224,8 +226,8 @@ module.exports = {
             'will match the range of the corresponding axis in data-coordinates space.',
             'Moreover, matching axes share auto-range values, category lists and',
             'histogram auto-bins.',
-            'Note that setting `matches` and `scaleratio` under a *range* `constrain`',
-            'to the same axis id is forbidden.'
+            'Note that setting axes simultaneously in both a `scaleanchor` and a `matches` constraint',
+            'is currently forbidden.'
         ].join(' ')
     },
     // ticks
diff --git a/src/plots/cartesian/layout_defaults.js b/src/plots/cartesian/layout_defaults.js
@@ -254,9 +254,9 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
 
     // sets of axes linked by `scaleanchor` along with the scaleratios compounded
     // together, populated in handleConstraintDefaults
-    layoutOut._axisConstraintGroups = [];
+    var constraintGroups = layoutOut._axisConstraintGroups = [];
     // similar to _axisConstraintGroups, but for matching axes
-    layoutOut._axisMatchGroups = [];
+    var matchGroups = layoutOut._axisMatchGroups = [];
 
     for(i = 0; i < axNames.length; i++) {
         axName = axNames[i];
@@ -267,20 +267,22 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
         handleConstraintDefaults(axLayoutIn, axLayoutOut, coerce, allAxisIds, layoutOut);
     }
 
-    for(i = 0; i < layoutOut._axisMatchGroups.length; i++) {
-        var group = layoutOut._axisMatchGroups[i];
+    for(i = 0; i < matchGroups.length; i++) {
+        var group = matchGroups[i];
         var rng = null;
         var autorange = null;
         var axId;
 
+        // find 'matching' range attrs
         for(axId in group) {
             axLayoutOut = layoutOut[id2name(axId)];
             if(!axLayoutOut.matches) {
                 rng = axLayoutOut.range;
                 autorange = axLayoutOut.autorange;
             }
         }
-
+        // if `ax.matches` values are reciprocal,
+        // pick values of first axis in group
         if(rng === null || autorange === null) {
             for(axId in group) {
                 axLayoutOut = layoutOut[id2name(axId)];
@@ -289,7 +291,7 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
                 break;
             }
         }
-
+        // apply matching range attrs
         for(axId in group) {
             axLayoutOut = layoutOut[id2name(axId)];
             if(axLayoutOut.matches) {
@@ -298,5 +300,26 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
             }
             axLayoutOut._matchGroup = group;
         }
+
+        // remove matching axis from scaleanchor constraint groups (for now)
+        if(constraintGroups.length) {
+            for(axId in group) {
+                for(j = 0; j < constraintGroups.length; j++) {
+                    var group2 = constraintGroups[j];
+                    for(var axId2 in group2) {
+                        if(axId === axId2) {
+                            Lib.warn('Axis ' + axId2 + ' is set with both ' +
+                                'a *scaleanchor* and *matches* constraint; ' +
+                                'ignoring the scale constraint.');
+
+                            delete group2[axId2];
+                            if(Object.keys(group2).length < 2) {
+                                constraintGroups.splice(j, 1);
+                            }
+                        }
+                    }
+                }
+            }
+        }
     }
 };
diff --git a/test/image/baselines/axes_scaleanchor-with-matches.png b/test/image/baselines/axes_scaleanchor-with-matches.png
diff --git a/test/image/mocks/axes_scaleanchor-with-matches.json b/test/image/mocks/axes_scaleanchor-with-matches.json
@@ -1,27 +1,29 @@
 {
   "data":[
     {"x": [0,1,1,0,0,1,1,2,2,3,3,2,2,3], "y": [0,0,1,1,3,3,2,2,3,3,1,1,0,0]},
-    {"x": [0,1,2,3], "y": [1,2,4,8], "yaxis":"y2"}
+    {"x": [0,1,2,3], "y": [1,2,4,8], "xaxis": "x2", "yaxis":"y2"}
   ],
   "layout":{
     "width": 500,
     "height": 500,
+    "title": {"text": "Bottom subplot has scaleanchor constrained axes<br>Top subplot has matching axes"},
     "xaxis": {
-        "constrain": "domain"
+        "constrain": "range"
     },
     "yaxis": {
         "scaleanchor": "x",
-        "constrain": "domain",
+        "constrain": "range",
         "domain": [0, 0.45],
-        "title": {"text": "1:1<br>matching y range above"}
+        "title": {"text": "1:1<br>x|y scale constrain range"}
+    },
+    "xaxis2": {
+        "anchor": "y2"
     },
     "yaxis2": {
-        "matches": "y",
-        "constrain": "domain",
-        "scaleanchor": "x",
-        "scaleratio": 0.2,
+        "anchor": "x2",
+        "matches": "x2",
         "domain": [0.55, 1],
-        "title": {"text": "1:5<br>matching y range below"}
+        "title": {"text": "matching x|y"}
     },
     "showlegend": false
   }
diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js
@@ -588,8 +588,8 @@ describe('Test axes', function() {
         });
 
         var warnTxt = ' to avoid either an infinite loop and possibly ' +
-            'inconsistent scaleratios, or because the targetaxis has ' +
-            'fixed range.';
+            'inconsistent scaleratios, or because the target axis has ' +
+            'fixed range or this axis declares a *matches* constraint.';
 
         it('breaks scaleanchor loops and drops conflicting ratios', function() {
             var warnings = [];
@@ -707,6 +707,48 @@ describe('Test axes', function() {
             expect(layoutOut._axisMatchGroups).toEqual([{x: 1, x2: 1}]);
         });
 
+        it('remove axes from constraint groups if they are in a match group', function() {
+            layoutIn = {
+                // this one is ok
+                xaxis: {},
+                yaxis: {scaleanchor: 'x'},
+                // this one too
+                xaxis2: {},
+                yaxis2: {matches: 'x2'},
+                // not these ones
+                xaxis3: {scaleanchor: 'x2'},
+                yaxis3: {scaleanchor: 'y2'}
+            };
+            layoutOut._subplots.cartesian.push('x2y2, x3y3');
+            layoutOut._subplots.xaxis.push('x2', 'x3');
+            layoutOut._subplots.yaxis.push('y2', 'y3');
+
+            supplyLayoutDefaults(layoutIn, layoutOut, fullData);
+
+            expect(layoutOut._axisMatchGroups.length).toBe(1);
+            expect(layoutOut._axisMatchGroups).toContain({x2: 1, y2: 1});
+
+            expect(layoutOut._axisConstraintGroups.length).toBe(1);
+            expect(layoutOut._axisConstraintGroups).toContain({x: 1, y: 1});
+        });
+
+        it('remove constraint group if they are one or zero items left in it', function() {
+            layoutIn = {
+                xaxis: {},
+                yaxis: {matches: 'x'},
+                xaxis2: {scaleanchor: 'y'}
+            };
+            layoutOut._subplots.cartesian.push('x2y');
+            layoutOut._subplots.xaxis.push('x2');
+
+            supplyLayoutDefaults(layoutIn, layoutOut, fullData);
+
+            expect(layoutOut._axisMatchGroups.length).toBe(1);
+            expect(layoutOut._axisMatchGroups).toContain({x: 1, y: 1});
+
+            expect(layoutOut._axisConstraintGroups.length).toBe(0);
+        });
+
         it('drops scaleanchor settings if either the axis or target has fixedrange', function() {
             // some of these will create warnings... not too important, so not going to test,
             // just want to keep the output clean
