uuv_simulator icon indicating copy to clipboard operation
uuv_simulator copied to clipboard

Python 2 and 3 compatibility

Open musamarcusso opened this issue 6 years ago • 11 comments

musamarcusso avatar Jul 06 '18 14:07 musamarcusso

Many scripts in the package need to be Python 3 compatible, too. Apart from, for example, applying the suggested changes from ROS Python 2 and 3 compatibility, does anyone have any idea on how to automate tests for Python 2/3 compatibility in Travis CI? As far as I now, the images used there still have Python 2.7 as default.

musamarcusso avatar Mar 20 '19 07:03 musamarcusso

Any objections to using the python-future package https://python-future.org/ for compat?

russkel avatar Mar 22 '19 05:03 russkel

No, I am using it already in the Python code I write now to be sure it will work. My biggest concern is that I can't easily set up the CI system to actually run the tests and code with Python 3 automatically to see if it fails or not. Some differences also regard packages that have been refactored for different Python versions or even some Python 3 dependencies that will not be found by rosdep. I usually try to do it manually but then it makes the review process a bit error prone and time consuming. I will start going through the Python scripts in the repo to check at least some obvious fixes for Python 3 combatibility, but I still don't know how to test everything on Travis

musamarcusso avatar Mar 22 '19 07:03 musamarcusso

Okay. Looking at the Travis CI runs, they take a long time (50+ minutes!) and seem to be timing out setting up the environment. I am not very familiar with Travis, but it does seem to allow using Docker images. If it's unable to set up the environment in a timely fashion maybe it should be migrated to use a Docker build environment, so it will build the env once and then cache it. ROS provides a number of docker files that could be built upon.

russkel avatar Mar 22 '19 08:03 russkel

Yes, I had to tweak some configurations because I was getting some errors (e.g. setting the maximum number of jobs to build the catkin workspace to 1), which also makes the process take a lot longer. The Travis configuration right now is already using the ROS Industrial CI infrastructure with caching, but I think using the free Travis CI has some limitations on the amount of resources you can use. Right now the main tests done (for kinetic, lunar and melodic) when building the packages are the unit-tests, pre-release tests and running catkin_lint to look for inconsistencies that might show up also in the ROS buildfarm. Unfortunately I don't know how to exactly integrate the Python 3 compatibility tests (if even there is such a thing) mostly because the ROS docker images use Ubuntu images as a base that use Python 2.7 per default. And as I said, after even setting Python 3.x as the default, it would also be necessary to locate the Python packages that might not be installed by rosdep and manually install them (I think) to really run all the tests for Python 3 seamlessly. I am doing some experiments with the Docker images here to see if there is a painless way to get there, but I am open to any ideas or suggestions in case somebody already ran into these issues :)

musamarcusso avatar Mar 22 '19 10:03 musamarcusso

The Travis configuration right now is already using the ROS Industrial CI infrastructure with caching, but I think using the free Travis CI has some limitations on the amount of resources you can use.

Oh I see! I only looked at one failed build log and it terminated as it was configuring/installing dependencies. If it is a resources issue then docker is unlikely to solve it. The main reason I suggested Docker was that all the environment building can be done once and cached, so when Travis CI is triggered, it simply pulls a bunch of cached Docker images (this step should take seconds) and then it can instantly compile uuvsimulator and runs the tests.

Unfortunately I don't know how to exactly integrate the Python 3 compatibility tests (if even there is such a thing) mostly because the ROS docker images use Ubuntu images as a base that use Python 2.7 per default. And as I said, after even setting Python 3.x as the default, it would also be necessary to locate the Python packages that might not be installed by rosdep and manually install them (I think) to really run all the tests for Python 3 seamlessly.

I apologise if I am misunderstanding, is that a problem? It could be arranged with a number of docker containers: three 2.7 containers running kinetic, lunar and melodic and one melodic container running 3.6. And yes, the uuvsimulator-py36 dockerfile can pull in the python3 dependencies via apt-get, pip or rosdep quite easily.

As for the actual tests, from what I have seen in other projects is they simply run the same unittests, but with the other python version, to test their code base. This will require decent code coverage to be successful.

Unfortunately I do not have my workstation with me so I can't attempt to get a py3 environment running right this moment, hopefully I get it back sometime this weekend.

russkel avatar Mar 22 '19 11:03 russkel

I think you are right. Maybe setting up some of the builds to just do something like alias python=python3 should be enough. I can't say how many packages will need to be installed separately. Maybe on the long run, however, making requests to add the missing Python 3 packages at rosdistro's list instead of setting up an extra installation step would be a good idea, too.

musamarcusso avatar Mar 22 '19 12:03 musamarcusso

Hello @musamarcusso I am currently working with docker at the moment and will try to find some time to create some custom Dockerfiles to do what I described above in the near future.

russkel avatar Apr 08 '19 00:04 russkel

Apparently one way to check (or debug) for the compatibility issue is to run the Python script file using

co = compile(open(filename).read(), filename, 'exec')

with Python 3. I will made the changes for the Python scripts (Python modules are next) in #357

musamarcusso avatar Apr 17 '19 23:04 musamarcusso

That should catch the syntax changes, however there may be some sneaky runtime incompatibilities.

FYI Looks like environment variables were added for python version https://github.com/osrf/docker_images/pull/256

Edit: On further inspection that's ROS2 and not very useful. Nevermind.

russkel avatar Apr 24 '19 05:04 russkel

It looks like it is currently not possible to test with Python2 and 3 with ros-industrial/industrial_ci. The problem is that ROS1 has to be rebuild from scratch to support Python3. The ros-industrial guys are working to support this. Have a look at this PR: https://github.com/ros-industrial/industrial_ci/pull/429

arturmiller avatar Dec 20 '19 21:12 arturmiller