Skip to content

Commit 294c1f3

Browse files
AffieAffie
authored
Merge pull request #295 from JuliaRobotics/feature/20Q1/sort#186
sortDFG wrapper close #186 and close #241
2 parents ad25c12 + 7302534 commit 294c1f3

File tree

3 files changed

+101
-124
lines changed

3 files changed

+101
-124
lines changed

‎src/Common.jl

Lines changed: 84 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11

22
import Base: *
33

4-
export sortVarNested, sortDFG
4+
export sortDFG
5+
#Natural less than defined for sorting
6+
export natural_lt
7+
58
export isPrior, lsfPriors
69
export getVariableType, getSofttype
710
export getFactorType, getfnctype
@@ -12,6 +15,7 @@ export findClosestTimestamp, findVariableNearTimestamp
1215
export addTags!
1316
export hasTags, hasTagsNeighbors
1417

18+
# FIXME remove! is it really needed? This is type piracy
1519
*(a::Symbol, b::AbstractString)::Symbol = Symbol(string(a,b))
1620

1721
## Utility functions for getting type names and modules (from IncrementalInference)
@@ -22,130 +26,107 @@ function _getname(t::T) where T
2226
T.name.name
2327
end
2428

25-
"""
26-
$(SIGNATURES)
27-
Test if all elements of the string is a number: Ex, "123" is true, "1_2" is false.
28-
"""
29-
allnums(str::S) where {S <: AbstractString} = occursin(Regex(string(["[0-9]" for j in 1:length(str)]...)), str)
29+
#TODO confirm this is only used in sortVarNested, then delete
30+
# """
31+
# $(SIGNATURES)
32+
# Test if all elements of the string is a number: Ex, "123" is true, "1_2" is false.
33+
# """
34+
# allnums(str::S) where {S <: AbstractString} = occursin(Regex(string(["[0-9]" for j in 1:length(str)]...)), str)
3035
# occursin(r"_+|,+|-+", node_idx)
3136

32-
isnestednum(str::S; delim='_') where {S <: AbstractString} = occursin(Regex("[0-9]+$(delim)[0-9]+"), str)
33-
34-
function sortnestedperm(strs::Vector{<:AbstractString}; delim='_')
35-
str12 = split.(strs, delim)
36-
sp1 = sortperm(parse.(Int,getindex.(str12,2)))
37-
sp2 = sortperm(parse.(Int,getindex.(str12,1)[sp1]))
38-
return sp1[sp2]
39-
end
37+
# isnestednum(str::S; delim='_') where {S <: AbstractString} = occursin(Regex("[0-9]+$(delim)[0-9]+"), str)
4038

41-
function getFirstNumericalOffset(st::AS) where AS <: AbstractString
42-
i = 1
43-
while !allnums(st[i:i]) i+=1; end
44-
return i
45-
end
39+
# function sortnestedperm(strs::Vector{<:AbstractString}; delim='_')
40+
# str12 = split.(strs, delim)
41+
# sp1 = sortperm(parse.(Int,getindex.(str12,2)))
42+
# sp2 = sortperm(parse.(Int,getindex.(str12,1)[sp1]))
43+
# return sp1[sp2]
44+
# end
4645

47-
"""
48-
$SIGNATURES
46+
# function getFirstNumericalOffset(st::AS) where AS <: AbstractString
47+
# i = 1
48+
# while !allnums(st[i:i]) i+=1; end
49+
# return i
50+
# end
51+
#
52+
# """
53+
# $SIGNATURES
54+
#
55+
# Sort a variable list which may have nested structure such as `:x1_2` -- does not sort for alphabetic characters.
56+
# """
57+
# function sortVarNested(vars::Vector{Symbol})::Vector{Symbol}
58+
# # whos nested and first numeric character offset
59+
# sv = string.(vars)
60+
# offsets = getFirstNumericalOffset.(sv)
61+
# masknested = isnestednum.(sv)
62+
# masknotnested = true .⊻ masknested
63+
#
64+
# # strip alphabetic characters from front
65+
# msv = sv[masknotnested]
66+
# msvO = offsets[masknotnested]
67+
# nsv = sv[masknested]
68+
# nsvO = offsets[masknested]
69+
#
70+
# # do nonnested list separately
71+
# nnreducelist = map((s,o) -> s[o:end], msv, msvO)
72+
# nnintlist = parse.(Int, nnreducelist)
73+
# nnp = sortperm(nnintlist)
74+
# nnNums = nnintlist[nnp] # used in mixing later
75+
# nonnested = msv[nnp]
76+
# smsv = vars[masknotnested][nnp]
77+
#
78+
# # do nested list separately
79+
# nestedreducelist = map((s,o) -> s[o:end], nsv, nsvO)
80+
# nestedp = sortnestedperm(nestedreducelist)
81+
# nesNums = parse.(Int, getindex.(split.(nestedreducelist[nestedp], '_'),1)) # used in mixing later
82+
# nested = nsv[nestedp]
83+
# snsv = vars[masknested][nestedp]
84+
#
85+
# # mix back together, pick next sorted item from either pile
86+
# retvars = Vector{Symbol}(undef, length(vars))
87+
# nni = 1
88+
# nesi = 1
89+
# lsmsv = length(smsv)
90+
# lsnsv = length(snsv)
91+
# MAXMAX = 999999999999
92+
# for i in 1:length(vars)
93+
# # inner ifs to ensure bounds and correct sorting at end of each list
94+
# if (nni<=lsmsv ? nnNums[nni] : MAXMAX) <= (nesi<=lsnsv ? nesNums[nesi] : MAXMAX)
95+
# retvars[i] = smsv[nni]
96+
# nni += 1
97+
# else
98+
# retvars[i] = snsv[nesi]
99+
# nesi += 1
100+
# end
101+
# end
102+
# return retvars
103+
# end
104+
#END TODO confirm this is only used in sortVarNested, then delete
49105

50-
Sort a variable list which may have nested structure such as `:x1_2` -- does not sort for alphabetic characters.
51-
"""
52-
function sortVarNested(vars::Vector{Symbol})::Vector{Symbol}
53-
# whos nested and first numeric character offset
54-
sv = string.(vars)
55-
offsets = getFirstNumericalOffset.(sv)
56-
masknested = isnestednum.(sv)
57-
masknotnested = true .⊻ masknested
58-
59-
# strip alphabetic characters from front
60-
msv = sv[masknotnested]
61-
msvO = offsets[masknotnested]
62-
nsv = sv[masknested]
63-
nsvO = offsets[masknested]
64-
65-
# do nonnested list separately
66-
nnreducelist = map((s,o) -> s[o:end], msv, msvO)
67-
nnintlist = parse.(Int, nnreducelist)
68-
nnp = sortperm(nnintlist)
69-
nnNums = nnintlist[nnp] # used in mixing later
70-
nonnested = msv[nnp]
71-
smsv = vars[masknotnested][nnp]
72-
73-
# do nested list separately
74-
nestedreducelist = map((s,o) -> s[o:end], nsv, nsvO)
75-
nestedp = sortnestedperm(nestedreducelist)
76-
nesNums = parse.(Int, getindex.(split.(nestedreducelist[nestedp], '_'),1)) # used in mixing later
77-
nested = nsv[nestedp]
78-
snsv = vars[masknested][nestedp]
79-
80-
# mix back together, pick next sorted item from either pile
81-
retvars = Vector{Symbol}(undef, length(vars))
82-
nni = 1
83-
nesi = 1
84-
lsmsv = length(smsv)
85-
lsnsv = length(snsv)
86-
MAXMAX = 999999999999
87-
for i in 1:length(vars)
88-
# inner ifs to ensure bounds and correct sorting at end of each list
89-
if (nni<=lsmsv ? nnNums[nni] : MAXMAX) <= (nesi<=lsnsv ? nesNums[nesi] : MAXMAX)
90-
retvars[i] = smsv[nni]
91-
nni += 1
92-
else
93-
retvars[i] = snsv[nesi]
94-
nesi += 1
95-
end
96-
end
97-
return retvars
98-
end
99106

100107
"""
101108
$SIGNATURES
102109
103-
Sort variable (factor) lists in a meaningful way, for example `[:april;:x1_3;:x1_6;]`
110+
Convenience wrapper for `Base.sort`.
111+
Sort variable (factor) lists in a meaningful way (by `timestamp`, `label`, etc), for example `[:april;:x1_3;:x1_6;]`
112+
Defaults to sorting by timestamp for variables and factors and using `natural_lt` for Symbols.
113+
See Base.sort for more detail.
104114
105115
Notes
106116
- Not fool proof, but does better than native sort.
107117
108118
Example
109119
110120
`sortDFG(ls(dfg))`
121+
`sortDFG(ls(dfg), by=getLabel, lt=natural_lt)`
111122
112123
Related
113124
114125
ls, lsf
115126
"""
116-
sortDFG(vars::Vector{Symbol})::Vector{Symbol} = sortVarNested(vars)
117-
118-
#TODO Is this deprecate in favor of getFactorType, or OBSOLETE?
119-
# """
120-
# $SIGNATURES
121-
#
122-
# Return the factor type used in a `::DFGFactor`.
123-
#
124-
# Notes:
125-
# - OBSOLETE, use newer getFactorType instead.
126-
#
127-
# Related
128-
#
129-
# getFactorType
130-
# """
131-
# function getfnctype(data::GenericFunctionNodeData)
132-
# # TODO what is this?
133-
# if typeof(data).name.name == :VariableNodeData
134-
# return VariableNodeData
135-
# end
136-
#
137-
# # this looks right
138-
# return data.fnc.usrfnc!
139-
# end
140-
# function getfnctype(fact::DFGFactor, solveKey::Symbol=:default)
141-
# data = getSolverData(fact) # TODO , solveKey)
142-
# return getfnctype(data)
143-
# end
144-
# function getfnctype(dfg::T, lbl::Symbol, solveKey::Symbol=:default) where T <: AbstractDFG
145-
# getfnctype(getFactor(dfg, exvertid))
146-
# end
127+
sortDFG(vars::Vector{<:DFGNode}; by=getTimestamp, kwargs...) = sort(vars; by=by, kwargs...)
128+
sortDFG(vars::Vector{Symbol}; lt=natural_lt, kwargs...)::Vector{Symbol} = sort(vars; lt=lt, kwargs...)
147129

148-
@deprecate getfnctype(args...) getFactorType(args...)
149130

150131
"""
151132
$SIGNATURES

‎src/Deprecated.jl

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -171,19 +171,7 @@ function buildSubgraphFromLabels(dfg::G,
171171
buildSubgraphFromLabels!(dfg, syms, subfg=subfg, solvable=solvable, allowedFactors=allowedFactors )
172172
end
173173

174-
# NOTE Fully depcrecate nodeCounter and labelDict from LightGraphs
175-
# Base.propertynames(x::LightDFG, private::Bool=false) =
176-
# (:g, :description, :userId, :robotId, :sessionId, :nodeCounter, :labelDict, :addHistory, :solverParams)
177-
# # (private ? fieldnames(typeof(x)) : ())...)
178-
#
179-
# Base.getproperty(x::LightDFG,f::Symbol) = begin
180-
# if f == :nodeCounter
181-
# @error "Field nodeCounter deprecated. returning number of nodes"
182-
# nv(x.g)
183-
# elseif f == :labelDict
184-
# @error "Field labelDict deprecated. Consider using exists(dfg,label) or getLabelDict(dfg) instead. Returning internals copy"
185-
# copy(x.g.labels.sym_int)
186-
# else
187-
# getfield(x,f)
188-
# end
189-
# end
174+
@deprecate sortVarNested(vars::Vector{Symbol}) sortDFG(vars)
175+
176+
#TODO Deprecated or obsolete?
177+
@deprecate getfnctype(args...) getFactorType(args...)

‎test/testBlocks.jl

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -767,13 +767,21 @@ function testGroup!(fg, v1, v2, f0, f1)
767767

768768
@testset "Sorting" begin
769769
unsorted = [:x1_3;:x1_6;:l1;:april1] #this will not work for :x1x2f1
770-
@test sortDFG(unsorted) == sortVarNested(unsorted)
771-
@test sort([:x1x2f1, :x1l1f1], lt=DistributedFactorGraphs.natural_lt) == [:x1l1f1, :x1x2f1]
770+
@test @test_deprecated sortVarNested(unsorted) == sortDFG(unsorted)
771+
@test sort([:x1x2f1, :x1l1f1], lt=natural_lt) == [:x1l1f1, :x1x2f1]
772+
773+
# NOTE Some of what is possible with sort and the wrappers
772774
l = [:a1, :X1, :b1c2, :x2_2, :c, :x1, :x10, :x1_1, :x10_10,:a, :x2_1, :xy3, :l1, :x1_2, :x1l1f1, Symbol("1a1"), :x1x2f1]
773-
@test sort(l, lt=DistributedFactorGraphs.natural_lt) == [Symbol("1a1"), :X1, :a, :a1, :b1c2, :c, :l1, :x1, :x1_1, :x1_2, :x1l1f1, :x1x2f1, :x2_1, :x2_2, :x10, :x10_10, :xy3]
774-
# NOTE Some of what is possible with sort
775-
@test getLabel.(sort(getVariables(fg), lt=DistributedFactorGraphs.natural_lt, by=getLabel)) == [:a, :b]
775+
@test sortDFG(l) == [Symbol("1a1"), :X1, :a, :a1, :b1c2, :c, :l1, :x1, :x1_1, :x1_2, :x1l1f1, :x1x2f1, :x2_1, :x2_2, :x10, :x10_10, :xy3]
776+
@test sort(l, lt=natural_lt) == [Symbol("1a1"), :X1, :a, :a1, :b1c2, :c, :l1, :x1, :x1_1, :x1_2, :x1l1f1, :x1x2f1, :x2_1, :x2_2, :x10, :x10_10, :xy3]
777+
778+
@test getLabel.(sortDFG(getVariables(fg), lt=natural_lt, by=getLabel)) == [:a, :b]
779+
@test getLabel.(sort(getVariables(fg), lt=natural_lt, by=getLabel)) == [:a, :b]
780+
781+
@test getLabel.(sortDFG(getFactors(fg))) == [:abf1, :af1]
776782
@test getLabel.(sort(getFactors(fg), by=getTimestamp)) == [:abf1, :af1]
783+
784+
@test getLabel.(sortDFG(vcat(getVariables(fg),getFactors(fg)), lt=natural_lt, by=getLabel)) == [:a,:abf1, :af1, :b]
777785
end
778786

779787
@testset "Some more helpers to sort out" begin

0 commit comments

Comments
 (0)