Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revise hierarchy referencing and fixup parent percentage at the root node of treemap and sunburst #4219

Merged
merged 5 commits into from
Sep 30, 2019
Merged

Revise hierarchy referencing and fixup parent percentage at the root node of treemap and sunburst #4219

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7a1c0e5
Correct percentages at root
archmoj Sep 26, 2019
422d5d3
copy one of the treemap baselines from the CI which started failing t…
archmoj Sep 26, 2019
df5e121
only return label from the helper - set the label of dummy root to blank
archmoj Sep 28, 2019
54360be
handle parent of root special case - combine two treemap hovertemplat…
archmoj Sep 28, 2019
7155917
dont show parent and percent of parent in the template
archmoj Sep 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 11 additions & 18 deletions src/traces/sunburst/fx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
var cd0 = cd[0];
var trace = cd0.trace;
var hierarchy = cd0.hierarchy;
var rootLabel = hierarchy.data.data.pid;
var readLabel = function(refPt) {
var l = helpers.getPtLabel(refPt);
return l === undefined ? rootLabel : l;
};

var isSunburst = trace.type === 'sunburst';
var isTreemap = trace.type === 'treemap';
Expand All @@ -53,7 +48,7 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
var isRoot = helpers.isHierarchyRoot(pt);
var parent = helpers.getParent(hierarchy, pt);

var val = helpers.getVal(pt);
var val = helpers.getValue(pt);

var _cast = function(astr) {
return Lib.castOption(traceNow, ptNumber, astr);
Expand Down Expand Up @@ -109,24 +104,22 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
}
};

if(parent) {
hoverPt.percentParent = pt.percentParent = val / helpers.getVal(parent);
hoverPt.parent = pt.parentString = readLabel(parent);
if(hasFlag('percent parent')) {
tx = helpers.formatPercent(hoverPt.percentParent, separators) + ' of ' + hoverPt.parent;
insertPercent();
}
hoverPt.percentParent = pt.percentParent = val / helpers.getValue(parent);
hoverPt.parent = pt.parentString = helpers.getPtLabel(parent);
if(hasFlag('percent parent')) {
tx = helpers.formatPercent(hoverPt.percentParent, separators) + ' of ' + hoverPt.parent;
insertPercent();
}

hoverPt.percentEntry = pt.percentEntry = val / helpers.getVal(entry);
hoverPt.entry = pt.entry = readLabel(entry);
hoverPt.percentEntry = pt.percentEntry = val / helpers.getValue(entry);
hoverPt.entry = pt.entry = helpers.getPtLabel(entry);
if(hasFlag('percent entry') && !isRoot && !pt.onPathbar) {
tx = helpers.formatPercent(hoverPt.percentEntry, separators) + ' of ' + hoverPt.entry;
insertPercent();
}

hoverPt.percentRoot = pt.percentRoot = val / helpers.getVal(hierarchy);
hoverPt.root = pt.root = readLabel(hierarchy);
hoverPt.percentRoot = pt.percentRoot = val / helpers.getValue(hierarchy);
hoverPt.root = pt.root = helpers.getPtLabel(hierarchy);
if(hasFlag('percent root') && !isRoot) {
tx = helpers.formatPercent(hoverPt.percentRoot, separators) + ' of ' + hoverPt.root;
insertPercent();
Expand Down Expand Up @@ -307,7 +300,7 @@ function makeEventData(pt, trace, keys) {
if(key in pt) out[key] = pt[key];
}
// handle special case of parent
if('parentString' in pt) out.parent = pt.parentString;
if('parentString' in pt && !helpers.isHierarchyRoot(pt)) out.parent = pt.parentString;

appendArrayPointValue(out, trace, cdi.i);

Expand Down
10 changes: 4 additions & 6 deletions src/traces/sunburst/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@ exports.getPtId = function(pt) {
};

exports.getPtLabel = function(pt) {
return pt.data.data.label;
var label = pt.data.data.label;
return label === undefined ? pt.data.data.pid : label;
etpinard marked this conversation as resolved.
Show resolved Hide resolved
};

exports.getVal = function(d) {
exports.getValue = function(d) {
return d.value;
};

Expand Down Expand Up @@ -152,10 +153,7 @@ function getParentId(pt) {
}

exports.getParent = function(hierarchy, pt) {
var parentId = getParentId(pt);
return parentId === '' ?
undefined :
exports.findEntryWithLevel(hierarchy, parentId);
return exports.findEntryWithLevel(hierarchy, getParentId(pt));
};

exports.listPath = function(d, keyStr) {
Expand Down
36 changes: 14 additions & 22 deletions src/traces/sunburst/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,15 +488,9 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {
var cd0 = cd[0];
var cdi = pt.data.data;
var hierarchy = cd0.hierarchy;
var rootLabel = hierarchy.data.data.pid;
var readLabel = function(refPt) {
var l = helpers.getPtLabel(refPt);
return l === undefined ? rootLabel : l;
};

var isRoot = helpers.isHierarchyRoot(pt);
var parent = helpers.getParent(hierarchy, pt);
var val = helpers.getVal(pt);
var val = helpers.getValue(pt);

if(!texttemplate) {
var parts = textinfo.split('+');
Expand Down Expand Up @@ -532,16 +526,16 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {
thisText.push(tx);
};

if(hasFlag('percent parent') && parent) {
percent = val / helpers.getVal(parent);
if(hasFlag('percent parent')) {
percent = val / helpers.getValue(parent);
addPercent('parent');
}
if(hasFlag('percent entry')) {
percent = val / helpers.getVal(entry);
percent = val / helpers.getValue(entry);
addPercent('entry');
}
if(hasFlag('percent root')) {
percent = val / helpers.getVal(hierarchy);
percent = val / helpers.getValue(hierarchy);
addPercent('root');
}
}
Expand All @@ -566,25 +560,23 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {

obj.currentPath = helpers.getPath(pt.data);

if(parent) {
obj.percentParent = val / helpers.getVal(parent);
obj.percentParentLabel = helpers.formatPercent(
obj.percentParent, separators
);
obj.parent = readLabel(parent);
}
obj.percentParent = val / helpers.getValue(parent);
obj.percentParentLabel = helpers.formatPercent(
obj.percentParent, separators
);
obj.parent = helpers.getPtLabel(parent);

obj.percentEntry = val / helpers.getVal(entry);
obj.percentEntry = val / helpers.getValue(entry);
obj.percentEntryLabel = helpers.formatPercent(
obj.percentEntry, separators
);
obj.entry = readLabel(entry);
obj.entry = helpers.getPtLabel(entry);

obj.percentRoot = val / helpers.getVal(hierarchy);
obj.percentRoot = val / helpers.getValue(hierarchy);
obj.percentRootLabel = helpers.formatPercent(
obj.percentRoot, separators
);
obj.root = readLabel(hierarchy);
obj.root = helpers.getPtLabel(hierarchy);

if(cdi.hasOwnProperty('color')) {
obj.color = cdi.color;
Expand Down
6 changes: 3 additions & 3 deletions test/jasmine/tests/sunburst_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ describe('Test sunburst texttemplate without `values` should work at root level:
['path: %{currentPath}', ['path: /', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve', 'path: Eve/Seth', 'path: Eve/Seth/', 'path: Eve/Awan/']],
['%{percentRoot} of %{root}', ['100% of Eve', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve']],
['%{percentEntry} of %{entry}', ['100% of Eve', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve']],
['%{percentParent} of %{parent}', ['%{percentParent} of %{parent}', '100% of Seth', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '50% of Seth', '100% of Awan']],
['%{percentParent} of %{parent}', ['100% of Eve', '100% of Seth', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '50% of Seth', '100% of Awan']],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait. Is this the root node sector? You're saying the parent of the root node is itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Fixed in 54360be.

Copy link
Contributor

@etpinard etpinard Sep 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get it. After 54360be, looks like %{parent} is now '', but %{percentParent} is still 100% for the root node.

Can we have '%{percentParent} of %{parent}' resolve as '%{percentParent} of %{parent}' for the root node (i.e. the percentParent and parent keys are not defined) like before?

I know this is not ideal, but I don't like filling in '%{percentParent} and %{parent} with wrong values for the root node just to make the template string look better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 7155917.

[
[
'label: %{label}',
Expand Down Expand Up @@ -1389,7 +1389,7 @@ describe('Test sunburst texttemplate with *total* `values` should work at root l
['path: %{currentPath}', ['path: /', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve', 'path: Eve/Seth', 'path: Eve/Seth/', 'path: Eve/Awan/']],
['%{percentRoot} of %{root}', ['100% of Eve', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '15% of Eve', '3% of Eve', '2% of Eve']],
['%{percentEntry} of %{entry}', ['100% of Eve', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '15% of Eve', '3% of Eve', '2% of Eve']],
['%{percentParent} of %{parent}', ['%{percentParent} of %{parent}', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '83% of Seth', '17% of Seth', '17% of Awan']],
['%{percentParent} of %{parent}', ['100% of Eve', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '83% of Seth', '17% of Seth', '17% of Awan']],
[
[
'label: %{label}',
Expand Down Expand Up @@ -1433,7 +1433,7 @@ describe('Test sunburst texttemplate with *remainder* `values` should work at ro
['path: %{currentPath}', ['path: /', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve/', 'path: Eve', 'path: Eve/Seth', 'path: Eve/Seth/', 'path: Eve/Awan/']],
['%{percentRoot} of %{root}', ['100% of Eve', '20% of Eve', '12% of Eve', '6% of Eve', '5% of Eve', '3% of Eve', '8% of Eve', '2% of Eve', '1% of Eve']],
['%{percentEntry} of %{entry}', ['100% of Eve', '20% of Eve', '12% of Eve', '6% of Eve', '5% of Eve', '3% of Eve', '8% of Eve', '2% of Eve', '1% of Eve']],
['%{percentParent} of %{parent}', ['%{percentParent} of %{parent}', '20% of Eve', '12% of Eve', '6% of Eve', '5% of Eve', '3% of Eve', '42% of Seth', '8% of Seth', '14% of Awan']],
['%{percentParent} of %{parent}', ['100% of Eve', '20% of Eve', '12% of Eve', '6% of Eve', '5% of Eve', '3% of Eve', '42% of Seth', '8% of Seth', '14% of Awan']],
[
[
'label: %{label}',
Expand Down
183 changes: 183 additions & 0 deletions test/jasmine/tests/treemap_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,189 @@ describe('Test treemap hover:', function() {
});
});

describe('Test treemap hover with levels', function() {
etpinard marked this conversation as resolved.
Show resolved Hide resolved
var gd;

var labels0 = ['Alpha', 'Bravo', 'Charlie', 'Delta', 'Echo', 'Foxtrot', 'Golf', 'Hotel', 'India', 'Juliet', 'Kilo', 'Lima', 'Mike', 'November', 'Oscar', 'Papa', 'Quebec', 'Romeo', 'Sierra', 'Tango', 'Uniform', 'Victor', 'Whiskey', 'X ray', 'Yankee', 'Zulu'];
var parents0 = ['', 'Alpha', 'Alpha', 'Charlie', 'Charlie', 'Charlie', 'Foxtrot', 'Foxtrot', 'Foxtrot', 'Foxtrot', 'Juliet', 'Juliet', 'Juliet', 'Juliet', 'Juliet', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Uniform', 'Uniform', 'Uniform', 'Uniform', 'Uniform', 'Uniform'];
var values0 = [40, 2, 38, 1.5, 2.5, 34, 1, 2, 3, 28, 1.25, 1.75, 2.25, 2.75, 20, 1, 1.5, 2, 2.5, 3, 10, 1, 1.5, 2, 2.5, 3];
var text0 = ['forty', 'two', 'thirty-eight', 'one and a half', 'two and a half', 'thirty-four', 'one', 'two', 'three', 'twenty-eight', 'one and twenty-five hundredths', 'one and seventy-five hundredths', 'two and twenty-five hundredths', 'two and seventy-five hundredths', 'twenty', 'one', 'one and a half', 'two', 'two and a half', 'three', 'ten', 'one', 'one and a half', 'two', 'two and a half', 'three'];

afterEach(destroyGraphDiv);

function run(spec) {
gd = createGraphDiv();

var data = (spec.traces || [{}]).map(function(t) {
t.type = 'treemap';
t.text = text0;
t.values = values0;
t.level = 'Oscar';
t.branchvalues = 'total';
t.hovertemplate = 'path = %{currentPath}<br>label = %{label}<br>text = %{text}<br>value = %{value}<br>ratio to %{parent} = %{percentParent}<br>ratio to %{entry} = %{percentEntry}<br>ratio to %{root} = %{percentRoot}';

if(!t.labels) t.labels = labels0.slice();
if(!t.parents) t.parents = parents0.slice();
return t;
});

var layout = Lib.extendFlat({
width: 500,
height: 500,
margin: {t: 0, b: 0, l: 0, r: 0, pad: 0}
}, spec.layout || {});

var exp = spec.exp || {};
var ptData = null;

return Plotly.plot(gd, data, layout)
.then(function() {
gd.once('plotly_hover', function(d) { ptData = d.points[0]; });
})
.then(hover(gd, spec.pos))
.then(function() {
assertHoverLabelContent(exp.label);

for(var k in exp.ptData) {
expect(ptData[k]).toBe(exp.ptData[k], 'pt event data key ' + k);
}

if(exp.style) {
var gd3 = d3.select(gd);
assertHoverLabelStyle(gd3.select('.hovertext'), exp.style);
}
});
}

[{
desc: 'entry',
pos: 0,
exp: {
label: {
name: 'trace 0',
nums: 'path = Alpha/Charlie/Foxtrot/Juliet/\nlabel = Oscar\ntext = twenty\nvalue = 20\nratio to Juliet = 0.7142857142857143\nratio to Oscar = 1\nratio to Alpha = 0.5',
},
ptData: {
curveNumber: 0,
pointNumber: 14,
label: 'Oscar',
parent: 'Juliet'
}
}
}, {
desc: 'leaf',
pos: 10,
exp: {
label: {
name: 'trace 0',
nums: 'path = Alpha/Charlie/Foxtrot/Juliet/Oscar/Uniform/\nlabel = X ray\ntext = two\nvalue = 2\nratio to Uniform = 0.2\nratio to Oscar = 0.1\nratio to Alpha = 0.05',
},
ptData: {
curveNumber: 0,
pointNumber: 23,
label: 'X ray',
parent: 'Uniform'
}
}
}]
.forEach(function(spec) {
it('should generate correct hover labels and event data - ' + spec.desc, function(done) {
run(spec).catch(failTest).then(done);
});
});
});

describe('Test treemap hover at root level', function() {
var gd;

var labels0 = ['Alpha', 'Bravo', 'Charlie', 'Delta', 'Echo', 'Foxtrot', 'Golf', 'Hotel', 'India', 'Juliet', 'Kilo', 'Lima', 'Mike', 'November', 'Oscar', 'Papa', 'Quebec', 'Romeo', 'Sierra', 'Tango', 'Uniform', 'Victor', 'Whiskey', 'X ray', 'Yankee', 'Zulu'];
var parents0 = ['', 'Alpha', 'Alpha', 'Charlie', 'Charlie', 'Charlie', 'Foxtrot', 'Foxtrot', 'Foxtrot', 'Foxtrot', 'Juliet', 'Juliet', 'Juliet', 'Juliet', 'Juliet', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Oscar', 'Uniform', 'Uniform', 'Uniform', 'Uniform', 'Uniform', 'Uniform'];
var values0 = [40, 2, 38, 1.5, 2.5, 34, 1, 2, 3, 28, 1.25, 1.75, 2.25, 2.75, 20, 1, 1.5, 2, 2.5, 3, 10, 1, 1.5, 2, 2.5, 3];
var text0 = ['forty', 'two', 'thirty-eight', 'one and a half', 'two and a half', 'thirty-four', 'one', 'two', 'three', 'twenty-eight', 'one and twenty-five hundredths', 'one and seventy-five hundredths', 'two and twenty-five hundredths', 'two and seventy-five hundredths', 'twenty', 'one', 'one and a half', 'two', 'two and a half', 'three', 'ten', 'one', 'one and a half', 'two', 'two and a half', 'three'];

afterEach(destroyGraphDiv);

function run(spec) {
gd = createGraphDiv();

var data = (spec.traces || [{}]).map(function(t) {
t.type = 'treemap';
t.text = text0;
t.values = values0;
t.branchvalues = 'total';
t.hovertemplate = 'path = %{currentPath}<br>label = %{label}<br>text = %{text}<br>value = %{value}<br>ratio to %{parent} = %{percentParent}<br>ratio to %{entry} = %{percentEntry}<br>ratio to %{root} = %{percentRoot}';

if(!t.labels) t.labels = labels0.slice();
if(!t.parents) t.parents = parents0.slice();
return t;
});

var layout = Lib.extendFlat({
width: 500,
height: 500,
margin: {t: 0, b: 0, l: 0, r: 0, pad: 0}
}, spec.layout || {});

var exp = spec.exp || {};
var ptData = null;

return Plotly.plot(gd, data, layout)
.then(function() {
gd.once('plotly_hover', function(d) { ptData = d.points[0]; });
})
.then(hover(gd, spec.pos))
.then(function() {
assertHoverLabelContent(exp.label);

for(var k in exp.ptData) {
expect(ptData[k]).toBe(exp.ptData[k], 'pt event data key ' + k);
}

if(exp.style) {
var gd3 = d3.select(gd);
assertHoverLabelStyle(gd3.select('.hovertext'), exp.style);
}
});
}

[{
desc: 'entry',
pos: 0,
exp: {
label: {
name: 'trace 0',
nums: 'path = /\nlabel = Alpha\ntext = forty\nvalue = 40\nratio to = 1\nratio to Alpha = 1\nratio to Alpha = 1',
},
ptData: {
curveNumber: 0,
pointNumber: 0,
label: 'Alpha',
parent: ''
}
}
}, {
desc: 'leaf',
pos: 10,
exp: {
label: {
name: 'trace 0',
nums: 'path = Alpha/Charlie/Foxtrot/\nlabel = Golf\ntext = one\nvalue = 1\nratio to Foxtrot = 0.029411764705882353\nratio to Alpha = 0.025\nratio to Alpha = 0.025',
},
ptData: {
curveNumber: 0,
pointNumber: 6,
label: 'Golf',
parent: 'Foxtrot'
}
}
}]
.forEach(function(spec) {
it('should generate correct hover labels and event data - ' + spec.desc, function(done) {
run(spec).catch(failTest).then(done);
});
});
});

describe('Test treemap hover lifecycle:', function() {
var gd;
var hoverData;
Expand Down