change delete return type (#1134)

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

Author: Affie

NEWS.md

@@ -1,5 +1,8 @@
 Listing news on any major breaking changes in DFG.  For regular changes, see integrated Github.com project milestones for DFG.
 
+# v0.27
+- `delete` returns number of nodes deleted and no longer the object that was deleted.
+
 # v0.26
 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`.
 

ext/DFGPlots.jl

@@ -24,7 +24,7 @@ end
 function DFGPlotProps()
     return DFGPlotProps(
         (var = colorant"lightgreen", fac = colorant"cyan3"),
-        (var = 40.0, fac = 20.0),
+        (var = 50.0, fac = 20.0),
         (var = :circle, fac = :rect),
         GraphMakie.Stress(),
         true,

src/DataBlobs/services/BlobEntry.jl

@@ -206,7 +206,8 @@ Notes:
 - users responsibility to delete data in db before deleting entry
 """
 function deleteBlobEntry!(var::AbstractDFGVariable, key::Symbol)
-    return pop!(var.dataDict, key)
+    pop!(var.dataDict, key)
+    return 1
 end
 
 function deleteBlobEntry!(var::VariableDFG, key::Symbol)
@@ -217,7 +218,8 @@ function deleteBlobEntry!(var::VariableDFG, key::Symbol)
             ),
         )
     end
-    return deleteat!(var.blobEntries, findfirst(x -> x.label == key, var.blobEntries))
+    deleteat!(var.blobEntries, findfirst(x -> x.label == key, var.blobEntries))
+    return 1
 end
 
 function deleteBlobEntry!(dfg::AbstractDFG, label::Symbol, key::Symbol)

src/DataBlobs/services/BlobStores.jl

@@ -128,23 +128,13 @@ end
 # also creates an originId as uuid4
 addBlob!(store::AbstractBlobStore, data) = addBlob!(store, uuid4(), data)
 
-#fallback as not all blobStores use filename
-function addBlob!(store::AbstractBlobStore, blobId::UUID, data, ::String)
-    return addBlob!(store, blobId, data)
-end
-
-function addBlob!(store::AbstractBlobStore{T}, data::T, ::String) where {T}
-    return addBlob!(store, uuid4(), data)
-end
-
 #update
-function updateBlob!(dfg::AbstractDFG, entry::BlobEntry, data::T) where {T}
-    return updateBlob!(getBlobStore(dfg, entry.blobstore), entry, data)
+function updateBlob!(dfg::AbstractDFG, entry::BlobEntry, data)
+    return updateBlob!(getBlobStore(dfg, entry.blobstore), entry.blobId, data)
 end
 
 function updateBlob!(store::AbstractBlobStore, entry::BlobEntry, data)
-    blobId = isnothing(entry.blobId) ? entry.originId : entry.blobId
-    return updateBlob!(store, blobId, data)
+    return updateBlob!(store, entry.blobId, data)
 end
 #delete
 function deleteBlob!(dfg::AbstractDFG, entry::BlobEntry)
@@ -241,10 +231,8 @@ end
 
 function deleteBlob!(store::FolderStore{T}, blobId::UUID) where {T}
     blobfilename = joinpath(store.folder, string(blobId))
-
-    data = getBlob(store, blobId)
     rm(blobfilename)
-    return data
+    return 1
 end
 
 #hasBlob or existsBlob?
@@ -292,7 +280,8 @@ function updateBlob!(store::InMemoryBlobStore{T}, blobId::UUID, data::T) where {
 end
 
 function deleteBlob!(store::InMemoryBlobStore, blobId::UUID)
-    return pop!(store.blobs, blobId)
+    pop!(store.blobs, blobId)
+    return 1
 end
 
 hasBlob(store::InMemoryBlobStore, blobId::UUID) = haskey(store.blobs, blobId)
@@ -430,7 +419,8 @@ function updateBlob!(store::RowBlobStore{T}, blobId::UUID, blob::T) where {T}
 end
 
 function deleteBlob!(store::RowBlobStore, blobId::UUID)
-    return getfield(pop!(store.blobs, blobId), :blob)
+    getfield(pop!(store.blobs, blobId), :blob)
+    return 1
 end
 
 hasBlob(store::RowBlobStore, blobId::UUID) = haskey(store.blobs, blobId)

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

@@ -276,9 +276,10 @@ function updateData!(
 end
 
 function deleteData!(dfg::AbstractDFG, vLbl::Symbol, bLbl::Symbol)
-    de = deleteBlobEntry!(dfg, vLbl, bLbl)
-    db = deleteBlob!(dfg, de)
-    return de => db
+    de = getBlobEntry(dfg, vLbl, bLbl)
+    deleteBlobEntry!(dfg, vLbl, bLbl)
+    deleteBlob!(dfg, de)
+    return 2
 end
 
 function deleteData!(
@@ -296,7 +297,8 @@ function deleteData!(
     vLbl::Symbol,
     bLbl::Symbol,
 )
-    de = deleteBlobEntry!(dfg, vLbl, bLbl)
-    db = deleteBlob!(blobstore, de)
-    return de => db
+    de = getBlobEntry(dfg, vLbl, bLbl)
+    deleteBlobEntry!(dfg, vLbl, bLbl)
+    deleteBlob!(blobstore, de)
+    return 2
 end

src/Deprecated.jl

@@ -1,3 +1,11 @@
+## ================================================================================
+## Deprecated in v0.27
+##=================================================================================
+
+@deprecate getNeighborhood(args...; kwargs...) listNeighborhood(args...; kwargs...)
+@deprecate addBlob!(store::AbstractBlobStore, blobId::UUID, data, ::String) addBlob!(store, blobId, data)
+@deprecate addBlob!(store::AbstractBlobStore{T}, data::T, ::String) where {T} addBlob!(store, uuid4(), data)
+
 ## ================================================================================
 ## Deprecated in v0.25
 ##=================================================================================
@@ -62,8 +70,3 @@ DFGSummary(args) = error("DFGSummary is deprecated")
 
 @deprecate lsfWho(dfg::AbstractDFG, type::Symbol) lsf(dfg, getfield(Main, type))
 
-## ================================================================================
-## Deprecated in v0.23
-##=================================================================================
-#NOTE free up getNeighbors to return the variables or factors
-@deprecate getNeighbors(args...; kwargs...) listNeighbors(args...; kwargs...)

src/GraphsDFG/services/GraphsDFG.jl

@@ -174,21 +174,18 @@ function deleteVariable!(dfg::GraphsDFG, label::Symbol)#::Tuple{AbstractDFGVaria
 
     deleteNeighbors = true # reserved, orphaned factors are not supported at this time
     if deleteNeighbors
-        neigfacs = map(l -> deleteFactor!(dfg, l), listNeighbors(dfg, label))
+        del_facs = map(l -> deleteFactor!(dfg, l), listNeighbors(dfg, label))
     end
-    variable = dfg.g.variables[label]
     rem_vertex!(dfg.g, dfg.g.labels[label])
-
-    return variable, neigfacs
+    return sum(del_facs) + 1
 end
 
 function deleteFactor!(dfg::GraphsDFG, label::Symbol; suppressGetFactor::Bool = false)
     if !haskey(dfg.g.factors, label)
         error("Factor label '$(label)' does not exist in the factor graph")
     end
-    factor = dfg.g.factors[label]
     rem_vertex!(dfg.g, dfg.g.labels[label])
-    return factor
+    return 1
 end
 
 function getVariables(
@@ -319,7 +316,7 @@ function listNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int = 0)
     return neighbors_ll::Vector{Symbol}
 end
 
-function getNeighborhood(
+function listNeighborhood(
     dfg::GraphsDFG,
     variableFactorLabels::Vector{Symbol},
     distance::Int;

src/services/AbstractDFG.jl

@@ -213,8 +213,15 @@ function updateGraphMetadata!(dfg::AbstractDFG, pair::Pair{Symbol, String})
     return push!(dfg.graphMetadata, pair)
 end
 
-deleteAgentMetadata!(dfg::AbstractDFG, key::Symbol) = pop!(dfg.agent.metadata, key)
-deleteGraphMetadata!(dfg::AbstractDFG, key::Symbol) = pop!(dfg.graphMetadata, key)
+function deleteAgentMetadata!(dfg::AbstractDFG, key::Symbol) 
+    pop!(dfg.agent.metadata, key)
+    return 1
+end
+
+function deleteGraphMetadata!(dfg::AbstractDFG, key::Symbol)
+    pop!(dfg.graphMetadata, key)
+    return 1
+end
 
 emptyAgentMetadata!(dfg::AbstractDFG) = empty!(dfg.agent.metadata)
 emptyGraphMetadata!(dfg::AbstractDFG) = empty!(dfg.graphMetadata)
@@ -263,7 +270,10 @@ end
 function updateBlobStore!(dfg::AbstractDFG, bs::AbstractBlobStore)
     return push!(dfg.blobStores, getLabel(bs) => bs)
 end
-deleteBlobStore!(dfg::AbstractDFG, key::Symbol) = pop!(dfg.blobStores, key)
+function deleteBlobStore!(dfg::AbstractDFG, key::Symbol)
+    pop!(dfg.blobStores, key)
+    return 1
+end
 emptyBlobStore!(dfg::AbstractDFG) = empty!(dfg.blobStores)
 listBlobStores(dfg::AbstractDFG) = collect(keys(dfg.blobStores))
 
@@ -1366,8 +1376,8 @@ function buildSubgraph(
     #build up the neighborhood from variableFactorLabels
     allvarfacs = getNeighborhood(dfg, variableFactorLabels, distance; solvable = solvable)
 
-    variableLabels = intersect(listVariables(dfg), allvarfacs)
-    factorLabels = intersect(listFactors(dfg), allvarfacs)
+    variableLabels = intersect(allvarfacs, listVariables(dfg))
+    factorLabels = intersect(allvarfacs, listFactors(dfg))
     # Copy the section of graph we want
     destDFG = deepcopyGraph(G, dfg, variableLabels, factorLabels; graphLabel, kwargs...)
     return destDFG

src/services/DFGVariable.jl

@@ -570,9 +570,9 @@ Delete a Metadata entry at `key` for variable `label` in `dfg`
 """
 function deleteMetadata!(dfg::AbstractDFG, label::Symbol, key::Symbol)
     v = getVariable(dfg, label)
-    rval = pop!(v.smallData, key)
+    pop!(v.smallData, key)
     updateVariable!(dfg, v)
-    return rval
+    return 1
 end
 
 """
@@ -877,8 +877,8 @@ function deleteVariableSolverData!(
     if !haskey(var.solverDataDict, solveKey)
         throw(KeyError("VariableNodeData '$(solveKey)' does not exist"))
     end
-    vnd = pop!(var.solverDataDict, solveKey)
-    return vnd
+    pop!(var.solverDataDict, solveKey)
+    return 1
 end
 
 """
@@ -1073,8 +1073,8 @@ function deletePPE!(dfg::AbstractDFG, variablekey::Symbol, ppekey::Symbol = :def
     if !haskey(var.ppeDict, ppekey)
         throw(KeyError("VariableNodeData '$(ppekey)' does not exist"))
     end
-    vnd = pop!(var.ppeDict, ppekey)
-    return vnd
+    pop!(var.ppeDict, ppekey)
+    return 1
 end
 
 """

test/consol_DataEntryBlobTests.jl

@@ -105,11 +105,11 @@ gde, gdb = getData(dfg, :x1, :random)
 # @test incrDataLabelSuffix(dfg,:x1,:another) == :another_2 # TODO exand support for Regex likely search on labels
 # @test incrDataLabelSuffix(dfg,:x1,"random") == "random_1" # TODO expand support for label::String
 
-dde, ddb = deleteData!(dfg, :x1, :random)
-_, _ = deleteData!(dfg, :x1, :another_1)
+@test deleteData!(dfg, :x1, :random) == 2
+@test deleteData!(dfg, :x1, :another_1) == 2
 
-@test ade == gde == dde
-@test dataset1 == gdb == ddb
+@test ade == gde
+@test dataset1 == gdb
 
 ade2 = addData!(dfg, :x2, deepcopy(ade), dataset1)
 # ade3,adb3 = updateBlob!(dfg, :x2, deepcopy(ade), dataset1)
@@ -135,10 +135,10 @@ addBlobStore!(dfg, ds)
 
 ade = addData!(dfg, :default_inmemory_store, :x1, :random, dataset1)
 gde, gdb = getData(dfg, :x1, :random)
-dde, ddb = deleteData!(dfg, :x1, :random)
+@test deleteData!(dfg, :x1, :random) == 2
 
-@test ade == gde == dde
-@test dataset1 == gdb == ddb
+@test ade == gde
+@test dataset1 == gdb
 
 ade2 = addData!(dfg, :x2, deepcopy(ade), dataset1)
 # ade3,adb3 = updateBlob!(dfg, :x2, deepcopy(ade), dataset1)