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

python 2 support #12

Closed
haoxian-zhao opened this issue Dec 16, 2018 · 12 comments
Closed

python 2 support #12

haoxian-zhao opened this issue Dec 16, 2018 · 12 comments

Comments

@haoxian-zhao
Copy link

hi there, thanks for writing pampy and it looks very interesting. at the moment, only python2 is possible in my work environment, may I ask if there is chance we can have this backport to python2? thanks

@gchamon
Copy link

gchamon commented Dec 16, 2018

from readme:

Install
Currently it works only in Python >= 3.6 Because dict matching can work only in the latest Pythons.
I'm currently working on a backport with some minor syntax changes for Python2.

there is not only a chance but there is work being put into porting to Python 2

I wonder if 3to2 could help

@mcepl
Copy link

mcepl commented Dec 17, 2018

@Pixiez ??? https://pythonclock.org/

Where is the place you are wasting time in porting libraries to Python 2 instead of finishing porting of whole project to Python 3?

@mbarkhau
Copy link

Shameless plug: https://pypi.org/project/lib3to6/

@mbarkhau
Copy link

I started a proof of concept for python2.7 compatibility using lib3to6: mbarkhau@4e33ac3

I had to move the library into a src/ directory, and inline the helpers.py module, other than that the changes are mainly related to actual python2 compatibility issues, eg. str means something different.

Running make test shows that most tests, pass. I haven't looked into the remaining three that are failing.

$ make test
rm -f dist/*.whl
/home/mbarkhau/miniconda3/envs/pampy36/bin/python setup.py bdist_wheel;
running bdist_wheel
running build
running build_py
copying build/lib3to6_out/src/pampy/pampy.py -> build/lib/pampy
copying build/lib3to6_out/src/pampy/__init__.py -> build/lib/pampy
installing to build/bdist.linux-x86_64/wheel
running install
running install_lib
creating build/bdist.linux-x86_64/wheel
creating build/bdist.linux-x86_64/wheel/pampy
copying build/lib/pampy/pampy.py -> build/bdist.linux-x86_64/wheel/pampy
copying build/lib/pampy/__init__.py -> build/bdist.linux-x86_64/wheel/pampy
running install_egg_info
running egg_info
creating build/lib3to6_out/src/pampy.egg-info
writing build/lib3to6_out/src/pampy.egg-info/PKG-INFO
writing dependency_links to build/lib3to6_out/src/pampy.egg-info/dependency_links.txt
writing requirements to build/lib3to6_out/src/pampy.egg-info/requires.txt
writing top-level names to build/lib3to6_out/src/pampy.egg-info/top_level.txt
writing manifest file 'build/lib3to6_out/src/pampy.egg-info/SOURCES.txt'
reading manifest file 'build/lib3to6_out/src/pampy.egg-info/SOURCES.txt'
writing manifest file 'build/lib3to6_out/src/pampy.egg-info/SOURCES.txt'
Copying build/lib3to6_out/src/pampy.egg-info to build/bdist.linux-x86_64/wheel/pampy-0.1.10-py3.6.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/pampy-0.1.10.dist-info/WHEEL
creating 'dist/pampy-0.1.10-py2.py3-none-any.whl' and adding 'build/bdist.linux-x86_64/wheel' to it
adding 'pampy/__init__.py'
adding 'pampy/pampy.py'
adding 'pampy-0.1.10.dist-info/LICENSE'
adding 'pampy-0.1.10.dist-info/METADATA'
adding 'pampy-0.1.10.dist-info/WHEEL'
adding 'pampy-0.1.10.dist-info/top_level.txt'
adding 'pampy-0.1.10.dist-info/RECORD'
removing build/bdist.linux-x86_64/wheel
/home/mbarkhau/miniconda3/envs/pampy36/bin/python -m pip install dist/*.whl
Requirement already satisfied: pampy==0.1.10 from file:///mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/dist/pampy-0.1.10-py2.py3-none-any.whl in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (0.1.10)
Requirement already satisfied: six in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (from pampy==0.1.10) (1.12.0)
Requirement already satisfied: typing in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (from pampy==0.1.10) (3.6.6)
/home/mbarkhau/miniconda3/envs/pampy36/bin/python -m unittest discover -s compat_tests/
.............................................
----------------------------------------------------------------------
Ran 45 tests in 0.032s

OK
/home/mbarkhau/miniconda3/envs/pampy27/bin/python -m pip install dist/*.whl
Requirement already satisfied: pampy==0.1.10 from file:///mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/dist/pampy-0.1.10-py2.py3-none-any.whl in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (0.1.10)
Requirement already satisfied: typing in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (from pampy==0.1.10) (3.6.6)
Requirement already satisfied: six in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (from pampy==0.1.10) (1.12.0)
/home/mbarkhau/miniconda3/envs/pampy27/bin/python -m unittest discover -s compat_tests/
.......................FF......E.............
======================================================================
ERROR: test_parser (test_elaborate.PampyElaborateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/compat_tests/test_elaborate.py", line 45, in test_parser
    assert parser('x') == 'any string'
  File "/mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/compat_tests/test_elaborate.py", line 41, in parser
    [2, _], _], lambda a, b: '[1, [2, %s], %s]' % (a, b))
  File "/home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages/pampy/pampy.py", line 232, in match
    text_type(var))
MatchError: '_' not provided. This case is not handled:
x

======================================================================
FAIL: test_multi_underscore_ambiguous (test_dict.IterableTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/compat_tests/test_dict.py", line 49, in test_multi_underscore_ambiguous
    'c': 3}), (True, [1, 'b', 2]))
AssertionError: Tuples differ: (True, [1, u'c', 3]) != (True, [1, u'b', 2])

First differing element 1:
[1, u'c', 3]
[1, u'b', 2]

- (True, [1, u'c', 3])
?              ^   ^

+ (True, [1, u'b', 2])
?              ^   ^


======================================================================
FAIL: test_wild_dicts (test_dict.IterableTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/compat_tests/test_dict.py", line 64, in test_wild_dicts
    self.assertEqual(names, ['fuffy', 'puffy', 'buffy'])
AssertionError: Lists differ: [u'dog', u'pet', u'cat'] != [u'fuffy', u'puffy', u'buffy']

First differing element 0:
u'dog'
u'fuffy'

- [u'dog', u'pet', u'cat']
+ [u'fuffy', u'puffy', u'buffy']

----------------------------------------------------------------------
Ran 45 tests in 0.044s

FAILED (failures=2, errors=1)
makefile:15: recipe for target 'test' failed
make: *** [test] Error 1

@santinic
Copy link
Owner

Thank you @mbarkhau, I encourage you to keep trying. Unfortunately the some features of Pampy won't be working in Python2 (dataclasses support, and default ordered dictionaries).

To make it work you need to replace any pattern dictionary, into an Phython2 OrderedDict, otherwise precedence won't work. The fact you can use simple dictionaries {...} in pampy is because >=Python3.6 makes any dict an OrderedDict.

So every match(x, {'a':1, 'b':2}, action) in the tests, must become match(x, OrderedDict((['a',1],['b',2]]), action)

@mbarkhau
Copy link

Indeed. Swapping out the dictionaries in all failing tests to use OrderedDict did the trick. Now all tests pass on python2.7.

/home/mbarkhau/miniconda3/envs/pampy36/bin/python -m pip install dist/*.whl
Requirement already satisfied: pampy==0.1.10 from file:///mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/dist/pampy-0.1.10-py2.py3-none-any.whl in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (0.1.10)
Requirement already satisfied: six in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (from pampy==0.1.10) (1.12.0)
Requirement already satisfied: typing in /home/mbarkhau/miniconda3/envs/pampy36/lib/python3.6/site-packages (from pampy==0.1.10) (3.6.6)
/home/mbarkhau/miniconda3/envs/pampy36/bin/python -m unittest discover -s compat_tests/
.............................................
----------------------------------------------------------------------
Ran 45 tests in 0.050s

OK
/home/mbarkhau/miniconda3/envs/pampy27/bin/python -m pip install dist/*.whl
Requirement already satisfied: pampy==0.1.10 from file:///mnt/c/Users/ManuelBarkhau/Dropbox/projects/pampy/dist/pampy-0.1.10-py2.py3-none-any.whl in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (0.1.10)
Requirement already satisfied: typing in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (from pampy==0.1.10) (3.6.6)
Requirement already satisfied: six in /home/mbarkhau/miniconda3/envs/pampy27/lib/python2.7/site-packages (from pampy==0.1.10) (1.12.0)
/home/mbarkhau/miniconda3/envs/pampy27/bin/python -m unittest discover -s compat_tests/
.............................................
----------------------------------------------------------------------
Ran 45 tests in 0.041s

OK

@mbarkhau
Copy link

mbarkhau commented Dec 20, 2018

Sooo, do you think this is a viable path to go forward with?

"I'm currently working on a backport with some minor syntax changes for Python2."

Considering this, would you prefer to go forward with that effort rather than use this transpiler?

One thing I can think of that might be an issue is the CI setup with travis. For my projects I use the gitlab CI, with a docker image that has both python3.6 and 2.7 installed. There I generate a wheel with the transpiled code using the 3.6 interpreter and test the wheel using the 2.7 interpreter. I'm not sure how to do that with travis.

@santinic
Copy link
Owner

Mm I'm not actually working on it, I will remove that line. You have done a great job. But maybe you should just have two different projects, what do you think ? maybe people can pip install your fork if they need to use python2. I could link to it in the Readme.

@mbarkhau
Copy link

Sounds ok. Any name suggestions for the package? Maybe backports.pampy? Probably the module name should still be pampy though.

@mbarkhau
Copy link

Looking over the documentation here https://pypi.org/project/backports/, it appears the recommendation would actually to have the import also be backports.pampy.

@mbarkhau
Copy link

Well, here you go: https://pypi.org/project/backports.pampy/

$ pip install backports.pampy
...
Installing collected packages: backports.pampy
Successfully installed backports.pampy-0.1.10
$ ipython
Python 2.7.15 | packaged by conda-forge | (default, Oct 12 2018, 14:10:50)
...
In [1]: import backports.pampy as pampy
In [2]: pampy.__version__
Out[2]: u'0.1.10'
In [3]: from backports.pampy import match, HEAD, TAIL, _
In [4]: x = [1, 2, 3]
In [5]: match(x, [1, TAIL], lambda t: t)
Out[5]: [2, 3]
In [6]: match(x, [HEAD, TAIL],  lambda h, t: (h, t))
Out[6]: (1, [2, 3])

@santinic
Copy link
Owner

Thanks @mbarkhau.
I added a link to it in the README.
I'm not sure how much you want to keep track of Pampy changes, but we are adding a default param to the match() function soon. Thanks again

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

No branches or pull requests

5 participants