DistributedFactorGraphs.jl icon indicating copy to clipboard operation
DistributedFactorGraphs.jl copied to clipboard

Should we deprecate lsfWho in favor of just lsf

Open Affie opened this issue 5 years ago • 2 comments

From PR discussion: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/pull/705#discussion_r540034178

new name is: 'lsfTypes(fg, LinearRelative)'

@dehann There is no function like that. Only lsfTypes(dfg) -> Return Vector{Symbol} of all unique factor types in factor graph.

Did you perhaps mean just lsf as the test just above that?

Should we deprecate lsfWho in favour of just lsf

In this case lsfWho and lsf have almost identical functionality with different implementations. I would say we keep on only lsf for example lsf(fg, LinearRelative)

It can work if there is not a need for lsf(fg, :LinearRelative)

Affie avatar Dec 10 '20 10:12 Affie

yeah that's fine, agree with: don't pass a Symbol, just pass the type (especially to resolve dispatch). 'lsf(fg, Pose2Pose2)'

This function already exists somewhere because i use it quite often. Same goes for 'ls(fg, Pose3)'.

dehann avatar Dec 10 '20 15:12 dehann

@Affie , i agree with your idea to just have simple ls and lsf dispatch. bi quickly tried and this is already working:

using RoME

fg = generateCanonicalFG_Hexagonal(graphinit=false);

julia> lsf(fg, Pose2Pose2)
6-element Array{Symbol,1}:
 :x3x4f1
 :x4x5f1
 :x5x6f1
 :x0x1f1
 :x1x2f1
 :x2x3f1

If you don't mind, please check if there are any other loose bits around. We definitely need to deprecate things like lsfWho.

dehann avatar Dec 10 '20 22:12 dehann

  • [ ] Deprecate lsfWho for just lsf

Affie avatar Aug 28 '24 17:08 Affie