geomerty2_python3: 0.6.8-1 in 'melodic/distribution.yaml' [bloom]

[k-okada]

Increasing version of package(s) in repository geomerty2_python3 to 0.6.8-1:

• upstream repository: https://github.com/jsk-ros-pkg/geometry2_python3.git
• release repository: https://github.com/tork-a/geometry2_python3-release.git
• distro file: melodic/distribution.yaml
• bloom version: 0.11.1
• previous version for package: null

[k-okada]

same motivation as #32505, cc: @iory

[tfoote]

I'm very worried about how this is going to interact with the greater system.

THere's a lot of c++ code being compiled in here: https://github.com/jsk-ros-pkg/geometry_python3/blob/melodic-devel/tf_python3/CMakeLists.txt I'm worried about these colliding with

And looking here: https://github.com/jsk-ros-pkg/geometry_python3/blob/fc76a49b2edeea990661e3ffc549e2c5ab22681b/tf_conversions_python3/CMakeLists.txt#L56 it's installing tf_conversions to tf which looks wrong.

In general I think that this is a neat trick. But it's not a supported API and as such I think that these sort of hacks should not be released as full packages that look like here's an official python3 backport of geometry2 by maybe make up a jsk_ prefixed package or packages that holds the couple of things that you're building and installing and can hot patch them into your path.

I might even suggest rolling back the opencv_bridge previously done too. In general I don't want to prevent you from doing these things. But I don't want to get generic questions on ROS Answers that are asking how to make tf work under python3 because I found one tutorial that told me to use this officially released package that's basically a hack and requires hot patching your python path and I've never actually run. If you're doing all these install path mutations there's no need to use standard names. And when you give people the hotpatch documentation you can also point them to the alternative package names.

[clalancette]

I originally let in vision_opencv_python3 because there was no risk it would collide with anything else; it was just python code, and the package name was different from the official one, so there was no risk of conflict.

That said, I see the larger issue that the more of these we put in, the more we run the risk of having conflict and also diluting the messaging around Python 2 vs Python 3. Given that Melodic has only 1 year of support life left, trying to put Python 3 support in at this time is maybe not the best idea.

So my inclination is to close this out (along with #33329 and #33328). I'm ambivalent about whether we keep vision_opencv_python3 in the distribution or not. By itself, it probably isn't super useful. But it also shouldn't really hurt anything.

@k-okada does this all make sense to you?

[k-okada]

@clalancette @tfoote thanks for comments, and I fully understand your concerns, but here are my thoughts;

Given that Melodic has only 1 year of support life left, trying to put Python 3 support in at this time is maybe not the best idea.

I understand our approach is not the best, but at the same time, moving to Python3/Noetic of robotic system is not a easy job. It very different from move one package to Noetic/Python3. As you already know, the key of robotic system for us is "integration" and it consists of variety of packages. Thus, usually, it takes a few month to upgrade and test. But just to spend time to upgrade to keep current functionality is few rewards. Actually we use our PR2 with Indigo at this moment.
Of course we can use 2to3 tools, but if robot moves wrong it hard to tell if this is because of Python3, or there are already problem in existing software.
Moreover, Python3 changes str, unicode which has huge impact on Japanese speakers.

But I don't want to get generic questions on ROS Answers that are asking how to make tf work under python3 because I found one tutorial that told me to use this officially released package that's basically a hack and requires hot patching your python path

Me neither. But in the real life, people asking me to use Python3 on their robots, and they sometime breaks robot environment by "hotpatch", "hack" and so on, or stop using a real hardware and use dataset/simulator to show their results, which I really do not want to. So if we release this *_python3 packages, then we just tell them to depends on these packages and I do hot have to help them to fix their environment or convince them to use real hardware.

So for me, provide *_python3 packages is bad solution, but better than other hack/hotpatch approach, and spending time on discuss/fix Python2/Python3 Melodic/Noetic, UpgradeRobot discussion.

I'm very worried about how this is going to interact with the greater system.

This is my concern too. We do not intend to support all melodic packages with Python3. Our target is limited to tf

we need tf2_py and python_orocos_kdl because shared library generated in these packages depends on libpython2.7, so we need rebuild them.

As for tf and tf_conversions, when we import tf from python3 enabled packages, it try to load libpython2.7 and same for tf_conversions, so we need to rebuild them too.

We will have problem if we import tf2_geometry_msgs, tf2_kdl, tf2_sensor_msgs and tf2_ros, because they also load libpython2.7. But since we limited this project to tf, we do not support them. (See https://gist.github.com/k-okada/d6948cab5d1cd8fac8e22da2a8458d6e for how I checked with )

it's installing tf_conversions to tf which looks wrong.

Sorry, you're right. We should check more carefully, @iory could you fix the issue? here is the check result and script -> https://gist.github.com/k-okada/f6bd47a21c9f46b1eae4e8051f772191

[iory]

I programatically investigated what depends on python in the package related to this tf.
I examined the package that generates pythonlib.so in the geometry package by using rosinstall_generator.
I have decided that the one that uses PythonLibs in CMakeLists.txt depends on it.

mkdir -p /tmp/ros/geometry/src
cd /tmp/ros/geometry/src
rosinstall_generator geometry --rosdistro melodic --deps > .rosinstall
wstool up > /dev/null 2>&1
grep --include CMakeLists.txt "^[^#;].*PythonLibs.*" -n -i -r

The result of this command is:

orocos_kinematics_dynamics/python_orocos_kdl/CMakeLists.txt:11:find_package(PythonLibs ${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR} REQUIRED)
rospack/CMakeLists.txt:7:find_package(PythonLibs "${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR}" REQUIRED)
geometry2/tf2_py/CMakeLists.txt:13:find_package(PythonLibs 2 REQUIRED)
ros_comm/roslz4/CMakeLists.txt:44:find_package(PythonLibs "${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR}" REQUIRED)

roslz4 is used in rosbag, but there is no problem if it cannot be imported.
https://github.com/ros/ros_comm/blob/2a92e3d3ccc43ce3e9eac1b4e76bb7450c564aed/tools/rosbag/src/rosbag/bag.py#L70-L77

rospack cannot be imported with python in the first place.

From these things, to use tf in python3 in melodic environment,
only orokos_kinematics_dynamics/python_orocos_kdl and geometry2/tf2_py are enough.

As for tf and tf_conversions, when we import tf from python3 enabled packages, it try to load libpython2.7 and same for tf_conversions, so we need to rebuild them too.

Actually building tf and tf_conversion with python3 is not necessary if tf2_py and PyKDL(orokos_kinematics_dynamics/python_orocos_kdl) are built with python3.

This PR is not needed.

Only these two are required to use tf with python3 in the melodic environment.
#33329
#33330

Also, as @k-okada confirmed, I checked the packages installed on /opt/ros.
https://gist.github.com/k-okada/f6bd47a21c9f46b1eae4e8051f772191?permalink_comment_id=4198910#gistcomment-4198910

It is installed only on /opt/ros/melodic/lib/python3/ so it does not affect other packages.

[sloretz]

if robot moves wrong it hard to tell if this is because of Python3, or there are already problem in existing software

Wouldn't it also be hard to tell if the problem is caused by using Python 3 with Debian packages built with Python 2?

But in the real life, people asking me to use Python3 on their robots, and they sometime breaks robot environment by "hotpatch", "hack" and so on, or stop using a real hardware and use dataset/simulator to show their results, which I really do not want to. So if we release this *_python3 packages, then we just tell them to depends on these packages and I do hot have to help them to fix their environment or convince them to use real hardware.

So for me, provide *_python3 packages is bad solution, but better than other hack/hotpatch approach, and spending time on discuss/fix Python2/Python3 Melodic/Noetic, UpgradeRobot discussion.

If Python 3 and ROS 1 are both requirements, Spending time upgrading to ROS Noetic gets you a much longer support life. If upgrading to Noetic isn't worth the cost, then building ROS Melodic from source with Python 3 would give you the option of patching packages in your workspace past Melodic's EOL date.

[sloretz]

Hi @k-okada,

Would you be willing to prefix the repository and package names with your organization name, jsk_?

There was concern expressed internally that someone who saw and installed ros-melodic-tf2-py-python3 might open an issue on the geometry2 repo to report they can't import it, but someone who installed ros-melodic-jsk-tf2-py or ros-melodic-jsk-tf2-py-python3 would hopefully find the correct repo.

[audrow]

@k-okada, would you also update the meta data in the package.xml files in all of the packages? Specifically,

• name (as @sloretz) mentioned
• description: should mention that this is community supported, in addition to describing the package
• maintainer: should be someone in your organization
• url: should point to documentation you maintain (maybe your repo)

[k-okada]

@audrow thanks for comments, @iory could you change source repository?

[iory]

Thank you for your comments.
I updated the meta information.
Changed tf2_py to jsk_tf2_py_python3 and python_orocos_kdl to jsk_python_orocos_kdl_python3.

[k-okada]

@iroy thanks, @audrow @sloretz @tfoote I have fixed package name jsk_* and released in #33632 #33631

[sloretz]

continued in #33632