[MRG] Working towards python3 compatible codebase - #442

@mr-c @anton-khodak: this PR is almost ready to merge now. I will do minor annotation improvements by tomorrow. Please review.

branch is on my fork, but edits from maintainers are allowed :)

Issues created for avro-python3: https://issues.apache.org/jira/browse/AVRO-2046

PR to make avro-python2 compatible with Python 3: apache/avro#234

Update: I've updated the description of the issue on JIRA to better express our interests.

Update
So in order to invoke 2_to_3 automated convertor on avro-python2 during install we have to modify it's setup.py file. There is no way to do it from cwltool's setup.py file.

But the changes we need to make to avro-python2 setup.py are very minimal for it convert all the source files to Python 3. Here's the link to the setup.py and below is the diff:

 try:
-  from setuptools import setup
+  from setuptools import setup, find_packages
 except ImportError:
   from distutils.core import setup
 from sys import version_info
@@ -27,10 +27,10 @@ if version_info[:2] <= (2, 5):
 
 setup(
   name = 'avro',
-  version = '@AVRO_VERSION@',
-  packages = ['avro',],
-  package_dir = {'avro': 'src/avro'},
-  scripts = ["./scripts/avro"],
+  version = '1.0',
+  package_dir = find_packages(),
+  use_2to3=True,
 
   #include_package_data=True,
   package_data={'avro': ['LICENSE', 'NOTICE']},

2to3 might introduce regressions. But I've tested it with schema_salad and cwltool and it passes all unit tests in Python 3.

cc: @mr-c @anton-khodak

@manu-chroma Can you try out http://python-future.org/translation.html ?

@mr-c This looks like a great alternative. I've tested it locally and it works.

Just one problem here, it fails to convert this particular line https://github.com/apache/avro/blob/master/lang/py/src/avro/schema.py#L778. So I had to change it in avro source before it worked for me.

It passed both schema_salad and cwltool unit tests locally.

Possible solutions:

• We can try changing this particular line in the pip installed avro source on user's machine but this is not the best approach.
• Possbily make a PR to avro. This is pretty minimal change so maybe they might accept.

More info on the error: http://python-future.org/compatible_idioms.html#raising-exceptions

@manu-chroma 👍 to a PR against their repo. Shall I merge what you've done so far?

@mr-c Yes please 😄

Woohoo! Getting closer!

👍 to a PR against their repo.

I'm trying to make a PR which they would convincingly accept. The problem is this code when run in python3:

from past import autotranslate

autotranslate(['avro'])
import avro.schema

outputs

Traceback (most recent call last):
  File "late.py", line 4, in <module>
    import avro.schema
  File "<frozen importlib._bootstrap>", line 969, in _find_and_load
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 664, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 634, in _load_backward_compatible
  File "/home/manu/Desktop/test/venv3/lib/python3.5/site-packages/past/translation/__init__.py", line 402, in load_module
    code = compile(source, self.pathname, 'exec')
  File "/home/manu/Desktop/test/venv3/lib/python3.5/site-packages/avro/schema.py", line 782
    % (json_string, e)), None, sys.exc_info()[2]
                       ^
SyntaxError: invalid syntax

The way autotranslate works is it converts code into py2+3 compatible code.

raise SchemaParseException('Error parsing JSON: %s, error = %s'
                               % (json_string, e)), None, sys.exc_info()[2]

to make this line work in cross-compatible way, we need to use six lib. If I re-write this line to work with both py2+3 without using any external library, it suppresses a useful exception generated by sys.exc_info()[2] keyword.

Still trying to figure out how to change it in such a way that gets accepted upstream.

@manu-chroma I'd implement it as separate py2 and py3 paths as documented in http://python-future.org/compatible_idioms.html#raising-exceptions with an if sys.version_info[0] < 3: to switch between them

Yes, I had the same idea @mr-c but I was doubtful they would accept such a patch. Though it's definitely worth a shot.