Skip to content

change delete return type #1134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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`.

Expand Down
2 changes: 1 addition & 1 deletion ext/DFGPlots.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions src/DataBlobs/services/BlobEntry.jl
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@
- 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)
Expand All @@ -217,7 +218,8 @@
),
)
end
return deleteat!(var.blobEntries, findfirst(x -> x.label == key, var.blobEntries))
deleteat!(var.blobEntries, findfirst(x -> x.label == key, var.blobEntries))
return 1

Check warning on line 222 in src/DataBlobs/services/BlobEntry.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/BlobEntry.jl#L221-L222

Added lines #L221 - L222 were not covered by tests
end

function deleteBlobEntry!(dfg::AbstractDFG, label::Symbol, key::Symbol)
Expand Down
26 changes: 8 additions & 18 deletions src/DataBlobs/services/BlobStores.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,23 +128,13 @@
# 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)

Check warning on line 133 in src/DataBlobs/services/BlobStores.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/BlobStores.jl#L132-L133

Added lines #L132 - L133 were not covered by tests
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)
Expand Down Expand Up @@ -241,10 +231,8 @@

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?
Expand Down Expand Up @@ -292,7 +280,8 @@
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)
Expand Down Expand Up @@ -430,7 +419,8 @@
end

function deleteBlob!(store::RowBlobStore, blobId::UUID)
return getfield(pop!(store.blobs, blobId), :blob)
getfield(pop!(store.blobs, blobId), :blob)
return 1

Check warning on line 423 in src/DataBlobs/services/BlobStores.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/BlobStores.jl#L422-L423

Added lines #L422 - L423 were not covered by tests
end

hasBlob(store::RowBlobStore, blobId::UUID) = haskey(store.blobs, blobId)
Expand Down
14 changes: 8 additions & 6 deletions src/DataBlobs/services/HelpersDataWrapEntryBlob.jl
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,10 @@
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!(
Expand All @@ -296,7 +297,8 @@
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

Check warning on line 303 in src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl#L300-L303

Added lines #L300 - L303 were not covered by tests
end
13 changes: 8 additions & 5 deletions src/Deprecated.jl
Original file line number Diff line number Diff line change
@@ -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)
Comment on lines +6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
@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)
@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
##=================================================================================
Expand Down Expand Up @@ -62,8 +70,3 @@ DFGSummary(args) = error("DFGSummary is deprecated")

@deprecate lsfWho(dfg::AbstractDFG, type::Symbol) lsf(dfg, getfield(Main, type))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change

## ================================================================================
## Deprecated in v0.23
##=================================================================================
#NOTE free up getNeighbors to return the variables or factors
@deprecate getNeighbors(args...; kwargs...) listNeighbors(args...; kwargs...)
11 changes: 4 additions & 7 deletions src/GraphsDFG/services/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,18 @@

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(
Expand Down Expand Up @@ -319,7 +316,7 @@
return neighbors_ll::Vector{Symbol}
end

function getNeighborhood(
function listNeighborhood(

Check warning on line 319 in src/GraphsDFG/services/GraphsDFG.jl

View check run for this annotation

Codecov / codecov/patch

src/GraphsDFG/services/GraphsDFG.jl#L319

Added line #L319 was not covered by tests
dfg::GraphsDFG,
variableFactorLabels::Vector{Symbol},
distance::Int;
Expand Down
20 changes: 15 additions & 5 deletions src/services/AbstractDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
function deleteAgentMetadata!(dfg::AbstractDFG, key::Symbol)
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)
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/services/DFGVariable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand Down Expand Up @@ -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

"""
Expand Down Expand Up @@ -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

"""
Expand Down
14 changes: 7 additions & 7 deletions test/consol_DataEntryBlobTests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
14 changes: 4 additions & 10 deletions test/iifInterfaceTests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,11 @@ end
@test updateFactor!(dfg2, f2) == 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 dv3 == v3
@test dv3.label == v3.label
dv3 = deleteVariable!(dfg2, v3)
@test dv3 == 2
@test_throws ErrorException deleteVariable!(dfg2, v3)

@test issetequal(ls(dfg2), [:a, :b])
df2 = dv3facs[1]
#TODO write compare if we want to compare complete one, for now just label
# @test df2 == f2
@test df2.label == f2.label
@test_throws ErrorException deleteFactor!(dfg2, f2)

@test lsf(dfg2) == [:abf1]
Expand Down Expand Up @@ -290,10 +284,10 @@ end
@test listBlobEntries(dfg, :b) == Symbol[:key2]

#delete
@test deleteBlobEntry!(v1, :key1) == de1
@test deleteBlobEntry!(v1, :key1) == 1
@test listBlobEntries(v1) == Symbol[:key2]
#delete from ddfg
@test deleteBlobEntry!(dfg, :a, :key2) == de2_update
@test deleteBlobEntry!(dfg, :a, :key2) == 1
@test listBlobEntries(v1) == Symbol[]
end

Expand Down
Loading
Loading