Use Python 3 everywhere #12809

julianoes · 2019-08-26T13:03:49Z

Since Python 2 is retired in 4 months, we should move everything to 3.

@MaEtUgR do you mind checking appveyor? 😄

cclauss · 2019-09-21T12:24:52Z

At least 6 Python 3 syntax errors to cleanup...
flake8 testing of https://github.com/PX4/Firmware on Python 3.7.1

$ flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

./platforms/nuttx/Debug/Nuttx.py:394:6: F632 use ==/!= to compare str, bytes, and int literals
		if regs is 0:
     ^
./platforms/nuttx/Debug/Nuttx.py:419:14: F821 undefined name 'regmap'
		for reg in regmap:
             ^
./platforms/nuttx/Debug/Nuttx.py:420:24: F821 undefined name 'regmap'
			hex_addr= hex(int(a[regmap[reg]]))
                       ^
./platforms/nuttx/NuttX/tools/oldconfig.py:33:13: F821 undefined name 'raw_input'
    input = raw_input
            ^
./Tools/geotag_images_ulog.py:41:69: E999 SyntaxError: invalid syntax
    print "Usage: python geotag_images_ulog.py [logfile] [image dir]"
                                                                    ^
./Tools/fetch_file.py:85:19: E999 SyntaxError: invalid syntax
    print "Get %s:" % fn,
                  ^
./Tools/upload_log.py:43:15: F821 undefined name 'raw_input'
        ret = raw_input(ask_string)
              ^
./Tools/run-clang-tidy.py:57:57: E999 SyntaxError: invalid syntax
      print 'Error: could not find compilation database.'
                                                        ^
./Tools/px4moduledoc/srcparser.py:350:23: F821 undefined name 'reduce'
        getopt_args = reduce(lambda a, b: a + b[1], getopt_args, '').replace(':', '')
                      ^
./Tools/uorb_graph/create.py:521:36: E999 TabError: inconsistent use of tabs and spaces in indentation
	graph.render(file_name, view=False)
                                   ^
./integrationtests/python_src/px4_it/mavros/mavros_offboard_attctl_test.py:126:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_offboard_posctl_test.py:133:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_offboard_posctl_test.py:172:18: F821 undefined name 'xrange'
        for i in xrange(len(positions)):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:173:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:203:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:234:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:257:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:285:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:308:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:341:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mavros_test_common.py:377:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/mavros/mission_test.py:190:18: F821 undefined name 'xrange'
        for i in xrange(timeout * loop_freq):
                 ^
./integrationtests/python_src/px4_it/dronekit/MissionCheck.py:61:18: E999 SyntaxError: invalid syntax
print "Connecting"
                 ^
./src/lib/parameters/px4params/markdownout.py:50:20: F632 use ==/!= to compare str, bytes, and int literals
                if long_desc is not '':
                   ^
./src/modules/mavlink/mavlink_tests/mavlink_ftp_test_data.py:2:26: E999 SyntaxError: invalid syntax
print 'Arguments: file - ' + sys.argv[1] + ', length - ' + sys.argv[2]
                         ^
./msg/tools/uorb_rtps_classifier.py:198:27: F821 undefined name 'errno'
            if e.errno == errno.ENOENT:
                          ^
./msg/tools/uorb_rtps_classifier.py:199:31: F821 undefined name 'errno'
                raise IOError(errno.ENOENT, os.strerror(
                              ^
./msg/tools/uorb_rtps_classifier.py:200:21: F821 undefined name 'errno'
                    errno.ENOENT), yaml_file)
                    ^
./msg/tools/uorb_to_ros_rtps_ids.py:79:23: F821 undefined name 'errno'
        if e.errno == errno.ENOENT:
                      ^
./msg/tools/uorb_to_ros_rtps_ids.py:80:27: F821 undefined name 'errno'
            raise IOError(errno.ENOENT, os.strerror(errno.ENOENT), file)
                          ^
./msg/tools/uorb_to_ros_rtps_ids.py:80:53: F821 undefined name 'errno'
            raise IOError(errno.ENOENT, os.strerror(errno.ENOENT), file)
                                                    ^
./msg/tools/uorb_to_ros_rtps_ids.py:97:20: F821 undefined name 'basestring'
                v, basestring) else v for k, v in dictionary.iteritems()}
                   ^
./msg/tools/uorb_to_ros_rtps_ids.py:129:23: F821 undefined name 'errno'
        if e.errno == errno.ENOENT:
                      ^
./msg/tools/uorb_to_ros_rtps_ids.py:130:27: F821 undefined name 'errno'
            raise IOError(errno.ENOENT, os.strerror(errno.ENOENT), file)
                          ^
./msg/tools/uorb_to_ros_rtps_ids.py:130:53: F821 undefined name 'errno'
            raise IOError(errno.ENOENT, os.strerror(errno.ENOENT), file)
                                                    ^
./msg/tools/generate_microRTPS_bridge.py:246:25: F821 undefined name 'raw_input'
        _continue = str(raw_input("\nFiles in " + agent_out_dir +
                        ^
./msg/tools/generate_microRTPS_bridge.py:256:25: F821 undefined name 'raw_input'
        _continue = str(raw_input(
                        ^
6     E999 SyntaxError: invalid syntax
2     F632 use ==/!= to compare str, bytes, and int literals
29    F821 undefined name 'reduce'
37

E901,E999,F821,F822,F823 are the "showstopper" flake8 issues that can halt the runtime with a SyntaxError, NameError, etc. These 5 are different from most other flake8 issues which are merely "style violations" -- useful for readability but they do not effect runtime safety.

F821: undefined name name
F822: undefined name name in __all__
F823: local variable name referenced before assignment
E901: SyntaxError or IndentationError
E999: SyntaxError -- failed to compile a file into an Abstract Syntax Tree

lamping7 · 2019-10-18T04:15:23Z

The current linting for the stuff I've touched with testing was done with yapf. We've never created a standard here for linting in python as we have with astyle. Some of these things are part of the ROS integration tests which will stay at 2.7. This explains the xrange issues too. The print statement should be functions (from __future__ import print_function) and other similar issues.

So, I'm not sure how to review this. Are we to fix all this or just review the current stage as a step in the direction given by the title of the PR?

cclauss · 2019-10-18T04:26:34Z

The conflicts should be fixed.
It is not best practice to solve all Python 3 compatibility issues is a single pull request (unless the codebase is trivial) so if these changes do not break Python 2 compatibility and improve Python 3 compatibility then they should be reviewed favorably and landed.

75 days until Python 2 end of life.

lamping7

I tested this by simply running make from a clean pull. With Python 3.5, this worked after installing some dependencies. For reference I needed: pip3 install --user jinja2 catkin_pkg numpy toml pyyaml

lamping7 · 2019-10-18T13:43:27Z

@cclauss Can you open an issue with your comments here. I don't think they fall in line with what the purpose of this PR was -- python 3 for the build process -- and I don't want to drag it down.

cclauss · 2019-10-18T15:05:03Z

Agreed. Conflicts must be resolved and tests must pass.

TSC21 · 2019-11-20T12:21:35Z

I am not sure what's happening but I am totally confident that jinja2 is installed on the containers: http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2Fcontainers/detail/master/150/pipeline#step-32-log-724

cclauss · 2019-11-20T12:23:03Z

In several of these files we need to make the same text change numerous times. Would it be possible instead to define a variable that can be reused multiple times so that future modifications are simpler to make and to review?

TSC21 · 2019-11-20T12:29:45Z

In several of these files we need to make the same text change numerous times. Would it be possible instead to define a variable that can be reused multiple times so that future modifications are simpler to make and to review?

Are you referring to the container tags? The thing about it is that it's not mandatory that we update all the containers tag at the same time. There could even be situations where that doesn't happen.

cclauss · 2019-11-20T12:36:14Z

I am totally confident that jinja2 is installed on the containers
pip2 install Jinja2 makes enables that module for Python 2 but not for Python 3.
pip3 install Jinja2 makes enables that module for Python 3 but not for Python 2.

TSC21 · 2019-11-20T12:45:19Z

I am totally confident that jinja2 is installed on the containers
pip2 install Jinja2 makes enables that module for Python 2 but not for Python 3.
pip3 install Jinja2 makes enables that module for Python 3 but not for Python 2.

@cclauss what's your point here? The containers were updated: https://github.com/PX4/containers/blob/master/docker/Dockerfile_base-bionic#L60-L63

TSC21 · 2019-11-20T13:18:01Z

I am totally confident that jinja2 is installed on the containers
pip2 install Jinja2 makes enables that module for Python 2 but not for Python 3.
pip3 install Jinja2 makes enables that module for Python 3 but not for Python 2.

@cclauss what's your point here? The containers were updated: https://github.com/PX4/containers/blob/master/docker/Dockerfile_base-bionic#L60-L63

Looks like we missed Python3 in the first place: PX4/PX4-containers#211

MaEtUgR · 2019-11-21T17:02:13Z

@julianoes Good call. Well I never checked anything Python 3 for the Windows toolchain because at the time it was completely unsupported by the build. Now I could change the toolchain installation procedure to only include 3.

TSC21 · 2019-11-22T12:24:55Z

@julianoes we have another issue here: genmsg and gencpp seem to use Python 2 instead of Python 3. We need to update our fork on those as well.

TSC21 · 2019-11-22T15:50:07Z

Actually, we have already Python 3 compatibility added here: PX4/genmsg@1ad8e13. This just needs to get it and we should update both genmsg and gencpp modules.

MaEtUgR · 2019-11-22T16:29:42Z

@julianoes I found the necessary Cygwin toolchain changes locally and it builds this pr fine. I can create a pr on the toolchain repository and let CI generate a new toolchain installer soon. I'm assuming that I need to keep backward compatibility and leave python 2 working like before as well.

MaEtUgR · 2019-11-24T00:08:46Z

I

adjusted the Cygwin Toolchain according to my findings: PX4/PX4-windows-toolchain@5f60b57
made a new installer version 0.7: https://github.com/PX4/windows-toolchain/releases/tag/v0.7
rebased this pr on master such that appveyor builds the merge again without conflict
added a commit for appveyor to use that new toolcahain version: 0442295

@julianoes I would say the Windows Toolchain/CI is ready (first) 😉

Since Python 2 is retired in 4 months, we should move everything to 3.

for Python 3 support.

julianoes · 2020-01-15T09:41:57Z

This has been replaced by #13572 and #13949, closing.

julianoes requested a review from dagar August 26, 2019 13:03

This was referenced Aug 26, 2019

ubuntu.py: update to dual install to Python2 #12788

Merged

px4-dev: Try to move to Python 3 PX4/PX4-containers#199

Merged

weekly-digest bot mentioned this pull request Sep 1, 2019

Weekly Digest (25 August, 2019 - 1 September, 2019) #12858

Closed

weekly-digest bot mentioned this pull request Sep 22, 2019

Weekly Digest (15 September, 2019 - 22 September, 2019) #13002

Closed

TSC21 requested a review from lamping7 October 15, 2019 15:18

lamping7 previously approved these changes Oct 18, 2019

View reviewed changes

weekly-digest bot mentioned this pull request Oct 20, 2019

Weekly Digest (13 October, 2019 - 20 October, 2019) #13236

Closed

julianoes dismissed lamping7’s stale review via 4eaa675 October 24, 2019 06:34

julianoes force-pushed the pr-build-with-python3 branch from 4edecd0 to 4eaa675 Compare October 24, 2019 06:34

jbeyerstedt mentioned this pull request Nov 13, 2019

cmake: use python 3 as default PX4/PX4-SITL_gazebo-classic#368

Closed

TSC21 mentioned this pull request Nov 20, 2019

CI and scripting: bump container tags to 2019-11-19 #13537

Closed

TSC21 force-pushed the pr-build-with-python3 branch from 4eaa675 to 50ffc46 Compare November 20, 2019 12:10

TSC21 mentioned this pull request Nov 22, 2019

msg: remove gencpp and genmsg submodules; remove gencpp imports #13572

Merged

MaEtUgR force-pushed the pr-build-with-python3 branch from 721917c to 00bce6c Compare November 23, 2019 23:45

MaEtUgR force-pushed the pr-build-with-python3 branch from 00bce6c to 0442295 Compare November 23, 2019 23:55

TSC21 mentioned this pull request Nov 25, 2019

Add UTF-8 language encoding PX4/PX4-containers#214

Merged

julianoes and others added 4 commits December 18, 2019 11:21

Use Python 3 everywhere

514ddee

Since Python 2 is retired in 4 months, we should move everything to 3.

CI and scripting: bump container tags to 2019-11-19

7c62351

bump container tags to 2019-11-20

f39631f

appveyor: Switch to Cygwin Toolchain 0.7

89f39b3

for Python 3 support.

julianoes force-pushed the pr-build-with-python3 branch from 0442295 to 89f39b3 Compare December 18, 2019 10:21

julianoes mentioned this pull request Dec 18, 2019

docker: update base for armhf Dockerfile PX4/PX4-containers#216

Merged

weekly-digest bot mentioned this pull request Dec 22, 2019

Weekly Digest (15 December, 2019 - 22 December, 2019) #13770

Closed

julianoes closed this Jan 15, 2020

julianoes deleted the pr-build-with-python3 branch January 15, 2020 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Python 3 everywhere #12809

Use Python 3 everywhere #12809

julianoes commented Aug 26, 2019 •

edited

Loading

cclauss commented Sep 21, 2019

lamping7 commented Oct 18, 2019 •

edited

Loading

cclauss commented Oct 18, 2019 •

edited

Loading

lamping7 left a comment

lamping7 commented Oct 18, 2019

cclauss commented Oct 18, 2019

TSC21 commented Nov 20, 2019

cclauss commented Nov 20, 2019

TSC21 commented Nov 20, 2019

cclauss commented Nov 20, 2019

TSC21 commented Nov 20, 2019

TSC21 commented Nov 20, 2019 •

edited

Loading

MaEtUgR commented Nov 21, 2019

TSC21 commented Nov 22, 2019 •

edited

Loading

TSC21 commented Nov 22, 2019

MaEtUgR commented Nov 22, 2019 •

edited

Loading

MaEtUgR commented Nov 24, 2019

julianoes commented Jan 15, 2020

Use Python 3 everywhere #12809

Use Python 3 everywhere #12809

Conversation

julianoes commented Aug 26, 2019 • edited Loading

cclauss commented Sep 21, 2019

lamping7 commented Oct 18, 2019 • edited Loading

cclauss commented Oct 18, 2019 • edited Loading

lamping7 left a comment

Choose a reason for hiding this comment

lamping7 commented Oct 18, 2019

cclauss commented Oct 18, 2019

TSC21 commented Nov 20, 2019

cclauss commented Nov 20, 2019

TSC21 commented Nov 20, 2019

cclauss commented Nov 20, 2019

TSC21 commented Nov 20, 2019

TSC21 commented Nov 20, 2019 • edited Loading

MaEtUgR commented Nov 21, 2019

TSC21 commented Nov 22, 2019 • edited Loading

TSC21 commented Nov 22, 2019

MaEtUgR commented Nov 22, 2019 • edited Loading

MaEtUgR commented Nov 24, 2019

julianoes commented Jan 15, 2020

julianoes commented Aug 26, 2019 •

edited

Loading

lamping7 commented Oct 18, 2019 •

edited

Loading

cclauss commented Oct 18, 2019 •

edited

Loading

TSC21 commented Nov 20, 2019 •

edited

Loading

TSC21 commented Nov 22, 2019 •

edited

Loading

MaEtUgR commented Nov 22, 2019 •

edited

Loading