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

KeySet class in python is missing methods #1606

Closed
dwiel opened this issue Aug 18, 2023 · 4 comments · Fixed by borglab/wrap#163 or #1712
Closed

KeySet class in python is missing methods #1606

dwiel opened this issue Aug 18, 2023 · 4 comments · Fixed by borglab/wrap#163 or #1712
Labels
python Related to python wrapper

Comments

@dwiel
Copy link

dwiel commented Aug 18, 2023

Feature

The KeySet class in python should have more methods available like __iter__, __contains__, remove, etc.

Motivation

I'd like to be able to do things like:

for key in graph.keys():
    if key not in initial_estimate.keys():
        print(f"WARNING: missing key {key} in initial_estimate")
@ProfFan
Copy link
Collaborator

ProfFan commented Aug 27, 2023

Yeah this would be a great extension. We may need some special code to do this though, as KeySet is a bit more than a standard std::set. CC @varunagrawal

@varunagrawal
Copy link
Collaborator

I think the easiest way to achieve this is to replace gtsam::KeySet with std::set<gtsam::Key> (aka the superclass) in all the .i files?

@varunagrawal varunagrawal added the python Related to python wrapper label Oct 10, 2023
@varunagrawal
Copy link
Collaborator

A better way to do this is to integrate dunder methods (e.g. __iter__) into our wrapper library gtwrap. @dwiel can you compile a list of all the special methods needed?

@varunagrawal
Copy link
Collaborator

Accidentally closed this. This issue will be resolved when we merge #1712.

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

Successfully merging a pull request may close this issue.

3 participants