Cleanup some old deprecations (#1094)

Project.toml

@@ -1,6 +1,6 @@
 name = "DistributedFactorGraphs"
 uuid = "b5cc3c7e-6572-11e9-2517-99fb8daf2f04"
-version = "0.25.0"
+version = "0.24.1"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/FileDFG/services/FileDFG.jl

@@ -98,7 +98,6 @@ function loadDFG!(
     dfgLoadInto::AbstractDFG,
     dst::AbstractString;
     overwriteDFGMetadata::Bool = true,
-    useDeprExtract::Bool = false,
 )
 
     #
@@ -130,24 +129,16 @@ function loadDFG!(
         @info "loadDFG! detected a gzip $dstname -- unpacking via $loaddir now..."
         Base.rm(folder; recursive = true, force = true)
         # unzip the tar file
-
-        # TODO deprecated, remove. Kept for legacy support if older tarbals
-        if useDeprExtract
-            @warn "Old FileDFG compressed tar files are deprecated, load with useDeprExtract=true and use saveDFG again to update"
-            run(`tar -zxf $dstname -C $loaddir`)
-        else
-            tar_gz = open(dstname)
-            tar = CodecZlib.GzipDecompressorStream(tar_gz)
-            Tar.extract(tar, folder)
-            close(tar)
-        end
-
+        tar_gz = open(dstname)
+        tar = CodecZlib.GzipDecompressorStream(tar_gz)
+        Tar.extract(tar, folder)
+        close(tar)
         #or for non-compressed
         # Tar.extract(dstname, folder)
     end
 
     #GraphsDFG metadata
-    if overwriteDFGMetadata && !useDeprExtract
+    if overwriteDFGMetadata
         @assert isa(dfgLoadInto, GraphsDFG) "Only GraphsDFG metadata are supported"
         @info "loading dfg metadata"
         jstr = read("$folder/dfg.json", String)

src/GraphsDFG/FactorGraphs/FactorGraphs.jl

@@ -140,7 +140,8 @@ function addFactor!(
 
     for vlabel in variableLabels
         !haskey(g.labels, vlabel) && (
-            @error "Variable '$(vlabel)' not found in graph when creating Factor '$(factor.label)'"; return false
+            throw(KeyError(vlabel))
+            # @error "Variable '$(vlabel)' not found in graph when creating Factor '$(factor.label)'"; return false
         ) #TODO debug error or exception?
     end
 

src/entities/AbstractDFGSummary.jl

@@ -2,7 +2,6 @@
 $(TYPEDEF)
 Structure for a graph summary.
 """
-
 struct DFGSummary
     variables::Dict{Symbol, DFGVariableSummary}
     factors::Dict{Symbol, DFGFactorSummary}

src/entities/DFGFactor.jl

@@ -353,6 +353,16 @@ Base.@kwdef struct DFGFactorSummary <: AbstractDFGFactor
     timestamp::ZonedDateTime
 end
 
+function DFGFactorSummary(
+    label::Symbol,
+    variableOrderSymbols::Vector{Symbol};
+    timestamp::ZonedDateTime = now(localzone()),
+    tags::Set{Symbol} = Set{Symbol}(),
+    id::Union{UUID, Nothing} = nothing,
+)
+    return DFGFactorSummary(id, label, tags, variableOrderSymbols, timestamp)
+end
+
 ##------------------------------------------------------------------------------
 ## SkeletonDFGFactor lv0
 ##------------------------------------------------------------------------------
@@ -392,7 +402,7 @@ function SkeletonDFGFactor(
 end
 function SkeletonDFGFactor(
     label::Symbol,
-    variableOrderSymbols::Vector{Symbol} = Symbol[];
+    variableOrderSymbols::Vector{Symbol};
     id::Union{UUID, Nothing} = nothing,
     tags = Set{Symbol}(),
 )

src/entities/DFGVariable.jl

@@ -317,20 +317,10 @@ The default DFGVariable constructor.
 function DFGVariable(
     label::Symbol,
     T::Type{<:InferenceVariable};
-    estimateDict = nothing,
-    timestamp = now(localzone()),
+    timestamp::ZonedDateTime = now(localzone()),
     solvable::Union{Int, Base.RefValue{Int}} = Ref(1),
     kwargs...,
 )
-    #TODO deprecated, remove in v0.21 should have already been deprecated
-    if !isnothing(estimateDict)
-        error("Keyword argument `estimateDict` is deprecated use `ppeDict`")
-    end
-    if !isa(timestamp, ZonedDateTime)
-        @warn "timestamp<:DateTime is deprecated, timestamp must be a ZonedDateTime, using local zone."
-        timestamp = ZonedDateTime(timestamp, localzone())
-    end
-
     solvable isa Int && (solvable = Ref(solvable))
 
     N = getDimension(T)

src/services/AbstractDFG.jl

@@ -454,71 +454,6 @@ function getVariable(dfg::AbstractDFG, label::Symbol, solveKey::Symbol)
     return var
 end
 
-"""
-$(SIGNATURES)
-"""
-function addFactor!(
-    dfg::AbstractDFG,
-    variables::Vector{<:AbstractDFGVariable},
-    factor::F,
-) where {F <: AbstractDFGFactor}
-    Base.depwarn(
-        "addFactor!(dfg, variables, factor) is deprecated, use addFactor!(dfg, factor)",
-        :addFactor!,
-    )
-    variableLabels = map(v -> v.label, variables)
-
-    if factor isa DFGFactor
-        f = factor
-        newfactor = DFGFactor(
-            f.label,
-            f.timestamp,
-            f.nstime,
-            f.tags,
-            f.solverData,
-            f.solvable,
-            Tuple(variableLabels),
-        )
-        return addFactor!(dfg, newfactor)
-    else
-        resize!(factor._variableOrderSymbols, length(variableLabels))
-        factor._variableOrderSymbols .= variableLabels
-        return addFactor!(dfg, factor)
-    end
-end
-
-"""
-$(SIGNATURES)
-"""
-function addFactor!(
-    dfg::AbstractDFG,
-    variableLabels::Vector{Symbol},
-    factor::F,
-) where {F <: AbstractDFGFactor}
-    Base.depwarn(
-        "addFactor!(dfg, variables, factor) is deprecated, use addFactor!(dfg, factor)",
-        :addFactor!,
-    )
-
-    if factor isa DFGFactor
-        f = factor
-        newfactor = DFGFactor(
-            f.label,
-            f.timestamp,
-            f.nstime,
-            f.tags,
-            f.solverData,
-            f.solvable,
-            Tuple(variableLabels),
-        )
-        return addFactor!(dfg, newfactor)
-    else
-        resize!(factor._variableOrderSymbols, length(variableLabels))
-        factor._variableOrderSymbols .= variableLabels
-        return addFactor!(dfg, factor)
-    end
-end
-
 """
     $(SIGNATURES)
 Delete a referenced DFGVariable from the DFG.

src/services/CustomPrinting.jl

@@ -215,7 +215,7 @@ function Base.show(io::IO, dfg::AbstractDFG)
     println(io, "  Description: ", getDescription(dfg))
     println(io, "  Nr variables: ", length(ls(dfg)))
     println(io, "  Nr factors: ", length(lsf(dfg)))
-    println(io, "  Agent Metadata: ", keys(getAgentmetadata(dfg)))
+    println(io, "  Agent Metadata: ", keys(getAgentMetadata(dfg)))
     println(io, "  Graph Metadata: ", keys(getGraphMetadata(dfg)))
     return
 end

test/FactorGraphsTests.jl

@@ -66,7 +66,7 @@ end
         [:a, :b],
         SkeletonDFGFactor(:abf1, [:a, :b]),
     )
-    @test @test_logs (:error, r"not found") !FactorGraphs.addFactor!(
+    @test_throws KeyError FactorGraphs.addFactor!(
         fg,
         [:a, :c],
         SkeletonDFGFactor(:acf1, [:a, :c]),

test/GraphsDFGSummaryTypes.jl

@@ -14,15 +14,6 @@ function DistributedFactorGraphs.DFGVariableSummary(label::Symbol)
         Dict{Symbol, BlobEntry}(),
     )
 end
-function DistributedFactorGraphs.DFGFactorSummary(label::Symbol)
-    return DFGFactorSummary(
-        nothing,
-        label,
-        Set{Symbol}(),
-        Symbol[],
-        DistributedFactorGraphs.now(localzone()),
-    )
-end
 
 function DistributedFactorGraphs.DFGVariableSummary(
     label::Symbol,
@@ -54,10 +45,9 @@ dfg = GraphsDFG{NoSolverParams, VARTYPE, FACTYPE}()
 v1 = VARTYPE(:a)
 v2 = VARTYPE(:b)
 v3 = VARTYPE(:c)
-f0 = FACTYPE(:af1)
-f1 = FACTYPE(:abf1)
-f2 = FACTYPE(:bcf1)
-append!(f2._variableOrderSymbols, [:b, :c])
+f0 = FACTYPE(:af1, [:a])
+f1 = FACTYPE(:abf1, [:a, :b])
+f2 = FACTYPE(:bcf1, [:b, :c])
 
 union!(v1.tags, [:VARIABLE, :POSE])
 union!(v2.tags, [:VARIABLE, :LANDMARK])
@@ -150,7 +140,7 @@ end
     fg = GraphsDFG{NoSolverParams, VARTYPE, FACTYPE}()
     addVariable!(fg, VARTYPE(:a))
     addVariable!(fg, VARTYPE(:b))
-    addFactor!(fg, [:a, :b], FACTYPE(:abf1))
+    addFactor!(fg, FACTYPE(:abf1, [:a, :b]))
     addVariable!(fg, VARTYPE(:orphan))
 
     AdjacencyMatricesTestBlock(fg)

test/iifInterfaceTests.jl

@@ -73,11 +73,11 @@ end
     @test addVariable!(dfg2, v2) == v2
     @test @test_logs (:warn, r"exist") match_mode = :any updateVariable!(dfg2, v3) == v3
     @test_throws ErrorException addVariable!(dfg2, v3)
-    @test addFactor!(dfg2, [v1, v2], f1) == f1
-    @test_throws ErrorException addFactor!(dfg2, [v1, v2], f1)
+    @test addFactor!(dfg2, f1) == f1
+    @test_throws ErrorException addFactor!(dfg2, f1)
     # @test @test_logs (:warn, r"exist") updateFactor!(dfg2, f2) == f2
     @test updateFactor!(dfg2, f2) == f2
-    @test_throws ErrorException addFactor!(dfg2, [:b, :c], f2)
+    @test_throws ErrorException addFactor!(dfg2, f2)
 
     dv3, dv3facs = deleteVariable!(dfg2, v3)
     #TODO write compare if we want to compare complete one, for now just label

test/runtests.jl

@@ -117,7 +117,7 @@ struct NotImplementedDFG{T} <: AbstractDFG{T} end
 @testset "No Interface tests" begin
     dfg = NotImplementedDFG{NoSolverParams}()
     v1 = SkeletonDFGVariable(:v1)
-    f1 = SkeletonDFGFactor(:f1)
+    f1 = SkeletonDFGFactor(:f1, [:v1])
 
     @test_throws ErrorException exists(dfg, v1)
     @test_throws ErrorException exists(dfg, f1)

test/testBlocks.jl

@@ -474,11 +474,11 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2)
     @test getLabel(fg[getLabel(v1)]) == getLabel(v1)
 
     #TODO standardize this error and res also for that matter
-    @test_throws Exception addFactor!(fg, [:a, :nope], f1)
-    @test_throws Exception addFactor!(fg, [v1, v2, v3], f1)
+    fnope = DFGFactor{TestCCW{TestFunctorInferenceType1}}(:broken, [:a, :nope])
+    @test_throws KeyError addFactor!(fg, fnope)
 
-    @test addFactor!(fg, [v1, v2], f1) == f1
-    @test_throws ErrorException addFactor!(fg, [v1, v2], f1)
+    @test addFactor!(fg, f1) == f1
+    @test_throws ErrorException addFactor!(fg, f1)
 
     @test getLabel(fg[getLabel(f1)]) == getLabel(f1)
 
@@ -494,7 +494,7 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2)
         f2,
     ) === f2
     @test updateFactor!(fg, f2) === f2
-    @test_throws ErrorException addFactor!(fg, [:b, :c], f2)
+    @test_throws ErrorException addFactor!(fg, f2)
     #TODO Graphs.jl, but look at refactoring absract @test_throws ErrorException addFactor!(fg, f2)
 
     if f2 isa DFGFactor
@@ -1367,8 +1367,8 @@ function testGroup!(fg, v1, v2, f0, f1)
         @test setSolvable!(fg, f1.label, 0) == 0
         @test getSolvable(fg, f1.label) == 0
 
-        #TODO follow up on why f1 is no longer referenced, and remove next line
-        @test_broken getSolvable(f1) == 0
+        @test getSolvable(f1) == 0
+        setSolvable!(f1, 1)
 
         # isFactor and isVariable
         @test isFactor(fg, f1.label)
@@ -1494,7 +1494,10 @@ function connectivityTestGraph(
 
     else
         facs = map(
-            n -> addFactor!(dfg, [vars[n], vars[n + 1]], FACTYPE(Symbol("x$(n)x$(n+1)f1"))),
+            n -> addFactor!(
+                dfg,
+                FACTYPE(Symbol("x$(n)x$(n+1)f1"), [vars[n].label, vars[n + 1].label]),
+            ),
             1:(length(vars) - 1),
         )
     end
@@ -1698,7 +1701,7 @@ function ProducingDotFiles(
         if (FACTYPE == DFGFactor)
             f1 = DFGFactor{TestFunctorInferenceType1}(:abf1, [:a, :b])
         else
-            f1 = FACTYPE(:abf1)
+            f1 = FACTYPE(:abf1, [:a, :b])
         end
     end
 
@@ -1708,7 +1711,7 @@ function ProducingDotFiles(
     # ┌ Warning: addFactor!(dfg, variables, factor) is deprecated, use addFactor!(dfg, factor)
     # │   caller = ProducingDotFiles(testDFGAPI::Type{GraphsDFG}, v1::Nothing, v2::Nothing, f1::Nothing; VARTYPE::Type{DFGVariable}, FACTYPE::Type{DFGFactor}) at testBlocks.jl:1440
     # └ @ Main ~/.julia/dev/DistributedFactorGraphs/test/testBlocks.jl:1440
-    addFactor!(dotdfg, [v1, v2], f1)
+    addFactor!(dotdfg, f1)
     #NOTE hardcoded toDot will have different results so test Graphs seperately
     if testDFGAPI <: GraphsDFG || testDFGAPI <: GraphsDFG
         todotstr = toDot(dotdfg)