Pexpect-u #969

Merged
merged 6 commits into from Nov 27, 2011

Projects

None yet

3 participants

@takluyver
Member

This probably needs some careful thought, so it might not get in before 0.12.

pexpect doesn't work with Python 3. I've previously made a few small fixes so it works for piping process output to run external commands over ZMQ. It still failed for irunner, though. This brings in a more complete port of pexpect that I've been working on recently, called (for now) pexpect-u. It's compatible with Python 2.6 and up, including 2to3 conversion to Python 3.

I've contacted the author of pexpect about working these changes back in. But this could take a while - pexpect has previously supported very old versions of Python (the docs claim it's been tested on Python 1.5), so I'll have to see how much compromise is possible.

@minrk
Member
minrk commented Nov 9, 2011

Since this is a significant change, does that mean we should not import vanilla pexpect if it is present, and just use pexpect-u (on python-3 at least)?

@takluyver
Member

That's worth considering, although at this point approximately no-one is likely to have pexpect-u installed. I'm vaguely hoping to get some changes into pexpect itself, but I can't get in touch with the author. And I'm not sure what compromise he'd be willing to make on dropping support for older versions.

@minrk
Member
minrk commented Nov 9, 2011

But right now, we prioritize existing pexpect over bundled, so if users already have pexpect, your changes won't be used, even though they are necessary. So it would seem that the logic in external.pexpect.__init__ should change, at least for Python 3.

@takluyver
Member

It's only necessary on Python 3, and vanilla pexpect isn't compatible with Python 3, so it's sort of the user's problem if they've managed to install it there. On Python 2, the IPython code should work with either pexpect or pexpect-u.

@minrk
Member
minrk commented Nov 9, 2011

Okay, fair enough.

@fperez
Member
fperez commented Nov 26, 2011

@takluyver, just noticed this one is in need of a rebase...

@takluyver
Member

Rebased, and re-run tests on Python 2.7 and 3.2.

To explain these changes a bit further, I've separated the pexpect spawn class into spawn, which is a text interface, and spawnb, which is a bytes interface. For piped subprocesses, I've used spawnb, because our existing architecture expects bytes (and decodes it according to platform default encodings). For irunner, I've used the text interface (spawn), which will by default decode text with UTF-8 (which is a reasonable default on any modern unix-y system).

@fperez fperez merged commit 668e8a0 into ipython:master Nov 27, 2011
@fperez
Member
fperez commented Nov 27, 2011

Merged after final review on IRC with @takluyver. Great work, as usual :) Let's hope the pexpect author is responsive to these changes.

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