Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

add test code for multibyte string#530

Closed
k-okada wants to merge 1 commit intoros:indigo-develfrom
k-okada:add_unicode_test
Closed

add test code for multibyte string#530
k-okada wants to merge 1 commit intoros:indigo-develfrom
k-okada:add_unicode_test

Conversation

@k-okada
Copy link
Copy Markdown
Contributor

@k-okada k-okada commented Nov 9, 2014

I'm not sure if this is what you expected, In python, unicode data is transferred to str data.

@dirk-thomas
Copy link
Copy Markdown
Member

@ros-pull-request-builder retest this please

@k-okada
Copy link
Copy Markdown
Contributor Author

k-okada commented Mar 5, 2016

19:13:15 Build timed out (after 120 minutes). Marking the build as failed.
19:13:15 Build was aborted
19:13:15 [WARNINGS] Skipping pub

the built was failed due to time out.

Req = StringStringRequest

for name in [STRING_CAT_SERVICE_NAKED, STRING_CAT_SERVICE_WRAPPED]:
if sys.version < '3':
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very fragile way to check the major Python version. Please use sys.version_info.major for a numeric comparison.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the version check is even required. The u prefix syntax is valid in both Python 2 and 3. In 2, it produces a unicode, and in 3, a str. So I think the first block would be adequate for both versions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dirk-thomas changed to use sys.version_info.major
@mikepurvis as far as I remember, interpreting unicode has been changed between python 2 vs python 3

k-okada@p40-yoga:/tmp/ros_comm/test/test_rospy/test/rostest$ python
Python 2.7.6 (default, Oct 26 2016, 20:30:19)
[GCC 4.8.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> u'オープンソース' == 'オープンソース'
__main__:1: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them\
 as being unequal
False

and

k-okada@p40-yoga:~$ python3
Python 3.4.3 (default, Nov 17 2016, 01:08:31) 
[GCC 4.8.4] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> u'オープンソース' == 'オープンソース'
True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but look at what happens in the two cases:

if sys.version_info.major < '3':
    # Python 2, returns a <unicode>.
    resp_req = self._test(name, Cls, Req(String(u'ロボット'), Val(u'机器人')))
else:
    # Python 3, returns a <str>.
    resp_req = self._test(name, Cls, Req(String('ロボット'), Val('机器人')))

But you can use the u prefix in both, so the following can replace the above:

# Returns <unicode> in Python 2, <str> in Python 3.
resp_req = self._test(name, Cls, Req(String(u'ロボット'), Val(u'机器人')))

@dirk-thomas
Copy link
Copy Markdown
Member

@ros-pull-request-builder retest this please

@mikepurvis
Copy link
Copy Markdown
Member

Closing in favour of #985.

@mikepurvis mikepurvis closed this Feb 14, 2017
@k-okada k-okada deleted the add_unicode_test branch March 14, 2017 11:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants