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

(minor) Remove spawnu in spawn docstring in favour of spawn with encoding arg #395

Merged
merged 1 commit into from Dec 6, 2016

Conversation

Projects
None yet
2 participants
@AngusP
Contributor

AngusP commented Dec 4, 2016

Update docstring for class spawn.__init__ in pty_spawn.py to remove reference to deprecated spawnu in favour of spawn with encoding keyword argument

spawnu deprecated according to line 805 of pty_spawn.py and issue #159

Update docstring for class spawn.__init__ in pty_spawn.py to remove r…
…eference to deprecated spawnu in favour of spawn with ``encoding`` keyword argument

``spawnu`` deprecated according to line 805 of `pty_spawn.py` and issue #159
@takluyver

This comment has been minimized.

Member

takluyver commented Dec 4, 2016

Thanks, this is a good catch. I'm going to quibble over the wording a bit, however:

  • We can't ensure that the data the process sends is in any particular encoding - that's up to the subprocess. What the encoding parameter affects is how we encode data that we send to the pty (i.e. stdin of the subprocess), and how we try to decode data coming back. As a special case, when encoding=None, which is the default, the interface uses bytes rather than text.
  • I tend to avoid talking about stdout or stderr when running subprocesses in a pty, because we have just the one pty that combines both of them (and in fact stdin as well, but that's a bit different).
@AngusP

This comment has been minimized.

Contributor

AngusP commented Dec 6, 2016

@takluyver feel free to adjust the wording, I've only recently found this library so haven't familiarised with its internals.

@takluyver

This comment has been minimized.

Member

takluyver commented Dec 6, 2016

OK, thanks for spotting it, in any case.

@takluyver takluyver merged commit 1a2b41f into pexpect:master Dec 6, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment