Python 3 support #18

Merged
merged 7 commits into from Apr 15, 2014

Conversation

Projects
None yet
9 participants
@vsajip
Contributor

vsajip commented May 1, 2013

You might be interested in some changes I've made to see how Parsley can work with Python 3. The work was straightforward (a couple of hours) and all tests pass on 2.7 and 3.2 using a single code base which runs unchanged on >= 2.6 and 3.x. (The changed code will not work on Python <= 2.5 because of the incompatible exception syntax - though this could be hacked around, it makes the code ugly, so I didn't do it. I couldn't see from Parsley's docs or PyPI metadata what minimum Python version it works with.)

Even if you have no interest in merging these changes (as you don't use Python 3), I would be grateful for any comments you have.

@ambv

This comment has been minimized.

Show comment
Hide comment
@ambv

ambv May 4, 2013

👍 for Python 3 support.

ambv commented May 4, 2013

👍 for Python 3 support.

@orutherfurd

This comment has been minimized.

Show comment
Hide comment
@orutherfurd

orutherfurd Jul 15, 2013

Another 👍 for Python 3 support.

Another 👍 for Python 3 support.

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Aug 2, 2013

Contributor

This branch no longer cleanly applies to master. If you're interested in updating it, I can merge it afterward!

Also, what have you been using to run tests?

Contributor

habnabit commented Aug 2, 2013

This branch no longer cleanly applies to master. If you're interested in updating it, I can merge it afterward!

Also, what have you been using to run tests?

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Aug 2, 2013

Contributor

I've updated the python3 branch to merge recent changes in master. For testing, I created two venvs (one for Python 2.7, one for Python 3.2), installed nose and Twisted in each (since Twisted test infrastructure is used) and ran nosetests with each venv activated in turn, from the parley root directory.

Contributor

vsajip commented Aug 2, 2013

I've updated the python3 branch to merge recent changes in master. For testing, I created two venvs (one for Python 2.7, one for Python 3.2), installed nose and Twisted in each (since Twisted test infrastructure is used) and ran nosetests with each venv activated in turn, from the parley root directory.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Aug 21, 2013

Contributor

Bump: any reason for not merging yet? If you're going to merge, it's best done before master starts diverging too much ...

Contributor

vsajip commented Aug 21, 2013

Bump: any reason for not merging yet? If you're going to merge, it's best done before master starts diverging too much ...

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Aug 21, 2013

Contributor

Sorry, I was working on this and ended up starting to shave yaks. Basically I'm not sure if it's that our test setups aren't the same, but running nose for me locally still has failing tests for this branch. I was looking into getting a CI setup going so that this kind of thing can be done authoritatively and publicly.

Contributor

habnabit commented Aug 21, 2013

Sorry, I was working on this and ended up starting to shave yaks. Basically I'm not sure if it's that our test setups aren't the same, but running nose for me locally still has failing tests for this branch. I was looking into getting a CI setup going so that this kind of thing can be done authoritatively and publicly.

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Aug 22, 2013

Contributor

Okay, travis CI is set up now. If you can get a passing build on travis for 2.6/2.7 and some 3.x, this can get merged.

Contributor

habnabit commented Aug 22, 2013

Okay, travis CI is set up now. If you can get a passing build on travis for 2.6/2.7 and some 3.x, this can get merged.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 22, 2013

Coverage Status

Coverage remained the same when pulling ddf42d4 on vsajip:python3 into 30e706e on python-parsley:master.

Coverage Status

Coverage remained the same when pulling ddf42d4 on vsajip:python3 into 30e706e on python-parsley:master.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Aug 22, 2013

Contributor

There are Travis failures when twisted.trial.unittest is used, but not when unittest is used. They appear to be related to test infrastructure rather than parsley itself - some problem with Twisted on 3.x, perhaps?

See the last two commits - the only difference is due to the use of twisted.trial.unittest as opposed to unittest. It works fine locally in a venv with distribute 0.6.34, Twisted 13.0.0, nose 1.3.0 and zope.interface 4.0.5.

Contributor

vsajip commented Aug 22, 2013

There are Travis failures when twisted.trial.unittest is used, but not when unittest is used. They appear to be related to test infrastructure rather than parsley itself - some problem with Twisted on 3.x, perhaps?

See the last two commits - the only difference is due to the use of twisted.trial.unittest as opposed to unittest. It works fine locally in a venv with distribute 0.6.34, Twisted 13.0.0, nose 1.3.0 and zope.interface 4.0.5.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 22, 2013

Coverage Status

Coverage decreased (-0.9%) when pulling efb5830 on vsajip:python3 into 30e706e on python-parsley:master.

Coverage Status

Coverage decreased (-0.9%) when pulling efb5830 on vsajip:python3 into 30e706e on python-parsley:master.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Aug 22, 2013

Contributor

Everything works fine with nose, but the Twisted test infrastructure doesn't seem to work under Python 3. Digging further, for example, I find that the Twisted trial script has this code:

try:
    import _preamble
except ImportError:
    sys.exc_clear()

The only thing is, in Python 3.x sys has no exc_clear function. I can't fix this without shaving yaks myself :-(

Contributor

vsajip commented Aug 22, 2013

Everything works fine with nose, but the Twisted test infrastructure doesn't seem to work under Python 3. Digging further, for example, I find that the Twisted trial script has this code:

try:
    import _preamble
except ImportError:
    sys.exc_clear()

The only thing is, in Python 3.x sys has no exc_clear function. I can't fix this without shaving yaks myself :-(

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

+1 for python3 support, I'm currently using the vsajip/parsley branch with success. (with a small patch I must pull-request)

Contributor

bjonnh commented Nov 19, 2013

+1 for python3 support, I'm currently using the vsajip/parsley branch with success. (with a small patch I must pull-request)

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Nov 19, 2013

Contributor

bjonnh notifications@github.com writes:

+1 for python3 support, I'm currently using the vsajip/parsley branch with success. (with a small patch I must pull-request)

Would you mind doing so? I'm starting to look over the parsley stuff
again and this was a good reminder that I need to revisit
this. Hopefully your branch will pass on travis-ci!

Contributor

habnabit commented Nov 19, 2013

bjonnh notifications@github.com writes:

+1 for python3 support, I'm currently using the vsajip/parsley branch with success. (with a small patch I must pull-request)

Would you mind doing so? I'm starting to look over the parsley stuff
again and this was a good reminder that I need to revisit
this. Hopefully your branch will pass on travis-ci!

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

I don't think so, I just adapted some runtime errors…

Contributor

bjonnh commented Nov 19, 2013

I don't think so, I just adapted some runtime errors…

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

And I need to put all the new stuff that changed on parsley since the vsajip repo. But I don't know how to do that with github.

Contributor

bjonnh commented Nov 19, 2013

And I need to put all the new stuff that changed on parsley since the vsajip repo. But I don't know how to do that with github.

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Nov 19, 2013

Contributor

bjonnh notifications@github.com writes:

And I need to put all the new stuff that changed on parsley since the vsajip repo. But I don't know how to do that with github.

Which part are you having trouble with? I might be able to help.

Contributor

habnabit commented Nov 19, 2013

bjonnh notifications@github.com writes:

And I need to put all the new stuff that changed on parsley since the vsajip repo. But I don't know how to do that with github.

Which part are you having trouble with? I might be able to help.

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

Ok just check my parsley repository in the python3 branch. I have included all the last stuff. Now I can work on the travis stuff if I manage to understand how it works.

Contributor

bjonnh commented Nov 19, 2013

Ok just check my parsley repository in the python3 branch. I have included all the last stuff. Now I can work on the travis stuff if I manage to understand how it works.

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Nov 19, 2013

Contributor

bjonnh notifications@github.com writes:

Ok just check my parsley repository in the python3 branch. I have included all the last stuff. Now I can work on the travis stuff if I manage to understand how it works.

Travis is pretty easy--just post a pull request, and it'll automagically
run over your code in the pull request.

Contributor

habnabit commented Nov 19, 2013

bjonnh notifications@github.com writes:

Ok just check my parsley repository in the python3 branch. I have included all the last stuff. Now I can work on the travis stuff if I manage to understand how it works.

Travis is pretty easy--just post a pull request, and it'll automagically
run over your code in the pull request.

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

Ok I just added travis to my own project, I don't want to bother the parsley people with my tries.

Contributor

bjonnh commented Nov 19, 2013

Ok I just added travis to my own project, I don't want to bother the parsley people with my tries.

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

Mmm the problem is really in twisted. I've been able to run everything but the things that need twisted.

Contributor

bjonnh commented Nov 19, 2013

Mmm the problem is really in twisted. I've been able to run everything but the things that need twisted.

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Nov 19, 2013

Contributor

bjonnh notifications@github.com writes:

Mmm the problem is really in twisted. I've been able to run everything but the things that need twisted.

Yes--trial is not ported to python 3 yet. I was thinking you could use a
different test runner (py.test? since it's already being used) when on
python 3.

Contributor

habnabit commented Nov 19, 2013

bjonnh notifications@github.com writes:

Mmm the problem is really in twisted. I've been able to run everything but the things that need twisted.

Yes--trial is not ported to python 3 yet. I was thinking you could use a
different test runner (py.test? since it's already being used) when on
python 3.

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

py.test-3.3 test_parsley.py 2013-11-19 17:46:13 jo pts/12
================================================= test session starts =================================================platform linux -- Python 3.3.2 -- pytest-2.4.2
collected 5 items

test_parsley.py .....

============================================== 5 passed in 0.07 seconds ===============================================

Contributor

bjonnh commented Nov 19, 2013

py.test-3.3 test_parsley.py 2013-11-19 17:46:13 jo pts/12
================================================= test session starts =================================================platform linux -- Python 3.3.2 -- pytest-2.4.2
collected 5 items

test_parsley.py .....

============================================== 5 passed in 0.07 seconds ===============================================

@bjonnh

This comment has been minimized.

Show comment
Hide comment
@bjonnh

bjonnh Nov 19, 2013

Contributor

Hmm seem that pytest is failing too with a "AttributeError: 'function' object has no attribute 'im_func'"

Contributor

bjonnh commented Nov 19, 2013

Hmm seem that pytest is failing too with a "AttributeError: 'function' object has no attribute 'im_func'"

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Nov 20, 2013

Contributor

IIRC I had all tests working when I last looked at this, using unittest. I didn't do any work on this because of the Twisted dependency for tests, which I didn't want to mess with. I tend to find that the stdlib unittest is good enough for writing both unit and functional tests, but there's no point changing test infrastructure where upstream developers have different preferences.

N.B. The very first Python 3 port only took a couple of hours! The next lot of merging took longer only because of the test infrastructure chahges, and not because of inherent difficulties with the port.

Contributor

vsajip commented Nov 20, 2013

IIRC I had all tests working when I last looked at this, using unittest. I didn't do any work on this because of the Twisted dependency for tests, which I didn't want to mess with. I tend to find that the stdlib unittest is good enough for writing both unit and functional tests, but there's no point changing test infrastructure where upstream developers have different preferences.

N.B. The very first Python 3 port only took a couple of hours! The next lot of merging took longer only because of the test infrastructure chahges, and not because of inherent difficulties with the port.

@furrykef

This comment has been minimized.

Show comment
Hide comment
@furrykef

furrykef Jan 31, 2014

Is there a reason why parsley throws up a bunch of errors when I type "pip install parsley" in Python 3? Python 3.0 was released over five years ago. I really think it should be supported properly out of the box.

Is there a reason why parsley throws up a bunch of errors when I type "pip install parsley" in Python 3? Python 3.0 was released over five years ago. I really think it should be supported properly out of the box.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Jan 31, 2014

Contributor

@furrykef, The reason is that Parsley doesn't support Python 3 - but I guess your question was rhetorical. Though I made changes 9 months ago in my fork to fix this, and did a further update 5 months ago to merge upstream changes, my changes haven't been accepted. While Parsley itself worked fine with Python 3 last time I worked on it, Parsley's unit tests depend on Twisted, which AFAIK doesn't work under Python 3. The Parsley devs don't seem interested in merging my changes until the unit tests pass, and I'm not really up for porting Twisted to Python 3 just to keep them happy :-)

Contributor

vsajip commented Jan 31, 2014

@furrykef, The reason is that Parsley doesn't support Python 3 - but I guess your question was rhetorical. Though I made changes 9 months ago in my fork to fix this, and did a further update 5 months ago to merge upstream changes, my changes haven't been accepted. While Parsley itself worked fine with Python 3 last time I worked on it, Parsley's unit tests depend on Twisted, which AFAIK doesn't work under Python 3. The Parsley devs don't seem interested in merging my changes until the unit tests pass, and I'm not really up for porting Twisted to Python 3 just to keep them happy :-)

@furrykef

This comment has been minimized.

Show comment
Hide comment
@furrykef

furrykef Jan 31, 2014

OK, so why is the unit test suite using Twisted? Rather than porting Twisted to Python 3 perhaps we should simply eliminate that dependency.

OK, so why is the unit test suite using Twisted? Rather than porting Twisted to Python 3 perhaps we should simply eliminate that dependency.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Jan 31, 2014

Contributor

Sure, but that's seemingly a preference of the Parsley developers. Perhaps it's what they're familiar with - I know my preference is to keep dependencies to a minimum, and why a parsing library would depend on an asynchronous networking library is beyond me. (The other way round makes sense, of course, since network processing does involve parsing ...)

You're welcome to try with my fork (around 5 months since my last commit - it should be OK unless you need some recent features, or run into a deal-breaker when you try and use it ...)

Contributor

vsajip commented Jan 31, 2014

Sure, but that's seemingly a preference of the Parsley developers. Perhaps it's what they're familiar with - I know my preference is to keep dependencies to a minimum, and why a parsing library would depend on an asynchronous networking library is beyond me. (The other way round makes sense, of course, since network processing does involve parsing ...)

You're welcome to try with my fork (around 5 months since my last commit - it should be OK unless you need some recent features, or run into a deal-breaker when you try and use it ...)

@furrykef

This comment has been minimized.

Show comment
Hide comment
@furrykef

furrykef Feb 2, 2014

Then it would seem incumbent upon us to either persuade them to try a different testing system (I'd suggest nosetest, though I still have yet to look at how parsley's testing suite works) or to maintain our own Python 3-compatible fork of the software, complete with a package on PyPI and everything. It's time for the Python community to move forward, and we're not going to be able to if we keep sticking with technology that works only on 2.x.

furrykef commented Feb 2, 2014

Then it would seem incumbent upon us to either persuade them to try a different testing system (I'd suggest nosetest, though I still have yet to look at how parsley's testing suite works) or to maintain our own Python 3-compatible fork of the software, complete with a package on PyPI and everything. It's time for the Python community to move forward, and we're not going to be able to if we keep sticking with technology that works only on 2.x.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Feb 2, 2014

Contributor

The original porting took just a couple of hours, which I had no problem investing, but I don't think I've the bandwidth to provide the level of support a full parallel distribution would imply. (The projects I work on aren't heavily dependent on Parsley, but I like to do my bit to encourage Python 3.x adoption, when I can.) The software in my repository passed all tests with Python 2.7 and 3.2 when I last looked at it, using nosetests. You're welcome to use that code.

Contributor

vsajip commented Feb 2, 2014

The original porting took just a couple of hours, which I had no problem investing, but I don't think I've the bandwidth to provide the level of support a full parallel distribution would imply. (The projects I work on aren't heavily dependent on Parsley, but I like to do my bit to encourage Python 3.x adoption, when I can.) The software in my repository passed all tests with Python 2.7 and 3.2 when I last looked at it, using nosetests. You're welcome to use that code.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Mar 14, 2014

It looks like parsley has a way of integrating with Twisted to parse incoming data from the network:
http://parsley.readthedocs.org/en/latest/tutorial3.html#protocol-parsing

The tests rely on twisted in order to test that code, but it should be possible to skip or exclude those tests on Python 3 until Twisted is ready.

It looks like parsley has a way of integrating with Twisted to parse incoming data from the network:
http://parsley.readthedocs.org/en/latest/tutorial3.html#protocol-parsing

The tests rely on twisted in order to test that code, but it should be possible to skip or exclude those tests on Python 3 until Twisted is ready.

@vsajip

This comment has been minimized.

Show comment
Hide comment
@vsajip

vsajip Mar 14, 2014

Contributor

I haven't looked at the code recently, but the last time I looked, Parsley seemed to be using Twisted test harness code (twisted.trial.unittest) for everything. Did I misunderstand, or have things changed?

Contributor

vsajip commented Mar 14, 2014

I haven't looked at the code recently, but the last time I looked, Parsley seemed to be using Twisted test harness code (twisted.trial.unittest) for everything. Did I misunderstand, or have things changed?

@habnabit

This comment has been minimized.

Show comment
Hide comment
@habnabit

habnabit Mar 18, 2014

Contributor

Twisted's twisted.trial.testcase is ported to python 3, so that's not an issue. I just looked into this again and ran into a bug in py.test, the current test runner: https://bitbucket.org/hpk42/pytest/issue/483/function-object-has-no-attribute-im_func

When this is fixed in py.test and Twisted 14.0 is out (since it fixes pip3 install Twisted), I think this will be ready to merge, unless someone wants to take the time to remove the dependency on Twisted's TestCase subclass before then.

Contributor

habnabit commented Mar 18, 2014

Twisted's twisted.trial.testcase is ported to python 3, so that's not an issue. I just looked into this again and ran into a bug in py.test, the current test runner: https://bitbucket.org/hpk42/pytest/issue/483/function-object-has-no-attribute-im_func

When this is fixed in py.test and Twisted 14.0 is out (since it fixes pip3 install Twisted), I think this will be ready to merge, unless someone wants to take the time to remove the dependency on Twisted's TestCase subclass before then.

@MostAwesomeDude

This comment has been minimized.

Show comment
Hide comment
@MostAwesomeDude

MostAwesomeDude Apr 15, 2014

Contributor

Howdy! @habnabit, you and I are in the same room again today; could we bash this out? We could remove Parsley's Twisted dependency and then be done with it rather easily, I think.

Contributor

MostAwesomeDude commented Apr 15, 2014

Howdy! @habnabit, you and I are in the same room again today; could we bash this out? We could remove Parsley's Twisted dependency and then be done with it rather easily, I think.

@habnabit habnabit merged commit efb5830 into pyga:master Apr 15, 2014

1 check failed

default The Travis CI build failed
Details
@furrykef

This comment has been minimized.

Show comment
Hide comment
@furrykef

furrykef Jul 17, 2014

When is the Python 3-capable version going to be officially released? I don't want to release anything that uses it until users can just "pip install parsley".

When is the Python 3-capable version going to be officially released? I don't want to release anything that uses it until users can just "pip install parsley".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment