-
Notifications
You must be signed in to change notification settings - Fork 76
Add pybindings for VolumetricGridLookupField #657
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
base: gz-math8
Are you sure you want to change the base?
Conversation
Signed-off-by: gokulp01 <[email protected]>
Signed-off-by: gokulp01 <[email protected]>
Signed-off-by: gokulp01 <[email protected]>
Signed-off-by: gokulp01 <[email protected]>
Hi @scpeters / @adityapande-1995 / @ahcorde, just wanted to follow up on this (: If there’s anything more needed from my end, I’d be happy to address it! Thanks for your time and guidance. |
Signed-off-by: gokulp01 <[email protected]>
Hi @ahcorde thank you for the review! I’ve addressed your feedback. Let me know if there’s anything else to update. Thanks! |
using Class = gz::math::VolumetricGridLookupField<T, I>; | ||
using Vector3Type = gz::math::Vector3<T>; | ||
|
||
auto toString = [](const Class &si) { |
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.
si
is not used
|
||
auto toString = [](const Class &si) { | ||
std::stringstream stream; | ||
stream << "VolumetricGridLookupField"; |
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.
stream << "VolumetricGridLookupField"; | |
stream << si; |
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.
this doesn't work because VolumetricGridLookupField doesn't overload the <<
operator
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.
from a brief inspection, classes that don't overload <<
also don't define a python method for __str__
, so I think you could omit it for VolumetricGridLookupField
example:
Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Gokul <[email protected]>
Hi @gokulp01, there are some review comments that are still not addressed. Do you think you'll have time to get to them before the Jetty code freeze (see https://discourse.openrobotics.org/t/feature-freeze-for-gazebo-jetty/48187/3?u=azeey) |
Hi @azeey sorry, I had been v caught up--I will wrap it up ASAP! |
@gokulp01 we've had a change of policy regarding the feature/code freeze. I suggest targetting |
🎉 New feature
Closes #477
Summary
This pull request adds Python bindings for
VolumetricGridLookupField
, enabling its usage in Python.Changes:
CMakeLists.txt
to build the new Python bindingsInterpolationPoint
pybindings, which are dependencies forVolumetricGridLookupField
VolumetricGridLookupField
Details:
src/python_pybind11/CMakeLists.txt
: Modified to include new files in the buildsrc/python_pybind11/src/_gz_math_pybind11.cc
: Updated to include headers for the bindingssrc/python_pybind11/src/InterpolationPoint.cc
and.hh
: Provide pybindings forInterpolationPoint
src/python_pybind11/src/VolumetricGridLookupField.cc
and.hh
: Implement pybindings forVolumetricGridLookupField
src/python_pybind11/test/VolumetricGridLookupField_TEST.py
: Test file for validating the functionalityTest it
Build the project and test the bindings with
Expected Result:
VolumetricGridLookupField
bindings should function correctly in Python.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.