Skip to content
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

Make tf2_py python3 compatible again #257

Merged
merged 2 commits into from Jul 24, 2017

Conversation

@de-vri-es
Copy link
Contributor

de-vri-es commented Jul 16, 2017

120a532 reintroduced usage of PyString_FromString, which should be stringToPython from python_compat.h (which actually calls PyUnicode_FromString when compiled for python3). This PR does that so tf2_py can be compiled for python3 again.

It would be nice to have a python3 platform on CI somewhere to prevent this from accidentally happening again. I'm guessing that isn't as straightforward as one might hope though.

Additionally, this PR changes some print statements to print functions in assorted python scripts for python 3 compatibility.

@de-vri-es de-vri-es force-pushed the fizyr-forks:python3 branch from beff193 to edc7c29 Jul 16, 2017
@@ -34,6 +34,9 @@
#*
#* Author: Eitan Marder-Eppstein
#***********************************************************

from __futute__ import print_function

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Jul 16, 2017

Contributor

Looks like there is a typo: __futute__ => __future__. (same below)
Nice catch for the python functions!

This comment has been minimized.

Copy link
@de-vri-es

de-vri-es Jul 16, 2017

Author Contributor

Woops, thanks. Fixed.

@de-vri-es de-vri-es force-pushed the fizyr-forks:python3 branch from edc7c29 to 840caf5 Jul 16, 2017
@nuclearsandwich

This comment has been minimized.

Copy link
Member

nuclearsandwich commented Jul 17, 2017

@ros-pull-request-builder retest this please.

@tfoote

This comment has been minimized.

Copy link
Member

tfoote commented Jul 24, 2017

Thanks for the patch. Yeah it isn't straight forward for testing python3. We don't have an easy way to test python3 since it's not officially supported platform we don't have support for it in the ros_buildfarm targets. Starting to add the many different potential configurations starts to become combinatoric complexity for testing/support. We do get some advanced testing from users on Python3 default platforms like arch and gentoo.

@tfoote tfoote merged commit 6f721c3 into ros:indigo-devel Jul 24, 2017
4 checks passed
4 checks passed
Ipr__geometry2__ubuntu_trusty_amd64 Build finished.
Details
Jpr__geometry2__ubuntu_trusty_amd64 Build finished.
Details
Kpr__geometry2__ubuntu_xenial_amd64 Build finished.
Details
Lpr__geometry2__ubuntu_xenial_amd64 Build finished.
Details
@de-vri-es

This comment has been minimized.

Copy link
Contributor Author

de-vri-es commented Jul 25, 2017

We do get some advanced testing from users on Python3 default platforms like arch and gentoo.

Yeah, I noticed this when updating ros-kinetic-tf2-py in our local binary Arch Linux repository at work :) I tend to notice compilation problems a lot more than runtime python errors though, since we mainly use C++ for our own projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.