Proposer: Tully Foote
Present at review:
Question / concerns / comments
There currently is an implementation of swig wrappers around tf in the pytf directory. There are some simple examples of using it in test_pytf.py. And a launch file so you can see it in action.
The current structure is very poor. Everything is in the TransformListener file. It will be moved into the tf package. They module will be called pytf. And I'll reworkd the Make system into CMake.
The biggest question I have is how close the data types are to what people want. I've tried to use the robot_msgs and numpy formats. And extra conversions are available from the transformations.py file in tf.
- I like the "TransformTFToMsg" style of function naming much better than "numpy_transform_stamped_from_transform_stamped". Please change to "transform_stamped_msg_to_numpy"
In TransformListener, I don't feel like naming some methods "_full" is the right way to do polymorphism
- I do like how operator overloading for all the types (particularly for transforms) currently works in C++. Getting the Python code to look similar when doing complex operations is something I would like. Here is an example from plug_in/servo.cpp:
Seems like the functions could be more concise. For example: transform_stamped_from_numpy_transform_stamped -> stamped_from_numpy_stamped or tf_stamped_from_numpy_stamped
- _full methods should maybe be _with_fixed instead?
- Since you can't operate directly on pyTransform, why not just use numpy and only do the conversion behind the scenes? Doesn't seem like there's a lot of benefit to having pyTransform exposed to the user.
Can use just a 4x4 array, can easily be put into numpy. Would remove need for numpy dependency.
Two data types are ok since we can't work directly in the message data type.
Change API to take in numpy and spit out numpy data types since that's the data type you can manipulate. And it will reduce the number of conversions necessary for the user.
Want overloaded operators on the data types to make using transforms easy.
- See above in Stu's comments
If we use regular operators it will make things much simpler (rather than using numpy)
Hide private stuff with _
functions_full is bad
- either replace with default arguments or _with_fixed or _in_time
- Use _in_time
Shorten up converter function names to same syntax as tf, do it to transformations.py too.
Swigging bullet would be cool. To allow math like expressions. Could overhaul/add to transformations.py to add this sugar. This would make things much more powerful.
Examples of how to do things in C++ and parallels in python as a cookbook for transforms would be good.
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved