Fixes for Python 3 compatibility. #978
Conversation
From: https://github.com/aballier/ros_comm/pull/565 tcpros_service.py: encode error messages as bytes in python3 before packing them. test_rospy_tcpros_base.py: check against bytes equality for python3 compatibility. test_rospy_tcpros_pubsub.py: use bytes data instead of str for python3 compatibility. test_rospy_topics.py: check for bytes equality for python3 compatibility test_rosservice_command_line_online.py: pipes are bytes with python3, compare or decode accordingly. Fixes this test with python3. test_bag.py: use range() instead of xrange() so that it works with python 2 & 3. Since it is a test, performance does not matter much.
|
@dirk-thomas Does the Kinetic build somehow test Python 2 and 3, or just one of them? |
|
CI failures appear to be all related to roscpp parameter logic, which has nothing to do with this change. @ros-pull-request-builder retest this please. |
|
It appears similar tests failed on the dev job already: http://build.ros.org/view/Kdev/job/Kdev__ros_comm__ubuntu_xenial_amd64/96/ Likely related to the recent merges... |
|
Seems hard to believe— only three changes went in this morning, and none had anything to do with parameters, unless the select/poll changes are exposing races. But they all passed pre-merge, so that seems weird too. |
|
I am running a few more dev jobs for different commit hashes to figure out where the problem is. Locally I can't reproduce the failing tests 😟 |
|
See #981 for the result of the search. |
|
I just merged #981, which fixed the failing tests on the kinetic-devel branch. Therefore this should hopefully pass now... @ros-pull-request-builder retest this please |
|
We have a checkmark here now, but I'm still hazy on what exactly the buildfarm is able to test. AFAICT, even in Ubuntu Zesty, Wondering if Python 3 compatibility might be an argument for setting up a Travis job on this repo? |
|
We currently don't run CI with Python 3 and the buildfarm doesn't even have an option for it. It would be great to run this locally with Python 3. That being said reading the diff it looks good. Both kind of changes (range without X and popen returning bytes instead of strings) are common updates necessary. So I am fine merging this as is. |
|
Sounds good. Thanks for this contribution, @aballier! |
* tcpros_service.py: encode error messages as bytes in python3 before packing them. * test_rospy_tcpros_base.py: check against bytes equality for python3 compatibility. * test_rospy_tcpros_pubsub.py: use bytes data instead of str for python3 compatibility. * test_rospy_topics.py: check for bytes equality for python3 compatibility * test_rosservice_command_line_online.py: pipes are bytes with python3, compare or decode accordingly. Fixes this test with python3. * test_bag.py: use range() instead of xrange() so that it works with python 2 & 3. Since it is a test, performance does not matter much.
|
I repushed the squashed commit to retain original authorship, since github stripped it out when executing the merge (since I was the creator of the PR). |
tcpros_service.py: encode error messages as bytes in python3 before packing them.
test_rospy_tcpros_base.py: check against bytes equality for python3 compatibility.
test_rospy_tcpros_pubsub.py: use bytes data instead of str for python3 compatibility.
test_rospy_topics.py: check for bytes equality for python3 compatibility
test_rosservice_command_line_online.py: pipes are bytes with python3, compare or decode accordingly. Fixes this test with python3.
test_bag.py: use range() instead of xrange() so that it works with python 2 & 3. Since it is a test, performance does not matter much.
Replaces #565.