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

bpo-34605, pty: Avoid master/slave terms #9100

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@vstinner
Member

vstinner commented Sep 7, 2018

  • pty.spawn(): rename master_read parameter to parent_read

  • Rename pty.slave_open() to pty.child_open(), but keep an
    pty.slave_open alis to pty.child_open for backward compatibility

  • os.openpty(), os.forkpty(): rename master_fd/slave_fd
    to parent_fd/child_fd

  • Rename internal variables:

    • Rename master_fd/slave_fd to parent_fd/child_fd
    • Rename slave_name to child_name

https://bugs.python.org/issue34605

bpo-34605, pty: Avoid master/slave terms
* pty.spawn(): rename master_read parameter to parent_read
* Rename pty.slave_open() to pty.child_open(), but keep an
  pty.slave_open alis to pty.child_open for backward compatibility
* os.openpty(), os.forkpty(): rename master_fd/slave_fd
  to parent_fd/child_fd
* Rename internal variables:

  * Rename master_fd/slave_fd to parent_fd/child_fd
  * Rename slave_name to child_name
@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Sep 7, 2018

Member

pty.spawn(): rename master_read parameter to parent_read

This change is backward incompatible since technically it was possible to call pty.spawn(..., master_read=..., ...). Do we need to accept master_read as a keyword parameter but emit a deprecation warning?

Rename pty.slave_open() to pty.child_open(), but keep an pty.slave_open alis to pty.child_open for backward compatibility

I dislike keep the alias. The function isn't documented. Can't we just make the function private instead?

Member

vstinner commented Sep 7, 2018

pty.spawn(): rename master_read parameter to parent_read

This change is backward incompatible since technically it was possible to call pty.spawn(..., master_read=..., ...). Do we need to accept master_read as a keyword parameter but emit a deprecation warning?

Rename pty.slave_open() to pty.child_open(), but keep an pty.slave_open alis to pty.child_open for backward compatibility

I dislike keep the alias. The function isn't documented. Can't we just make the function private instead?

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Sep 7, 2018

Member

I didn't add a NEWS entry since I'm not sure if this PR is backward incompatible or not.

Member

vstinner commented Sep 7, 2018

I didn't add a NEWS entry since I'm not sure if this PR is backward incompatible or not.

@vstinner vstinner requested review from 1st1 and asvetlov as code owners Sep 7, 2018

@ammaraskar

This comment has been minimized.

Show comment
Hide comment
@ammaraskar

ammaraskar Sep 7, 2018

Contributor

(Adding my comment from bpo here since it pertains specifically to this PR)

If you look at all the current Linux man pages and documentation, they follow the master/slave terminology. Generally, Python documentation for underlying os functions like fork, stat etc are kept short because the OS documentation is the ultimate resource for them.

This change causes a deviation from the existing standard and will only serve to make things more confusing. Anyone who goes to google about a "pty leader" will find nothing. I would suggest deferring it until the actual OS documentation reflects this change as well.

Contributor

ammaraskar commented Sep 7, 2018

(Adding my comment from bpo here since it pertains specifically to this PR)

If you look at all the current Linux man pages and documentation, they follow the master/slave terminology. Generally, Python documentation for underlying os functions like fork, stat etc are kept short because the OS documentation is the ultimate resource for them.

This change causes a deviation from the existing standard and will only serve to make things more confusing. Anyone who goes to google about a "pty leader" will find nothing. I would suggest deferring it until the actual OS documentation reflects this change as well.

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Sep 7, 2018

Member

IMHO it's ok for Python to use a different terminology since "master" and "slave" are not really part of the PTY functions.

Member

vstinner commented Sep 7, 2018

IMHO it's ok for Python to use a different terminology since "master" and "slave" are not really part of the PTY functions.

@ammaraskar

This comment has been minimized.

Show comment
Hide comment

@1st1 1st1 removed their request for review Sep 7, 2018

@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Sep 7, 2018

Member

In C, the parameter names don't matter. You can use any name when you call the function.

In Python, "master" and "slave" are not part of the API neither. openpty() returns a tuple, that's it. The caller has to pick two names for the pair of file descriptors.

Member

vstinner commented Sep 7, 2018

In C, the parameter names don't matter. You can use any name when you call the function.

In Python, "master" and "slave" are not part of the API neither. openpty() returns a tuple, that's it. The caller has to pick two names for the pair of file descriptors.

@ammaraskar

This comment has been minimized.

Show comment
Hide comment
@ammaraskar

ammaraskar Sep 7, 2018

Contributor

My point is, googling for pty child descriptor is going to be a far worse search than pty slave descriptor. And sure, you can pass in whatever you want for the function argument names but the default argument names are used as implicit documentation. For example, fd is conventional for file descriptors and you can search for linux fd and get lots of relevant info.

Contributor

ammaraskar commented Sep 7, 2018

My point is, googling for pty child descriptor is going to be a far worse search than pty slave descriptor. And sure, you can pass in whatever you want for the function argument names but the default argument names are used as implicit documentation. For example, fd is conventional for file descriptors and you can search for linux fd and get lots of relevant info.

@suic86

This comment was marked as disruptive content.

Show comment
Hide comment
@suic86

suic86 Sep 8, 2018

Contributor

This is so sad. SJW terminology/ideology is infecting even Python. It's time look at a different language.

Contributor

suic86 commented Sep 8, 2018

This is so sad. SJW terminology/ideology is infecting even Python. It's time look at a different language.

@ammaraskar

This comment has been minimized.

Show comment
Hide comment
@ammaraskar

ammaraskar Sep 8, 2018

Contributor

Please keep motivational discussion on the bug tracker. Github is used for the technical code review.

Contributor

ammaraskar commented Sep 8, 2018

Please keep motivational discussion on the bug tracker. Github is used for the technical code review.

@rhettinger

This comment has been minimized.

Show comment
Hide comment
@rhettinger

rhettinger Sep 9, 2018

Collaborator

This change is unnecessary and ill-advised. IMO it makes the docs less usable. When "master/slave" is the actual terminology used in the problem domain, we need to keep the current wording. https://www.systutorials.com/docs/linux/man/4-pts/

Collaborator

rhettinger commented Sep 9, 2018

This change is unnecessary and ill-advised. IMO it makes the docs less usable. When "master/slave" is the actual terminology used in the problem domain, we need to keep the current wording. https://www.systutorials.com/docs/linux/man/4-pts/

@gvanrossum

This comment has been minimized.

Show comment
Hide comment
@gvanrossum

gvanrossum Sep 11, 2018

Member

Since this reflecting the terminology of the underlying UNIX pty mechanism it should not be changed.

Member

gvanrossum commented Sep 11, 2018

Since this reflecting the terminology of the underlying UNIX pty mechanism it should not be changed.

@gvanrossum gvanrossum closed this Sep 11, 2018

@vstinner vstinner deleted the vstinner:pty_master_slave branch Sep 13, 2018

@python python locked and limited conversation to collaborators Sep 13, 2018

@python python deleted a comment from schlamar Sep 13, 2018

@python python deleted a comment from decrepitude Sep 13, 2018

@python python deleted a comment from decrepitude Sep 13, 2018

@python python deleted a comment from owned139 Sep 13, 2018

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