allow to provide custom getValue() and getDelta() functions

[wonder-mice]

I have a huge data set that I embed in html as json object that follows d3-flamegraph specification. I also add several custom fields to it, such as syscall count, vmfault count, etc. I would like to have an easy way to choose what subset of data to display in a flame graph, preferably without regenerating the whole data structure (e.g. now I'll need to traverse the whole tree to place what I want into the value field).

Describe the solution you'd like
Some way to provide custom implementations of getValue() and getDelta() functions. This also will allow to make data more compact. E.g., for comparison view, I can have A and B values in the data (since I want to show them in a tooltip anyway), so I don't need to store delta and can compute it easily in getDelta() function.

[wonder-mice]

I assume I can to something like:

flamegraph.getValue = function(d) { d.A; }
flamegraph.getDelta = function(d) { d.B - d.A; }

but that seems not to work. Looks like some parts of the code access .v|.value fields directly, because when I do:

flamegraph.getValue = function(d) { 20; }
flamegraph.getDelta = function(d) { 20; }

Nodes width is still computed based on .v/.value. Maybe I'm doing something wrong though...

[wonder-mice]

OMG, I'm a JavaScript hacker :)
This seems to make it work:
wonder-mice@60c3415

[wonder-mice]

#115

[wonder-mice]

I would like to rename getValue and friends to reflect on what they operate on. Some of them work with data nodes (like getValue), while other work with d3 nodes (like getDelta). Is there any established terminology to distinguish them?

[spiermar]

Not that I'm aware of, but open to suggestions.

[wonder-mice]

Looks like found a "bug" - getChildren is used both with d3 nodes and with data nodes.

I suggest to name d3 nodes "nodes", since they are named that way all over d3 source code. Data nodes I would suggest to call 'items". Thus getItemValue, getNodeDelta, etc.

[wonder-mice]

Should default labelHandler show "self" value of a node or exact value provided in data object? It looks like currently it uses node.value which is always node's self value, since it's populated in update() function when calling root.sum(). It doesn't look like it's expected from default labelHandler - I would expect it to show what I provided in data.

[wonder-mice]

I guess I misread the node.sum() function in d3. Looks like node.value is total node value, including its children. However, passed function must return node's self value, which we compute by subtracting child nodes, which they add back in node.sum...

[wonder-mice]

Ah, I see, we need this trickery for if (d.fade || d.hide) to adjust node.value for zoom mode.

[wonder-mice]

When clicking on node to zoom in, this changes value in tooltip for all nodes on the path to clicked node:
http://martinspier.io/d3-flame-graph/

I think tooltip label shouldn't use node.value, node.value it's just defines node'a width on the chart and can be unrelated to item's data (e.g. nodes can be played out in log scale).

[spiermar]

Looks like found a "bug" - getChildren is used both with d3 nodes and with data nodes.

I suggest to name d3 nodes "nodes", since they are named that way all over d3 source code. Data nodes I would suggest to call 'items". Thus getItemValue, getNodeDelta, etc.

Saw that now.

[spiermar]

When clicking on node to zoom in, this changes value in tooltip for all nodes on the path to clicked node:
http://martinspier.io/d3-flame-graph/

I think tooltip label shouldn't use node.value, node.value it's just defines node'a width on the chart and can be unrelated to item's data (e.g. nodes can be played out in log scale).

If you have custom function like the proposed changes. Currently, value would be the same. Can you update the PR with the different functions?

[wonder-mice]

Currently, value would be the same

No, value is computed by node.sum() and will change by zooming.

[spiermar]

Currently, value would be the same

No, value is computed by node.sum() and will change by zooming.

When zooming, yes, but that was the intended effect. Having said that, it always bugged me, because I lose the notion of relative size when zooming. Might be a good time to change it. Let me check with Brendan what his thoughts are.

[wonder-mice]

I think node.value must have no relation to item's value, other than it should establish desired proportions of nodes in chart. Then it'll be easier to implement log scales and other effects. It'll also allow for some optimizations when computing these values.

[spiermar]

I think node.value must have no relation to item's value, other than it should establish desired proportions of nodes in chart. Then it'll be easier to implement log scales and other effects. It'll also allow for some optimizations when computing these values.

I think its important to differentiate between them, either by different functions or something else, but it's important to keep both for different use cases.

[wonder-mice]

What is the use case for node.value other than node's width?

[spiermar]

What is the use case for node.value other than node's width?

Exactly when I need to know the node's width.

[spiermar]

@wonder-mice prefer to treat those changes as a separate PR?

[wonder-mice]

Don't accept any PR from me for now. I need to rework the patch set, found other issues.

[spiermar]

Don't accept any PR from me for now. I need to rework the patch set, found other issues.

Ok, I'll wait for the new PR.

FYI, I'll be traveling next week, so there might be some delay on my response after Sat evening.

[wonder-mice]

Check this out:
#120

It has some differences visible from outside though:

• No transitionDuration / transitionEase functions. Must supply custom CSS class instead to control transitions. Don't have setter for that yet though.
• Tooltip doesn't have arrows because I didn't have tome to replicate this part of behavior and I think it's not super important