-
Notifications
You must be signed in to change notification settings - Fork 10
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
Parse dunder methods in interface file #163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much appreciated feature!
See comments, but as a whole much like!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See unresolved comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment
elif method.name == 'contains': | ||
function_call = f"return self->find({method.args.args_list[0].name}) != self->end();" | ||
function_call = f"return std::find(self->begin(), self->end(), {method.args.args_list[0].name}) != self->end();" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the back-and-forth, but realizing that std::find(..) is always O(n) whereas for many data structures (e.g. std::set) self->find(obj)
should be O(1), so probably using the ->find()
version would be better in this case? Overkill approach I guess would be SFINAE, but that seems excessive in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make up your mind, my guy 😂
I figured the find method would be faster but whoever designed the STL didn't think to make find a method for vectors and lists.
Also, this feature is not crucial. If people want efficiency, then cast to a Python set object (which is easily doable now since we support iteration).
Added support for specifying particular double-underscore aka
dunder
methods. This allows giving support for those methods particularly for classes that should behave like python containers e.g.gtsam::KeySet
.This will be used to fix borglab/gtsam#1606