Skip to content

Commit

Permalink
[bugfix] Base.eltype works on graph types as well as instances (#144)
Browse files Browse the repository at this point in the history
* Cover bug in tests

* Change eltype to act on type

* Implement eltype on AbstractGraph instead

* Fix eltype extension

* Proper subtyping

* Proper subtyping on nonbacktracking

* Remove useless exception since eltype is no longer in interface

* Fix typos

* Remove eltype in generic test graph tyoe

---------

Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com>
  • Loading branch information
lmondada and gdalle authored Jun 16, 2023
1 parent 1ca400d commit 11f54ad
Show file tree
Hide file tree
Showing 9 changed files with 9 additions and 15 deletions.
1 change: 0 additions & 1 deletion docs/src/ecosystem/interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ This section is designed to guide developers who wish to write their own graph s
All Graphs.jl functions rely on a standard API to function. As long as your graph structure is a subtype of [`AbstractGraph`](@ref) and implements the following API functions with the given return values, all functions within the Graphs.jl package should just work:

- [`edges`](@ref)
- [Base.eltype](https://docs.julialang.org/en/latest/base/collections/#Base.eltype)
- [`edgetype`](@ref) (example: `edgetype(g::CustomGraph) = Graphs.SimpleEdge{eltype(g)})`)
- [`has_edge`](@ref)
- [`has_vertex`](@ref)
Expand Down
1 change: 0 additions & 1 deletion src/SimpleGraphs/simpledigraph.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ function SimpleDiGraph(
return SimpleDiGraph{T}(ne, fadjlist, badjlist)
end

eltype(x::SimpleDiGraph{T}) where {T} = T

# DiGraph{UInt8}(6), DiGraph{Int16}(7), DiGraph{Int8}()
"""
Expand Down
2 changes: 0 additions & 2 deletions src/SimpleGraphs/simplegraph.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ function SimpleGraph(ne, fadjlist::Vector{Vector{T}}) where {T}
return SimpleGraph{T}(ne, fadjlist)
end

eltype(x::SimpleGraph{T}) where {T} = T

# Graph{UInt8}(6), Graph{Int16}(7), Graph{UInt8}()
"""
SimpleGraph{T}(n=0)
Expand Down
3 changes: 0 additions & 3 deletions src/Test/Test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ end
Graphs.is_directed(::Type{<:GenericGraph}) = false
Graphs.is_directed(::Type{<:GenericDiGraph}) = true

Base.eltype(g::GenericGraph) = eltype(g.g)
Base.eltype(g::GenericDiGraph) = eltype(g.g)

Graphs.edges(g::GenericGraph) = (GenericEdge(e) for e in Graphs.edges(g.g))
Graphs.edges(g::GenericDiGraph) = (GenericEdge(e) for e in Graphs.edges(g.g))

Expand Down
4 changes: 2 additions & 2 deletions src/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ Return the type of graph `g`'s edge
edgetype(g::AbstractGraph) = _NI("edgetype")

"""
eltype(g)
eltype(G)
Return the type of the graph's vertices (must be <: Integer)
"""
eltype(g::AbstractGraph) = _NI("eltype")
eltype(::Type{<:AbstractGraph{T}}) where {T} = T

"""
nv(g)
Expand Down
2 changes: 1 addition & 1 deletion src/linalg/nonbacktracking.jl
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ end

size(nbt::Nonbacktracking) = (nbt.m, nbt.m)
size(nbt::Nonbacktracking, i::Number) = size(nbt)[i]
eltype(nbt::Nonbacktracking) = Float64
eltype(::Type{<:Nonbacktracking}) = Float64
issymmetric(nbt::Nonbacktracking) = false

function *(nbt::Nonbacktracking, x::Vector{T}) where {T<:Number}
Expand Down
2 changes: 1 addition & 1 deletion test/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ mutable struct DummyEdge <: AbstractEdge{Int} end
@test_throws Graphs.NotImplementedError edgefun2edges(dummyedge, dummyedge)
end

for graphfunbasic in [nv, ne, vertices, edges, is_directed, edgetype, eltype]
for graphfunbasic in [nv, ne, vertices, edges, is_directed, edgetype]
@test_throws Graphs.NotImplementedError graphfunbasic(dummygraph)
end

Expand Down
1 change: 1 addition & 0 deletions test/linalg/spectral.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Matrix(nbt::Nonbacktracking) = Matrix(sparse(nbt))
@test size(n10, 1) == n10.m
@test size(n10, 2) == n10.m
@test eltype(n10) == Float64
@test eltype(typeof(n10)) == Float64
@test !issymmetric(n10)

contract!(z, n10, v)
Expand Down
8 changes: 4 additions & 4 deletions test/simplegraphs/simplegraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ using Random: Random
T = @inferred(eltype(g))
@test @inferred(nv(SimpleGraph{T}(6))) == 6

@test @inferred(eltype(SimpleGraph(T))) == T
@test @inferred(eltype(SimpleGraph{T})) == T
@test @inferred(eltype(SimpleGraph{T}(adjmx1))) == T

ga = SimpleGraph(10)
Expand Down Expand Up @@ -156,7 +156,7 @@ using Random: Random
T = @inferred(eltype(g))
@test @inferred(nv(SimpleDiGraph{T}(6))) == 6

@test @inferred(eltype(SimpleDiGraph(T))) == T
@test @inferred(eltype(SimpleDiGraph{T})) == T
@test @inferred(eltype(SimpleDiGraph{T}(adjmx2))) == T

ga = SimpleDiGraph(10)
Expand Down Expand Up @@ -271,7 +271,7 @@ using Random: Random
edge_set_any = Set{Any}(edge_list)

g1 = @inferred SimpleDiGraph(edge_list)
# we can't infer the return type of SimpleDiGraphFromIterator at the moment
# we can't infer the return type of SimpleDiGraphFromIterator at the moment
g2 = SimpleDiGraphFromIterator(edge_list)
g3 = SimpleDiGraphFromIterator(edge_iter)
g4 = SimpleDiGraphFromIterator(edge_set)
Expand Down Expand Up @@ -312,7 +312,7 @@ using Random: Random
))
end

# check if multiple edges && multiple self-loops result in the
# check if multiple edges && multiple self-loops result in the
# correct number of edges & vertices
# edges using integers < 1 should be ignored
g_undir = SimpleGraph(0)
Expand Down

0 comments on commit 11f54ad

Please sign in to comment.