8000 PR feedback: moved circularity check into calc.js · osvik/plotly.js@9214c92 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9214c92

Browse files
monferamonfera
committed
PR feedback: moved circularity check into calc.js
1 parent af1c43b commit 9214c92

File tree

3 files changed

+127
-96
lines changed

3 files changed

+127
-96
lines changed

‎src/traces/sankey/calc.js

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,44 @@
88

99
'use strict';
1010

11-
module.exports = function calc() {
12-
return [{}];
11+
var tarjan = require('strongly-connected-components');
12+
var Lib = require('../../lib');
13+
14+
function circularityPresent(nodeList, sources, targets) {
15+
16+
var nodes = nodeList.map(function() {return [];});
17+
18+
for(var i = 0; i < Math.min(sources.length, targets.length); i++) {
19+
if(sources[i] === targets[i]) {
20+
return true; // self-link which is also a scc of one
21+
}
22+
nodes[sources[i]].push(targets[i]);
23+
}
24+
25+
var scc = tarjan(nodes);
26+
27+
// Tarján's strongly connected components algorithm coded by Mikola Lysenko
28+
// returns at least one non-singular component if there's circularity in the graph
29+
return scc.components.some(function(c) {
30+
return c.length > 1;
31+
});
32+
}
33+
34+
module.exports = function calc(gd, trace) {
35+
36+
if(circularityPresent(trace.node.label, trace.link.source, trace.link.target)) {
37+
Lib.error('Circularity is present in the Sankey data. Removing all nodes and links.');
38+
trace.link.label = [];
39+
trace.link.source = [];
40+
trace.link.target = [];
41+
trace.link.value = [];
42+
trace.link.color = [];
43+
trace.node.label = [];
44+
trace.node.color = [];
45+
}
46+
47+
return [{
48+
link: trace.link,
49+
node: trace.node
50+
}];
1351
};

‎src/traces/sankey/defaults.js

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,27 +13,6 @@ var attributes = require('./attributes');
1313
var colors = require('../../components/color/attributes').defaults;
1414
var Color = require('../../components/color');
1515
var tinycolor = require('tinycolor2');
16-
var tarjan = require('strongly-connected-components');
17-
18-
function circularityPresent(nodeList, sources, targets) {
19-
20-
var nodes = nodeList.map(function() {return [];});
21-
22-
for(var i = 0; i < Math.min(sources.length, targets.length); i++) {
23-
if(sources[i] === targets[i]) {
24-
return true; // self-link which is also a scc of one
25-
}
26-
nodes[sources[i]].push(targets[i]);
27-
}
28-
29-
var scc = tarjan(nodes);
30-
31-
// Tarján's strongly connected components algorithm coded by Mikola Lysenko
32-
// returns at least one non-singular component if there's circularity in the graph
33-
return scc.components.some(function(c) {
34-
return c.length > 1;
35-
});
36-
}
3716

3817
module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
3918

@@ -85,15 +64,4 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
8564
if(traceOut.node.label.some(missing)) {
8665
Lib.warn('Some of the nodes are neither sources nor targets, they will not be displayed.');
8766
}
88-
89-
if(circularityPresent(traceOut.node.label, traceOut.link.source, traceOut.link.target)) {
90-
Lib.error('Circularity is present in the Sankey data. Removing all nodes and links.');
91-
traceOut.link.label = [];
92-
traceOut.link.source = [];
93-
traceOut.link.target = [];
94-
traceOut.link.value = [];
95-
traceOut.link.color = [];
96-
traceOut.node.label = [];
97-
traceOut.node.color = [];
98-
}
9967
};

‎test/jasmine/tests/sankey_test.js

Lines changed: 87 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
var attributes = require('@src/traces/sankey/attributes');
12
var Lib = require('@src/lib');
3+
var mock = require('@mocks/energy.json');
4+
var Plots = require('@src/plots/plots');
25
var Sankey = require('@src/traces/sankey');
3-
var attributes = require('@src/traces/sankey/attributes');
46

57
describe('sankey tests', function() {
68

@@ -46,48 +48,6 @@ describe('sankey tests', function() {
4648
});
4749
});
4850

49-
describe('remove nodes if encountering circularity', function() {
50-
51-
it('removing a single self-pointing node', function() {
52-
var fullTrace = _supply({
53-
node: {
54-
label: ['a']
55-
},
56-
link: {
57-
value: [1],
58-
source: [0],
59-
target: [0]
60-
}
61-
});
62-
63-
expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
64-
expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
65-
expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
66-
expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
67-
68-
});
69-
70-
it('removing everything if detecting a circle', function() {
71-
var fullTrace = _supply({
72-
node: {
73-
label: ['a', 'b', 'c', 'd', 'e']
74-
},
75-
link: {
76-
value: [1, 1, 1, 1, 1, 1, 1, 1],
77-
source: [0, 1, 2, 3],
78-
target: [1, 2, 0, 4]
79-
}
80-
});
81-
82-
expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
83-
expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
84-
expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
85-
expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
86 10000 -
87-
});
88-
89-
});
90-
9151
describe('log warning if issue is encountered with graph structure',
9252
function() {
9353

@@ -111,31 +71,20 @@ describe('sankey tests', function() {
11171

11272
expect(warnings.length).toEqual(1);
11373
});
74+
});
11475

115-
it('circularity is detected', function() {
116-
117-
var errors = [];
118-
spyOn(Lib, 'error').and.callFake(function(msg) {
119-
errors.push(msg);
120-
});
121-
122-
_supply({
123-
node: {
124-
label: ['a', 'b', 'c']
125-
},
126-
link: {
127-
value: [1, 1, 1],
128-
source: [0, 1, 2],
129-
target: [1, 2, 0]
130-
}
131-
});
76+
describe('sankey global defaults', function() {
13277

133-
expect(errors.length).toEqual(1);
134-
});
78+
it('should not coerce trace opacity', function() {
79+
var gd = Lib.extendDeep({}, mock);
13580

81+
Plots.supplyDefaults(gd);
13682

83+
expect(gd._fullData[0].opacity).toBeUndefined();
13784
});
13885

86+
});
87+
13988
describe('sankey defaults', function() {
14089

14190
it('\'Sankey\' specification should have proper arrays where mandatory',
@@ -223,4 +172,80 @@ describe('sankey tests', function() {
223172
});
224173

225174
});
175+
176+
describe('sankey calc', function() {
177+
178+
function _calc(trace) {
179+
var gd = { data: [trace] };
180+
181+
Plots.supplyDefaults(gd);
182+
var fullTrace = gd._fullData[0];
183+
Sankey.calc(gd, fullTrace);
184+
return fullTrace;
185+
}
186+
187+
var base = { type: 'sankey' };
188+
189+
it('circularity is detected', function() {
190+
191+
var errors = [];
192+
spyOn(Lib, 'error').and.callFake(function(msg) {
193+
errors.push(msg);
194+
});
195+
196+
_calc(Lib.extendDeep({}, base, {
197+
node: {
198+
label: ['a', 'b', 'c']
199+
},
200+
link: {
201+
value: [1, 1, 1],
202+
source: [0, 1, 2],
203+
target: [1, 2, 0]
204+
}
205+
}));
206+
207+
expect(errors.length).toEqual(1);
208+
});
209+
210+
describe('remove nodes if encountering circularity', function() {
211+
212+
it('removing a single self-pointing node', function() {
213+
var fullTrace = _calc(Lib.extendDeep({}, base, {
214+
node: {
215+
label: ['a']
216+
},
217+
link: {
218+
value: [1],
219+
source: [0],
220+
target: [0]
221+
}
222+
}));
223+
224+
expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
225+
expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
226+
expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
227+
expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
228+
229+
});
230+
231+
it('removing everything if detecting a circle', function() {
232+
var fullTrace = _calc(Lib.extendDeep({}, base, {
233+
node: {
234+
label: ['a', 'b', 'c', 'd', 'e']
235+
},
236+
link: {
237+
value: [1, 1, 1, 1, 1, 1, 1, 1],
238+
source: [0, 1, 2, 3],
239+
target: [1, 2, 0, 4]
240+
}
241+
}));
242+
243+
expect(fullTrace.node.label).toEqual([], 'node label(s) removed');
244+
expect(fullTrace.link.value).toEqual([], 'link value(s) removed');
245+
expect(fullTrace.link.source).toEqual([], 'link source(s) removed');
246+
expect(fullTrace.link.target).toEqual([], 'link target(s) removed');
247+
248+
});
249+
});
250+
});
226251
});

0 commit comments

Comments
 (0)
0