Added Visual Python interface #950
Conversation
Signed-off-by: ahcorde <ahcorde@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #950 +/- ##
=======================================
Coverage 66.66% 66.66%
=======================================
Files 2 2
Lines 27 27
=======================================
Hits 18 18
Misses 9 9 Continue to review full report at Codecov.
|
Signed-off-by: ahcorde <ahcorde@gmail.com>
| "Get the transparency value of the visual") | ||
| .def("set_transparency", &sdf::Visual::SetTransparency, | ||
| "Set the transparency value for the visual") | ||
| .def("geometry", &sdf::Visual::Geom, |
There was a problem hiding this comment.
The name of this function is different from the C++ version. Not sure what our policy is on this. @scpeters?
There was a problem hiding this comment.
I think it's ok to deviate from the C++ API. We could consider depreciating the C++ names and offering new APIs where it seems like an improvement (such as Geom -> Geometry)
| "Get a pointer to the collisions's geometry.") | ||
| .def("set_geometry", &sdf::Visual::SetGeom, | ||
| "Set the collision's geometry") | ||
| .def("raw_pose", &sdf::Visual::RawPose, |
There was a problem hiding this comment.
This uses return_value_policy::automatic, which makes a copy of lvalue references. Is that what we want? This applies to all the functions that return lvalue references.
There was a problem hiding this comment.
But it's a const reference, I thought is fine to return this as a copy. @scpeters ?
There was a problem hiding this comment.
Most of the items we return are small objects, so returning a copy makes sense. I just wanted to make sure we were doing this intentionally. Technically, it would be a change in behavior in that if I get a raw pose by calling raw_pose then call set_raw_pose, it wouldn't affect the raw pose I obtained earlier whereas in C++ it would.
It would be good to document these kind of differences between the C++ and Python API somewhere. Are we generating python docs and uploading them somewhere for ign-math? If we don't have the infrastructure setup yet, maybe we should put a README in the src/python.
Signed-off-by: ahcorde <ahcorde@gmail.com>
| "Get the transparency value of the visual") | ||
| .def("set_transparency", &sdf::Visual::SetTransparency, | ||
| "Set the transparency value for the visual") | ||
| .def("geometry", &sdf::Visual::Geom, |
| "Get a pointer to the collisions's geometry.") | ||
| .def("set_geometry", &sdf::Visual::SetGeom, | ||
| "Set the collision's geometry") | ||
| .def("raw_pose", &sdf::Visual::RawPose, |
There was a problem hiding this comment.
Most of the items we return are small objects, so returning a copy makes sense. I just wanted to make sure we were doing this intentionally. Technically, it would be a change in behavior in that if I get a raw pose by calling raw_pose then call set_raw_pose, it wouldn't affect the raw pose I obtained earlier whereas in C++ it would.
It would be good to document these kind of differences between the C++ and Python API somewhere. Are we generating python docs and uploading them somewhere for ign-math? If we don't have the infrastructure setup yet, maybe we should put a README in the src/python.
Signed-off-by: ahcorde ahcorde@gmail.com
🎉 New feature
This PR is part of this meta ticket #931
Require:
Summary
Added Visual Python interface
Test it
Checklist
codecheckpassed (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-bymessages.