-
Notifications
You must be signed in to change notification settings - Fork 530
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
Add Python3 compatibility #300
Add Python3 compatibility #300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for PR @kartikmohta, please check a comment in the code.
if sys.version_info >= (3, 0): | ||
string_types = str | ||
else: | ||
string_types = (str, unicode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this into rosbridge_library/utils/__init__.py
and import from there? Then, we can avoid copy and pasting of the same functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that seems better 👍
* First pass at Python 3 compatibility * message_conversion: Only call encode on a Python2 str or bytes type * protocol.py: Changes for dict in Python3. Compatible with Python 2 too. * More Python 3 fixes, all tests pass * Move definition of string_types to rosbridge_library.util
All the unit tests seem to pass with both Python 2 and 3.