Treemap new trace type #4185

archmoj · 2019-09-11T18:37:35Z

Fix #1038.
This PR adds treemap traces to plotly.js library.
This new trace inherits various logic from the plotly.js code base and is quite similar to its brother sunburst.
Since colorscale and displaying current path & percentages were within the requirements for our treemap, those features are also added to sunburst. Also a count attribute is added to both traces which allows counting the numbers of leaves, branches or both when values array is not provided.

One main extra feature of treemap is the barpath i.e. for displaying the current path of the visible portion of the hierarchical map. It may also be useful for zooming out of the graph.

Codepens in addition to new JSON mocks added which could be displayed here!

A
B
C
D

Please also note that commit 52ded31 touches on refactoring base_plot files of traces namely pie, funnelarea, indicator and 'sunburst`.

List of TODOs:

texttemplate additions appear to be incomplete for percentages
hovertemplate may require improvement (or bypassing) when hovered on pathbar elements.

@plotly/plotly_js
@nicolaskruchten
@Mahdis-z

… option to count branches - revise descriptions and editTypes

now no need to include empty string to hide last label on the barpath

src/traces/treemap/plot.js

src/traces/treemap/attributes.js

etpinard · 2019-09-12T17:45:28Z

src/traces/treemap/attributes.js

+            ].join(' ')
+        },
+
+        opacity: {


Can we reuse sunburstAttrs.marker.opacity here?

sunburst does not have a marker.opacity attribute. But we can reuse marker.line and marker.colors.

Done in 6dad958.

Ok, thanks for the info.

Now I see that marker.opacity can multiply branch.opacity:

plotly.js/src/traces/treemap/style.js

Line 43 in eb54f24

opacity = trace.marker.opacity * Math.pow(trace.branch.opacity, pt.height);

That's interesting, but wouldn't having marker.opacity (when set) override branch.opacity be more user friendly in your mind? In particular, if we allow marker.opacity to be arrayOk, this could allow users to set the opacity of a sector pretty easily.

What do you think @archmoj ?

A very good call!
It helped me rethink a couple of things in 0012fa1.
Now the API is more similar to sunburst where both have a leaf.opacity option.

marker.depthfade?

It also occurs to me that opacity might not actually be quite the effect we want - perhaps it would be better to use the opaque color that would result from blending the marker color with the background color in the same proportion? For the bottom-most layer the effect is equivalent, but anything stacked above it currently effectively is more opaque (if the two colors are the same) or blended to a different color (if using a colorscale or explicit colors). Both of these I think are problematic:

if the color has meaning, a faded version of that color is probably still easy to mentally connect with the original, keeping its interpretation the same, but a version blended with a different color could shift the implied meaning of that color.

making stacked layers more opaque kind of defeats the purpose - it means that the final effective opacity drops off too slowly at first, then more quickly toward the back of the stack.

Anyway if we extend the analogy I mentioned this morning of "looking at layers of hills in the distance" - those hills are not transparent, they are faded by the air in front of them - which is exactly the same effect as blending with the background color.

@alexcjohnson Thanks very much for the feedback. Is there a place in plotly.js code that a similar colour blending is applied?

look for color.combine

plotly.js/src/components/color/index.js

Lines 46 to 62 in 56f6983

color.combine = function(front, back) {

var fc = tinycolor(front).toRgb();

if(fc.a === 1) return tinycolor(front).toRgbString();

var bc = tinycolor(back || background).toRgb();

var bcflat = bc.a === 1 ? bc : {

r: 255 * (1 - bc.a) + bc.r * bc.a,

g: 255 * (1 - bc.a) + bc.g * bc.a,

b: 255 * (1 - bc.a) + bc.b * bc.a

};

var fcflat = {

r: bcflat.r * (1 - fc.a) + fc.r * fc.a,

g: bcflat.g * (1 - fc.a) + fc.g * fc.a,

b: bcflat.b * (1 - fc.a) + fc.b * fc.a

};

return tinycolor(fcflat).toRgbString();

};

New attempt in 4c7a092 - which I'm a fan of 👌

src/traces/treemap/attributes.js

test/jasmine/tests/treemap_test.js

src/traces/treemap/defaults.js

src/traces/treemap/draw_ancestors.js

src/traces/treemap/index.js

src/traces/treemap/partition.js

src/traces/treemap/plot.js

etpinard · 2019-09-12T18:55:50Z

test/jasmine/tests/treemap_test.js

+        .catch(failTest)
+        .then(done);
+    });
+});


TODO:

add tests for texttemplate using @antoinerg 's checkTextTemplate wrapper

add tests for tweens either using @antoinerg 's checkTransition or using the same hack as in sunburst_test.js

still TODO.

Looks like the texttemplate part is now done.

- do not display the edges of the empty root unless hovered - replace empty string with space to have better transitions

- add restyle tests for treemap opacities - fix style.js - add texttemplate jasmine test for treemap - add treemap tween interaction tests

etpinard · 2019-09-24T14:00:07Z

test/jasmine/tests/sunburst_test.js

@@ -507,7 +507,7 @@ describe('Test sunburst hover:', function() {
                curveNumber: 0,
                pointNumber: 0,
                label: 'Eve',
-                parent: ''
+                parent: '"root"'


This doesn't look right. parents[0] is '' in the input trace, so I think we should honour that in the event data.

OK if we return an empty string here?

plotly.js/src/traces/sunburst/helpers.js

Line 154 in 217c0f4

return str === '' ? '"root"' : str;

What are the side-effects?

Simply getting blank string (nothing) referring to the dummy root label in hover; which I think is fine.

Fixed in 8416883.

etpinard · 2019-09-24T14:00:42Z

src/traces/sunburst/plot.js

+    };
+
+    var getVal = function(d) {
+        if(d.hasOwnProperty('hierarchy')) return d.hierarchy.value;


Where does d get a hierarchy key?

Here:

plotly.js/src/traces/treemap/plot.js

Lines 90 to 93 in 217c0f4

entry._parent = (numAncestors ?

ancestorNodes[numAncestors - 1] :

hierarchy // parent of root is itself

).data;

Removed in 8416883.

etpinard · 2019-09-24T14:44:43Z

src/traces/sunburst/plot.js

-        Registry.call('animate', gd, frame, animOpts);
-    });
-}
+    if(trace.type === 'treemap' && helpers.isHeader(pt, trace)) {


I don't think we need this if block. Header nodes don't appear to call formatSliceLabel from:

plotly.js/src/traces/treemap/draw_descendants.js

Line 167 in 217c0f4

if(isHeader && noRoomForHeader) return;

... with the formatSliceLabel below:

plotly.js/src/traces/treemap/draw_descendants.js

Lines 167 to 169 in 217c0f4

if(isHeader && noRoomForHeader) return;

var tx = formatSliceLabel(pt, entry, trace, cd, fullLayout) || ' ';

Good call. Moved to treemap/draw_descendants.js in 8416883.

etpinard · 2019-09-24T15:05:50Z

The diff in sunburst/plot.js is pretty difficult to read, so I'll make comments from the file tree off 217c0f4

Why can we just use helpers.getPtId here:

plotly.js/src/traces/sunburst/plot.js

Lines 495 to 511 in 217c0f4

    
           var getLabel = function(d) { 
        
               var id = d.id; 
        
               if(d.data) { 
        
                   id = d.data.id; 
        
                   if(d.data.data) { 
        
                       id = d.data.data.id; 
        
                   } 
        
               } 
        
               if(!id) return ''; 
        
               for(var q = 0; q < cd.length; q++) { 
        
                   if(cd[q].label === id) { 
        
                       return id; 
        
                   } 
        
               } 
        
               return ''; 
        
           };

juggling between d.data.id and d.data.data.id looks pretty dangerous to my eyes.

We should try to find a better condition than d.hasOwnProperty('hierarchy') here:

plotly.js/src/traces/sunburst/plot.js

Line 514 in 217c0f4

if(d.hasOwnProperty('hierarchy')) return d.hierarchy.value;

We never set hierarchy on sliceData items, so this is very confusing.

It's probably better to check that the denominator is greater than 0 before computing the fraction here:

plotly.js/src/traces/sunburst/plot.js

Lines 518 to 521 in 217c0f4

    
           var calcPercent = function() { 
        
               var result = (trace.branchvalues ? cdi.v : cdi.value) / getVal(ref); 
        
               return isFinite(result) ? result : 1; 
        
           };

instead of checking if the fraction is finite.

I don't understand some of the differences between the logic in formatSliceLabel and in sunburst/fx.js. For example,

plotly.js/src/traces/sunburst/plot.js

Line 620 in 217c0f4

obj.entry = helpers.getLabelString(getLabel(ref));

vs

plotly.js/src/traces/sunburst/fx.js

Line 126 in 217c0f4

hoverPt.entry = pt.entry = helpers.getLabelString(ref1.data.data.label);

@archmoj can you explain why the "text" part appears more complicated than the "hover" part?

- move treemap header checks from sunburst plot to treemap/draw_ancestors - cleanup dirty _parents and hierarchy links for pt - display blank label as blank

archmoj · 2019-09-24T19:32:33Z

@etpinard thanks very much for the review.
Reworked in
8416883.
Now the hover and plot code look pretty much the same.

etpinard · 2019-09-24T20:02:55Z

Reworked in
8416883.
Now the hover and plot code look pretty much the same.

Thanks @archmoj - this looks much (much) better 😄

I have a few questions about this block:

plotly.js/src/traces/sunburst/fx.js

Lines 53 to 64 in 8416883

    
           var parent; 
        
           var parentLabel; 
        
           var parentValue; 
        
           if(isEntry) { 
        
               parent = pt.data.parent; 
        
               parentLabel = parent ? parent.data.label : helpers.getPtLabel(hierarchy); 
        
               parentValue = parent ? helpers.getVal(parent.data) : helpers.getVal(hierarchy); 
        
           } else { 
        
               parent = pt.parent; 
        
               parentLabel = helpers.getPtLabel(parent); 
        
               parentValue = helpers.getVal(parent); 
        
           }

(and it's equivalent in formatSliceLabel)

what's the difference between pt.data.parent and pt.parent?
why does helpers.getVal(parent); not return the same result as helpers.getVal(parent.data)?

archmoj · 2019-09-24T22:13:02Z

what's the difference between pt.data.parent and pt.parent?

@etpinard
E.g. with level being set when hovering on or plotting the entry node, we need to know the parent of the entry (to compute percentages from). But that is not available on the entry itself where pt.parent is equal to null. But the parent of the entry is accessible via pt.data.parent.

etpinard · 2019-09-24T22:20:33Z

But that is not available on the entry itself where pt.parent is equal to null. But the parent of the entry is accessible via pt.data.parent.

Ok, this sounds right. Thanks!

I think you could also use pt.data.data.parent. If so, this would be even clearer as pt.data.data always correspond to gd.calcdata[i][j] item. Elsewhere in the sunburst code, you should be able to find plenty of var cdi = pt.data.data; statements.

- add functions to get parents info above entry node - use computed d.value everywhere - add more jasmine tests to test percentgaes and current path at root and desired level

…nstead of pt.data.parent

archmoj · 2019-09-25T22:29:30Z

src/traces/treemap/plot.js

+        return x + ',' + y;
+    }
+
+    function noNaN(path) {


@etpinard it looks we don't need this extra check anymore.

etpinard · 2019-09-25T22:51:16Z

Time to merge this thing. Amazing work @archmoj - sorry for being a little picky there that last two days.

💃 💃 💃 💃 💃 💃

To sum up, this PR:

adds a new treemap trace type (Treemap trace #1038) with:
- click transition support
- on-hover effect
- 6 different packing algorithms
- pathbar (that can be optionally removed)
- several padding attributes
- support for per-sector values, branchvalues, colorscale, hover/text info/template, colorways
- a level-to-level sector opacity gradient
- ... all this while generalising and reusing much of the sunburst logic
adds helpers Plots.plotBasePlot and Plots.cleanBasePlot
adds attribute count to sunburst traces Display counted number of leaves/branches on sunburst/treemap using textinfo, texttemplate, hoverinfo and hovertemplate #4174
adds colorscale support to sunburst traces
adds a bunch more hoverinfo and textinfo flags to sunburst traces

archmoj added 7 commits September 10, 2019 16:37

bar textpad const

ec6a9de

refactor base_plot files

52ded31

changes to sunburst attributes - add percentages - add colorscale and…

320f81a

… option to count branches - revise descriptions and editTypes

prep to add treemap to the lib

06d93ae

treemap mocks

6cc8590

treemap jasmine tests

6a595b6

treemap code

671e864

archmoj added status: in progress feature something new labels Sep 11, 2019

etpinard added this to the v1.50.0 milestone Sep 11, 2019

archmoj removed the status: in progress label Sep 12, 2019

correct current path when there is no parent

648167d

now no need to include empty string to hide last label on the barpath

archmoj commented Sep 12, 2019

View reviewed changes

src/traces/treemap/plot.js Outdated Show resolved Hide resolved