API Standardization through testing (#276)

* API standardization through testing WIP

* deprecated clean up

* listVariables, listFactors, add update return type

* **TODO removed compares == to be fixed later**

* fix for julia 1.0 in test

* Exists,  getVariables/factors isVar/Fac, move attic

* no interface tests

* TODO compare and confirm pack/unpack changes. Compare skiped, rest pass

* IIF_TESTS does not work on julia 1.0

Author: Affie

.travis.yml

@@ -13,13 +13,13 @@ services:
     - neo4j
 
 julia:
-    - 1.0
-#FIXME#    - 1.2
+#FIXME#    - 1.0
+    - 1.2
 #FIXME#    - 1.3
 #FIXME#    - nightly
 
 env:
-    - IIF_TEST=false
+    - IIF_TEST=true
 
 #FIXME#jobs:
 #FIXME#  include:

src/CloudGraphsDFG/services/CGStructure.jl

@@ -282,7 +282,7 @@ function copySession!(sourceDFG::CloudGraphsDFG, destDFG::Union{Nothing, CloudGr
     if destDFG == nothing
         destDFG = _getDuplicatedEmptyDFG(sourceDFG)
     end
-    _copyIntoGraph!(sourceDFG, destDFG, union(getVariableIds(sourceDFG), getFactorIds(sourceDFG)), true)
+    _copyIntoGraph!(sourceDFG, destDFG, union(listVariables(sourceDFG), listFactors(sourceDFG)), true)
     return destDFG
 end
 """

src/CloudGraphsDFG/services/CloudGraphsDFG.jl

@@ -69,7 +69,7 @@ isVariable(dfg::CloudGraphsDFG, sym::Symbol)::Bool =
 isFactor(dfg::CloudGraphsDFG, sym::Symbol)::Bool =
     _getNodeCount(dfg.neo4jInstance, ["FACTOR", dfg.userId, dfg.robotId, dfg.sessionId, String(sym)]) == 1
 
-function addVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)::Bool
+function addVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)::DFGVariable
     if exists(dfg, variable)
         error("Variable '$(variable.label)' already exists in the factor graph")
     end
@@ -89,10 +89,14 @@ function addVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)::Bool
     # Track insertion
     push!(dfg.addHistory, variable.label)
 
-    return true
+    return variable
+end
+
+function addFactor!(dfg::CloudGraphsDFG, factor::DFGFactor)
+    addFactor!(dfg, factor._variableOrderSymbols, factor)
 end
 
-function addFactor!(dfg::CloudGraphsDFG, variables::Vector{DFGVariable}, factor::DFGFactor)::Bool
+function addFactor!(dfg::CloudGraphsDFG, variables::Vector{<:DFGVariable}, factor::DFGFactor)::DFGFactor
     if exists(dfg, factor)
         error("Factor '$(factor.label)' already exists in the factor graph")
     end
@@ -120,10 +124,10 @@ function addFactor!(dfg::CloudGraphsDFG, variables::Vector{DFGVariable}, factor:
     # Track insertion only for variables
     # push!(dfg.addHistory, factor.label
 
-    return true
+    return factor
 end
 
-function addFactor!(dfg::CloudGraphsDFG, variableIds::Vector{Symbol}, factor::DFGFactor)::Bool
+function addFactor!(dfg::CloudGraphsDFG, variableIds::Vector{Symbol}, factor::DFGFactor)::DFGFactor
     variables = map(vId -> getVariable(dfg, vId), variableIds)
     return addFactor!(dfg, variables, factor)
 end
@@ -187,7 +191,8 @@ end
 
 function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)::DFGVariable
     if !exists(dfg, variable)
-        error("Variable label '$(variable.label)' does not exist in the factor graph")
+        @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
+        return addVariable!(dfg, variable)
     end
     nodeId = _tryGetNeoNodeIdFromNodeLabel(dfg.neo4jInstance, dfg.userId, dfg.robotId, dfg.sessionId, variable.label)
     # Update the node ID
@@ -217,7 +222,8 @@ end
 
 function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor)::DFGFactor
     if !exists(dfg, factor)
-        error("Factor label '$(factor.label)' does not exist in the factor graph")
+        @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
+        return addFactor!(dfg, factor)
     end
     nodeId = _tryGetNeoNodeIdFromNodeLabel(dfg.neo4jInstance, dfg.userId, dfg.robotId, dfg.sessionId, factor.label)
     # Update the _internalId
@@ -316,7 +322,7 @@ end
 deleteFactor!(dfg::CloudGraphsDFG, factor::DFGFactor)::DFGFactor = deleteFactor!(dfg, factor.label)
 
 function getVariables(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; tags::Vector{Symbol}=Symbol[], solvable::Int=0)::Vector{DFGVariable}
-    variableIds = getVariableIds(dfg, regexFilter, tags=tags, solvable=solvable)
+    variableIds = listVariables(dfg, regexFilter, tags=tags, solvable=solvable)
     # TODO: Optimize to use tags in query here!
     variables = map(vId->getVariable(dfg, vId), variableIds)
     if length(tags) > 0
@@ -326,7 +332,7 @@ function getVariables(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=no
     return variables
 end
 
-function getVariableIds(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; tags::Vector{Symbol}=Symbol[], solvable::Int=0)::Vector{Symbol}
+function listVariables(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; tags::Vector{Symbol}=Symbol[], solvable::Int=0)::Vector{Symbol}
     # Optimized for DB call
     tagsFilter = length(tags) > 0 ? " and "*join("node:".*String.(tags), " or ") : ""
     if regexFilter == nothing
@@ -337,11 +343,11 @@ function getVariableIds(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=
 end
 
 function getFactors(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; solvable::Int=0)::Vector{DFGFactor}
-    factorIds = getFactorIds(dfg, regexFilter, solvable=solvable)
+    factorIds = listFactors(dfg, regexFilter, solvable=solvable)
     return map(vId->getFactor(dfg, vId), factorIds)
 end
 
-function getFactorIds(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; solvable::Int=0)::Vector{Symbol}
+function listFactors(dfg::CloudGraphsDFG, regexFilter::Union{Nothing, Regex}=nothing; solvable::Int=0)::Vector{Symbol}
     # Optimized for DB call
     if regexFilter == nothing
         return _getLabelsFromCyphonQuery(dfg.neo4jInstance, "(node:$(dfg.userId):$(dfg.robotId):$(dfg.sessionId):FACTOR) where node.solvable >= $solvable")
@@ -353,8 +359,8 @@ end
 function isFullyConnected(dfg::CloudGraphsDFG)::Bool
     # If the total number of nodes == total number of distinct connected nodes, then it is fully connected
     # Total nodes
-    varIds = getVariableIds(dfg)
-    factIds = getFactorIds(dfg)
+    varIds = listVariables(dfg)
+    factIds = listFactors(dfg)
     length(varIds) + length(factIds) == 0 && return false
 
     # Total connected nodes - thank you Neo4j for 0..* awesomeness!!
@@ -424,8 +430,8 @@ function getSubgraph(dfg::CloudGraphsDFG,
 end
 
 function getIncidenceMatrix(dfg::CloudGraphsDFG; solvable::Int=0)::Matrix{Union{Nothing, Symbol}}
-    varLabels = sort(getVariableIds(dfg, solvable=solvable))
-    factLabels = sort(getFactorIds(dfg, solvable=solvable))
+    varLabels = sort(listVariables(dfg, solvable=solvable))
+    factLabels = sort(listFactors(dfg, solvable=solvable))
     vDict = Dict(varLabels .=> [1:length(varLabels)...].+1)
     fDict = Dict(factLabels .=> [1:length(factLabels)...].+1)
 
@@ -454,8 +460,8 @@ function getIncidenceMatrix(dfg::CloudGraphsDFG; solvable::Int=0)::Matrix{Union{
 end
 
 function getIncidenceMatrixSparse(dfg::CloudGraphsDFG; solvable::Int=0)::Tuple{SparseMatrixCSC, Vector{Symbol}, Vector{Symbol}}
-    varLabels = getVariableIds(dfg, solvable=solvable)
-    factLabels = getFactorIds(dfg, solvable=solvable)
+    varLabels = listVariables(dfg, solvable=solvable)
+    factLabels = listFactors(dfg, solvable=solvable)
     vDict = Dict(varLabels .=> [1:length(varLabels)...])
     fDict = Dict(factLabels .=> [1:length(factLabels)...])
 

src/Common.jl

@@ -348,7 +348,7 @@ DevNotes:
 
 Related
 
-ls, getVariableIds, findClosestTimestamp
+ls, listVariables, findClosestTimestamp
 """
 function findVariableNearTimestamp(dfg::AbstractDFG,
                                    timest::DateTime,
@@ -359,8 +359,8 @@ function findVariableNearTimestamp(dfg::AbstractDFG,
                                    number::Int=1  )::Vector{Tuple{Vector{Symbol}, Millisecond}}
   #
   # get the variable labels based on filters
-  # syms = getVariableIds(dfg, regexFilter, tags=tags, solvable=solvable)
-  syms = getVariableIds(dfg, regexFilter, tags=tags, solvable=solvable)
+  # syms = listVariables(dfg, regexFilter, tags=tags, solvable=solvable)
+  syms = listVariables(dfg, regexFilter, tags=tags, solvable=solvable)
   # compile timestamps with label
   # vars = map( x->getVariable(dfg, x), syms )
   timeset = map(x->(getTimestamp(getVariable(dfg,x)), x), syms)
@@ -406,7 +406,8 @@ end
 Add tags to a variable or factor
 """
 function addTags!(dfg::InMemoryDFGTypes, sym::Symbol, tags::Vector{Symbol})
-  union!(getTags(getFactor(fg, sym)), tags)
+  getFnc = isVariable(dfg,sym) ? getVariable : getFactor
+  union!(getTags(getFnc(dfg, sym)), tags)
 end
 
 

src/CommonAccessors.jl

@@ -22,7 +22,7 @@ $SIGNATURES
 
 Set the tags for a node.
 """
-function setTags!(f::DataLevel0, tags::Vector{Symbol})
+function setTags!(f::DataLevel0, tags::Union{Vector{Symbol},Set{Symbol}})
   empty!(f.tags)
   union!(f.tags, tags)
 end
@@ -53,6 +53,14 @@ end
 
 setTimestamp!(f::FactorDataLevel1, ts::DateTime) = f.timestamp = ts
 
+function setTimestamp(f::DFGFactor, ts::DateTime)
+    return DFGFactor(f.label, ts, f.tags, f.solverData, f.solvable, f._dfgNodeParams, f._variableOrderSymbols)
+end
+
+function setTimestamp(f::DFGFactorSummary, ts::DateTime)
+    return DFGFactorSummary(f.label, ts, f.tags, f._internalId, f._variableOrderSymbols)
+end
+
 """
 $SIGNATURES
 

src/DFGPlots/DFGPlots.jl

@@ -88,7 +88,7 @@ More information at [GraphPlot.jl](https://github.com/JuliaGraphs/GraphPlot.jl)
 function dfgplot(dfg::AbstractDFG, p::DFGPlotProps = DFGPlotProps())
     # TODO implement convert functions
     ldfg = LightDFG{NoSolverParams}()
-    DistributedFactorGraphs._copyIntoGraph!(dfg, ldfg, union(getVariableIds(dfg), getFactorIds(dfg)), true, copyGraphMetadata=false)
+    DistributedFactorGraphs._copyIntoGraph!(dfg, ldfg, union(listVariables(dfg), listFactors(dfg)), true, copyGraphMetadata=false)
     dfgplot(ldfg, p)
 end