Tribler: Python 3 compatibility

[devos50]

This issue hosts all discussion regarding the upgrade to Python 3. It follows the methodology of https://github.com/Tribler/py-ipv8/issues/265.

Everyone is welcome to help us to move our code base to Python 3. While pull requests are more than welcome, please keep the following things in mind:

• make sure that your (new) code is compatible with both Python 2 and Python 3. To help developers with identifying code that is not backward compatible, we run a few checks on your code. Please address issues raised by these checkers accordingly, or motivate why something is not worth fixing.
• keep your pull requests small and concise. This makes it easier for us to review, or revert some changes in case a (major) error slipped through.
• if you have a question, please leave a comment on this issue.

Python 3 migration guidelines:

• use the six library and the six.moves library as much as possible.
• for hex encoding/decoding, use hexlify and unhexlify in the binascii package. We found out this is significantly faster than using the encode or decode functions in codecs.

[devos50]

Also, we have to remove the pymdht submodule if we are going to do this (in favour of EgbertDHT). Otherwise, we have to port pymdht to Python 3, which requires a significant amount of engineering effort.

[qstokkink]

As I discovered the Python 3 static analysis is in no way enough to claim a class is ported, we should also do test driven porting with Python 3 actually running the tests.

We could do two things here:

1. First fix all of the static analysis issues and then move on to the tests.
2. Port to Python 3 per module, making sure the tests pass with Python 3 for this specific module.

From my experience I prefer (2) as this gives us more insight into the remaining work.

[cclauss]

Create a branch and try __futurize --stage1 -w .__ to get a sense of the _initial_ changes that will be required

[ichorid]

Some interesting links to note:
Twisted is compatible with asyncio (e.g. it is officially supported mixing them together):
https://meejah.ca/blog/python3-twisted-and-asyncio
this provides us with a clear migration path: de-Dispersification -> Python3 -> twisted+asyncio -> asyncio

Comparison of Twisted concepts and resulting code to Asyncio:
https://asyncio.readthedocs.io/en/latest/twisted.html

[cclauss]

I agree with the approach suggested above by @qstokkink although, I like to do 1. and 2. in that order.

Python 2 end of life is now just one year away and the current "big bang" approach to porting to Python 3 is not working. Currently the __python3__ branch is 1128 commits behind the __devel__ branch and has not been modified since May of 2017. Why can't we do this port the recommended way that Facebook, Dropbox, and many others large and small have successfully used to complete the port to Python 3?

Why can't we make large or small __pull requests to the devel branch__ that improve Python 3 compatibility without breaking Python 2 compatibility or functionality. Each of these PRs will need to pass the test suite and review process so we will have confidence that they do not break Python 2. Later on, there will be other changes required around dependencies, etc. but those changes can come after we have a codebase that has no Python 3 syntax errors and undefined names and passes the test suite on both Python 2 and Python 3.

Your thoughts on adopting this approach?

[ichorid]

@cclauss , basically you propose to add as much Python3 compatibility to the current code base as possible without breaking it, and then do a 'Big Bang' transition?

[cclauss]

Yes. I think the "bang" will be much smaller that way.

[synctext]

Yes, the py3 issue has not getting much attention. Current focus is finishing the complete GUI overhaul and refactoring of our core. Then we'll get it done.

[cclauss]

Fix Python 3 Syntax Errors: python3 -m flake8 . --select=E999

See ~#4091~ and ~#4092~
Test results:

• https://github.com/cclauss/Tribler-tribler-tester
• https://github.com/cclauss/Flake8-for-Tribler-Core

Fix Undefined names: python3 -m flake8 . --select=F821

Python 2: 2 -- See #4093
Python 3: 384 -- __buffer, cmp, long, unicode__, etc.

• [x] Python files at the root and utility scripts (.sonar, doc, win, twisted/plugins)
• [x] Tribler/Test
• [x] Tribler/Main
• Tribler/Core
  
  
  
  • [x] (root)
  
  
  • [x] APIImplementation
  
  
  • [x] CacheDB
  
  
  • [x] Category
  
  
  • [x] Config
  
  
  • [x] CreditMining
  
  
  • [x] DecentralizedTracking
  
  
  • [x] Libtorrent
  
  
  • [x] Modules
  
  
  • [x] Socks5
  
  
  • [x] TFTP
  
  
  • [x] TorrentChecker
  
  
  • [x] Upgrade
  
  
  • [x] Utilities
  
  
  • [x] Video
  
  
• TriblerGUI
  
  
  
  • [x] (root)
  
  
  • [x] dialogs
  
  
  • [x] widgets
  
  

Once __flake8__ reports no syntax errors and undefined names on either Python 2 and Python 3, then we can plan the rest of the migration. 391 days until Python 2 end of life.

[devos50]

We had some discussion in the team and we will accept and review proposed changes for Python 3 compatibility. Your help in this upgrade and knowledge is much appreciated since we are currently focussing on other aspects of Tribler (including a big GUI overhaul). That said, we would advise you to:

• please keep your pull requests small and containable.
• please start with the low-hanging fruits like in #4091.
• don't touch any code in the Dispersy or pymdht submodule. These submodules will be removed in the nearby future.
• please work towards some functional Python 3 unit tests.
• the IPv8 networking library has already been ported to Python 3, see https://github.com/Tribler/py-ipv8/issues/265. It would be nice if you follow the methodology of @qstokkink.

[cclauss]

Sounds good. I will try to follow this advise. If PyLint complains then I will attempt to fix but will avoid adding pylint directives. Is it possible in your CI to add Python 3 testing in __allow_failures__ mode so that we can see results on Py3 but it does not fail your builds?

[devos50]

Yes, I will try to add this in a moment.

[devos50]

First ten unit tests are working on my python3_category2 branch:

root@leaseweb2:~/tribler# nosetests Tribler/Test/Core/Category/test_category.py -v -x
/usr/lib/python3.5/importlib/_bootstrap.py:222: RuntimeWarning: to-Python converter for boost::shared_ptr<libtorrent::alert> already registered; second conversion method ignored.
  return f(*args, **kwds)
If no C{methodName} argument is passed to the constructor, L{run} will ... ok
test_calculate_category_invalid_announce_list ... ok
test_calculate_category_multi_file ... ok
test_calculate_category_single_file ... ok
test_calculate_category_xxx ... ok
test_category_names_none_names ... ok
test_cmp_rank ... ok
test_get_category_names ... ok
test_get_family_filter_sql ... ok
test_non_existent_conf_file ... ok

----------------------------------------------------------------------
Ran 10 tests in 2.035s

OK

Depends on #4094 and a few more changes in the Tribler/Core/Category package. I will make the Python 3 unit tester part of the PR pipeline. They will run a small subset of all tests, which will increase as we do more work on the Python 3 migration.

[cclauss]

Could we please at least temporarily add two tests to the CI?

1. python2 -m flake8 . --count --show-source --statistics --select=E901,E999,F821,F822,F823 --exclude=.git,./TriblerGUI/vlc.py # On Python 2
2. python3 -m flake8 . --count --show-source --statistics --select=E901,E999,F821,F822,F823 --exclude=.git,./TriblerGUI/vlc.py # On Python 3 in allow_failures mode?

Once #4093 Is fixed we will be able to remove the __--exclude__ option. This will keep any regressions from popping in on the Python 2 side and will give us visibility on the Python 3 progress. Thanks!!

[devos50]

@cclauss could you clarify a bit what on these statements? In particular, should the tests fail when it gives a specific output?

I'll have a look tomorrow.

[cclauss]

Those tests all currently all pass on Python 2 and we want them to continue passing while we do the porting because we do not want the complexity of new syntax errors or undefined names. On Python 3, the syntax errors (E999) should all be fixed but we have 300+ _undefined names_ (F821) which will be things like: __basestring, buffer, cmp, raw_input, reload, unichr, unicode, xrange__ that were remove in Python 3.

With the __-count__ option, a passing test will merely print __0__ and if you remove that option, a passing test will print nothing at all.

__E901,E999,F821,F822,F823__ are the "_showstopper_" flake8 issues that can halt the runtime with a SyntaxError, NameError, etc. Most other flake8 issues 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

[Dmole]

https://pythonclock.org/

https://fedoraproject.org/wiki/Releases/30/ChangeSet#Mass_Python_2_Package_Removal

[cclauss]

I upgraded the status list above. Still work to do...

[cclauss]

OK... We are now down to the _dirty dozen_... These are the 12 remaining _showstopper__ flake8 issues that would halt the runtime if they were executed on Python 3. There are other Python 3 compatibility issues beyond these 12 _undefined names_ but flake8 can not detect them so we will need to find them using other tools.

Two of the issues I fixed upstream in VLC some months ago (#4093) so we will need to re-vendor VLC.

The remaining 10 are code of the form __unicode(s, encoding)__ which has no direct analog in Python 3 because __unicode()__ was removed because all __str__ in Python 3 are Unicode utf-8 by default.

If there is someone who is really experienced with Unicode in Python, please propose fixes.

flake8 testing of https://github.com/Tribler/tribler on Python 3.7.1

$ __flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics__

./Tribler/Core/TorrentDef.py:552:24: F821 undefined name 'unicode'
                return unicode(self.metainfo["info"]["name.utf-8"], "UTF-8")
                       ^
./Tribler/Core/TorrentDef.py:561:28: F821 undefined name 'unicode'
                    return unicode(self.metainfo["info"]["name"], self.metainfo["encoding"])
                           ^
./Tribler/Core/TorrentDef.py:580:24: F821 undefined name 'unicode'
                return unicode(self.metainfo["info"]["name"], "UTF-8")
                       ^
./Tribler/Core/TorrentDef.py:651:38: F821 undefined name 'unicode'
                        yield join(*[unicode(element, "UTF-8") for element in file_dict["path.utf-8"]]), file_dict["length"]
                                     ^
./Tribler/Core/TorrentDef.py:662:42: F821 undefined name 'unicode'
                            yield join(*[unicode(element, encoding) for element in file_dict["path"]]), file_dict["length"]
                                         ^
./Tribler/Core/TorrentDef.py:684:38: F821 undefined name 'unicode'
                        yield join(*[unicode(element, "UTF-8") for element in file_dict["path"]]), file_dict["length"]
                                     ^
./Tribler/Core/Utilities/unicode.py:43:24: F821 undefined name 'unicode'
                return unicode(s, encoding)
                       ^
./Tribler/Core/Utilities/torrent_utils.py:112:30: F821 undefined name 'unicode'
            params['name'] = unicode(params['name'], 'utf-8')
                             ^
./Tribler/Core/Utilities/torrent_utils.py:115:53: F821 undefined name 'unicode'
        torrent_file_name = os.path.join(base_path, unicode(t1['info']['name'], 'utf-8') + postfix)
                                                    ^
./Tribler/Core/Modules/restapi/create_torrent_endpoint.py:60:31: F821 undefined name 'unicode'
            file_path_list = [unicode(f, 'utf-8') for f in parameters['files[]']]
                              ^
./TriblerGUI/vlc.py:4784:50: F821 undefined name 'MediaDiscovererDescription'
                   ctypes.POINTER(ctypes.POINTER(MediaDiscovererDescription)))
                                                 ^
./TriblerGUI/vlc.py:4796:41: F821 undefined name 'MediaDiscovererDescription'
                   None, ctypes.POINTER(MediaDiscovererDescription), ctypes.c_size_t)
                                        ^
12    F821 undefined name 'unicode'
12

[Dmole]

...propose fixes...

Throw experience out the window, make an alpha build without unicode() and let users report bugs :)

[devos50]

@cclauss thanks for all your work so far! We would highly appreciate if you could help us fix the remaining Python 3 compatibility issues (beyond the ones discoverable with flake8) 👍

Regarding these showstoppers, I will try to include a new version of the VLC bindings today. Regarding the unicode(s, enc) calls, I should discuss this with the team what we should do with this. If I remember correctly, we have a cast_to_unicode_utf8 method somewhere which is both Python 2 and 3 compatible. We could use that one?

[cclauss]

I used __six.ensure_text()__ in #4287 and it seems to work but I would recommend that someone who really grocks Unicode (i.e. _not me_) tries to rationalize __six.ensure_text()__, __cast_to_unicode_utf8()__, and __dunno2unicode()__. The use of bare exceptions in _Tribler/Core/Utilities/unicode.py_ bug me to no end.

[qstokkink]

The core issue is that we have 14 years of contributions, doing whatever works to please the unicode gods. We have 50 different methods to do roughly the same string/unicode transformations.

Honestly, looking back, I believe it would have been better if we had created our own String class which would be just an array of unsigned shorts (or store it all in a raw_unicode_escape encoded string). Only at the very edges of our system would we then convert our own String to an str or unicode with optional character set.

[cclauss]

Python 3 has _explicit_ __bytes__ and all __str__ are Unicode for exactly these reasons.

Your String approach seems to be the __Unicode sandwich__ turned on its head.

[qstokkink]

Python 3 has explicit bytes and all str are Unicode for exactly these reasons.

In theory, yes. But in practice it is still a big clusterf*ck of character maps and broken conversions. I might have to propose this beautiful new String standard for Python 4.

[cclauss]

As of 12h12 on 05 March 2019 this repo is _syntax compatible_ with Python 3.

No syntax errors and no undefined names. This does ___not___ mean that the port to Python 3 is complete but it is a minor milestone on that path.

Next up will be to run https://python-modernize.readthedocs.io on various files and directories to see what kind of changes that it recommends. So far, we have only been __linting__ (running flake8 on Python 3 and pylint --py3k on Python 2) for Py3 issues but we now need to step up our testing to:

1. run pylint tests on Python 3 (like we do today for flake8) and
2. run Linux, Mac, and/or Windows tests on Python 3.

[qstokkink]

I think we run Python 3.6 on the testing machine.

[devos50]

We are running Python 3.5 on our development machine.

I just setup a job to run all the Python 3 tests on this machine. You can find it here: https://jenkins-ci.tribler.org/job/test_tribler_python3. Results:

Ran 794 tests in 240.218s
FAILED (errors=273, failures=14)

We can slowly start to fix the easy issues now I guess?

[cclauss]

Nice. Yes.

File "/home/jenkins/.local/lib/python3.5/site-packages/pony/orm/dbapiprovider.py", line 647, in validate
if isinstance(val, str): return buffer(val)

What version of https://pypi.org/project/pony are we using? Where are our dependency versions in general?

[qstokkink]

Here's the just-give-me-something-that-works string class prototype:

https://gist.github.com/qstokkink/91d2b22a647a14d16726dd3745aaa063

[qstokkink]

With #4707, the issue of porting to Python 3 has been resolved. (Note that this is not to say we won't run into any Python 3 incompatibilities in the future).