use constructor and not convert

Author: Affie

src/Deprecated.jl

@@ -95,4 +95,23 @@ Base.setproperty!(x::DFGFactor,f::Symbol, val) = begin
     end
 
 
-#
+#NOTE deprecate in favor of constructors because its not lossless: https://docs.julialang.org/en/v1/manual/conversion-and-promotion/#Conversion-vs.-Construction-1
+function Base.convert(::Type{DFGVariableSummary}, v::DFGVariable)
+    Base.depwarn("convert to type DFGVariableSummary is deprecated use the constructor", :convert)
+    return DFGVariableSummary(v)
+end
+
+function Base.convert(::Type{SkeletonDFGVariable}, v::VariableDataLevel1)
+    Base.depwarn("convert to type SkeletonDFGVariable is deprecated use the constructor", :convert)
+    return SkeletonDFGVariable(v)
+end
+
+function Base.convert(::Type{DFGFactorSummary}, f::DFGFactor)
+    Base.depwarn("convert to type DFGFactorSummary is deprecated use the constructor", :convert)
+    return DFGFactorSummary(f)
+end
+
+function Base.convert(::Type{SkeletonDFGFactor}, f::FactorDataLevel1)
+    Base.depwarn("convert to type SkeletonDFGFactor is deprecated use the constructor", :convert)
+    return SkeletonDFGFactor(f)
+end

src/LightDFG/entities/LightDFG.jl

@@ -66,3 +66,14 @@ LightDFG(description::String,
          sessionData::Dict{Symbol, String},
          solverParams::AbstractParams) =
          LightDFG(FactorGraph{Int,DFGVariable,DFGFactor}(), description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], solverParams)
+
+
+LightDFG{T,V,F}(description::String,
+                userId::String,
+                robotId::String,
+                sessionId::String,
+                userData::Dict{Symbol, String},
+                robotData::Dict{Symbol, String},
+                sessionData::Dict{Symbol, String},
+                solverParams::T) where {T <: AbstractParams, V <:AbstractDFGVariable, F<:AbstractDFGFactor} =
+                LightDFG(FactorGraph{Int,V,F}(), description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], solverParams)

src/LightDFG/services/LightDFG.jl

@@ -38,6 +38,12 @@ function addVariable!(dfg::LightDFG{<:AbstractParams, V, <:AbstractDFGFactor}, v
     return variable
 end
 
+function addVariable!(dfg::LightDFG{<:AbstractParams, VD, <:AbstractDFGFactor},
+                      variable::AbstractDFGVariable)::VD where VD <: AbstractDFGVariable
+    return addVariable!(dfg, VD(variable))
+end
+
+
 #moved to abstract
 # function addFactor!(dfg::LightDFG{<:AbstractParams, V, F}, variables::Vector{<:V}, factor::F)::F where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
 #
@@ -86,6 +92,11 @@ function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F}, f
     return factor
 end
 
+function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F},
+                      factor::AbstractDFGFactor)::F where F <: AbstractDFGFactor
+    return addFactor!(dfg, F(factor))
+end
+
 function getVariable(dfg::LightDFG, label::Symbol)::AbstractDFGVariable
     if !haskey(dfg.g.variables, label)
         error("Variable label '$(label)' does not exist in the factor graph")
@@ -257,7 +268,7 @@ end
 function _copyIntoGraph!(sourceDFG::LightDFG{<:AbstractParams, V, F}, destDFG::LightDFG{<:AbstractParams, V, F}, ns::Vector{Int}, includeOrphanFactors::Bool=false)::Nothing where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
 
     includeOrphanFactors && (@error "Adding orphaned factors is not supported")
-    
+
     #kan ek die in bulk copy, soos graph en dan nuwe map maak
     # Add all variables first,
     labels = [sourceDFG.g.labels[i] for i in ns]

src/entities/DFGFactor.jl

@@ -183,13 +183,11 @@ const FactorDataLevel1 = Union{DFGFactor, DFGFactorSummary}
 const FactorDataLevel2 = Union{DFGFactor}
 
 ##==============================================================================
-## Convert
+## Convertion constructors
 ##==============================================================================
-function Base.convert(::Type{DFGFactorSummary}, f::DFGFactor)
-    return DFGFactorSummary(f.label, f.timestamp, deepcopy(f.tags), f._dfgNodeParams._internalId, deepcopy(f._variableOrderSymbols))
-end
 
-#TODO TEST
-function Base.convert(::Type{SkeletonDFGFactor}, f::FactorDataLevel1)
-    return SkeletonDFGFactor(f.label, deepcopy(f.tags), deepcopy(f._variableOrderSymbols))
-end
+DFGFactorSummary(f::DFGFactor) =
+    DFGFactorSummary(f.label, f.timestamp, deepcopy(f.tags), f._dfgNodeParams._internalId, deepcopy(f._variableOrderSymbols))
+
+SkeletonDFGFactor(f::FactorDataLevel1) =
+    SkeletonDFGFactor(f.label, deepcopy(f.tags), deepcopy(f._variableOrderSymbols))

src/entities/DFGVariable.jl

@@ -309,14 +309,11 @@ const VariableDataLevel1 = Union{DFGVariable, DFGVariableSummary}
 const VariableDataLevel2 = Union{DFGVariable}
 
 ##==============================================================================
-## converters
+## Convertion constructors
 ##==============================================================================
 
-function Base.convert(::Type{DFGVariableSummary}, v::DFGVariable)
-    return DFGVariableSummary(v.label, v.timestamp, deepcopy(v.tags), deepcopy(v.ppeDict), Symbol(typeof(getSofttype(v))), v.bigData, v._internalId)
-end
+DFGVariableSummary(v::DFGVariable) =
+        DFGVariableSummary(v.label, v.timestamp, deepcopy(v.tags), deepcopy(v.ppeDict), Symbol(typeof(getSofttype(v))), v.bigData, v._internalId)
 
-#TODO Test
-function Base.convert(::Type{SkeletonDFGVariable}, v::VariableDataLevel1)
-    return SkeletonDFGVariable(v.label, deepcopy(v.tags))
-end
+SkeletonDFGVariable(v::VariableDataLevel1) =
+            SkeletonDFGVariable(v.label, deepcopy(v.tags))

src/services/AbstractDFG.jl

@@ -1023,9 +1023,8 @@ Produces a dot-format of the graph for visualization.
 """
 function toDot(dfg::AbstractDFG)::String
     #TODO implement convert
-    # graphsdfg = GraphsDFG{NoSolverParams}()
-    # DistributedFactorGraphs._copyIntoGraph!(dfg, graphsdfg, union(listVariables(dfg), listFactors(dfg)), true)
-    graphsdfg = convert(GraphsDFG{NoSolverParams}, dfg)
+    graphsdfg = GraphsDFG{NoSolverParams}()
+    DistributedFactorGraphs._copyIntoGraph!(dfg, graphsdfg, union(listVariables(dfg), listFactors(dfg)), true)
     # Calls down to GraphsDFG.toDot
     return toDot(graphsdfg)
 end
@@ -1065,8 +1064,8 @@ Get a summary of the graph (first-class citizens of variables and factors).
 Returns a DFGSummary.
 """
 function getSummary(dfg::G)::DFGSummary where {G <: AbstractDFG}
-    vars = map(v -> convert(DFGVariableSummary, v), getVariables(dfg))
-    facts = map(f -> convert(DFGFactorSummary, f), getFactors(dfg))
+    vars = map(v -> DFGVariableSummary(v), getVariables(dfg))
+    facts = map(f -> DFGFactorSummary(f), getFactors(dfg))
     return DFGSummary(
         Dict(map(v->v.label, vars) .=> vars),
         Dict(map(f->f.label, facts) .=> facts),
@@ -1087,11 +1086,12 @@ function getSummaryGraph(dfg::G)::LightDFG{NoSolverParams, DFGVariableSummary, D
         userId=dfg.userId,
         robotId=dfg.robotId,
         sessionId=dfg.sessionId)
-    for v in getVariables(dfg)
-        newV = addVariable!(summaryDfg, convert(DFGVariableSummary, v))
-    end
-    for f in getFactors(dfg)
-        addFactor!(summaryDfg, getNeighbors(dfg, f), convert(DFGFactorSummary, f))
-    end
+    deepcopyGraph!(summaryDfg, dfg)
+    # for v in getVariables(dfg)
+    #     newV = addVariable!(summaryDfg, DFGVariableSummary(v))
+    # end
+    # for f in getFactors(dfg)
+    #     addFactor!(summaryDfg, getNeighbors(dfg, f), DFGFactorSummary(f))
+    # end
     return summaryDfg
 end

src/services/copyFunctions.jl

@@ -77,5 +77,3 @@ function deepcopyGraph( ::Type{T},
     copyGraph!(destDFG, sourceDFG, variableFactorLabels; deepcopyNodes=true, kwargs...)
     return destDFG
 end
-
-Base.convert(::Type{T}, fg::AbstractDFG) where T <: AbstractDFG = deepcopyGraph(T, fg)

test/interfaceTests.jl

@@ -94,8 +94,13 @@ end
 
     fgcopy = testDFGAPI()
     DFG._copyIntoGraph!(fg, fgcopy, union(ls(fg), lsf(fg)))
+    @test getVariableOrder(fg,:f1) == getVariableOrder(fgcopy,:f1)
 
+    #test copyGraph, deepcopyGraph[!]
+    fgcopy = testDFGAPI()
+    DFG.deepcopyGraph!(fgcopy, fg)
     @test getVariableOrder(fg,:f1) == getVariableOrder(fgcopy,:f1)
 
+    CopyFunctionsTest(testDFGAPI)
 
 end

test/testBlocks.jl

@@ -1083,3 +1083,65 @@ function ConnectivityTest(testDFGAPI; kwargs...)
         @test hasOrphans(dfg) == true
     end
 end
+
+
+function CopyFunctionsTest(testDFGAPI; kwargs...)
+
+    # testDFGAPI = LightDFG
+    # kwargs = ()
+
+    dfg, verts, facs = connectivityTestGraph(testDFGAPI; kwargs...)
+
+    varlbls = ls(dfg)
+    faclbls = lsf(dfg)
+
+    dcdfg = DFG.deepcopyGraph(LightDFG, dfg)
+
+    @test issetequal(ls(dcdfg), varlbls)
+    @test issetequal(lsf(dcdfg), faclbls)
+
+    lbls = [:x2, :x3, :x2x3f1]
+    dcdfg_part = DFG.deepcopyGraph(LightDFG, dfg, lbls)
+
+    @test issetequal(union(ls(dcdfg_part), lsf(dcdfg_part)), lbls)
+
+    # deepcopy subgraph ignoring orphans
+    @test_logs (:warn, r"orphan") dcdfg_part = DFG.deepcopyGraph(LightDFG, dfg, union(lbls, [:x1x2f1]))
+    @test issetequal(union(ls(dcdfg_part), lsf(dcdfg_part)), lbls)
+
+    # deepcopy subgraph with 2 parts
+    lbls = [:x2, :x3, :x2x3f1, :x5, :x6, :x5x6f1, :x10]
+    dcdfg_part = DFG.deepcopyGraph(LightDFG, dfg, lbls)
+    @test issetequal(union(ls(dcdfg_part), lsf(dcdfg_part)), lbls)
+    @test !isFullyConnected(dcdfg_part)
+    # dfgplot(dcdfg_part)
+
+    # not found errors
+    @test_throws ErrorException DFG.deepcopyGraph(LightDFG, dfg, [:x0, :a])
+
+    # already exists errors
+    dcdfg_part = DFG.deepcopyGraph(LightDFG, dfg, [:x1, :x2, :x3, :x1x2f1, :x2x3f1])
+    @test_throws ErrorException DFG.deepcopyGraph!(dcdfg_part, dfg, [:x4, :x2, :x3, :x1x2f1, :x2x3f1])
+    @test_throws ErrorException DFG.deepcopyGraph!(dcdfg_part, dfg, [:x1x2f1])
+
+    # same but overwrite destination
+    DFG.deepcopyGraph!(dcdfg_part, dfg, [:x4, :x2, :x3, :x1x2f1, :x2x3f1]; overwriteDest = true)
+
+    DFG.deepcopyGraph!(dcdfg_part, dfg, [:x1x2f1]; overwriteDest=true)
+
+
+    # convert to...
+    # condfg = convert(GraphsDFG, dfg)
+    # @test condfg isa GraphsDFG
+    # @test issetequal(ls(condfg), varlbls)
+    # @test issetequal(lsf(condfg), faclbls)
+    #
+    # condfg = convert(LightDFG, dfg)
+    # @test condfg isa LightDFG
+    # @test issetequal(ls(condfg), varlbls)
+    # @test issetequal(lsf(condfg), faclbls)
+
+    # GraphsDFG(dfg::AbstractDFG) = convert(GraphsDFG,dfg)
+    # GraphsDFG(dfg)
+
+end