search

ros / dynamic_reconfigure

BSD 3-Clause "New" or "Revised" License
48 stars 111 forks source link

Python3 compatibility #104

Closed vytasrgl closed 6 years ago

vytasrgl commented 6 years ago

Current python version is really close to be python3 compatible. Similar to roscomm i think would be great to have it compatible for py3

mikaelarguedas commented 6 years ago

Thanks @vytasrgl for reporting it. Can you point out the parts you identified that are not python3 compatible?

vytasrgl commented 6 years ago

I wasn't able to run full test but having fixed iteritems in: https://github.com/ros/dynamic_reconfigure/blob/17e1d4bf6a538d2def097a7f70b139ccd41e9f78/src/dynamic_reconfigure/encoding.py#L78 i was able to run reconfigure server inside python 3.6.

zchen24 commented 6 years ago

Also parameter_generator.py L180: ret.append(string.upper(x))

string.upper is not compatible

mikaelarguedas commented 6 years ago

thank you both for the feedback!

@vytasrgl : https://github.com/ros/dynamic_reconfigure/pull/105/commits/c03c68f5736f2a9600e8a111688548ee3d653d38 should address the dictionaries issues.

@zchen24: As parameter_generator.py has been deprecated since the migration from rosbuild to catkin, I am not planning on making changes to it. If you know of a package that still uses it, please let me know. thanks!

mikaelarguedas commented 6 years ago

with #105 merged I'm going to close this. Feel free to comment here or open a new issue if you find any other incompatibility.

Thanks!

Serafadam commented 6 years ago

camera_aravis still uses parameter_generator.py, I have the same remarks as @zchen24 - the string method causes compile errors

mikaelarguedas commented 6 years ago

thanks @Brudasoe for following up.

I'm not opposed to update parameter_generator.py to be python3 compliant. But I strongly recommend submitting Pull Requests to any package you come across that still use the deprecated rosbuild version. As that version is deprecated it is not at feature parity with the supported one and can develop bugs that have been fixed in the supported version

Serafadam commented 6 years ago

@mikaelarguedas do you have any pointers as to how proceed with updating this code so that it will not use parameter_generator.py? Currently when I update its string methods to python3 version the compilation goes through it, but after a while it displays the information that the dynamic reconfigure files were not created (even though there is a message about generating them).

mikaelarguedas commented 6 years ago

@Brudasoe I missed that comment. The way to use the new version is to import parameter_generator_catkin instad of parameter_generator in the .cfg file. You can find an example at:

https://github.com/ros/dynamic_reconfigure/blob/a574a3b79b49d8efaf6a4a0b748b002026dd64c0/cfg/Test.cfg#L37