Standardize get dfg variable variable/factor types (#1080)

* standardize get dfg variable variable/factor types

* implement getFactors(dfg, labels)

---------

Co-authored-by: Johannes Terblanche <[email protected]>

Author: Affie

src/Common.jl

@@ -24,7 +24,7 @@ end
 splitbynum(x::AbstractString) = split(x, r"(?<=\D)(?=\d)|(?<=\d)(?=\D)")
 #parse to Int
 function numstringtonum(arr::Vector{<:AbstractString})
-    return [(n = tryparse(Int, e)) != nothing ? n : e for e in arr]
+    return [(n = tryparse(Int, e)) !== nothing ? n : e for e in arr]
 end
 #natural less than
 function natural_lt(x::T, y::T) where {T <: AbstractString}

src/FileDFG/services/FileDFG.jl

@@ -174,7 +174,7 @@ function loadDFG!(
     end
     # FIXME, why is this treated different from VariableSkeleton, VariableSummary?
     # FIXME, still creates type instability on `variables` as either `::Variable` or `::DFGVariable`
-    if isa(dfgLoadInto, GraphsDFG) && GraphsDFGs._variablestype(dfgLoadInto) == Variable
+    if isa(dfgLoadInto, GraphsDFG) && getTypeDFGVariables(dfgLoadInto) == Variable
         variables = packedvars
     else
         variables = unpackVariable.(packedvars)
@@ -190,7 +190,7 @@ function loadDFG!(
         return JSON3.read(jstr, PackedFactor)
     end
     # FIXME, still creates type instability on `variables` as either `::Factor` or `::DFGFactor{<:}`
-    if isa(dfgLoadInto, GraphsDFG) && GraphsDFGs._factorstype(dfgLoadInto) == PackedFactor
+    if isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) == PackedFactor
         factors = packedfacts
     else
         factors = unpackFactor.(dfgLoadInto, packedfacts)
@@ -201,7 +201,7 @@ function loadDFG!(
     # # Adding factors
     map(f -> addFactor!(dfgLoadInto, f), factors)
 
-    if isa(dfgLoadInto, GraphsDFG) && GraphsDFGs._factorstype(dfgLoadInto) != PackedFactor
+    if isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) != PackedFactor
         # Finally, rebuild the CCW's for the factors to completely reinflate them
         # NOTE CREATES A NEW DFGFactor IF  CCW TYPE CHANGES
         @info "Rebuilding CCW's for the factors..."

src/GraphsDFG/GraphsDFG.jl

@@ -52,7 +52,9 @@ import ...DistributedFactorGraphs:
     getSessionBlobEntry,
     getSessionBlobEntries,
     addSessionBlobEntry!,
-    addSessionBlobEntries!
+    addSessionBlobEntries!,
+    getTypeDFGVariables,
+    getTypeDFGFactors
 
 include("FactorGraphs/FactorGraphs.jl")
 using .FactorGraphs

src/GraphsDFG/services/GraphsDFGSerialization.jl

@@ -29,8 +29,8 @@ function StructTypes.subtypes(::Type{PackedGraphsDFG})
     return NamedTuple(map(s -> nameof(s) => PackedGraphsDFG{s}, subs))
 end
 
-_variablestype(fg::GraphsDFG{<:AbstractParams, T, <:AbstractDFGFactor}) where {T} = T
-_factorstype(fg::GraphsDFG{<:AbstractParams, <:AbstractDFGVariable, T}) where {T} = T
+getTypeDFGVariables(fg::GraphsDFG{<:AbstractParams, T, <:AbstractDFGFactor}) where {T} = T
+getTypeDFGFactors(fg::GraphsDFG{<:AbstractParams, <:AbstractDFGVariable, T}) where {T} = T
 
 ##
 """
@@ -52,8 +52,8 @@ function packDFGMetadata(fg::GraphsDFG)
 
     props = (k => getproperty(fg, k) for k in commonfields)
     return PackedGraphsDFG(;
-        typePackedVariable = _variablestype(fg) == Variable,
-        typePackedFactor = _factorstype(fg) == PackedFactor,
+        typePackedVariable = getTypeDFGVariables(fg) == Variable,
+        typePackedFactor = getTypeDFGFactors(fg) == PackedFactor,
         blobStores,
         props...,
     )

src/services/AbstractDFG.jl

@@ -101,6 +101,18 @@ function rebuildFactorMetadata!(
     return error("rebuildFactorMetadata! is not implemented for $(typeof(dfg))")
 end
 
+"""
+    $(SIGNATURES)
+Function to get the type of the variables in the DFG.
+"""
+function getTypeDFGVariables end
+
+"""
+    $(SIGNATURES)
+Function to get the type of the factors in the DFG.
+"""
+function getTypeDFGFactors end
+
 ##------------------------------------------------------------------------------
 ## Setters
 ##------------------------------------------------------------------------------
@@ -373,6 +385,10 @@ function getFactors(
     return error("getFactors not implemented for $(typeof(dfg))")
 end
 
+function getFactors(dfg::AbstractDFG, labels::Vector{Symbol})
+    return map(label -> getFactor(dfg, label), labels)
+end
+
 ##------------------------------------------------------------------------------
 ## Checking Types
 ##------------------------------------------------------------------------------
@@ -1079,9 +1095,8 @@ function copyGraph!(
     showprogress::Bool = verbose,
 )
     # Split into variables and factors
-    sourceVariables = map(vId -> getVariable(sourceDFG, vId), variableLabels)
-    sourceFactors = map(fId -> getFactor(sourceDFG, fId), factorLabels)
-
+    sourceVariables = getVariables(sourceDFG, variableLabels)
+    sourceFactors = getFactors(sourceDFG, factorLabels)
     # Now we have to add all variables first,
     @showprogress desc = "copy variables" enabled = showprogress for variable in
                                                                      sourceVariables

src/services/Serialization.jl

@@ -245,7 +245,10 @@ function unpackVariable(variable::PackedVariable; skipVersionCheck::Bool = false
     )
 end
 
+DFGVariable(v::DFGVariable) = v
 DFGVariable(v::Variable) = unpackVariable(v)
+Variable(v::Variable) = v
+Variable(v::DFGVariable) = packVariable(v)
 
 ##==============================================================================
 ## Factor Packing and unpacking