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_nodes! gives unexpected results when deleting multiple nodes at the root #70

Closed bspanoghe closed 5 months ago

bspanoghe commented 5 months ago

The function delete_nodes! seems to have an issue when deleting the root node as well as one or more nodes that would become its successor root node:

using MultiScaleTreeGraph
mtg = Node(MutableNodeMTG("/", "Scene", 0, 0), Dict())
insert_child!(mtg, MutableNodeMTG("/", "Plant", 0, 1))
insert_child!(mtg[1], MutableNodeMTG("/", "Internode", 0, 2))
insert_child!(mtg[1][1], MutableNodeMTG("<", "Leaf", 0, 2))
insert_child!(mtg[1][1], MutableNodeMTG("<", "Leaf", 0, 2))

new_mtg = delete_nodes!(mtg, filter_fun = node -> node_mtg(node).scale != 2)
length(new_mtg) == 3 # false

The issue seems to arise from the fact that the function only checks the root node once before starting to work recursively from leaves to root:

function delete_nodes!

...

if filtered
    node = delete_node!(node)
    # Don't go further if all == false
    !all && return
end

delete_nodes!_(node, scale, symbol, link, all, filter_fun, child_link_fun)
...

Changing the function to continue working from root to leaves until the node does not need to be deleted:

function delete_nodes!(
    node;
    scale=nothing,
    symbol=nothing,
    link=nothing,
    all::Bool=true, # like continue in the R package, but actually the opposite
    filter_fun=nothing,
    child_link_fun=new_child_link
    )

    # Check the filters once, and then compute the descendants recursively using `descendants_`
    check_filters(node, scale=scale, symbol=symbol, link=link)
    filtered = is_filtered(node, scale, symbol, link, filter_fun)

    while filtered
        node = delete_node!(node)
        filtered = is_filtered(node, scale, symbol, link, filter_fun)

        # Don't go further if all == false
        !all && return
    end

    delete_nodes!_(node, scale, symbol, link, all, filter_fun, child_link_fun)

    return node
end

fixes the issue for this example:

using MultiScaleTreeGraph
mtg = Node(MutableNodeMTG("/", "Scene", 0, 0), Dict())
insert_child!(mtg, MutableNodeMTG("/", "Plant", 0, 1))
insert_child!(mtg[1], MutableNodeMTG("/", "Internode", 0, 2))
insert_child!(mtg[1][1], MutableNodeMTG("<", "Leaf", 0, 2))
insert_child!(mtg[1][1], MutableNodeMTG("<", "Leaf", 0, 2))

new_mtg = delete_nodes!(mtg, filter_fun = node -> node_mtg(node).scale != 2)
length(new_mtg) == 3 # true