-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Review Python policy for Hydro #506
Comments
The updated REP 3 says:
Exactly what does this mean? A literal reading implies that all ROS packages containing any Python code must run unmodified with both a Python 2.7 interpreter and a Python 3.3 interpreter. I don't believe that literal interpretation is reasonable, or even possible. While an enthusiastic Python user for several years now, I am no language expert. So, I've done some reading on the net. Here are some pages by knowledgeable people discussing the Python migration problem:
In summary, various migration methods are discussed, but no Python experts recommend releasing any non-trivial "bilingual" code that attempts to support both Python versions in a single source tree. Deep semantic incompatibilities such as Unicode string handling seem to dictate against that approach. The usual recommendation for maintaining a body of code that must work with both versions is to maintain the source in a Python 2.7 branch; use the automated 2to3 script to translate to Python 3 syntax with no manual editing; make all fixes in the original branch; then release separate tarballs for the two versions. That poses obvious problems for ROS Hydro code that must support both Precise with a Python 2.7 default interpreter, and Raring with Python 3.3 by default in a single source release. I can think of various ways to address these problems, but first I want to raise the issues and get input from others. Some people have been working on Python 3 ports of various ROS scripts for a while now. I hope they will describe the problems encountered, and suggest how best to proceed. |
This was discussed in yesterdays platform group meeting, here are ideas from the meeting: One point made was that of the core ROS python stuff, considerable effort has already been made to provide dual compatibility by the members of the MORSE project. I know I have done this also for vcstools and rosinstall. So in the meeting some confidence existed that for Hydro this is doable for a "core" set of packages. All other "non-core" packages will either be migrated to python3 in due time, and until then only be usable on systems where python2 is the default python interpreter. So it is up to users to not use ringtail or downgrade the ringtail default interpreter, if any package they want to use is not py3k compatible. How users are supposed to know that beforehand was not discussed. Also we mentioned that a problem is that debian packages may get released for ringtail even if they have no py3k compatible python code. To prevent that, the idea is to change the build scripts such that when automated testing fails, the build fails (which s not the case currently, apparently). Other than that, I also recommend this link as a resource: |
Converting all the Python code in the packages I maintain seems like a big job to get done, tested and released in only two months. But, it is the user confusion Thibault mentions that I worry most about. From an external perspective, it will appear that large segments of "the ROS system" just don't work. Users don't distinguish between core ROS components and various packages they want to use. Despite that, we may need to attack the Python migration problem differently for different classes of scripts:
|
Thibault's link to the Ubuntu migration page is helpful. Apparently, "bilingual" scripts have become more feasible since the Python 2.7 and 3.3 releases. Many of the links I posted were from the Python 2.6 and 3.0 time frame, when that was more difficult. Fortunately Hydro specifies both 2.7 and 3.3, so many scripts will be able to take advantage of some newer co-existence features. I expect most of us would prefer a bilingual solution, when it is workable, although some developers needing to support Python 2.6 or older may not have that option. All recommendations emphasize the importance of good unit test coverage before starting the port to Python 3. We need to provide a clean way for developers to run ROS unit tests with the |
I decided to experiment with a simple unique identifier module. Some observations:
/usr/lib/pymodules/python2.7/rospkg/manifest.py:196: DeprecationWarning: Overriding __eq__ blocks inheritance of __hash__ in 3.x
class Platform(object):
$ python3 tests/test_unique_id.py
Traceback (most recent call last):
File "tests/test_unique_id.py", line 4, in <module>
import roslib; roslib.load_manifest(PKG)
File "/opt/ros/groovy/lib/python2.7/dist-packages/roslib/__init__.py", line 50, in <module>
from roslib.launcher import load_manifest
File "/opt/ros/groovy/lib/python2.7/dist-packages/roslib/launcher.py", line 42, in <module>
import rospkg
ImportError: No module named rospkg
|
Severin Lemaignan, me and other members of the MORSE community together with Ken Conley ported ROS messaging (which involved mostly code in roslib and rospy) to work on Python2 and 3 (we started with 3.1, but meanwhile are using 3.3). We are now using this code since Diamondback and it works fine apart from some minor patches we had to do from time to time (like fixing rospack and catkin for Py2 and 3). The Ubuntu Ubuntu migration page that Thibault posted seems to cover most of the issues, as mentioned there, I can confirm that string/bytes handling can be a pain.. So one should really listen to the last point "Clarify your data model" in the "Before you start" section of the Ubuntu migration page. From my experience, this migration is doable but the effort should not be underestimated! 2 month seems rather like a tough schedule... |
Based on MORSE requirements for ROS support, the last ROS "core" component to require large porting effort to py3 is actionlib (and maybe tf? Michael?). |
Yes, tf and actionlib will have to be patched as well (although I guess this should be a minor effort). We also didn't patch the tools in the ros_comm package (rosgraph, rosconsole, etc.). I didn't look into it in detail, but I guess most of them will have to be patched which could require some more effort... |
Based on the MORSE work, I propose the following Hydro release plan:
|
Just an update, the SMACH hydro-devel branch is ready for Python3 testing (and maintains 2.7 compatibility): ros/executive_smach@fdf99b1 |
Until bilingual versions of roslib and rospkg are released, only Did you try that with SMACH, Jonathan? |
@jack-oquin When I run the tests with |
That's my status, too. If the basic roslib and rospkg updates could be released, there is much more that could be tested. That should top the priority queue for Python 3 migration. |
Conclusion: Core tools will be bilingual (2.7, 3.3 compatible) If something is known not to work for either python2 or python3 it should be annotated in the description. This can be closed when we document this somewhere. |
If we want tooling support we should revisit that. |
I volunteer to make an update for REP-0003, explaining that only the core tools will work with Python 3. For users to actually run ROS on Raring, we will need to recommend configuring the default Python version to 2.7. Once people start migrating actual robotics packages (I-turtle??), users will definitely need tool help to figure out what they can run in a Python 3 system. |
REP-0003 update submitted with Python 3 support explanation. Please review and provide comments. |
@kargm and @severin-lemaignan:
|
They are checked in into the ROS trunk. Basically standard ros core and rospy allow messaging with python3 since Electric. However, we didn't test for code coverage with py3, so there might still be some issues, but we are using messaging as well as services with Python3 since Diamondback and it seems to work fine... (for Diamondback and the first Electric release, you still had to overlay ros core and rospy, but in one maintenance update of Electric, all of our patches made it into the trunk) |
As far as I know, everything has been already merged upstream by Ken a while ago See for instance: ros/genpy#3 Currently we only have on "MORSE specific" version of a ROS core component: tfMessage.py |
On Hydro and Groovy rosunit seems to be broken. At least, it uses the old-style print command. So, should I presume that is a new problem and open an issue for it? |
The hydro-devel branch of rosunit does not work with python 3.2 on Precise.
Since Raring is not yet supported on Hydro, converted code needs to be tested with 3.2, which is less compatible than 3.3, which supports u"string" syntax. |
The deeper I delve into this problem, the harder it looks.
|
So on closer reading of the Python 3 Ubuntu page it seems that we've gotten ahead of ourselves. Or Ubuntu has rolled back their ambition to roll out Python 3. https://wiki.ubuntu.com/Python/3 It looks like they are now targeting 14.04 to be the first distro without 2.7 installed, but at least in the mean time 2.7 will continue to be installed, and remain the default. Dirk did some testing on a Raring vm and confirmed the following:
Based on this Raring from our point of view should be no different than Precise or Quantal, however we should keep up the effort to make our code bilingual. This just takes the short term pressure off for the next release. |
That is a huge relief. I was getting really worried about the viability of Hydro on Raring. I bet they were surprised by the level of difficulty, like we were. The Hydro Python 3 scare was probably a blessing. It encouraged us to think about how to do the transition, and to realize how big an infrastructure change is likely to come with it. Some of those updates should probably be released before Ubuntu 14.04 and J-turtle. I'll make another REP-0003 update reflecting this. |
REP 003 updated in ros-infrastructure/rep#35 |
REP 3 discussion for Python 3 compatibility is continuing on ros-infrastructure/rep#64 |
As raised by @jack-oquin in ros-infrastructure/rep#26 the python decision could use a review by ros-users. I'll circulate this to get feedback. I'm opening this ticket here so we can close #411 and have a default option for now.
The text was updated successfully, but these errors were encountered: