Standardize delete! and merge!

Project.toml

@@ -65,7 +65,7 @@ SHA = "0.7, 1"
 SparseArrays = "1.11"
 StaticArrays = "1"
 Statistics = "1.11"
-StructUtils = "2.6.0"
+StructUtils = "=2.7.0"
 Tables = "1.11.1"
 Tar = "1.9"
 TensorCast = "0.3.3, 0.4"

docs/src/BuildingGraphs.md

@@ -116,7 +116,7 @@ To list all variables or factors (instead of just their labels), use the
 Traversing and Querying functions for finding the relationships and building subtraphs include:  
 
 - [`listNeighbors`](@ref)
-- [`buildSubgraph`](@ref)
+- [`getSubgraph`](@ref)
 - [`getBiadjacencyMatrix`](@ref)
 
 ## Getting (Reading) Variables and Factors

src/DataBlobs/services/BlobEntry.jl

@@ -46,6 +46,7 @@ function mergeBlobentry!(node, entry::Blobentry)
 end
 
 function mergeBlobentries!(node, entries::Vector{Blobentry})
+    #TODO optimize with something like: merge!(refBlobentries(node), entries)
     mergeBlobentry!.(node, entries)
     return length(entries)
 end
@@ -54,16 +55,15 @@ end
     $(SIGNATURES)
 """
 function deleteBlobentry!(node, label::Symbol)
-    !haskey(refBlobentries(node), label) && throw(LabelNotFoundError("Blobentry", label))
+    !haskey(refBlobentries(node), label) && return 0
     pop!(refBlobentries(node), label)
     return 1
 end
 
 deleteBlobentry!(node, entry) = deleteBlobentry!(node, getLabel(entry))
 
 function deleteBlobentries!(node, labels::Vector{Symbol})
-    deleteBlobentry!.(node, labels)
-    return length(labels)
+    return sum(deleteBlobentry!.(node, labels))
 end
 
 """

src/DataBlobs/services/BlobStores.jl

@@ -172,15 +172,12 @@ function deleteBlob!(store::FolderStore{T}, blobid::UUID) where {T}
         rm(blobfilename)
         # Create a tombstone marker
         open(tombstonefile, "w") do f
-            return write(f, "deleted")
+            return write(f, string("DELETED: ", now(UTC)))
         end
         return 1
-    elseif isfile(tombstonefile)
-        # Already deleted
-        return 0
     else
-        # Not found
-        throw(IdNotFoundError("Blob", blobid))
+        # Already deleted or doesn't exist
+        return 0
     end
 end
 
@@ -235,9 +232,7 @@ function addBlob!(store::InMemoryBlobstore{T}, blobid::UUID, data::T) where {T}
 end
 
 function deleteBlob!(store::InMemoryBlobstore, blobid::UUID)
-    if !haskey(store.blobs, blobid)
-        throw(IdNotFoundError("Blob", blobid))
-    end
+    !haskey(store.blobs, blobid) && return 0
     pop!(store.blobs, blobid)
     return 1
 end
@@ -372,9 +367,7 @@ function addBlob!(store::RowBlobstore{T}, blobid::UUID, blob::T) where {T}
 end
 
 function deleteBlob!(store::RowBlobstore, blobid::UUID)
-    if !haskey(store.blobs, blobid)
-        throw(IdNotFoundError("Blob", blobid))
-    end
+    !haskey(store.blobs, blobid) && return 0
     pop!(store.blobs, blobid)
     return 1
 end

src/Deprecated.jl

@@ -4,6 +4,7 @@
 export FactorCompute
 const FactorCompute = FactorDFG
 
+#TODO maybe keep for Bloblet type assert later.
 const MetadataTypes = Union{
     Int,
     Float64,
@@ -424,3 +425,205 @@ function getCoordinates(
     X = ManifoldsBase.log(M, p0, p)
     return ManifoldsBase.get_coordinates(M, p0, X, basis)
 end
+
+# old merge and copy functions, will be replaced by cleaner merge!, sync!, and copyto! functions
+
+# """
+#     $(SIGNATURES)
+# Merger sourceDFG to destDFG given an optional list of variables and factors and distance.
+# Notes:
+# - Nodes already in the destination graph are updated from sourceDFG.
+# - Orphaned factors (where the subgraph does not contain all the related variables) are not included.
+# Related:
+# - [`copyGraph!`](@ref)
+# - [`buildSubgraph`](@ref)
+# - [`listNeighborhood`](@ref)
+# - [`deepcopyGraph`](@ref)
+# """
+
+##==============================================================================
+## Copy Functions #TODO replace with sync
+##==============================================================================
+
+"""
+    $(SIGNATURES)
+Common function for copying nodes from one graph into another graph.
+This is overridden in specialized implementations for performance.
+Orphaned factors are not added, with a warning if verbose.
+Set `overwriteDest` to overwrite existing variables and factors in the destination DFG.
+NOTE: `copyGraphMetadata` is deprecated – use agent/graph Bloblets instead.
+Related:
+- [`deepcopyGraph`](@ref)
+- [`deepcopyGraph!`](@ref)
+- [`buildSubgraph`](@ref)
+- [`listNeighborhood`](@ref)
+- [`mergeGraph!`](@ref)
+"""
+#
+function copyGraph!(
+    destDFG::AbstractDFG,
+    sourceDFG::AbstractDFG,
+    variableLabels::AbstractVector{Symbol} = listVariables(sourceDFG),
+    factorLabels::AbstractVector{Symbol} = listFactors(sourceDFG);
+    copyGraphMetadata::Bool = false,
+    overwriteDest::Bool = false,
+    deepcopyNodes::Bool = false,
+    verbose::Bool = false,
+    showprogress::Bool = verbose,
+)
+    # Split into variables and factors
+    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
+
+        variableCopy = deepcopyNodes ? deepcopy(variable) : variable
+        if !hasVariable(destDFG, variable.label)
+            addVariable!(destDFG, variableCopy)
+        elseif overwriteDest
+            mergeVariable!(destDFG, variableCopy)
+        else
+            throw(LabelExistsError("Variable", variable.label))
+        end
+    end
+    # And then all factors to the destDFG.
+    @showprogress desc = "copy factors" enabled = showprogress for factor in sourceFactors
+        # Get the original factor variables (we need them to create it)
+        sourceFactorVariableIds = collect(factor.variableorder)
+        # Find the labels and associated variables in our new subgraph
+        factVariableIds = Symbol[]
+        for variable in sourceFactorVariableIds
+            if hasVariable(destDFG, variable)
+                push!(factVariableIds, variable)
+            end
+        end
+        # Only if we have all of them should we add it (otherwise strange things may happen on evaluation)
+        if length(factVariableIds) == length(sourceFactorVariableIds)
+            factorCopy = deepcopyNodes ? deepcopy(factor) : factor
+            if !hasFactor(destDFG, factor.label)
+                addFactor!(destDFG, factorCopy)
+            elseif overwriteDest
+                mergeFactor!(destDFG, factorCopy)
+            else
+                throw(LabelExistsError("Factor", factor.label))
+            end
+        elseif verbose
+            @warn "Factor $(factor.label) will be an orphan in the destination graph, and therefore not added."
+        end
+    end
+
+    if copyGraphMetadata
+        error(
+            "copyGraphMetadata keyword has been removed – metadata APIs were replaced by Bloblets. " *
+            "Copy agent/graph Bloblets manually before calling copyGraph!",
+        )
+    end
+    return nothing
+end
+
+"""
+    $(SIGNATURES)
+Copy nodes from one graph into another graph by making deepcopies.
+see [`copyGraph!`](@ref) for more detail.
+Related:
+- [`deepcopyGraph`](@ref)
+- [`buildSubgraph`](@ref)
+- [`listNeighborhood`](@ref)
+- [`mergeGraph!`](@ref)
+"""
+#
+function deepcopyGraph!(
+    destDFG::AbstractDFG,
+    sourceDFG::AbstractDFG,
+    variableLabels::Vector{Symbol} = ls(sourceDFG),
+    factorLabels::Vector{Symbol} = lsf(sourceDFG);
+    kwargs...,
+)
+    return copyGraph!(
+        destDFG,
+        sourceDFG,
+        variableLabels,
+        factorLabels;
+        deepcopyNodes = true,
+        kwargs...,
+    )
+end
+
+"""
+    $(SIGNATURES)
+Copy nodes from one graph into a new graph by making deepcopies.
+see [`copyGraph!`](@ref) for more detail.
+Related:
+- [`deepcopyGraph!`](@ref)
+- [`buildSubgraph`](@ref)
+- [`listNeighborhood`](@ref)
+- [`mergeGraph!`](@ref)
+"""
+#
+function deepcopyGraph(
+    ::Type{T},
+    sourceDFG::AbstractDFG,
+    variableLabels::Vector{Symbol} = ls(sourceDFG),
+    factorLabels::Vector{Symbol} = lsf(sourceDFG);
+    graphLabel::Symbol = Symbol(getGraphLabel(sourceDFG), "_cp_$(string(uuid4())[1:6])"),
+    kwargs...,
+) where {T <: AbstractDFG}
+    destDFG = T(;
+        solverParams = getSolverParams(sourceDFG),
+        graph = sourceDFG.graph,
+        agent = sourceDFG.agent,
+        graphLabel,
+    )
+    copyGraph!(
+        destDFG,
+        sourceDFG,
+        variableLabels,
+        factorLabels;
+        deepcopyNodes = true,
+        kwargs...,
+    )
+    return destDFG
+end
+
+function mergeGraph!(
+    destDFG::AbstractDFG,
+    sourceDFG::AbstractDFG,
+    variableLabels::Vector{Symbol},
+    factorLabels::Vector{Symbol} = lsf(sourceDFG),
+    distance::Int = 0;
+    solvableFilter = nothing,
+    tagsFilter = nothing,
+    kwargs...,
+)
+    Base.depwarn(
+        """
+            mergeGraph! with variableLabels, factorLabels, and distance is deprecated. 
+            For now use function composition ending with mergeGraph!,
+            syncGraph!(Coming soon) will replace this functionality.
+        """,
+        :mergeGraph!,
+    )
+    # find neighbors at distance to add
+    sourceVariables, sourceFactors = listNeighborhood(
+        sourceDFG,
+        union(variableLabels, factorLabels),
+        distance;
+        solvableFilter,
+        tagsFilter,
+    )
+
+    copyGraph!(
+        destDFG,
+        sourceDFG,
+        sourceVariables,
+        sourceFactors;
+        deepcopyNodes = true,
+        overwriteDest = true,
+        kwargs...,
+    )
+
+    return destDFG
+end
+
+@deprecate buildSubgraph(args...; kwargs...) getSubgraph(args...; kwargs...)

src/DistributedFactorGraphs.jl

@@ -77,7 +77,7 @@ export AbstractStateType, StateType
 ##------------------------------------------------------------------------------
 ## Types
 ##------------------------------------------------------------------------------
-#TODO types are not yet stable - also, we might not export types such as VariableCompute
+#TODO types are not yet stable - also, we might not export all types
 # Variables
 export VariableDFG
 export VariableSummary
@@ -181,7 +181,7 @@ export listFactors
 export listStates
 
 ##
-export getObservation
+public getObservation
 
 ##------------------------------------------------------------------------------
 # Tags
@@ -307,10 +307,10 @@ export listFactorBloblets
 export listGraphBloblets
 export listAgentBloblets
 
-# export hasVariableBloblet
-# export hasFactorBloblet
-# export hasGraphBloblet
-# export hasAgentBloblet
+export hasVariableBloblet
+export hasFactorBloblet
+export hasGraphBloblet
+export hasAgentBloblet
 
 ## v1 name, signiture, and return
 
@@ -485,7 +485,7 @@ const unstable_functions::Vector{Symbol} = [
     :natural_lt, #TODO do we export stable functions such as natural_lt or just mark as public
     :sortDFG, #TODO do we export stable functions such as natural_lt or just mark as public
     :mergeGraph!,
-    :buildSubgraph,
+    :getSubgraph,
     :incrDataLabelSuffix,# TODO somewhat used, do we deprecate?
 
     # set # TODO what to do here, maybe `ref` verb + setproperty.  
@@ -509,7 +509,7 @@ const unstable_functions::Vector{Symbol} = [
     :unpackDistribution,
     :hasTagsNeighbors,
     # :updateBlobstore!,## TODO deprecated or obsolete
-    :emptyMetadata!, #TODO maybe deprecate for just deleteMetadata!
+    # :emptyMetadata!, #TODO maybe deprecate for just deleteMetadata!
     # :emptyBlobstore!, #TODO maybe deprecate for just deleteBlobstore!
     :MetadataTypes, #maybe make public after metadata stable
     :getVariableTypeName,

src/GraphsDFG/GraphsDFG.jl

@@ -11,6 +11,7 @@ using ...DistributedFactorGraphs:
     Agent,
     LabelNotFoundError,
     LabelExistsError,
+    MergeConflictError,
     Graphroot,
     AbstractGraphVariable,
     AbstractGraphFactor,
@@ -25,7 +26,8 @@ using ...DistributedFactorGraphs:
     Blobentries,
     FolderStore,
     refTags,
-    listTags
+    listTags,
+    patch!
 
 # import DFG functions to extend
 import ...DistributedFactorGraphs:
@@ -53,7 +55,6 @@ import ...DistributedFactorGraphs:
     isConnected,
     listNeighbors,
     buildSubgraph,
-    copyGraph!,
     getBiadjacencyMatrix,
     toDot,
     toDotFile,