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

delete_node! errors on root nodes #68

Closed bspanoghe closed 5 months ago

bspanoghe commented 5 months ago

Hi

The delete_node! function returns MethodError: no method matching link(::Nothing) when using it to remove the root node.

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

new_mtg = delete_node!(mtg) # errors

The problem seems to arise from the fact that the child node of the root has its parent set to nothing before applying the link! function. Changing the order as follows:

function delete_node_fixed!(node::Node{N,A}; child_link_fun=new_child_link) where {N<:AbstractNodeMTG,A}
    if isroot(node)
        if length(children(node)) == 1
            # If it has only one child, make it the new root:
            chnode = children(node)[1]
            # Add to the new root the mandatory root attributes:
            root_attrs = Dict(
                :symbols => node[:symbols],
                :scales => node[:scales],
                :description => node[:description]
            )

            append!(chnode, root_attrs)

            link!(chnode, child_link_fun(chnode))
            reparent!(chnode, nothing)

            node_return = chnode
        else
            error("Can't delete the root node if it has several children")
        end
    else
        parent_node = parent(node)

        if !isleaf(node)
            # We re-parent the children to the parent of the node.
            for chnode in children(node)
                # Updating the link of the children:
                link!(chnode, child_link_fun(chnode))
                addchild!(parent_node, chnode; force=true)
            end
        end

        # Delete the node as child of his parent:
        deleteat!(children(parent_node), findfirst(x -> node_id(x) == node_id(node), children(parent_node)))
        node_return = parent_node
    end

    node = nothing

    return node_return
end

seems to fix the issue

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

new_mtg = delete_node_fixed!(mtg) # no error
new_mtg

Version info: Julia Version 1.10.2, MultiScaleTreeGraph v0.13.1

VEZY commented 5 months ago

Hi @bspanoghe, thanks for the issue. Do you want to make a pull request with your proposition, or should I make the fix?

VEZY commented 5 months ago

Closed in #69