added AbstractNode, StableNode, and a bunch of docs

Author: ExpandingMan

docs/src/faq.md

@@ -66,29 +66,3 @@ map leaves.
 
 Introducing a new type becomes necessary to ensure that it can accommodate arbitrary output types.
 
-# Why is my code type unstable?
-Guaranteeing type stability when iterating over trees is challenging to say the least.  There are
-several major obstacles
-- The children of a tree node do not, in general, have the same type as their parent.
-- Even if it is easy to infer the type of a node's immediate children, it is usually much harder to
-    infer the types of the node's more distant descendants.
-- Navigating a tree requires inferring not just the types of the children but the types of the
-    children's *iteration states*.  To make matters worse, Julia's `Base` does not include traits
-    for describing these, and the `Base` iteration protocol makes very few assumptions about them.
-
-All of this means that you are unlikely to get type-stable code from AbstractTrees.jl without some
-effort.
-
-The simplest way around this is to define the `NodeType` trait and `nodetype` (analogous to
-`Base.IteratorEltype` and `eltype`):
-```julia
-AbstractTrees.NodeType(::Type{<:ExampleNode}) = HasNodeType()
-AbstractTrees.nodetype(::Type{<:ExampleNode}) = ExampleNode
-```
-which is equivalent to asserting that all nodes of a tree are of the same type.  Performance
-critical code must ensure that it is possible to construct such a tree, which may not be trivial.
-
-Note that even after defining `Base.eltype` it might still be difficult to achieve type-stability
-due to the aforementioned difficulties with iteration states.  The most reliable around this is to
-ensure that the object returned by `children` is indexable and that the node has the
-`IndexedChildren` state.  This guarantees that `Int` can always be used as an iteration state.

docs/src/index.md

@@ -145,6 +145,33 @@ prevsiblingindex
 rootindex
 ```
 
+## The `AbstractNode` Type
+It is not required that objects implementing the AbstractTrees.jl interface are of this type, but it
+can be used to indicate that an object *must* implement the interface.
+```@docs
+AbstractNode
+```
+
+## Type Stability and Performance
+Because of the recursive nature of trees it can be quite challenging to achieve type stability when
+traversing it in any way such as iterating over nodes.  Only trees which guarantee that all nodes
+are of the same type (with [`HasNodeType`](@ref)) can be type stable.
+
+To make it easier to convert trees with non-uniform node types this package provides the
+`StableNode` type.
+```@docs
+StableNode
+```
+
+To achieve the same performance with custom node types be sure to define at least
+```julia
+AbstractTrees.NodeType(::Type{<:ExampleNode}) = HasNodeType()
+AbstractTrees.nodetype(::Type{<:ExampleNode}) = ExampleNode
+```
+
+In some circumstances it is also more efficient for nodes to have [`ChildIndexing`](@ref) since this
+also guarantees the type of the iteration state of the iterator returned by `children`.
+
 ## Additional Functions
 ```@docs
 getdescendant

docs/src/iteration.md

@@ -34,6 +34,7 @@ PostOrderDFS
 Leaves
 Siblings
 StatelessBFS
+treemap
 ```
 
 ### Iterator States

src/AbstractTrees.jl

@@ -19,11 +19,6 @@ include("iteration.jl")
 include("builtins.jl")
 include("printing.jl")
 
-# Julia 1.0 support (delete when we no longer support it)
-if !isdefined(Base, :isnothing)
-    isnothing(x) = x === nothing
-end
-
 
 #interface
 export ParentLinks, StoredParents, ImplicitParents
@@ -34,6 +29,8 @@ export nodetype, nodevalue, nodevalues, children, parentlinks, siblinglinks, chi
 #extended interface
 export nextsibling, prevsibling
 
+export AbstractNode, StableNode
+
 # properties
 export ischild, isroot, isroot, intree, isdescendant, treesize, treebreadth, treeheight, descendleft, getroot
 

src/base.jl

@@ -191,3 +191,85 @@ function getroot(::StoredParents, node)
         p = parent(p)
     end
 end
+
+
+"""
+    AbstractNode{T}
+
+Abstract type of tree nodes that implement the AbstractTrees.jl interface.
+
+It is *NOT* necessary for tree nodes to inherit from this type to implement the AbstractTrees.jl interface.
+Conversely, all `AbstractNode` types are required to satisfy the AbstractTrees.jl interface (i.e. they must
+at least define [`children`](@ref)).
+
+Package developers should keep in mind when writing methods that most trees *will not* be of this type.
+Therefore, any functions which are intended to work on any tree should not dispatch on `AbstractNode`.
+
+The type parameter `T` is the type of the [`nodevalue`](@ref) of the concrete type descented from `AbstractNode`.
+"""
+abstract type AbstractNode{T} end
+
+function Base.show(io::IO, node::AbstractNode)
+    print(io, typeof(node), "(")
+    show(io, nodevalue(node))
+    print(io, ", nchildren=", length(children(node)), ")")
+end
+
+Base.show(io::IO, ::MIME"text/plain", node::AbstractNode) = print_tree(io, node)
+
+
+"""
+    StableNode{T} <: AbstractNode{T}
+
+A node belonging to a tree in which all nodes are of type `StableNode{T}`.  This type is provided so that
+trees with [`NodeTypeUnknown`](@ref) can implement methods to be converted to type-stable trees with indexable
+`children` which allow for efficient traversal and iteration.
+
+## Constructors
+```julia
+StableNode{T}(x::T, ch)
+StableNode(x, ch=())
+StableNode(𝒻, T, node)
+```
+
+## Arguments
+- `x`: the value of the constructed node, returned by [`nodevalue`](@ref).
+- `ch`: the children of the node, each must be of type `StableNode`.
+- `𝒻`: A function which, when called on the node of a tree returns a value which should be wrapped
+    by a `StableNode`.  The return value of `𝒻` must be convertable to `T` (see example).
+- `T`: The value type of the `StableNode`s in a tree.
+- `node`: A node from a tree which is to be used to construct the `StableNode` tree.
+
+## Examples
+```julia
+t = [1, [2,3]]
+
+node = StableNode(Union{Int,Nothing}, t) do n
+    n isa Integer ? convert(Int, n) : nothing
+end
+```
+In the above example `node` is a tree with [`HasNodeType`](@ref), nodes of type `StableNode{Union{Int,Nothing}}`.
+The nodes in the new tree corresponding to arrays have value `nothing` while other nodes have their
+corresponding `Int` value.
+"""
+struct StableNode{T} <: AbstractNode{T}
+    value::T
+    children::Vector{StableNode{T}}
+
+    # this ensures proper handling of all cases for iterables ch
+    StableNode{T}(x::T, ch) where {T} = new{T}(x, collect(StableNode{T}, ch))
+end
+
+nodevalue(n::StableNode) = n.value
+
+children(n::StableNode) = n.children
+
+NodeType(::Type{<:StableNode}) = HasNodeType()
+nodetype(::Type{StableNode{T}}) where {T} = StableNode{T}
+
+ChildIndexing(::Type{<:StableNode}) = IndexedChildren()
+
+function StableNode{T}(𝒻, node) where {T}
+    StableNode{T}(convert(T, 𝒻(node)), map(n -> StableNode{T}(𝒻, n), children(node)))
+end
+

src/cursors.jl

@@ -81,6 +81,17 @@ nodevalue(tc::TreeCursor) = tc.node
 parent(tc::TreeCursor) = tc.parent
 
 
+#====================================================================================================
+Note for developers:
+
+The following code for `TreeCursor` types contains a fair amount of code duplication.
+In particular, some of the cursors can probably be combined (e.g. ImplicitCursor and StableCursor).
+
+This duplication is deliberate: cursors can get very subtle and it is rather easy to break them,
+break their type stability or cause O(tree_depth) recursive compilation costs.
+====================================================================================================#
+
+
 """
     TrivialCursor{N,P} <: TreeCursor{N,P}
 

src/iteration.jl

@@ -430,7 +430,7 @@ Every `MapNode` is itself a tree with the [`IndexedChildren`](@ref) trait and th
 
 Use [`AbstractTrees.nodevalue`](@ref) or `mapnode.value` to obtain the wrapped value.
 """
-struct MapNode{T,C}
+struct MapNode{T,C} <: AbstractNode{T}
     value::T
     children::C
 
@@ -450,13 +450,6 @@ nodevalue(μ::MapNode) = μ.value
 
 ChildIndexing(::MapNode) = IndexedChildren()
 
-function Base.show(io::IO, μ::MapNode)
-    print(io, typeof(μ))
-    print(io, "(", μ.value, ")")
-end
-
-Base.show(io::IO, ::MIME"text/plain", μ::MapNode) = print_tree(io, μ)
-
 
 """
     treemap(f, node)
@@ -479,6 +472,8 @@ It's very easy to write an `f` that makes `treemap` stack-overflow.  To avoid th
 termiantes, i.e. that sometimes it returns empty `children`.  For example, if `f(n) = (nothing, [0; children(n)])` will
 stack-overflow because every node will have at least 1 child.
 
+To create a tree with [`HasNodeType`](@ref) which enables efficient iteration, see [`StableNode`](@ref) instead.
+
 ## Examples
 ```julia
 julia> t = [1, [2, 3]];

test/builtins.jl

@@ -86,3 +86,25 @@ end
     @test nodevalue.(PostOrderDFS(b)) == [0, 1, 0, 2, 0, 3, nothing, nothing, nothing]
 end
 
+@testset "StableNode" begin
+    t = [1,[2,3,[4,5]]]
+
+    n = StableNode{Union{Int,Nothing}}(t) do m
+        m isa Integer ? convert(Int, m) : nothing
+    end
+
+    @test treeheight(n) == 3
+    @test treebreadth(n) == 5
+
+    @test typeof(TreeCursor(n)) == AbstractTrees.StableIndexedCursor{StableNode{Union{Int,Nothing}}}
+
+    ls = @inferred collect(Leaves(n))
+    ls = nodevalue.(ls)
+    @test eltype(ls) <: Union{Nothing,Int}
+    @test nodevalue.(ls) == 1:5
+
+    ns = @inferred collect(PreOrderDFS(n))
+    ns = nodevalue.(ns)
+    @test eltype(ns) == Union{Nothing,Int}
+    @test ns == [nothing, 1, nothing, 2, 3, nothing, 4, 5]
+end