VEZY / MultiScaleTreeGraph.jl

Read, analyse, compute, write and convert MTG files
https://vezy.github.io/MultiScaleTreeGraph.jl/stable
MIT License
10 stars 1 forks source link

insert_generation! does not update parent #72

Closed bspanoghe closed 5 months ago

bspanoghe commented 5 months ago

The function insert_generation! adds a new node to a graph and makes the previous children of its parent node its own children, but it does not update the parent of these child nodes:

using MultiScaleTreeGraph
mtg = Node(MutableNodeMTG("/", "foo", 0, 0), Dict())
insert_child!(mtg, MutableNodeMTG("/", "bar", 0, 0))

insert_generation!(mtg, MutableNodeMTG("/", "xyzzy", 0, 0))
parent(mtg[1][1]) == mtg[1] # false

Changing the function to include the parent update of the child nodes:

function insert_generation!(node::Node{N,A}, template, attr_fun=node -> A(), maxid=[max_id(node)]) where {N<:AbstractNodeMTG,A}

    maxid[1] += 1

    new_node = Node(
        maxid[1],
        node,
        children(node),
        new_node_MTG(node, template),
        copy(attr_fun(node)),
        Dict{String,Vector{Node{N,A}}}()
    )

    # Add the new node as the only child of the node:
    rechildren!(node, Node{N,A}[new_node])

    # Add the new node as parent of the children
    for chnode in children(new_node)
        setfield!(chnode, :parent, new_node)
    end

    return node
end

fixes this:

using MultiScaleTreeGraph
mtg = Node(MutableNodeMTG("/", "foo", 0, 0), Dict())
insert_child!(mtg, MutableNodeMTG("/", "bar", 0, 0))

insert_generation!(mtg, MutableNodeMTG("/", "xyzzy", 0, 0))
parent(mtg[1][1]) == mtg[1] # true

Strangely, using reparent! in the for loop instead of setfield! does not work, despite seeming to be the intended function here. I've looked into this but can't find the issue. Since I can't properly fix the issue I will not open a PR for this.

VEZY commented 5 months ago

Thank you for the issue and the fix @bspanoghe ! It works on my computer when I use reparent!. I'll make the PR (with you as co-author) and we'll see what it says. But in any case, that would be nice if you could tell me if you can reproduce the error, and in that case, which version of Julia you're using.

bspanoghe commented 5 months ago

After the merge it works perfectly fine for me, I must have done something weird. Thank you!