Skip to content

Conversation

@v4hn
Copy link
Contributor

@v4hn v4hn commented Oct 10, 2019

This fixes the remaining issues coming up when generating https://github.com/ros-planning/moveit_tutorials .

It would be nice to see a melodic release for these fixes.

def _generate_msg_text_from_spec(package, spec, msg_context, buff=None, indent=0):
if buff is None:
buff = cStringIO.StringIO()
buff = io.StringIO()
Copy link
Member

Choose a reason for hiding this comment

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

For full compatibility we have a conditional import to do this: http://wiki.ros.org/python_2_and_3_compatible_code

"""
config_list = [c for c in rd_configs.itervalues() if c['builder'] != 'rosmake']
try:
rd_configs_values = rd_configs.values()
Copy link
Member

Choose a reason for hiding this comment

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

I believe that we should be able to just use values() in both 2.7 and 3.x

http://wiki.ros.org/python_2_and_3_compatible_code

"""
for k, v in tempvars.iteritems():
try:
items = tempvars.items()
Copy link
Member

Choose a reason for hiding this comment

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

I believe that we should be able to just use items() in both 2.7 and 3.x

http://wiki.ros.org/python_2_and_3_compatible_code

@v4hn v4hn force-pushed the pr-master-support-python3 branch from a22610c to 7a5b362 Compare November 8, 2019 06:51
@v4hn
Copy link
Contributor Author

v4hn commented Nov 8, 2019

Thanks for the review @tfoote, I addressed your feedback.
Would be nice to see this merged soon.

@tfoote tfoote merged commit ec884e5 into ros-infrastructure:master Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants