Support for Python 3

[george-hopkins]

This PR implements support for Python 3. As I'm not very familiar with whole code base, I would be glad to receive some feedback if everything still works as expected. All unit tests pass on 2.7, 3.3 and 3.5. In addition the decoding was tested against an other implementation.

• [x] Wait for zooko/pyutil#4 tpltnt/pyutil#1 to get merged and released (ports pyutil to Python 3)

[george-hopkins]

@daira Thanks for reviewing! I updated the code accordingly. Please feel free to comment if you find any further issues.

It seems that the comments don't show up here as they were made before this PR was created.

[george-hopkins]

@ThomasWaldmann Thank you for putting so much effort in reviewing this pull-request! In retrospect, it might be more sensible to ditch Python 2 support altogether. This would simplify maintenance quite a bit.

@zooko, @vu3rdd What are your views on this proposal? I'm happy to address the issues mentioned as soon as possible if there is an interest in this PR in general.

[ThomasWaldmann]

@george-hopkins 2.7.x and 3.4+ are quite reasonable choices right now. YMMV. :)

[warner]

Yay for py3 support! I'd second @ThomasWaldmann suggestion to go with 2.7 and >=3.3 or >=3.4. I'm particularly keen to maintain 2.7 support because Tahoe-LAFS is still 2.7-only, and I think Tahoe remains the largest customer of zfec.

[warner]

I'll definitely land this once it's ready. Some ideas for a rewritten branch:

• start with a commit that removes support for py2.4/2.5 (which I think just means changing setup.py's metadata claims)
• then a commit that switches from absolute imports to relative imports
• then one to fix that compare-int-vs-None problem
• then some commits to replace the py3-incompatible syntax issues: print functions, "except X,y" -> "except X as y". This is a good place to add from __future__ import lines
• then a commit to add six and maybe change some of the bytes/unicodes to use it. Also things like changing import cStringIO to use the six.moves equivalent
• then fix the rest of the py3 incompatibilities
• then finally add py3 to setup.py and travis and tox

The general idea is the tree should build and test correctly after every commit. But I'll be ok if it doesn't actually achieve that, as long as the sequence is fairly easy to follow,

That travis line that installs pyutil from a git repo.. what's that for? I want to get rid of that. Do we need a new pyutil release with something fixed? What do we need pyutil for anyways? (I vaguely remember talking with zooko a long while back about maybe splitting the CLI utilities out of zfec and into a separate package, because I seem to remember that pyutil was only needed for the CLI tools).

[george-hopkins]

@warner Thank you for your feedback! I'm happy to incorporate the suggested changes if the PR has a chance of getting merged.

As for pyutil: As pyutil needs to be ported to Python 3 as well, I created zooko/pyutil#4. If you can help to get it merged, that would be great! As soon as it's merged, we can get rid of the include from my personal repository here (used it to see if Travis succeeds on all versions).

[sbellem]

May we just know whether there are near-term plans to move this PR forward and eventually merge it?

I could provide some help if needed.

[tpltnt]

I am waiting for @george-hopkins to do the PR for Python 3 support against my fork so I can build and push to pypi. This would remove the major blocker and move the Python 3 port of zfec forward.

[tpltnt]

I am happy to announce pyutil version 3.0.0 now supports Python 3 thanks to @george-hopkins . The module is already published on pypi.

[george-hopkins]

With the new release of pyutil (thanks to @tpltnt!) the patch is now ready to be reviewed. The updated patch already includes some fixes for the issues mentioned earlier.

[tpltnt]

The code looks good to me.

[nimamox]

Has it been merged to the main branch? I couldn't use pip3 to install zfec. EDIT: I get an error during compilation on macOS from @george-hopkins fork.

$ pip3 install git+https://github.com/george-hopkins/zfec
...
    zfec/_fecmodule.c:633:12: warning: incompatible integer to pointer conversion assigning to 'PyObject *' (aka 'struct _object *') from 'int' [-Wint-conversion]
        module = Py_InitModule3("_fec", fec_functions, fec__doc__);
               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    zfec/_fecmodule.c:635:7: error: non-void function 'init_fec' should return a value [-Wreturn-type]
          return;
          ^
    33 warnings and 5 errors generated.
    error: command 'clang' failed with exit status 1

[warner]

Looks good to me, local tests pass on macOS. There's a problem with extending tox to include py3 (the "trialcoverage" dependency doesn't work on py3), but we can fix that later. Will land momentarily. Thanks everyone!

[george-hopkins]

@warner Thank you for merging and the clean up you had to do before the release!

@nimamox I kept my porting efforts in a separate branch "python3". However, with the new release you can simply install it with pip install zfec.