Skip to content

Commit

Permalink
Merge pull request #1024 from JuliaRobotics/23Q3/maint/listNeighbors
Browse files Browse the repository at this point in the history
Deprecate getNeighbors for listNeighbors
  • Loading branch information
Affie committed Jul 13, 2023
2 parents 311fa2a + 132f95b commit 5d6a35f
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 46 deletions.
2 changes: 1 addition & 1 deletion docs/src/BuildingGraphs.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ To list all variables or factors (instead of just their labels), use the

Traversing and Querying functions for finding the relationships and building subtraphs include:

- [`getNeighbors`](@ref)
- [`listNeighbors`](@ref)
- [`buildSubgraph`](@ref)
- [`getBiadjacencyMatrix`](@ref)

Expand Down
6 changes: 6 additions & 0 deletions src/Deprecated.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@

## ================================================================================
## Remove in v0.24
##=================================================================================
#NOTE free up getNeighbors to return the variables or factors
@deprecate getNeighbors(args...; kwargs...) listNeighbors(args...; kwargs...)


## ================================================================================
## Remove in v0.23
Expand Down
2 changes: 1 addition & 1 deletion src/DistributedFactorGraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export packVariableNodeData, unpackVariableNodeData

export getSolvedCount, isSolved, setSolvedCount!, isInitialized, isMarginalized, setMarginalized!

export getNeighborhood, getNeighbors, _getDuplicatedEmptyDFG
export getNeighborhood, listNeighbors, _getDuplicatedEmptyDFG
export findFactorsBetweenNaive
export copyGraph!, deepcopyGraph, deepcopyGraph!, buildSubgraph, mergeGraph!

Expand Down
2 changes: 1 addition & 1 deletion src/GraphsDFG/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import ...DistributedFactorGraphs: setSolverParams!,
listFactors,
lsf,
isConnected,
getNeighbors,
listNeighbors,
buildSubgraph,
copyGraph!,
getBiadjacencyMatrix,
Expand Down
6 changes: 3 additions & 3 deletions src/GraphsDFG/services/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function deleteVariable!(dfg::GraphsDFG, label::Symbol)#::Tuple{AbstractDFGVaria

deleteNeighbors = true # reserved, orphaned factors are not supported at this time
if deleteNeighbors
neigfacs = map(l->deleteFactor!(dfg, l), getNeighbors(dfg, label))
neigfacs = map(l->deleteFactor!(dfg, l), listNeighbors(dfg, label))
end
variable = dfg.g.variables[label]
rem_vertex!(dfg.g, dfg.g.labels[label])
Expand Down Expand Up @@ -231,7 +231,7 @@ function _isSolvable(dfg::GraphsDFG, label::Symbol, ready::Int)::Bool
return false
end

function getNeighbors(dfg::GraphsDFG, node::DFGNode; solvable::Int=0)
function listNeighbors(dfg::GraphsDFG, node::DFGNode; solvable::Int=0)
label = node.label
if !exists(dfg, label)
error("Variable/factor with label '$(node.label)' does not exist in the factor graph")
Expand All @@ -252,7 +252,7 @@ function getNeighbors(dfg::GraphsDFG, node::DFGNode; solvable::Int=0)
end


function getNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int=0)::Vector{Symbol}
function listNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int=0)::Vector{Symbol}
if !exists(dfg, label)
error("Variable/factor with label '$(label)' does not exist in the factor graph")
end
Expand Down
4 changes: 2 additions & 2 deletions src/entities/DFGFactor.jl
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ Base.@kwdef struct DFGFactorSummary <: AbstractDFGFactor
"""Factor tags, e.g [:FACTOR].
Accessors: [`getTags`](@ref), [`mergeTags!`](@ref), and [`removeTags!`](@ref)"""
tags::Set{Symbol}
"""Internal cache of the ordering of the neighbor variables. Rather use getNeighbors to get the list as this is an internal value.
"""Internal cache of the ordering of the neighbor variables. Rather use listNeighbors to get the list as this is an internal value.
Accessors: [`getVariableOrder`](@ref)"""
_variableOrderSymbols::Vector{Symbol}
"""Variable timestamp.
Expand Down Expand Up @@ -261,7 +261,7 @@ Base.@kwdef struct SkeletonDFGFactor <: AbstractDFGFactor
"""Factor tags, e.g [:FACTOR].
Accessors: [`getTags`](@ref), [`mergeTags!`](@ref), and [`removeTags!`](@ref)"""
tags::Set{Symbol}
"""Internal cache of the ordering of the neighbor variables. Rather use getNeighbors to get the list as this is an internal value.
"""Internal cache of the ordering of the neighbor variables. Rather use listNeighbors to get the list as this is an internal value.
Accessors: [`getVariableOrder`](@ref)"""
_variableOrderSymbols::Vector{Symbol}
end
Expand Down
22 changes: 11 additions & 11 deletions src/services/AbstractDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ end
$(SIGNATURES)
Retrieve a list of labels of the immediate neighbors around a given variable or factor specified by its label.
"""
function getNeighbors(dfg::AbstractDFG, label::Symbol; solvable::Int=0)
error("getNeighbors not implemented for $(typeof(dfg))")
function listNeighbors(dfg::AbstractDFG, label::Symbol; solvable::Int=0)
error("listNeighbors not implemented for $(typeof(dfg))")
end


Expand Down Expand Up @@ -479,8 +479,8 @@ end
## Connectivity Alias
##------------------------------------------------------------------------------

function getNeighbors(dfg::AbstractDFG, node::DFGNode; solvable::Int=0)
getNeighbors(dfg, node.label, solvable=solvable)
function listNeighbors(dfg::AbstractDFG, node::DFGNode; solvable::Int=0)
listNeighbors(dfg, node.label; solvable)
end


Expand Down Expand Up @@ -621,14 +621,14 @@ end
Retrieve a list of labels of the immediate neighbors around a given variable or factor.
"""
function ls(dfg::G, node::T; solvable::Int=0) where {G <: AbstractDFG, T <: DFGNode}
return getNeighbors(dfg, node, solvable=solvable)
return listNeighbors(dfg, node, solvable=solvable)
end
function ls(dfg::G, label::Symbol; solvable::Int=0) where G <: AbstractDFG
return getNeighbors(dfg, label, solvable=solvable)
return listNeighbors(dfg, label, solvable=solvable)
end

function lsf(dfg::G, label::Symbol; solvable::Int=0)::Vector{Symbol} where G <: AbstractDFG
return getNeighbors(dfg, label, solvable=solvable)
return listNeighbors(dfg, label, solvable=solvable)
end

## list by types
Expand Down Expand Up @@ -1146,7 +1146,7 @@ function getNeighborhood(dfg::AbstractDFG, label::Symbol, distance::Int)
for dist in 1:distance
newNeighbors = Set{Symbol}()
for node in curList
neighbors = getNeighbors(dfg, node)
neighbors = listNeighbors(dfg, node)
for neighbor in neighbors
push!(neighborList, neighbor)
push!(newNeighbors, neighbor)
Expand Down Expand Up @@ -1313,7 +1313,7 @@ function getAdjacencyMatrixSymbols(dfg::AbstractDFG; solvable::Int=0)
adjMat[2:end, 1] = factLabels
adjMat[1, 2:end] = varLabels
for (fIndex, factLabel) in enumerate(factLabels)
factVars = getNeighbors(dfg, getFactor(dfg, factLabel), solvable=solvable)
factVars = listNeighbors(dfg, getFactor(dfg, factLabel), solvable=solvable)
map(vLabel -> adjMat[fIndex+1,vDict[vLabel]] = factLabel, factVars)
end
return adjMat
Expand Down Expand Up @@ -1341,7 +1341,7 @@ function getBiadjacencyMatrix(dfg::AbstractDFG; solvable::Int=0)
adjMat = spzeros(Int, length(factLabels), length(varLabels))

for (fIndex, factLabel) in enumerate(factLabels)
factVars = getNeighbors(dfg, getFactor(dfg, factLabel), solvable=solvable)
factVars = listNeighbors(dfg, getFactor(dfg, factLabel), solvable=solvable)
map(vLabel -> adjMat[fIndex,vDict[vLabel]] = 1, factVars)
end
return (B=adjMat, varLabels=varLabels, facLabels=factLabels)
Expand Down Expand Up @@ -1426,7 +1426,7 @@ function getSummaryGraph(dfg::G) where {G <: AbstractDFG}
# newV = addVariable!(summaryDfg, DFGVariableSummary(v))
# end
# for f in getFactors(dfg)
# addFactor!(summaryDfg, getNeighbors(dfg, f), DFGFactorSummary(f))
# addFactor!(summaryDfg, listNeighbors(dfg, f), DFGFactorSummary(f))
# end
return summaryDfg
end
2 changes: 1 addition & 1 deletion src/services/DFGFactor.jl
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ end
$SIGNATURES
Get the variable ordering for this factor.
Should be equivalent to getNeighbors unless something was deleted in the graph.
Should be equivalent to listNeighbors unless something was deleted in the graph.
"""
getVariableOrder(fct::DFGFactor) = fct._variableOrderSymbols::Vector{Symbol}
getVariableOrder(dfg::AbstractDFG, fct::Symbol) = getVariableOrder(getFactor(dfg, fct))
Expand Down
24 changes: 12 additions & 12 deletions test/iifInterfaceTests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -401,22 +401,22 @@ facts = map(n ->
# Trivial test to validate that intersect([], []) returns order of first parameter
@test intersect([:x3, :x2, :x1], [:x1, :x2]) == [:x2, :x1]
# Get neighbors tests
@test getNeighbors(dfg, verts[1]) == [:x1x2f1]
neighbors = getNeighbors(dfg, getFactor(dfg, :x1x2f1))
@test listNeighbors(dfg, verts[1]) == [:x1x2f1]
neighbors = listNeighbors(dfg, getFactor(dfg, :x1x2f1))
@test neighbors == [:x1, :x2]
# Testing aliases
@test getNeighbors(dfg, getFactor(dfg, :x1x2f1)) == ls(dfg, getFactor(dfg, :x1x2f1))
@test getNeighbors(dfg, :x1x2f1) == ls(dfg, :x1x2f1)
@test listNeighbors(dfg, getFactor(dfg, :x1x2f1)) == ls(dfg, getFactor(dfg, :x1x2f1))
@test listNeighbors(dfg, :x1x2f1) == ls(dfg, :x1x2f1)

# solvable checks
@test getNeighbors(dfg, :x5, solvable=1) == Symbol[]
@test symdiff(getNeighbors(dfg, :x5, solvable=0), [:x4x5f1,:x5x6f1]) == []
@test symdiff(getNeighbors(dfg, :x5),[:x4x5f1,:x5x6f1]) == []
@test getNeighbors(dfg, :x7x8f1, solvable=0) == [:x7, :x8]
@test getNeighbors(dfg, :x7x8f1, solvable=1) == [:x7]
@test getNeighbors(dfg, verts[1], solvable=0) == [:x1x2f1]
@test getNeighbors(dfg, verts[1], solvable=1) == Symbol[]
@test getNeighbors(dfg, verts[1]) == [:x1x2f1]
@test listNeighbors(dfg, :x5, solvable=1) == Symbol[]
@test symdiff(listNeighbors(dfg, :x5, solvable=0), [:x4x5f1,:x5x6f1]) == []
@test symdiff(listNeighbors(dfg, :x5),[:x4x5f1,:x5x6f1]) == []
@test listNeighbors(dfg, :x7x8f1, solvable=0) == [:x7, :x8]
@test listNeighbors(dfg, :x7x8f1, solvable=1) == [:x7]
@test listNeighbors(dfg, verts[1], solvable=0) == [:x1x2f1]
@test listNeighbors(dfg, verts[1], solvable=1) == Symbol[]
@test listNeighbors(dfg, verts[1]) == [:x1x2f1]

end

Expand Down
4 changes: 2 additions & 2 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ struct NotImplementedDFG{T} <: AbstractDFG{T} end
@test_throws ErrorException getVariables(dfg)
@test_throws ErrorException getFactors(dfg)
@test_throws ErrorException isConnected(dfg)
@test_throws ErrorException getNeighbors(dfg, v1)
@test_throws ErrorException getNeighbors(dfg, :a)
@test_throws ErrorException listNeighbors(dfg, v1)
@test_throws ErrorException listNeighbors(dfg, :a)

@test_throws ErrorException _getDuplicatedEmptyDFG(dfg)

Expand Down
24 changes: 12 additions & 12 deletions test/testBlocks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1366,24 +1366,24 @@ function GettingNeighbors(testDFGAPI; VARTYPE=DFGVariable, FACTYPE=DFGFactor)
# Trivial test to validate that intersect([], []) returns order of first parameter
@test intersect([:x3, :x2, :x1], [:x1, :x2]) == [:x2, :x1]
# Get neighbors tests
@test getNeighbors(dfg, verts[1]) == [:x1x2f1]
neighbors = getNeighbors(dfg, getFactor(dfg, :x1x2f1))
@test listNeighbors(dfg, verts[1]) == [:x1x2f1]
neighbors = listNeighbors(dfg, getFactor(dfg, :x1x2f1))
@test neighbors == [:x1, :x2]
# Testing aliases
@test getNeighbors(dfg, getFactor(dfg, :x1x2f1)) == ls(dfg, getFactor(dfg, :x1x2f1))
@test getNeighbors(dfg, :x1x2f1) == ls(dfg, :x1x2f1)
@test listNeighbors(dfg, getFactor(dfg, :x1x2f1)) == ls(dfg, getFactor(dfg, :x1x2f1))
@test listNeighbors(dfg, :x1x2f1) == ls(dfg, :x1x2f1)

# Solvable
#TODO if not a GraphsDFG with and summary or skeleton
if VARTYPE == DFGVariable
@test getNeighbors(dfg, :x5, solvable=2) == Symbol[]
@test issetequal(getNeighbors(dfg, :x5, solvable=0), [:x4x5f1,:x5x6f1])
@test issetequal(getNeighbors(dfg, :x5), [:x4x5f1,:x5x6f1])
@test getNeighbors(dfg, :x7x8f1, solvable=0) == [:x7, :x8]
@test getNeighbors(dfg, :x7x8f1, solvable=1) == [:x7]
@test getNeighbors(dfg, verts[1], solvable=0) == [:x1x2f1]
@test getNeighbors(dfg, verts[1], solvable=2) == Symbol[]
@test getNeighbors(dfg, verts[1]) == [:x1x2f1]
@test listNeighbors(dfg, :x5, solvable=2) == Symbol[]
@test issetequal(listNeighbors(dfg, :x5, solvable=0), [:x4x5f1,:x5x6f1])
@test issetequal(listNeighbors(dfg, :x5), [:x4x5f1,:x5x6f1])
@test listNeighbors(dfg, :x7x8f1, solvable=0) == [:x7, :x8]
@test listNeighbors(dfg, :x7x8f1, solvable=1) == [:x7]
@test listNeighbors(dfg, verts[1], solvable=0) == [:x1x2f1]
@test listNeighbors(dfg, verts[1], solvable=2) == Symbol[]
@test listNeighbors(dfg, verts[1]) == [:x1x2f1]
end

end
Expand Down

0 comments on commit 5d6a35f

Please sign in to comment.