BREAKING: Merge pull request #81 from VEZY/symbols-for-links-and-symbol-names

Use symbols for mtg link and symbol. Breaking as now `symbol(node)` and `link(node)` return a symbol instead of a `String`

Author: VEZY

docs/src/tutorials/2.descendants_ancestors_filters.md

@@ -133,10 +133,10 @@ descendants(mtg, :Length, scale = 3)
 
 ### Filter by symbol
 
-If we need only the leaves, we would filter by their symbol (*i.e.* "Leaf"):
+If we need only the leaves, we would filter by their symbol (*i.e.* :Leaf):
 
 ```@example usepkg
-descendants(mtg, :Length, symbol = "Leaf")
+descendants(mtg, :Length, symbol = :Leaf)
 ```
 
 ### Filter by anything
@@ -156,14 +156,14 @@ descendants(mtg, :Length, filter_fun = x -> x[:Width] === nothing ? false : x[:W
 Because `filter_fun` takes a node as input, we can even filter on the node's parent. Let's say for example we want the values for the :Length, but only for the nodes that are children of a an Internode that follows another node:
 
 ```@example usepkg
-descendants(mtg, :Length, filter_fun = node -> !isroot(node) && symbol(parent(node)) == "Internode" && link(parent(node)) == "<")
+descendants(mtg, :Length, filter_fun = node -> !isroot(node) && symbol(parent(node)) == :Internode && link(parent(node)) == :<)
 ```
 
 In this example it returns only one value, because there is only one node that corresponds to this criteria: The Leaf with id 7.
 
 We could apply the same kind of filtering on the node's children, or any combination of topological information and attributes.
 
-Note that we first test if the node is not the root node, because the root node does not have a parent. We then test if the parent's symbol is "Internode" and if the link is "<".
+Note that we first test if the node is not the root node, because the root node does not have a parent. We then test if the parent's symbol is :Internode and if the link is :<.
 
 ### Filter helpers
 

docs/src/tutorials/3.transform_mtg.md

@@ -213,7 +213,7 @@ transform!(mtg, node -> isleaf(node) ? println(node_id(node)," is a leaf") : not
 We can also use this form to mutate the MTG of a node (which is not possible with Form 2). Here's an example where we change the "Internode" symbol into "I":
 
 ```@example usepkg
-transform!(mtg, node -> symbol!(node, "I"), symbol = "Internode")
+transform!(mtg, node -> symbol!(node, :I), symbol = :Internode)
 
 mtg
 ```
@@ -255,7 +255,7 @@ DataFrame(mtg_select)
 [`transform!`](@ref) and [`select!`](@ref) use [`traverse!`](@ref) under the hood to apply a function call to each node of an MTG. [`traverse!`](@ref) is just a little bit less easy to use as it only accepts Form 4. We can obtain the exact same results as the last example of [`transform!`](@ref) using the same call with [`traverse!`](@ref). Let's change the `Leaf` symbol into `L`:
 
 ```@example usepkg
-traverse!(mtg, node -> symbol!(node, "L"), symbol = "Leaf")
+traverse!(mtg, node -> symbol!(node, :L), symbol = :Leaf)
 
 mtg
 ```
@@ -275,7 +275,7 @@ end
 For users coming from R, we also provide the `@mutate_mtg!` macro that is similar to [`transform!`](@ref) but uses a more `tidyverse`-alike syntax. All values coming from the MTG node must be preceded by a `node.`, as with the `.data$` in the `tidyverse`. The names of the attributes are shortened to just `node.attr_name` instead of `node_attributes(node).attr_name` though. Here's an example usage:
 
 ```@example usepkg
-@mutate_mtg!(mtg, volume = π * 2 * node.Length, symbol = "I")
+@mutate_mtg!(mtg, volume = π * 2 * node.Length, symbol = :I)
 ```
 
 We see that we first name the new attribute and assign the result of the computation. Constants are provided as is, and values coming from the nodes are prefixes by `node.`.

docs/src/tutorials/6.add_remove_nodes.md

@@ -100,7 +100,7 @@ Those functions use a `NodeMTG` (or `MutableNodeMTG`), and automatically:
 ```@example usepkg
 mtg_2 = deepcopy(mtg)
 
-insert_parent!(mtg_2, NodeMTG("/", "Scene", 0, 0))
+insert_parent!(mtg_2, NodeMTG(:/, :Scene, 0, 0))
 
 mtg_2 = get_root(mtg_2)
 ```
@@ -114,7 +114,7 @@ insert_parent!(
     mtg_2,
     node -> (
         link = link(node),
-        symbol = "Scene",
+        symbol = :Scene,
         index = index(node),
         scale = scale(node) - 1
     )
@@ -147,7 +147,7 @@ mtg_2 = deepcopy(mtg)
 
 insert_child!(
     mtg_2,
-    NodeMTG("/", "Axis", 0, 2),
+    NodeMTG(:/, :Axis, 0, 2),
     node -> Dict{Symbol, Any}(:length => 2, :area =>  0.1)
 )
 
@@ -161,7 +161,7 @@ mtg_2 = deepcopy(mtg)
 
 insert_child!(
     mtg_2,
-    NodeMTG("/", "Axis", 0, 2),
+    NodeMTG(:/, :Axis, 0, 2),
     node -> Dict{Symbol, Any}(:total_length => sum(descendants(node, :length, ignore_nothing = true)))
 )
 
@@ -265,23 +265,23 @@ For example if we need to insert new Flower nodes as parents of each Leaf, we wo
 
 ```@example usepkg
 mtg_4 = deepcopy(mtg)
-template = MutableNodeMTG("+", "Flower", 0, 2)
-insert_parents!(mtg_4, template, symbol = "Leaf")
+template = MutableNodeMTG(:+, :Flower, 0, 2)
+insert_parents!(mtg_4, template, symbol = :Leaf)
 ```
 
 Similarly, we can add a new child to leaves using [`insert_children!`](@ref):
 
 ```@example usepkg
-template = MutableNodeMTG("/", "Leaflet", 0, 3)
-insert_children!(mtg_4, template, symbol = "Leaf")
+template = MutableNodeMTG(:/, :Leaflet, 0, 3)
+insert_children!(mtg_4, template, symbol = :Leaf)
 ```
 
 Usually, the flower is positioned as a sibling of the leaf though. To do so, we can use [`insert_siblings!`](@ref):
 
 ```@example usepkg
 mtg_5 = deepcopy(mtg)
-template = MutableNodeMTG("+", "Flower", 0, 2)
-insert_siblings!(mtg_5, template, symbol = "Leaf")
+template = MutableNodeMTG(:+, :Flower, 0, 2)
+insert_siblings!(mtg_5, template, symbol = :Leaf)
 ```
 
 ### Compute the template on the fly
@@ -291,8 +291,8 @@ The template for the `NodeMTG` can also be computed on the fly for more complex
 ```@example usepkg
 insert_children!(
     mtg_5,
-    node -> if node_id(node) == 3 MutableNodeMTG("/", "Spear", 0, 3) else MutableNodeMTG("/", "Leaflet", 0, 3) end,
-    symbol = "Leaf"
+    node -> if node_id(node) == 3 MutableNodeMTG(:/, :Spear, 0, 3) else MutableNodeMTG(:/, :Leaflet, 0, 3) end,
+    symbol = :Leaf
 )
 ```
 
@@ -303,9 +303,9 @@ The same is true for the attributes. We can provide them as is:
 ```@example usepkg
 insert_siblings!(
     mtg_5,
-    MutableNodeMTG("+", "Leaf", 0, 2),
+    MutableNodeMTG(:+, :Leaf, 0, 2),
     Dict{Symbol, Any}(:area => 0.1),
-    symbol = "Leaf"
+    symbol = :Leaf
 )
 ```
 
@@ -314,9 +314,9 @@ Or compute them based on the node on which we insert the new nodes. For example
 ```@example usepkg
 insert_siblings!(
     mtg_5,
-    MutableNodeMTG("+", "Leaf", 0, 2),
+    MutableNodeMTG(:+, :Leaf, 0, 2),
     node -> node[:area] === nothing  ? nothing : Dict{Symbol, Any}(:area => node[:area] * 2),
-    symbol = "Leaf"
+    symbol = :Leaf
 )
 ```
 

docs/src/tutorials/7.performance_considerations.md

@@ -22,6 +22,13 @@ The MTG encoding is the type used to store the MTG information about the node, *
 
 By default, MultiScaleTreeGraph.jl uses a mutable encoding ([`MutableNodeMTG`](@ref)), which allows for modifying this information. However, if the user does not need to modify these, it is recommended to use an immutable encoding instead ([`NodeMTG`](@ref)). This will improve performance significantly.
 
+The internal representation of MTG `symbol` and `link` values is based on `Symbol` for faster comparisons and lower repeated allocations. For backward compatibility, string inputs are still accepted everywhere (constructors and filters), but using symbols in performance-critical code is recommended:
+
+```julia
+NodeMTG(:/, :Internode, 1, 2)
+traverse(mtg, x -> x, symbol=:Internode, link=:<)
+```
+
 ### Traversal: node caching
 
 MultiScaleTreeGraph.jl traverses all nodes by default when performing tree traversal. The traversal is done in a recursive manner so it is performant, but not always as fast as it could be. For example, we could have a very large tree with only two leaves at the top. In this case, we would traverse all nodes in the tree, even though we only need to traverse two nodes.
@@ -34,10 +41,10 @@ To improve performance, it is possible to cache any type of `traversal`, includi
 To cache a traversal, you can use [`cache_nodes!`](@ref). For example, if you want to cache all the **leaf** nodes in the MTG, you can do:
 
 ```julia
-cache_nodes!(mtg, symbol = "Leaf")
+cache_nodes!(mtg, symbol = :Leaf)
 ```
 
-This will cache all the nodes with the symbol `"Leaf"` in the MTG. Then, the tree traversal functions will use the cached traversal to iterate over the nodes.
+This will cache all the nodes with the symbol `:Leaf` in the MTG. Then, the tree traversal functions will use the cached traversal to iterate over the nodes.
 
 !!! tip
     Tree traversal is *very* fast, so caching nodes is not always necessary. Caching should be used when the traversal is needed **multiple times**, and the traversal is sparse, *i.e.* a lot of nodes are filtered-out.

src/compute_MTG/ancestors.jl

@@ -15,7 +15,7 @@ Make it a `Symbol` for faster computation time.
 ## Keyword Arguments
 
 - `scale = nothing`: The scale to filter-in (i.e. to keep). Usually a Tuple-alike of integers.
-- `symbol = nothing`: The symbol to filter-in. Usually a Tuple-alike of Strings.
+- `symbol = nothing`: The symbol to filter-in. Usually a Tuple-alike of Symbols.
 - `link = nothing`: The link with the previous node to filter-in. Usually a Tuple-alike of Char.
 - `all::Bool = true`: Return all filtered-in nodes (`true`), or stop at the first node that
 is filtered out (`false`).
@@ -56,8 +56,8 @@ ancestors(leaf_node, :XX, scale = 1, type = Float64)
 ancestors(leaf_node, :Length, scale = 3, type = Float64)
 
 # Filter by symbol:
-ancestors(leaf_node, :Length, symbol = "Internode")
-ancestors(leaf_node, :Length, symbol = ("Axis","Internode"))
+ancestors(leaf_node, :Length, symbol = :Internode)
+ancestors(leaf_node, :Length, symbol = (:Axis,:Internode))
 ```
 """
 function ancestors(
@@ -71,6 +71,8 @@ function ancestors(
     recursivity_level=-1,
     ignore_nothing=false,
     type::Union{Union,DataType}=Any)
+    symbol = normalize_symbol_filter(symbol)
+    link = normalize_link_filter(link)
 
     # Check the filters once, and then compute the ancestors recursively using `ancestors_`
     check_filters(node, scale=scale, symbol=symbol, link=link)
@@ -146,6 +148,8 @@ function ancestors(
     filter_fun=nothing,
     recursivity_level=-1
 )
+    symbol = normalize_symbol_filter(symbol)
+    link = normalize_link_filter(link)
 
     # Check the filters once, and then compute the ancestors recursively using `ancestors_`
     check_filters(node, scale=scale, symbol=symbol, link=link)
@@ -219,6 +223,8 @@ function ancestors!(
     ignore_nothing=false,
     type::Union{Union,DataType}=Any,
 )
+    symbol = normalize_symbol_filter(symbol)
+    link = normalize_link_filter(link)
     check_filters(node, scale=scale, symbol=symbol, link=link)
     filter_fun_ = filter_fun_nothing(filter_fun, ignore_nothing, key)
     use_no_filter = no_node_filters(scale, symbol, link, filter_fun_)
@@ -251,6 +257,8 @@ function ancestors!(
     filter_fun=nothing,
     recursivity_level=-1,
 )
+    symbol = normalize_symbol_filter(symbol)
+    link = normalize_link_filter(link)
     check_filters(node, scale=scale, symbol=symbol, link=link)
     use_no_filter = no_node_filters(scale, symbol, link, filter_fun)
 

src/compute_MTG/caching.jl

@@ -44,16 +44,18 @@ file = joinpath(dirname(dirname(pathof(MultiScaleTreeGraph))), "test", "files",
 mtg = read_mtg(file, Dict)
 
 # Cache all leaf nodes:
-cache_nodes!(mtg, symbol="Leaf")
+cache_nodes!(mtg, symbol=:Leaf)
 
 # Cached nodes are stored in the traversal_cache field of the mtg (here, the two leaves):
 @test MultiScaleTreeGraph.node_traversal_cache(mtg)["_cache_c0bffb8cc8a9b075e40d26be9c2cac6349f2a790"] == [get_node(mtg, 5), get_node(mtg, 7)]
 
 # Then you can use the cached nodes in a traversal:
-traverse(mtg, x -> symbol(x), symbol="Leaf") == ["Leaf", "Leaf"]
+traverse(mtg, x -> symbol(x), symbol=:Leaf) == [:Leaf, :Leaf]
 ```
 """
 function cache_nodes!(node; scale=nothing, symbol=nothing, link=nothing, filter_fun=nothing, all=true, overwrite=false)
+    symbol = normalize_symbol_filter(symbol)
+    link = normalize_link_filter(link)
     # The cache is already present:
     if length(node_traversal_cache(node)) != 0 && haskey(node_traversal_cache(node), cache_name(scale, symbol, link, all, filter_fun))
         if !overwrite
@@ -71,4 +73,4 @@ function cache_nodes!(node; scale=nothing, symbol=nothing, link=nothing, filter_
     )
 
     return nothing
-end
+end

src/compute_MTG/check_filters.jl

@@ -8,12 +8,33 @@ Check if the filters are consistant with the mtg onto which they are applied
 ```julia
 check_filters(mtg, scale = 1)
 check_filters(mtg, scale = (1,2))
-check_filters(mtg, scale = (1,2), symbol = "Leaf", link = "<")
+check_filters(mtg, scale = (1,2), symbol = :Leaf, link = :<)
 ```
 """
 @inline no_node_filters(scale, symbol, link, filter_fun=nothing) =
     isnothing(scale) && isnothing(symbol) && isnothing(link) && isnothing(filter_fun)
 
+@inline normalize_symbol_filter(filter::Nothing) = nothing
+@inline normalize_symbol_filter(filter::Symbol) = filter
+@inline normalize_symbol_filter(filter::AbstractString) = Symbol(filter)
+@inline normalize_symbol_filter(filter::Char) = Symbol(filter)
+@inline function normalize_symbol_filter(filter::T) where {T<:Union{Tuple,AbstractArray}}
+    map(normalize_symbol_filter, filter)
+end
+
+@inline normalize_link_filter(filter::Nothing) = nothing
+@inline normalize_link_filter(filter::Symbol) = filter
+@inline normalize_link_filter(filter::AbstractString) = Symbol(filter)
+@inline normalize_link_filter(filter::Char) = Symbol(filter)
+@inline function normalize_link_filter(filter::T) where {T<:Union{Tuple,AbstractArray}}
+    map(normalize_link_filter, filter)
+end
+
+@inline normalize_symbol_allowed(filters::Nothing) = nothing
+@inline function normalize_symbol_allowed(filters::T) where {T<:Union{Tuple,AbstractArray}}
+    map(normalize_symbol_filter, filters)
+end
+
 function check_filters(node::Node{N,A}; scale=nothing, symbol=nothing, link=nothing) where {N<:AbstractNodeMTG,A}
     no_node_filters(scale, symbol, link) && return nothing
 
@@ -24,11 +45,11 @@ function check_filters(node::Node{N,A}; scale=nothing, symbol=nothing, link=noth
     end
 
     if root_node[:symbols] !== nothing
-        check_filter(N, :symbol, symbol, unique(root_node[:symbols]))
+        check_filter(N, :symbol, normalize_symbol_filter(symbol), unique(normalize_symbol_allowed(root_node[:symbols])))
     end
 
     if root_node[:link] !== nothing
-        check_filter(N, :link, link, ("/", "<", "+"))
+        check_filter(N, :link, normalize_link_filter(link), (:/, :<, :+))
     end
 
     return nothing
@@ -37,7 +58,11 @@ end
 function check_filter(nodetype, type::Symbol, filter, filters)
     if !isnothing(filter)
         filter_type = fieldtype(nodetype, type)
-        !(typeof(filter) <: filter_type) &&
+        filter_ok = typeof(filter) <: filter_type
+        if type == :symbol || type == :link
+            filter_ok = filter_ok || typeof(filter) <: Union{Symbol,AbstractString,Char}
+        end
+        !filter_ok &&
             @warn "The $type argument should be of type $filter_type"
         if !(filter in filters)
             @warn "The $type argument should be one of: $filters, and you provided $filter."
@@ -58,9 +83,9 @@ end
 Is a node filtered in ? Returns `true` if the node is kept, `false` if it is filtered-out.
 """
 @inline function is_filtered(node, mtg_scale, mtg_symbol, mtg_link, filter_fun)
-
-    link_keep = isnothing(mtg_link) || is_filtered(mtg_link, link(node))
-    symbol_keep = isnothing(mtg_symbol) || is_filtered(mtg_symbol, symbol(node))
+    node_mtg_ = node_mtg(node)
+    link_keep = isnothing(mtg_link) || is_filtered(mtg_link, getfield(node_mtg_, :link))
+    symbol_keep = isnothing(mtg_symbol) || is_filtered(mtg_symbol, getfield(node_mtg_, :symbol))
     scale_keep = isnothing(mtg_scale) || is_filtered(mtg_scale, scale(node))
     filter_fun_keep = isnothing(filter_fun) || filter_fun(node)
 
@@ -75,8 +100,42 @@ end
     value in filter
 end
 
-@inline function is_filtered(filter::String, value)
-    value in (filter,)
+@inline function is_filtered(filter::AbstractString, value::Symbol)
+    Symbol(filter) === value
+end
+
+@inline function is_filtered(filter::AbstractString, value::AbstractString)
+    filter == value
+end
+
+@inline function is_filtered(filter::AbstractString, value)
+    filter == value
+end
+
+@inline function is_filtered(filter::Symbol, value::AbstractString)
+    filter === Symbol(value)
+end
+
+@inline function is_filtered(filter::Symbol, value::Symbol)
+    filter === value
+end
+
+@inline function is_filtered(filter::Symbol, value)
+    filter === value
+end
+
+@inline function is_filtered(filter::T, value::Symbol) where {T<:Union{Tuple,AbstractArray}}
+    for f in filter
+        is_filtered(f, value) && return true
+    end
+    return false
+end
+
+@inline function is_filtered(filter::T, value::AbstractString) where {T<:Union{Tuple,AbstractArray}}
+    for f in filter
+        is_filtered(f, value) && return true
+    end
+    return false
 end
 
 @inline function is_filtered(filter, value::T) where {T<:Union{Tuple,Array}}