Skip to content
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

Python3 supported? #940

Closed
fnoop opened this issue Feb 5, 2018 · 10 comments
Closed

Python3 supported? #940

fnoop opened this issue Feb 5, 2018 · 10 comments

Comments

@fnoop
Copy link
Contributor

fnoop commented Feb 5, 2018

Issue details

Traceback (most recent call last):
  File "./test-rosparams", line 13, in <module>
    param_received, param_list = param_get_all(False)
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/mavros/param.py", line 167, in param_get_all
    sorted((Parameter(k, v) for k, v in params.iteritems()),
AttributeError: 'dict' object has no attribute 'iteritems'

param_get_all() function fails as python3 doesn't have iteritems. Possible solutions are to change to items() or import iteritems from future:
from future.utils import iteritems

MAVROS version and platform

Mavros: 0.22
ROS: Kinetic
Ubuntu: 16.04

Autopilot type and version

[ x ] ArduPilot
[ ] PX4

Version: 3.5

@vooon
Copy link
Member

vooon commented Feb 5, 2018

Not officially supported, ROS still uses 2.7.
2to3 may help.

@fnoop
Copy link
Contributor Author

fnoop commented Nov 28, 2018

This is still a problem. From:
http://www.ros.org/reps/rep-0003.html#python

Melodic
Ubuntu will most likely stop supporting Python 2 in release 20.04. To make sure ROS will be able to support that version of Ubuntu, ROS Python packages starting with Melodic Morenia are highly encouraged to support both Python 2.7 and Python 3.5 or later. During the development of Melodic there will be work undertaken to support both Python 2 and Python 3 (including rosdep keys) so ROS package developers can more easily test with either version of Python.

Python 2 EOL is fast approaching, and there are quite a few python modules/frameworks that really depend on Python 3 or at least significantly encourage them (through nice features only available in Python 3). We are currently writing a full web GCS which significantly leverages MAVROS, however we are having to move to Python 3 to follow support of other libraries/frameworks and the lack of Python 3 support in MAVROS is a dealbreaker. ROS 2 would be an option as it only supports Python 3, but MAVROS does not support ROS 2 either.
Is there any plan or timeline for MAVROS to support Python 3 and/or ROS 2?

@khancyr
Copy link
Contributor

khancyr commented Nov 28, 2018

Hello,

Using futur, the support for python3 shouldn't be long as MAVROS don't have much pure python code. I will see to do it this weekend!

@fnoop
Copy link
Contributor Author

fnoop commented Nov 28, 2018

@khancyr How do you deal with something like the error in this issue? Because in this case 'params' is from:
params = rospy.get_param(mavros.get_topic('param'))
So the params object doesn't have iteritems - it doesn't do any good importing iteritems in mavros/param.py from future.utils or six.

@fnoop
Copy link
Contributor Author

fnoop commented Nov 28, 2018

Useful cheat sheet: http://python-future.org/compatible_idioms.html

Option 2 for items() seems most suitable for this situation:

# Python 2 and 3: option 2
from future.utils import viewitems
for (key, value) in viewitems(heights):   # also behaves like a set

Option 1 is inefficient, Option 2 doesn't work because of the above

@fnoop
Copy link
Contributor Author

fnoop commented Nov 28, 2018

Now getting:

[dev] [mav@goodrobots ~/code/ros]$ ./test-py3.py
Traceback (most recent call last):
  File "./test-py3.py", line 21, in <module>
    param_received, param_list = param_get_all(False)
  File "/opt/ros/melodic/lib/python2.7/dist-packages/mavros/param.py", line 170, in param_get_all
    cmp=lambda x, y: cmp(x.param_id, y.param_id))
TypeError: 'cmp' is an invalid keyword argument for this function

Need to convert old cmp param to key: https://docs.python.org/3/library/functools.html#functools.cmp_to_key

Importing these in mavros/param.py:

from past.builtins import cmp
import functools

And then changing the sorted param in param_get_all() return to:

            sorted((Parameter(k, v) for k, v in viewitems(params)),
                   key=functools.cmp_to_key(lambda x, y: cmp(x.param_id, y.param_id)))
            )

Fixes and works in both python2 and python3 - a little messy, maybe there's a cleaner method.

fnoop added a commit to fnoop/mavros that referenced this issue Nov 28, 2018
@fnoop
Copy link
Contributor Author

fnoop commented Nov 28, 2018

Raised PR here with fix:
#1127

@vooon
Copy link
Member

vooon commented Nov 28, 2018

Unfortunately i do not have any plans. In my current project i just use 3.6+...
I think that usage of .items() is good enough.

fnoop added a commit to fnoop/mavros that referenced this issue Nov 28, 2018
fnoop added a commit to fnoop/mavros that referenced this issue Nov 28, 2018
Simplify python3 fixes, mavlink#940

Remove unnecessary functools
vooon pushed a commit that referenced this issue Nov 29, 2018
vooon pushed a commit that referenced this issue Nov 29, 2018
Simplify python3 fixes, #940

Remove unnecessary functools
@amilcarlucas
Copy link
Contributor

The PR is now merged, can this be closed?

@fnoop
Copy link
Contributor Author

fnoop commented Feb 5, 2019

Yes, working great in python3 now. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants