link.js: "nodeById" uses array position instead of "index" property
runeksvendsen opened this issue · 6 comments
Running initialize with a nodes array of
[{"name":"Two","index":2}, {"name":"One","index":1}, {"name":"Zero","index":0}]
results in a nodeById map which maps 0 to the string "Two". This is because the index property of a node is ignored, and instead its position in the nodes array is used as the index.
As a result, the order of the objects inside the nodes array is what determines the source and target of links, and not -- as one would expect -- the index property of the node in question.
the node's index is set (in this case, overwritten) in https://github.com/d3/d3-force/blob/master/src/simulation.js#L65
@Fil If I have the following data:
nodes = [{"name":"Two","index":2}, {"name":"One","index":1}, {"name":"Zero","index":0}]
links = [{"source": 2, "target": 1}]
where in the code is the source and target properties of links updated to reflect the new node indices?
In the above case the new node index for {"name":"Two","index":2} would be 0, so the link inside links should have its source property changed to reflect this. Otherwise the link will start at {"name":"Zero","index":0} instead of {"name":"Two","index":2}.
The change in links happens at https://github.com/d3/d3-force/blob/master/src/link.js#L60 ; this is after the nodes have been set and "indexed".
When source and target are given as indices (ie not objects), then the nodes are taken from the nodes list, and the "index" set in https://github.com/d3/d3-force/blob/master/src/simulation.js#L65 is used.
I believe I've reproduced the bug here: https://observablehq.com/@runeksvendsen/force-directed-graph
This draws a link between "One" and "Zero" when using a nodes array of
[ {"name":"Three", "index":3},
{"name":"Two", "index":2},
{"name":"One", "index":1},
{"name":"Zero", "index":0} ]
and a links array of [{"source": 2, "target": 3}].
Exactly, that's what happens: all the nodes' indexes are applied on initialization based on their position in the nodes array (and overwriting any "index" property they might already have); then links are computed based on that index, ie the order of the nodes.
In other words any "index" property on nodes is not taken into account, and even discarded/replaced by the array index.
It is unfortunate in your case (but you can fix by either sorting nodes to have a correct index, or assign source and target as nodes yourself, rather than as indices). The behavior is properly documented:
Each node must be an object. The following properties are assigned by the simulation:
- index - the node’s zero-based index into nodes
Thank you for your time! I did not realize this.
The reason I was confused is because the Vega "force directed graph"-example (https://vega.github.io/vega/examples/force-directed-layout/) uses node data which contains an index property (https://vega.github.io/editor/data/miserables.json).
I will submit a PR to the Vega repo suggesting to correct this.