balanced tree #1610
balanced tree #1610
Conversation
src/transforms/tree.js
Outdated
| treeData[treeIndex] = { | ||
| data: node.data, | ||
| name: nodeName(node), | ||
| path: nodePath(node), | ||
| internal: nodeInternal(node), | ||
| depth: node.depth, // nodeDepth(node) | ||
| height: node.height // nodeHeight(node) | ||
| }; |
There was a problem hiding this comment.
This breaks backwards compatibility, and I think we want to preserve the original data (which is why I originally designed it this way).
Can we not use the existing node:internal method because that only works for channels that are visible to the tree transform? I would think that we could pass filter: "node:internal" as an option to the treeNode transform? (And we could have a node:external or node:leaf for the opposite meaning.) But maybe we have to pull out the filter option and do something special; I haven’t tested yet.
There was a problem hiding this comment.
I couldn't find how to filter inside the transform; the naive approach removes those nodes too early, even before the determination of the tree—basically crashing with a "missing root". There's certainly a better way.
There was a problem hiding this comment.
I guess the confusion is whether the filter applies before or after the tree is constructed? I guess the filter operation should run before the tree layout… 🤔 Maybe we could have a treeFilter option that runs after the tree layout, and supports the same node:internal method?
src/marks/tree.js
Outdated
| ...options | ||
| } = {} | ||
| ) { | ||
| if (dx === undefined) dx = maybeTreeAnchor(options.treeAnchor).dx; | ||
| if (textAnchor !== undefined) throw new Error("textAnchor is not a configurable tree option"); | ||
| textLayout = keyword( | ||
| textLayout === undefined ? (options.treeLayout === undefined ? "mirrored" : "normal") : textLayout, |
There was a problem hiding this comment.
I think we should check whether treeLayout is Tree or Cluster here. I’ll make this change and then merge. (I was just thinking this would be nice for the D3 documentation!)
mbostock
force-pushed
the
fil/balanced-tree
branch
from
56729e3 to
8cb8b43
Compare
Co-authored-by: Mike Bostock <[email protected]>
Co-authored-by: Mike Bostock <[email protected]>
todo:
closes #1589
what do we want to expose as data? I decided on an object with the "node:*" values, but that's tbd.(unchanged)we'll probably need to make this configurable (opt-in or out, or smarter anyway, maybe it could be set by the layout), otherwise flare-indent is going sideways.