Merge pull request #356 from JuliaRobotics/fix/1Q20/354

fix integration tests 354

Author: dehann

.travis.yml

@@ -41,7 +41,7 @@ matrix:
   allow_failures:
   - julia: nightly
   - arch: arm64
-  - env: IIF_TEST=true
+  #- env: IIF_TEST=true
 
 # Set the password for Neo4j to neo4j:test
 before_script:

docs/src/GraphData.md

@@ -111,7 +111,7 @@ Example of PPE operations:
 # Add a new PPE of type MeanMaxPPE to :x0
 ppe = MeanMaxPPE(:default, [0.0], [0.0], [0.0])
 addPPE!(dfg, :x0, ppe)
-@show listPPE(dfg, :x0)
+@show listPPEs(dfg, :x0)
 # Get the data back - note that this is a reference to above.
 v = getPPE(dfg, :x0, :default)
 # Delete it

src/CloudGraphsDFG/services/CloudGraphsDFG.jl

@@ -181,7 +181,7 @@ function getVariable(dfg::CloudGraphsDFG, label::Union{Symbol, String})::DFGVari
     variable = unpackVariable(dfg, props)
 
     # TODO - make this get PPE's in batch
-    for ppe in listPPE(dfg, label)
+    for ppe in listPPEs(dfg, label)
         variable.ppeDict[ppe] = getPPE(dfg, label, ppe)
     end
     return variable
@@ -220,11 +220,12 @@ function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)::DFGVariabl
     return variable
 end
 
-function mergeUpdateVariableSolverData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable)::DFGVariable
+function mergeVariableData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable)::DFGVariable
     if !exists(dfg, sourceVariable)
         error("Source variable '$(sourceVariable.label)' doesn't exist in the graph.")
     end
     for (k,v) in sourceVariable.ppeDict
+        # TODO what is happening inside, is this truely an update or is it a merge? (API consistency hounds are apon you)
         updatePPE!(dfg, getLabel(sourceVariable), v, k)
     end
     for (k,v) in sourceVariable.solverDataDict
@@ -233,6 +234,11 @@ function mergeUpdateVariableSolverData!(dfg::CloudGraphsDFG, sourceVariable::DFG
     return sourceVariable
 end
 
+function mergeUpdateVariableSolverData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable)::DFGVariable
+  @warn "Deprecated CloudGraphsDFG.mergeUpdateVariableSolverData!, use mergeVariableData! instead."
+  mergeVariableData!(dfg, sourceVariable)
+end
+
 function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor)::DFGFactor
     if !exists(dfg, factor)
         @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
@@ -435,7 +441,7 @@ end
 
 ### PPEs with DB calls
 
-function listPPE(dfg::CloudGraphsDFG, variablekey::Symbol; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::Vector{Symbol}
+function listPPEs(dfg::CloudGraphsDFG, variablekey::Symbol; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::Vector{Symbol}
     query = "match (ppe:$(join(_getLabelsForType(dfg, MeanMaxPPE, parentKey=variablekey),':'))) return ppe.solverKey"
     @debug "[Query] PPE read query:\r\n$query"
     result = nothing
@@ -469,7 +475,7 @@ function getPPE(dfg::CloudGraphsDFG, variablekey::Symbol, ppekey::Symbol=:defaul
 end
 
 function addPPE!(dfg::CloudGraphsDFG, variablekey::Symbol, ppe::P, ppekey::Symbol=:default; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::AbstractPointParametricEst where P <: AbstractPointParametricEst
-    if ppekey in listPPE(dfg, variablekey, currentTransaction=currentTransaction)
+    if ppekey in listPPEs(dfg, variablekey, currentTransaction=currentTransaction)
         error("PPE '$(ppekey)' already exists")
     end
     return updatePPE!(dfg, variablekey, ppe, ppekey, currentTransaction=currentTransaction)

src/Deprecated.jl

@@ -12,6 +12,7 @@
 
 @deprecate getFactorIds(dfg, regexFilter=nothing; solvable=0) listFactors(dfg, regexFilter, solvable=solvable)
 
+@deprecate listPPE(args...) listPPEs(args...)
 
 export getLabelDict
 getLabelDict(dfg::AbstractDFG) = error("getLabelDict is deprecated, consider using listing functions")

src/DistributedFactorGraphs.jl

@@ -141,17 +141,15 @@ export getMaxPPE, getMeanPPE, getSuggestedPPE, getLastUpdatedTimestamp
 
 # accessors
 
-export getPPEDict
+export getPPEDict, getVariablePPEDict, getVariablePPE
 
 # CRUD & SET
 export getPPE,
-       getPPEs,
        getVariablePPE,
-       getVariablePPEs,
        addPPE!,
        updatePPE!,
        deletePPE!,
-       listPPE,
+       listPPEs,
        mergePPEs! #TODO look at rename to just mergePPE to match with cloud?
 
 

src/needsahome.jl

@@ -104,13 +104,6 @@ printVariable(dfg::AbstractDFG, sym::Symbol, solveKey::Symbol=:default) = print(
 print(dfg::AbstractDFG, sym::Symbol) = isVariable(dfg,sym) ? printVariable(dfg, sym) : printFactor(dfg, sym)
 
 
-#NOT too many aliases on PPE, brought back from deprecated DF
-
-getVariablePPEs(v::VariableDataLevel1) = getPPEDict(v)
-
-getPPEs(vari::VariableDataLevel1) = getPPEDict(v)
-
-getVariablePPE(args...) = getPPE(args...)
 
 ## KEEPING COMMENT, WANT TO BE CONSOLIDATED WITH FUNCTION ABOVE -- KEEPING ONLY ONE FOR MAINTAINABILITY
 ## STILL NEEDS TO BE CONSOLIDATED WITH `DFG._copyIntoGraph`

src/services/DFGVariable.jl

@@ -196,7 +196,7 @@ end
 """
     $SIGNATURES
 
-Get the PPE dictionary for a variable. Its direct use is not recomended.
+Get the PPE dictionary for a variable.  Recommended to use CRUD operations instead, [`getPPE`](@ref), [`addPPE!`](@ref), [`updatePPE!`](@ref), [`deletePPE!`](@ref).
 """
 getPPEDict(v::VariableDataLevel1) = v.ppeDict
 
@@ -219,14 +219,25 @@ function getPPE(vari::VariableDataLevel1, solveKey::Symbol=:default)
     # return haskey(ppeDict, solveKey) ? ppeDict[solveKey] : nothing
 end
 
+# afew more aliases on PPE, brought back from deprecated DF
+
+"""
+    $SIGNATURES
+
+Return full dictionary of PPEs in a variable, recommended to rather use CRUD: [`getPPE`](@ref), 
+"""
+getVariablePPEDict(vari::VariableDataLevel1) = getPPEDict(vari)
+
+getVariablePPE(args...) = getPPE(args...)
+
 ##------------------------------------------------------------------------------
 ## solverDataDict
 ##------------------------------------------------------------------------------
 
 """
     $SIGNATURES
 
-Get solver data dictionary for a variable.
+Get solver data dictionary for a variable.  Advised to use graph CRUD operations instead.
 """
 getSolverDataDict(v::DFGVariable) = v.solverDataDict
 
@@ -427,6 +438,9 @@ function mergeVariableSolverData!(destVariable::DFGVariable, sourceVariable::DFG
     return destVariable
 end
 
+mergeVariableSolverData!(dfg::AbstractDFG, sourceVariable::DFGVariable) =     mergeVariableSolverData!(getVariable(dfg,getLabel(sourceVariable)), sourceVariable)
+
+
 ##==============================================================================
 ## Point Parametric Estimates
 ##==============================================================================
@@ -478,7 +492,7 @@ addPPE!(dfg::AbstractDFG, sourceVariable::DFGVariable, ppekey::Symbol=:default)
 
 """
     $(SIGNATURES)
-Update PPE data if it exists, otherwise add it.
+Update PPE data if it exists, otherwise add it -- one call per `key::Symbol=:default`.
 """
 function updatePPE!(dfg::AbstractDFG, variablekey::Symbol, ppe::P, ppekey::Symbol=:default)::P where P <: AbstractPointParametricEst
 
@@ -539,7 +553,7 @@ deletePPE!(dfg::AbstractDFG, sourceVariable::DFGVariable, ppekey::Symbol=:defaul
     $(SIGNATURES)
 List all the PPE data keys in the variable.
 """
-function listPPE(dfg::AbstractDFG, variablekey::Symbol)::Vector{Symbol}
+function listPPEs(dfg::AbstractDFG, variablekey::Symbol)::Vector{Symbol}
     v = getVariable(dfg, variablekey)
     return collect(keys(v.ppeDict))
 end

test/LightDFGSummaryTypes.jl

@@ -40,7 +40,7 @@ end
 
     if VARTYPE == DFGVariableSummary
         @test getTimestamp(v1) == v1.timestamp
-        @test getVariablePPEs(v1) == v1.ppeDict
+        @test getVariablePPEDict(v1) == v1.ppeDict
         @test_throws KeyError getVariablePPE(v1, :notfound)
         @test getSofttypename(v1) == :Pose2
         @test getInternalId(v1) == v1._internalId

test/fileDFGTests.jl

@@ -40,7 +40,7 @@ using Test
 
         copyDfg = DistributedFactorGraphs._getDuplicatedEmptyDFG(dfg)
         @info "Going to load $filename"
-        retDFG = loadDFG(filename, Main, copyDfg, loaddir="/tmp")
+        retDFG = loadDFG(filename, Main, copyDfg) #, loaddir="/tmp")
 
         @test issetequal(ls(dfg), ls(retDFG))
         @test issetequal(lsf(dfg), lsf(retDFG))

test/iifInterfaceTests.jl

@@ -132,10 +132,6 @@ end
     @test @test_deprecated getFactorIds(dfg) == listFactors(dfg)
 
 
-    @test @test_deprecated getfnctype(f1.solverData) === getFactorType(f1.solverData)
-    @test @test_deprecated getfnctype(f1) === getFactorType(f1)
-    @test @test_deprecated getfnctype(dfg, :abf1) === getFactorType(dfg, :abf1)
-
     @test getFactorType(f1.solverData) === f1.solverData.fnc.usrfnc!
     @test getFactorType(f1) === f1.solverData.fnc.usrfnc!
     @test getFactorType(dfg, :abf1) === f1.solverData.fnc.usrfnc!
@@ -186,15 +182,15 @@ end
     @test getLabel(v1) == v1.label
     @test getTags(v1) == v1.tags
     @test getTimestamp(v1) == v1.timestamp
-    @test getVariablePPEs(v1) == v1.ppeDict
+    @test getVariablePPEDict(v1) == v1.ppeDict
     @test_throws Exception DistributedFactorGraphs.getVariablePPE(v1, :notfound)
-    @test solverData(v1) === v1.solverDataDict[:default]
     @test getSolverData(v1) === v1.solverDataDict[:default]
-    @test solverData(v1, :default) === v1.solverDataDict[:default]
+    @test getSolverData(v1) === v1.solverDataDict[:default]
+    @test getSolverData(v1, :default) === v1.solverDataDict[:default]
     @test getSolverDataDict(v1) == v1.solverDataDict
     @test getInternalId(v1) == v1._internalId
     # legacy compat test
-    @test getVariablePPEs(v1) == v1.estimateDict # changed to .ppeDict -- delete by DFG v0.7
+    @test getVariablePPEDict(v1) == v1.ppeDict # changed to .ppeDict -- delete by DFG v0.7
 
 
     @test typeof(getSofttype(v1)) == ContinuousScalar
@@ -203,10 +199,10 @@ end
 
     @test getLabel(f1) == f1.label
     @test getTags(f1) == f1.tags
-    @test solverData(f1) == f1.solverData
+    @test getSolverData(f1) == f1.solverData
 
     # Internal function
-    @test @test_deprecated internalId(f1) == f1._internalId
+    # @test @test_deprecated internalId(f1) == f1._internalId
 
     @test getSolverParams(dfg) != nothing
     @test setSolverParams!(dfg, getSolverParams(dfg)) == getSolverParams(dfg)
@@ -284,42 +280,48 @@ end
     var1 = getVariable(dfg, :a)
     #make a copy and simulate external changes
     newvar = deepcopy(var1)
-    getVariablePPEs(newvar)[:default] = MeanMaxPPE(:default, [150.0], [100.0], [50.0])
+    getVariablePPEDict(newvar)[:default] = MeanMaxPPE(:default, [150.0], [100.0], [50.0])
     #update
-    mergeUpdateVariableSolverData!(dfg, newvar)
+    # mergeUpdateVariableSolverData!(dfg, newvar)
+    # mergeVariableSolverData!(getVariable(dfg,getLabel(newvar)), newvar)
+    # mergeVariableData!(getVariable(dfg,getLabel(newvar)), newvar)
+    mergeVariableData!(dfg, newvar)
 
     #Check if variable is updated
     var1 = getVariable(dfg, :a)
-    @test getVariablePPEs(newvar) == getVariablePPEs(var1)
+    @test getVariablePPEDict(newvar) == getVariablePPEDict(var1)
 
     # Add a new estimate.
-    getVariablePPEs(newvar)[:second] = MeanMaxPPE(:second, [15.0], [10.0], [5.0])
+    getVariablePPEDict(newvar)[:second] = MeanMaxPPE(:second, [15.0], [10.0], [5.0])
 
     # Confirm they're different
-    @test getVariablePPEs(newvar) != getVariablePPEs(var1)
+    @test getVariablePPEDict(newvar) != getVariablePPEDict(var1)
     # Persist it.
-    mergeUpdateVariableSolverData!(dfg, newvar)
+    # mergeUpdateVariableSolverData!(dfg, newvar)
+    # mergeVariableSolverData!(getVariable(dfg, getLabel(newvar)), newvar)
+    mergeVariableData!(dfg, newvar)
     # Get the latest
     var1 = getVariable(dfg, :a)
-    @test symdiff(collect(keys(getVariablePPEs(var1))), [:default, :second]) == Symbol[]
+    @test symdiff(collect(keys(getVariablePPEDict(var1))), [:default, :second]) == Symbol[]
 
     #Check if variable is updated
-    @test getVariablePPEs(newvar) == getVariablePPEs(var1)
+    @test getVariablePPEDict(newvar) == getVariablePPEDict(var1)
 
 
     # Delete :default and replace to see if new ones can be added
-    delete!(getVariablePPEs(newvar), :default)
+    delete!(getVariablePPEDict(newvar), :default)
     #confirm delete
-    @test symdiff(collect(keys(getVariablePPEs(newvar))), [:second]) == Symbol[]
-    # Persist it.
-    mergeUpdateVariableSolverData!(dfg, newvar)
+    @test symdiff(collect(keys(getVariablePPEDict(newvar))), [:second]) == Symbol[]
+    # Persist it., and test
+    mergeVariableData!(dfg, newvar)
+    # mergeUpdateVariableSolverData!(dfg, newvar)  #357 #358
 
     # Get the latest and confirm they're the same, :second
     var1 = getVariable(dfg, :a)
 
     # TODO issue #166
-    @test getVariablePPEs(newvar) != getVariablePPEs(var1)
-    @test collect(keys(getVariablePPEs(var1))) ==  [:default, :second]
+    @test getVariablePPEDict(newvar) != getVariablePPEDict(var1)
+    @test collect(keys(getVariablePPEDict(var1))) ==  [:default, :second]
 
     # @test symdiff(collect(keys(getVariablePPE(getVariable(dfg, :a)))), [:default, :second]) == Symbol[]
 end
@@ -395,7 +397,7 @@ verts = map(n -> addVariable!(dfg, Symbol("x$n"), ContinuousScalar, labels = [:P
 #TODO fix this to use accessors
 setSolvable!(verts[7], 1)
 setSolvable!(verts[8], 0)
-solverData(verts[8]).solveInProgress = 1
+getSolverData(verts[8]).solveInProgress = 1
 #call update to set it on cloud
 updateVariable!(dfg, verts[7])
 updateVariable!(dfg, verts[8])