Add warn_if_absent to update verbs (#648)

Author: Affie

src/CloudGraphsDFG/services/CloudGraphsDFG.jl

@@ -164,15 +164,15 @@ function addVariable!(dfg::CloudGraphsDFG, variable::DFGVariable)
         error("Variable '$(variable.label)' already exists in the factor graph")
     end
 
-    ret = updateVariable!(dfg, variable, skipAddError=true)
+    ret = updateVariable!(dfg, variable, warn_if_absent=false)
 
     # Do a variable update
     return ret
 end
 
-function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; skipAddError::Bool=false)
+function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; warn_if_absent::Bool=true)
     exist = exists(dfg, variable)
-    !skipAddError && !exist && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
+    warn_if_absent && !exist && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
 
     # Create/update the base variable
     # NOTE: We are not merging the variable.tags into the labels anymore. We can index by that but not
@@ -196,7 +196,7 @@ function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; skipAddErro
         # length(result.results[1]["data"][1]["row"]) != 1 && error("Cannot update or add variable '$(getLabel(variable))'")
 
         # Merge the PPE's, SolverData, and BigData
-        mergeVariableData!(dfg, variable; currentTransaction=tx, skipExistenceCheck=true)
+        mergeVariableData!(dfg, variable; currentTransaction=tx, warn_if_absent=false)
 
         commit(tx)
     catch ex
@@ -220,7 +220,7 @@ function addFactor!(dfg::CloudGraphsDFG, factor::DFGFactor)
     end
 
     # Do a variable update
-    return updateFactor!(dfg, factor, skipAddError=true)
+    return updateFactor!(dfg, factor, warn_if_absent=false)
 end
 
 function getVariable(dfg::CloudGraphsDFG, label::Union{Symbol, String})
@@ -247,8 +247,8 @@ function getVariable(dfg::CloudGraphsDFG, label::Union{Symbol, String})
     return variable
 end
 
-function mergeVariableData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, skipExistenceCheck::Bool=false)
-    if !skipExistenceCheck & !exists(dfg, sourceVariable, currentTransaction=currentTransaction)
+function mergeVariableData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, warn_if_absent::Bool=true)
+    if warn_if_absent && !exists(dfg, sourceVariable, currentTransaction=currentTransaction)
         error("Source variable '$(sourceVariable.label)' doesn't exist in the graph.")
     end
     for (k,v) in sourceVariable.ppeDict
@@ -273,9 +273,9 @@ function getFactor(dfg::CloudGraphsDFG, label::Union{Symbol, String})
     return rebuildFactorMetadata!(dfg, unpackFactor(dfg, props))
 end
 
-function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor; skipAddError::Bool=false)
+function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor; warn_if_absent::Bool=true)
     exist = exists(dfg, factor)
-    !skipAddError && !exist && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
+    warn_if_absent && !exist && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
 
     if exist
         # Check that the neighbors are the same
@@ -599,10 +599,11 @@ end
 function updatePPE!(
         dfg::CloudGraphsDFG,
         variablekey::Symbol,
-        ppe::P;
-        currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::P where
-        {P <: AbstractPointParametricEst}
-    if !(ppe.solveKey in listPPEs(dfg, variablekey, currentTransaction=currentTransaction))
+        ppe::AbstractPointParametricEst;
+        currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing,
+        warn_if_absent::Bool=true)
+
+    if warn_if_absent && !(ppe.solveKey in listPPEs(dfg, variablekey, currentTransaction=currentTransaction))
         @warn "PPE '$(ppe.solveKey)' does not exist, adding"
     end
     softType = getSofttype(dfg, variablekey, currentTransaction=currentTransaction)
@@ -618,10 +619,13 @@ function updatePPE!(
         currentTransaction=currentTransaction))
 end
 
-function updatePPE!(dfg::CloudGraphsDFG, sourceVariables::Vector{<:DFGVariable}, ppekey::Symbol=:default; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)
+function updatePPE!(dfg::CloudGraphsDFG, sourceVariables::Vector{<:DFGVariable}, ppekey::Symbol=:default; 
+                    currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing,
+                    warn_if_absent::Bool=true)
+    
     tx = currentTransaction == nothing ? transaction(dfg.neo4jInstance.connection) : currentTransaction
     for var in sourceVariables
-        updatePPE!(dfg, var.label, getPPE(var, ppekey), currentTransaction=tx)
+        updatePPE!(dfg, var.label, getPPE(var, ppekey), currentTransaction=tx, warn_if_absent=warn_if_absent)
     end
     if currentTransaction == nothing
         result = commit(tx)
@@ -694,8 +698,10 @@ function addDataEntry!(dfg::CloudGraphsDFG, label::Symbol, bde::BlobStoreEntry;
             packed)
 end
 
-function updateDataEntry!(dfg::CloudGraphsDFG, label::Symbol,  bde::BlobStoreEntry; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)
-    if !(bde.label in listDataEntries(dfg, label, currentTransaction=currentTransaction))
+function updateDataEntry!(dfg::CloudGraphsDFG, label::Symbol,  bde::BlobStoreEntry;
+                          currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing,
+                          warn_if_absent::Bool=true)
+    if warn_if_absent && !(bde.label in listDataEntries(dfg, label, currentTransaction=currentTransaction))
         @warn "Data label '$(bde.label)' does not exist, adding"
     end
     packed = _matchmergeVariableSubnode!(
@@ -773,8 +779,9 @@ function updateVariableSolverData!(dfg::CloudGraphsDFG,
                                 vnd::VariableNodeData,
                                 useCopy::Bool=true,
                                 fields::Vector{Symbol}=Symbol[];
+                                warn_if_absent::Bool=true,
                                 currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::VariableNodeData
-    if !(vnd.solveKey in listVariableSolverData(dfg, variablekey, currentTransaction=currentTransaction))
+    if warn_if_absent && !(vnd.solveKey in listVariableSolverData(dfg, variablekey, currentTransaction=currentTransaction))
         @warn "Solver data '$(vnd.solveKey)' does not exist, adding rather than updating."
     end
     # TODO: Update this to use the selective parameters from fields.
@@ -847,10 +854,11 @@ function updateVariableSolverData!(dfg::CloudGraphsDFG,
                                    sourceVariables::Vector{<:DFGVariable},
                                    solvekey::Symbol=:default,
                                    useCopy::Bool=true,
-                                   fields::Vector{Symbol}=Symbol[] )
+                                   fields::Vector{Symbol}=Symbol[];
+                                   warn_if_absent::Bool=true )
     #TODO: Do in bulk for speed.
     for var in sourceVariables
-        updateVariableSolverData!(dfg, var.label, getSolverData(var, solvekey), solvekey, useCopy, fields)
+        updateVariableSolverData!(dfg, var.label, getSolverData(var, solvekey), solvekey, useCopy, fields; warn_if_absent=warn_if_absent)
     end
 end
 

src/Deprecated.jl

@@ -104,5 +104,24 @@ end
 
 @deprecate setSmallData!(v::DFGVariable, smallData::Dict{String, String}) setSmallData!(v, createSymbolDict(smallData)) 
 
+#update deprecation with warn_if_absent
+function updateVariableSolverData!(dfg::AbstractDFG,
+                                   variablekey::Symbol,
+                                   vnd::VariableNodeData,
+                                   useCopy::Bool,
+                                   fields::Vector{Symbol},
+                                   verbose::Bool)
+  Base.depwarn("updateVariableSolverData! argument verbose is deprecated in favor of keyword argument `warn_if_absent`, see #643", :updateVariableSolverData!)
+  updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields; warn_if_absent = verbose)
+end
 
-
+function updateVariableSolverData!(dfg::AbstractDFG,
+                                   variablekey::Symbol,
+                                   vnd::VariableNodeData,
+                                   solveKey::Symbol,
+                                   useCopy::Bool,
+                                   fields::Vector{Symbol},
+                                   verbose::Bool)
+  Base.depwarn("updateVariableSolverData! argument verbose is deprecated in favor of keyword argument `warn_if_absent`, see #643", :updateVariableSolverData!)
+  updateVariableSolverData!(dfg, variablekey, vnd, solveKey, useCopy, fields; warn_if_absent = verbose)
+end

src/LightDFG/services/LightDFG.jl

@@ -90,37 +90,37 @@ function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F}, f
 end
 
 function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F},
-                      factor::AbstractDFGFactor)::F where F <: AbstractDFGFactor
+                      factor::AbstractDFGFactor) where F <: AbstractDFGFactor
     return addFactor!(dfg, F(factor))
 end
 
-function getVariable(dfg::LightDFG, label::Symbol)::AbstractDFGVariable
+function getVariable(dfg::LightDFG, label::Symbol)
     if !haskey(dfg.g.variables, label)
         error("Variable label '$(label)' does not exist in the factor graph")
     end
 
     return dfg.g.variables[label]
 end
 
-function getFactor(dfg::LightDFG, label::Symbol)::AbstractDFGFactor
+function getFactor(dfg::LightDFG, label::Symbol)
     if !haskey(dfg.g.factors, label)
         error("Factor label '$(label)' does not exist in the factor graph")
     end
     return dfg.g.factors[label]
 end
 
-function updateVariable!(dfg::LightDFG, variable::V)::V where V <: AbstractDFGVariable
+function updateVariable!(dfg::LightDFG, variable::AbstractDFGVariable; warn_if_absent::Bool=true)
     if !haskey(dfg.g.variables, variable.label)
-        @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
+        warn_if_absent && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
         return addVariable!(dfg, variable)
     end
     dfg.g.variables[variable.label] = variable
     return variable
 end
 
-function updateFactor!(dfg::LightDFG, factor::F)::F where F <: AbstractDFGFactor
+function updateFactor!(dfg::LightDFG, factor::AbstractDFGFactor; warn_if_absent::Bool=true)
     if !haskey(dfg.g.factors, factor.label)
-        @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
+        warn_if_absent && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
         return addFactor!(dfg, factor)
     end
 

src/services/DFGVariable.jl

@@ -345,9 +345,9 @@ end
     $(SIGNATURES)
 Update a small data pair `key=>value` for variable `label` in `dfg`
 """
-function updateSmallData!(dfg::AbstractDFG, label::Symbol, pair::Pair{Symbol, <:SmallDataTypes})
+function updateSmallData!(dfg::AbstractDFG, label::Symbol, pair::Pair{Symbol, <:SmallDataTypes}; warn_if_absent::Bool=true)
     v = getVariable(dfg, label)
-    !haskey(v.smallData, pair.first) && @warn("$(pair.first) does not exist, adding.")
+    warn_if_absent && !haskey(v.smallData, pair.first) && @warn("$(pair.first) does not exist, adding.")
     push!(v.smallData, pair)
     updateVariable!(dfg, v)
     return v.smallData #or pair TODO
@@ -478,13 +478,12 @@ function updateVariableSolverData!(dfg::AbstractDFG,
                                    variablekey::Symbol,
                                    vnd::VariableNodeData,
                                    useCopy::Bool=true,
-                                   fields::Vector{Symbol}=Symbol[],
-                                   verbose::Bool=true  )
+                                   fields::Vector{Symbol}=Symbol[]; 
+                                   warn_if_absent::Bool=true)
     #This is basically just setSolverData
     var = getVariable(dfg, variablekey)
-    if verbose && !haskey(var.solverDataDict, vnd.solveKey)
-        @warn "VariableNodeData '$(vnd.solveKey)' does not exist, adding"
-    end
+
+    warn_if_absent && !haskey(var.solverDataDict, vnd.solveKey) && @warn "VariableNodeData '$(vnd.solveKey)' does not exist, adding"
 
     # for InMemoryDFGTypes do memory copy or repointing, for cloud this would be an different kind of update.
     usevnd = useCopy ? deepcopy(vnd) : vnd
@@ -510,23 +509,22 @@ function updateVariableSolverData!(dfg::AbstractDFG,
     return var.solverDataDict[vnd.solveKey]
 end
 
-
 function updateVariableSolverData!(dfg::AbstractDFG,
                                    variablekey::Symbol,
                                    vnd::VariableNodeData,
                                    solveKey::Symbol,
                                    useCopy::Bool=true,
-                                   fields::Vector{Symbol}=Symbol[],
-                                   verbose::Bool=true)
+                                   fields::Vector{Symbol}=Symbol[];
+                                   warn_if_absent::Bool=true)
 
     # TODO not very clean
     if vnd.solveKey != solveKey
         @warn("updateVariableSolverData with solveKey parameter might change in the future, see DFG #565. Future warnings are suppressed", maxlog=1) 
         usevnd = useCopy ? deepcopy(vnd) : vnd
         usevnd.solveKey = solveKey
-        return updateVariableSolverData!(dfg, variablekey, usevnd, useCopy, fields, verbose)
+        return updateVariableSolverData!(dfg, variablekey, usevnd, useCopy, fields; warn_if_absent=warn_if_absent)
     else
-        return updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields, verbose)
+        return updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields; warn_if_absent=warn_if_absent)
     end
 end
 
@@ -535,17 +533,19 @@ updateVariableSolverData!(dfg::AbstractDFG,
                           sourceVariable::DFGVariable,
                           solveKey::Symbol=:default,
                           useCopy::Bool=true,
-                          fields::Vector{Symbol}=Symbol[] ) =
-    updateVariableSolverData!(dfg, sourceVariable.label, getSolverData(sourceVariable, solveKey), useCopy, fields)
+                          fields::Vector{Symbol}=Symbol[];
+                          warn_if_absent::Bool=true ) =
+    updateVariableSolverData!(dfg, sourceVariable.label, getSolverData(sourceVariable, solveKey), useCopy, fields; warn_if_absent=warn_if_absent)
 
 function updateVariableSolverData!(dfg::AbstractDFG,
                                    sourceVariables::Vector{<:DFGVariable},
                                    solveKey::Symbol=:default,
                                    useCopy::Bool=true,
-                                   fields::Vector{Symbol}=Symbol[]  )
+                                   fields::Vector{Symbol}=Symbol[];
+                                   warn_if_absent::Bool=true)
     #I think cloud would do this in bulk for speed
     for var in sourceVariables
-        updateVariableSolverData!(dfg, var.label, getSolverData(var, solveKey), useCopy, fields)
+        updateVariableSolverData!(dfg, var.label, getSolverData(var, solveKey), useCopy, fields; warn_if_absent=warn_if_absent)
     end
 end
 
@@ -563,7 +563,7 @@ function deepcopySolvekeys!(dfg::AbstractDFG,
   for x in labels
       sd = deepcopy(getSolverData(getVariable(dfg,x), src))
       sd.solveKey = dest
-      updateVariableSolverData!(dfg, x, sd, true, Symbol[], verbose )
+      updateVariableSolverData!(dfg, x, sd, true, Symbol[]; warn_if_absent=verbose )
   end
 end
 const deepcopySupersolve! = deepcopySolvekeys!
@@ -670,9 +670,9 @@ addPPE!(dfg::AbstractDFG, sourceVariable::DFGVariable, ppekey::Symbol=:default)
     $(SIGNATURES)
 Update PPE data if it exists, otherwise add it -- one call per `key::Symbol=:default`.
 """
-function updatePPE!(dfg::AbstractDFG, variablekey::Symbol, ppe::P)::P where {P <: AbstractPointParametricEst}
+function updatePPE!(dfg::AbstractDFG, variablekey::Symbol, ppe::AbstractPointParametricEst; warn_if_absent::Bool=true)
     var = getVariable(dfg, variablekey)
-    if !haskey(var.ppeDict, ppe.solveKey)
+    if warn_if_absent && !haskey(var.ppeDict, ppe.solveKey)
         @warn "PPE '$(ppe.solveKey)' does not exist, adding"
     end
     #for InMemoryDFGTypes, cloud would update here
@@ -685,17 +685,17 @@ end
 Update PPE data if it exists, otherwise add it.
 NOTE: Copies the PPE data.
 """
-updatePPE!(dfg::AbstractDFG, sourceVariable::VariableDataLevel1, ppekey::Symbol=:default) =
-    updatePPE!(dfg, sourceVariable.label, deepcopy(getPPE(sourceVariable, ppekey)))
+updatePPE!(dfg::AbstractDFG, sourceVariable::VariableDataLevel1, ppekey::Symbol=:default; warn_if_absent::Bool=true) =
+    updatePPE!(dfg, sourceVariable.label, deepcopy(getPPE(sourceVariable, ppekey)); warn_if_absent=warn_if_absent)
 
 """
     $(SIGNATURES)
 Update PPE data if it exists, otherwise add it.
 """
-function updatePPE!(dfg::AbstractDFG, sourceVariables::Vector{<:VariableDataLevel1}, ppekey::Symbol=:default)
+function updatePPE!(dfg::AbstractDFG, sourceVariables::Vector{<:VariableDataLevel1}, ppekey::Symbol=:default; warn_if_absent::Bool=true)
     #I think cloud would do this in bulk for speed
     for var in sourceVariables
-        updatePPE!(dfg, var.label, getPPE(dfg, var, ppekey))
+        updatePPE!(dfg, var.label, getPPE(dfg, var, ppekey); warn_if_absent=warn_if_absent)
     end
 end