Bug fixes

.github/workflows/CompatHelper.yaml

@@ -19,7 +19,7 @@ jobs:
         os:
           - ubuntu-latest
     steps:
-      - uses: julia-actions/setup-julia@latest
+      - uses: julia-actions/setup-julia@v1
         with:
           version: ${{ matrix.julia-version }}
           arch: ${{ matrix.arch }}

.github/workflows/testing.yaml

@@ -15,7 +15,7 @@ jobs:
     strategy:
       matrix:
         julia-version:
-          - '1.4'
+          - '1.6'
           - '1'
         os:
           - ubuntu-latest
@@ -38,7 +38,7 @@ jobs:
         arch: ${{ matrix.arch }}
     - name: Install R
       if: matrix.os == 'macOS-latest'
-      uses: r-lib/actions/setup-r@master
+      uses: r-lib/actions/setup-r@v2
       with:
         r-version: ${{ matrix.R-version }}
     - name: Install ape

.gitignore

@@ -1,4 +1,5 @@
 *.jl.cov
 *.jl.*.cov
 *.jl.mem
-Manifest.toml
+/Manifest.toml
+test/Manifest.toml

Project.toml

@@ -1,15 +1,15 @@
 name = "Phylo"
 uuid = "aea672f4-3940-5932-aa44-993d1c3ff149"
 author = ["Richard Reeve <Richard.Reeve@glasgow.ac.uk>"]
-version = "0.4.21"
+version = "0.4.22"
 
 [deps]
 AxisArrays = "39de3d68-74b9-583c-8d2d-e117c070f3a9"
 DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
 Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
 IterableTables = "1c8ee90f-4401-5389-894e-7a04a3dc0f4d"
-LightGraphs = "093fc24a-ae57-5d10-9952-331d41423f4d"
+Graphs = "86223c79-3864-5bf0-83f7-82e725a168b6"
 Missings = "e1d29d7a-bbdc-5cf2-9ac0-f12de2c33e28"
 Printf = "de0858da-6303-5e67-8744-51eddeeeb8d7"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
@@ -22,13 +22,13 @@ Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d"
 
 [compat]
 AxisArrays = "0.4"
-DataFrames = "0.21, 0.22, 1.0"
+DataFrames = "^1"
 DataStructures = "0.17, 0.18"
 Distributions = "0.24, 0.25"
 IterableTables = "^1"
-LightGraphs = "^1"
-Missings = "0.4, 1.0"
-RecipesBase = "0.7, 0.8, ^1"
+Graphs = "^1"
+Missings = "^1"
+RecipesBase = "^1"
 Requires = "^1"
 SimpleTraits = "0.9"
 Tokenize = "0.5"

src/API.jl

@@ -63,7 +63,7 @@ _matchnodetype(::Type{<:AbstractTree{TT, RT, NL, N, B}},
                ::Type{NL}) where {TT, RT, NL, N, B} = !_prefernodeobjects(N)
 _matchnodetypes(::Type{<:AbstractTree}, ::Type{<:Any}, ::Type{<:Any}) = false
 _matchnodetypes(::Type{T}, ::Type{N}, ::Type{N}) where {T, N} =
-    _prefernodeobjects(T, N)
+    _matchnodetype(T, N)
 
     """
     _matchbranchtype(::Type{<:AbstractTree}, ::Type{<:AbstractBranch})
@@ -213,6 +213,7 @@ Returns a vector of nodes for a OneTree tree. Either _getnodes() must be
 implemented for any OneTree tree type.
 """
 function _getnodes end
+_getnodes(::T) where T <: AbstractTree = error("No _getnodes() method for tree type $T")
 _getnodes(tree::AbstractTree{OneTree}, order::TraversalOrder) =
     _traversal(tree, order)
 
@@ -223,11 +224,12 @@ Returns a vector of node names for a OneTree tree. Can
 be implemented for any OneTree tree type, especially PreferNodeObjects trees.
 """
 function _getnodenames end
+_getnodenames(::T) where T <: AbstractTree = error("No _getnodes() method for tree type $T")
 @traitfn _getnodenames(tree::T, order::TraversalOrder) where
-{T <: AbstractTree{OneTree}; !PreferNodeObjects{T}} = _getnodes(tree)
+{T <: AbstractTree{OneTree}; !PreferNodeObjects{T}} = _getnodes(tree, order)
 @traitfn _getnodenames(tree::T, order::TraversalOrder) where
 {T <: AbstractTree{OneTree}; PreferNodeObjects{T}} =
-    _getnodename.(tree, _getnodes(tree))
+    _getnodename.(tree, _getnodes(tree, order))
 
 """
     _nnodes(::AbstractTree)
@@ -352,7 +354,7 @@ _hasnode(tree::AbstractTree{OneTree, RT, NL, N},
          node::N) where {RT, NL, N} = node ∈ _getnodes(tree)
 
 """
-    _createnode!(tree::AbstractTree, nodename)
+    _createnode!(tree::AbstractTree, nodename[, data])
 
 Must be implemented for any AbstractTree subtype.
 """
@@ -412,6 +414,7 @@ Returns a vector of branches for a OneTree tree. Either _getbranches() or
 _getbranchnames() must be implemented for any OneTree tree type.
 """
 function _getbranches end
+_getbranches(::T) where T = error("No _getbranches() method for tree type $T")
 
 """
     _nbranches(::AbstractTree)
@@ -452,8 +455,8 @@ _hasbranch(tree::AbstractTree{OneTree},
                                           branch ∈ _getbranches(tree)
 
 """
-    _createbranch!(tree::AbstractTree, source, destination,
-                   length, data)
+    _createbranch!(tree::AbstractTree, source, destination[,
+                   length][, data])
 
 Create a new branch and add it to a tree. Must be implemented for any
 AbstractTree subtype.
@@ -513,12 +516,16 @@ Return an iterable object containing nodes in given order - preorder, inorder,
 postorder or breadthfirst
 """
 function _traversal end
-_traversal(tree::AbstractTree{OneTree, <: Rooted}, order::TraversalOrder) =
-    (order == anyorder ? _getnodes(tree) :
-     _traversal(tree, order, collect(_getroots(tree))))
+function _traversal(tree::T, order::TraversalOrder) where {T <: AbstractTree{OneTree, <: Rooted}}
+    if order == anyorder
+        return _getnodes(tree)
+    else
+        return _traversal(tree, order, collect(_getroots(tree)))
+    end
+end
 function _traversal(tree::AbstractTree{OneTree, <: Rooted},
-                    order::TraversalOrder, todo,
-                    sofar = eltype(todo)[])
+                    order::TraversalOrder, todo::Vector,
+                    sofar::Vector = eltype(todo)[])
     while !isempty(todo)
         if order == Phylo.breadthfirst
             append!(sofar, todo)
@@ -665,13 +672,15 @@ _degree(tree::AbstractTree{OneTree, <: Rooted}, node) =
 Must be implemented for any AbstractNode subtype.
 """
 function _hasinbound end
+_hasinbound(::T, ::N) where {T, N} = error("No _hasinbound() function for types $T, $N")
 
 """
     _getinbound(tree::AbstractTree, node::AbstractNode)
 
 Get the inbound connection. Must be implemented for any rooted AbstractNode subtype.
 """
 function _getinbound end
+_getinbound(::T, ::B) where {T, B} = error("No _getinbound() function for $T, $B")
 
 """
     _getparent(tree::AbstractTree, node)
@@ -705,6 +714,7 @@ Returns the outbound connections of a rooted node. Must be implemented for any
 rooted AbstractNode subtype.
 """
 function _getoutbounds end
+_getoutbounds(::T, ::N) where {T, N} = error("No _getoutbounds() function for $T, $N")
 
 """
     _addoutbound!(tree::AbstractTree, node::AbstractNode, branch)
@@ -730,8 +740,10 @@ Return the child node(s) for this node. May be implemented for any rooted
 AbstractNode subtype.
 """
 function _getchildren end
-_getchildren(tree::AbstractTree{OneTree, <: Rooted}, node) =
-    [_dst(tree, branch) for branch in _getoutbounds(tree, node)]
+_getchildren(tree::T, node::N) where {RT <: Rooted, NL, N <: AbstractNode, B, T <: AbstractTree{OneTree, RT, NL, N, B}} =
+    N[_dst(tree, branch) for branch in _getoutbounds(tree, node)]
+_getchildren(tree::T, node::NL) where {RT <: Rooted, NL, N <: AbstractNode, B, T <: AbstractTree{OneTree, RT, NL, N, B}} =
+    NL[_dst(tree, branch) for branch in _getoutbounds(tree, node)]
 
 """
     _getconnections(tree::AbstractTree, node::AbstractNode)
@@ -817,6 +829,7 @@ Return source node for a branch. Must be implemented for any rooted
 AbstractBranch subtype.
 """
 function _src end
+_src(::T, ::B) where {T, B} = error("No _src() function for $T, $B")
 
 """
     _dst(branch::AbstractBranch)
@@ -825,6 +838,7 @@ Return destination node for a branch. Must be implemented for any rooted
 AbstractBranch subtype.
 """
 function _dst end
+_dst(::T, ::B) where {T, B} = error("No _dst() function for $T, $B")
 
 """
     _conns(tree::AbstractTree, branch::AbstractBranch)
@@ -893,13 +907,15 @@ function _branchdatatype end
 _branchdatatype(::Type{<:AbstractTree}) = Nothing
 
 function _getnodedata end
+_getnodedata(::T, ::N) where {T, N} = error("No _getnodedata() function for $T, $N")
 _getnodedata(tree::AbstractTree{OneTree}, node, label) =
     _getnodedata(tree, node)[label]
 function _setnodedata! end
 _setnodedata!(tree::AbstractTree{OneTree}, node, label, value) =
     (_getnodedata(tree, node)[label] = value)
 
 function _getbranchdata end
+_getbranchdata(::T, ::B) where {T, B} = error("No _getbranchdata() function for $T, $B")
 _getbranchdata(tree::AbstractTree, branch, label) =
     _getbranchdata(tree, branch)[label]
 function _setbranchdata! end

src/Interface.jl

@@ -1,5 +1,5 @@
 using Phylo.API
-import LightGraphs: src, dst, indegree, outdegree, degree
+import Graphs: src, dst, indegree, outdegree, degree
 using SimpleTraits
 
 # AbstractTree/Node/Branch type methods

src/LinkTree.jl

@@ -339,7 +339,7 @@ function _createbranch!(tree::LinkTree{Unrooted, NL, N, B},
     {NL, N, Data, LenUnits,
      B <: LinkBranch{Unrooted, NL, Data, LenUnits}}
     id = ismissing(name) ? length(tree.branches) + 1 : name
-    branch = LinkBranch(id, tree, from, to, len, data)
+    branch = LinkBranch(id, from, to, len, data)
     push!(tree.branches, branch)
     _addconnection!(tree, from, branch)
     _addconnection!(tree, to, branch)

src/Phylo.jl

@@ -70,7 +70,7 @@ export HoldsNodeData, MatchTreeNameType
 
 # AbstractNode methods
 export _isleaf, _isroot, _isinternal, _isunattached
-export _indegree, _hasinboundspace, _outdegree, _hasoutboundspace, _degree
+export _indegree, _hasinboundspace, _outdegree, _hasoutboundspace, _hasspace, _degree
 export _hasinbound, _getinbound, _addinbound!, _removeinbound!
 export _getoutbounds, _addoutbound!, _removeoutbound!
 export _getconnections, _addconnection!, _removeconnection!

src/rand.jl

@@ -278,7 +278,7 @@ function BrownianTrait(tree::T, trait::String, start::N = 0.0;
                        σ² = missing, σ = missing, f::Function = identity) where
     {T <: AbstractTree, N <: Number}
     iscontinuous(N) ||
-        throw(TypeError("Type $N must be continuous for a Gaussian Trait"))
+        throw(TypeError(:BrownianTrait, "Type $N must be continuous for a Gaussian Trait", AbstractFloat, N))
     if ismissing(σ)
         σ = sqrt(σ²)
     end
@@ -415,7 +415,7 @@ function enum_rand(rng::AbstractRNG, current::TRAIT,
     while i < tot
         @inbounds pi = p[i]
         if pi < rp
-            if rand(rng) < pi / rp
+            if rand(rng, Float64) < pi / rp
                 return traits[i]
             end
             rp -= pi
@@ -486,7 +486,7 @@ end
 
 function enum_rand(rng::AbstractRNG, current::TRAIT,
                    p_stay::Float64) where TRAIT <: Enum
-    if rand(rng) < p_stay
+    if rand(rng, Float64) < p_stay
         return current
     end
 
@@ -499,7 +499,7 @@ function enum_rand(rng::AbstractRNG, current::TRAIT,
     while i < num
         passed |= (from == i)
         if remain > 1
-            if rand(rng) < inv(remain)
+            if rand(rng, Float64) < inv(remain)
                 return traits[i+passed]
             end
             i += 1

src/rcall.jl

@@ -4,11 +4,11 @@ using .RCall: protect, unprotect, rcall_p, RClass, isObject, isS4
 import .RCall: rcopy
 
 function rcopy(::Type{T}, rt::Ptr{VecSxp}) where T <: AbstractTree
-    if !isObject(rt) || isS4(rt) || rcopy(rcall_p(:class, rt)) != "phylo"
+    if !isObject(rt) || isS4(rt) || rcopy(String, rcall_p(:class, rt)) != "phylo"
         error("Object is not of S3 phylo class, aborting")
     end
 
-    if !rcopy(rcall_p(Symbol("is.rooted"), rt))
+    if !rcopy(Bool, rcall_p(Symbol("is.rooted"), rt))
         error("Cannot currently translate unrooted trees")
     end
 
@@ -21,7 +21,7 @@ function rcopy(::Type{T}, rt::Ptr{VecSxp}) where T <: AbstractTree
     nontips = nnode
     append!(nodes, getnodename.(tree, createnodes!(tree, nontips)))
 
-    for edge in 1:size(edges, 1)
+    for edge in Base.axes(edges, 1)
         createbranch!(tree,
                       nodes[edges[edge, 1]], nodes[edges[edge, 2]],
                       lengths[edge])

test/Project.toml

@@ -1,7 +1,6 @@
 [deps]
 DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 IterableTables = "1c8ee90f-4401-5389-894e-7a04a3dc0f4d"
-JuliaDB = "a93385a2-3734-596a-9a66-3cfbb77141e6"
 RCall = "6f49c342-dc21-5d91-9882-a32aef131414"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

test/run_rcall.jl

@@ -1,4 +1,4 @@
-module ValidateRCall_ape
+module ValidateRCall_wrapped
 using Test
 using Phylo
 using RCall

test/test_API.jl

@@ -0,0 +1,36 @@
+module TestAPI
+
+using Phylo
+using Phylo.API
+using Test
+
+struct TestBranch <: Phylo.AbstractBranch{OneRoot, String}
+end
+
+struct TestNode <: Phylo.AbstractNode{OneRoot, String}
+end
+
+struct TestTree <: Phylo.AbstractTree{OneTree, OneRoot, String, TestNode, TestBranch}
+end
+
+@testset "Check errors" begin
+    tt = TestTree();
+    tn = TestNode();
+    tb = TestBranch();
+
+    @test_throws ErrorException _getnodes(tt)
+    @test_throws ErrorException _getnodenames(tt)
+    @test_throws ErrorException _getbranches(tb)
+    @test_throws ErrorException _getnodes(tt)
+    @test_throws ErrorException _hasinbound(tt, tn)
+    @test_throws ErrorException _getinbound(tt, tn)
+    @test_throws ErrorException _getoutbounds(tt, tn)
+    @test_throws ErrorException _addconnection!(tt, tn, tn)
+    @test_throws ErrorException _removeconnection!(tt, tn, tb)
+    @test_throws ErrorException _src(tt, tb)
+    @test_throws ErrorException _dst(tt, tb)
+    @test_throws ErrorException _getnodedata(tt, tn)
+    @test_throws ErrorException _getbranchdata(tt, tb)
+end
+
+end

test/test_Interface.jl

@@ -28,12 +28,14 @@ using Test
         push!(othernodes, also)
         @test Set(othernodes) ⊆ Set(getnodenames(tree))
         innodes = append!(copy(species), othernodes)
-        allnodes = reverse(innodes)
+        allnodes = copy(innodes)
         pop!(innodes)
         @test_throws Exception getroot(tree)
         branches = map(innodes) do node
             itr = Iterators.filter(name -> hasoutboundspace(tree, name) &&
-                                   name != node, allnodes)
+                                   name != node &&
+                                   name ∉ getdescendants(tree, node) &&
+                                   name ∉ species, allnodes)
             getbranch(tree, createbranch!(tree, first(itr), node))
         end
         @test_nowarn getroot(tree)