Skip to content
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

Should we deprecate lsfWho in favor of just lsf #707

Closed
Affie opened this issue Dec 10, 2020 · 3 comments · Fixed by #1092
Closed

Should we deprecate lsfWho in favor of just lsf #707

Affie opened this issue Dec 10, 2020 · 3 comments · Fixed by #1092

Comments

@Affie
Copy link
Member

Affie commented Dec 10, 2020

From PR discussion: #705 (comment)

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 Affie changed the title @dehann There is no function like that. Should we deprecate lsfWho in favor of just lsf Dec 10, 2020
@Affie Affie added this to the v0.0.x milestone Dec 10, 2020
@dehann
Copy link
Member

dehann commented Dec 10, 2020

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
Copy link
Member

dehann commented Dec 10, 2020

@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.

@Affie Affie self-assigned this Aug 28, 2024
@Affie
Copy link
Member Author

Affie commented Aug 28, 2024

  • Deprecate lsfWho for just lsf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants