PR feedback: moved circularity check into calc.js

Author: monfera

src/traces/sankey/calc.js

@@ -8,6 +8,44 @@
 
 'use strict';
 
-module.exports = function calc() {
-    return [{}];
+var tarjan = require('strongly-connected-components');
+var Lib = require('../../lib');
+
+function circularityPresent(nodeList, sources, targets) {
+
+    var nodes = nodeList.map(function() {return [];});
+
+    for(var i = 0; i < Math.min(sources.length, targets.length); i++) {
+        if(sources[i] === targets[i]) {
+            return true; // self-link which is also a scc of one
+        }
+        nodes[sources[i]].push(targets[i]);
+    }
+
+    var scc = tarjan(nodes);
+
+    // Tarján's strongly connected components algorithm coded by Mikola Lysenko
+    // returns at least one non-singular component if there's circularity in the graph
+    return scc.components.some(function(c) {
+        return c.length > 1;
+    });
+}
+
+module.exports = function calc(gd, trace) {
+
+    if(circularityPresent(trace.node.label, trace.link.source, trace.link.target)) {
+        Lib.error('Circularity is present in the Sankey data. Removing all nodes and links.');
+        trace.link.label = [];
+        trace.link.source = [];
+        trace.link.target = [];
+        trace.link.value = [];
+        trace.link.color = [];
+        trace.node.label = [];
+        trace.node.color = [];
+    }
+
+    return [{
+        link: trace.link,
+        node: trace.node
+    }];
 };

src/traces/sankey/defaults.js

@@ -13,27 +13,6 @@ var attributes = require('./attributes');
 var colors = require('../../components/color/attributes').defaults;
 var Color = require('../../components/color');
 var tinycolor = require('tinycolor2');
-var tarjan = require('strongly-connected-components');
-
-function circularityPresent(nodeList, sources, targets) {
-
-    var nodes = nodeList.map(function() {return [];});
-
-    for(var i = 0; i < Math.min(sources.length, targets.length); i++) {
-        if(sources[i] === targets[i]) {
-            return true; // self-link which is also a scc of one
-        }
-        nodes[sources[i]].push(targets[i]);
-    }
-
-    var scc = tarjan(nodes);
-
-    // Tarján's strongly connected components algorithm coded by Mikola Lysenko
-    // returns at least one non-singular component if there's circularity in the graph
-    return scc.components.some(function(c) {
-        return c.length > 1;
-    });
-}
 
 module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
 
@@ -85,15 +64,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     if(traceOut.node.label.some(missing)) {
         Lib.warn('Some of the nodes are neither sources nor targets, they will not be displayed.');
     }
-
-    if(circularityPresent(traceOut.node.label, traceOut.link.source, traceOut.link.target)) {
-        Lib.error('Circularity is present in the Sankey data. Removing all nodes and links.');
-        traceOut.link.label = [];
-        traceOut.link.source = [];
-        traceOut.link.target = [];
-        traceOut.link.value = [];
-        traceOut.link.color = [];
-        traceOut.node.label = [];
-        traceOut.node.color = [];
-    }
 };

test/jasmine/tests/sankey_test.js

@@ -1,6 +1,8 @@
+var attributes = require('@src/traces/sankey/attributes');
 var Lib = require('@src/lib');
+var mock = require('@mocks/energy.json');
+var Plots = require('@src/plots/plots');
 var Sankey = require('@src/traces/sankey');
-var attributes = require('@src/traces/sankey/attributes');
 
 describe('sankey tests', function() {
 
@@ -46,48 +48,6 @@ describe('sankey tests', function() {
         });
     });
 
-    describe('remove nodes if encountering circularity', function() {
-
-        it('removing a single self-pointing node', function() {
-            var fullTrace = _supply({
-                node: {
-                    label: ['a']
-                },
-                link: {
-                    value: [1],
-                    source: [0],
-                    target: [0]
-                }
-            });
-
-            expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
-            expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
-            expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
-            expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
-
-        });
-
-        it('removing everything if detecting a circle', function() {
-            var fullTrace = _supply({
-                node: {
-                    label: ['a', 'b', 'c', 'd', 'e']
-                },
-                link: {
-                    value: [1, 1, 1, 1, 1, 1, 1, 1],
-                    source: [0, 1, 2, 3],
-                    target: [1, 2, 0, 4]
-                }
-            });
-
-            expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
-            expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
-            expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
-            expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
-
-        });
-
-    });
-
     describe('log warning if issue is encountered with graph structure',
         function() {
 
@@ -111,31 +71,20 @@ describe('sankey tests', function() {
 
                 expect(warnings.length).toEqual(1);
             });
+        });
 
-            it('circularity is detected', function() {
-
-                var errors = [];
-                spyOn(Lib, 'error').and.callFake(function(msg) {
-                    errors.push(msg);
-                });
-
-                _supply({
-                    node: {
-                        label: ['a', 'b', 'c']
-                    },
-                    link: {
-                        value: [1, 1, 1],
-                        source: [0, 1, 2],
-                        target: [1, 2, 0]
-                    }
-                });
+    describe('sankey global defaults', function() {
 
-                expect(errors.length).toEqual(1);
-            });
+        it('should not coerce trace opacity', function() {
+            var gd = Lib.extendDeep({}, mock);
 
+            Plots.supplyDefaults(gd);
 
+            expect(gd._fullData[0].opacity).toBeUndefined();
         });
 
+    });
+
     describe('sankey defaults', function() {
 
         it('\'Sankey\' specification should have proper arrays where mandatory',
@@ -223,4 +172,80 @@ describe('sankey tests', function() {
         });
 
     });
+
+    describe('sankey calc', function() {
+
+        function _calc(trace) {
+            var gd = { data: [trace] };
+
+            Plots.supplyDefaults(gd);
+            var fullTrace = gd._fullData[0];
+            Sankey.calc(gd, fullTrace);
+            return fullTrace;
+        }
+
+        var base = { type: 'sankey' };
+
+        it('circularity is detected', function() {
+
+            var errors = [];
+            spyOn(Lib, 'error').and.callFake(function(msg) {
+                errors.push(msg);
+            });
+
+            _calc(Lib.extendDeep({}, base, {
+                node: {
+                    label: ['a', 'b', 'c']
+                },
+                link: {
+                    value: [1, 1, 1],
+                    source: [0, 1, 2],
+                    target: [1, 2, 0]
+                }
+            }));
+
+            expect(errors.length).toEqual(1);
+        });
+
+        describe('remove nodes if encountering circularity', function() {
+
+            it('removing a single self-pointing node', function() {
+                var fullTrace = _calc(Lib.extendDeep({}, base, {
+                    node: {
+                        label: ['a']
+                    },
+                    link: {
+                        value: [1],
+                        source: [0],
+                        target: [0]
+                    }
+                }));
+
+                expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
+                expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
+                expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
+                expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
+
+            });
+
+            it('removing everything if detecting a circle', function() {
+                var fullTrace = _calc(Lib.extendDeep({}, base, {
+                    node: {
+                        label: ['a', 'b', 'c', 'd', 'e']
+                    },
+                    link: {
+                        value: [1, 1, 1, 1, 1, 1, 1, 1],
+                        source: [0, 1, 2, 3],
+                        target: [1, 2, 0, 4]
+                    }
+                }));
+
+                expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
+                expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
+                expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
+                expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
+
+            });
+        });
+    });
 });