classification
Title: subprocess __all__ is incomplete
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: a.badger, eric.araujo, georg.brandl, gregory.p.smith, martin.panter, orsenthil, pitrou, python-dev, r.david.murray, serhiy.storchaka, terry.reedy
Priority: normal Keywords:

Created on 2011-01-05 20:52 by a.badger, last changed 2016-04-16 14:36 by martin.panter. This issue is now closed.

Messages (17)
msg125468 - (view) Author: Toshio Kuratomi (a.badger) * Date: 2011-01-05 20:52
I have a compatibility module for subprocess in python-2.7 for people who are stuck on python-2.4 (without check_call) and they got a traceback from trying to use compat.subprocess.list2cmdline().

In order to use the stdlib's subprocess if it's of a recent enough version, I check the version and import the symbols from there using from subprocess import * in the compat module.  Unfortunately, one of the people is using list2cmdline() in their code and list2cmdline() is not in __all__.  Comparing the output, there's a few things not in __all__ in both python-2.7 and in python-3.1:

(From python-2.7, but python-3.1 boils down to the same list):

>>> sorted([d for d in  dir (subprocess) if not d.startswith('_')])
['CalledProcessError', 'MAXFD', 'PIPE', 'Popen', 'STDOUT', 'call', 'check_call', 'check_output', 'errno', 'fcntl', 'gc', 'list2cmdline', 'mswindows', 'os', 'pickle', 'select', 'signal', 'sys', 'traceback', 'types']
>>> sorted(subprocess.__all__)
['CalledProcessError', 'PIPE', 'Popen', 'STDOUT', 'call', 'check_call', 'check_output']

So, MAXFD, list2cmdline, and mswindows seem to be left out.

These could either be made private (prepend with "_"), or added to __all__ to resolve this bug.  (I note that searching for "subprocess.any of those three" leads to some hits so whether or not they're intended to be public, they are being used :-(
msg125470 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-05 21:05
IMO none of these three are meant to be public, and neither are they documented.  (Although the docs make a reference to "the list2cmdline *method*", which should probably just be removed.)

I remember a thread on python-dev about public-API-ness.  Did we really conclude that all non-underscored names must be public and therefore added to __all__?
msg125476 - (view) Author: Toshio Kuratomi (a.badger) * Date: 2011-01-05 22:11
IIRC, it was more along the lines of: all private names should be underscored.  The difference being that we get to choose whether currently non-underscored names should get underscored, should be deprecated and then underscored, or should be made public, put into __all__, and properly documented.

I think there was general agreement that leaving them non-underscored but expecting people to treat them as private wasn't a good idea.
msg125482 - (view) Author: Toshio Kuratomi (a.badger) * Date: 2011-01-05 23:46
For other's reference, there were three threads in November2010 that touch on this:

  :About removing argparse.__all__ or adding more methods to it:
    http://mail.python.org/pipermail/python-dev/2010-November/105147.html

  :Removing tk interface in pydoc:
    http://mail.python.org/pipermail/python-dev/2010-November/105375.html

The most on topic thread is the one with Subject:
  :[Python-Dev] Breaking undocumented API:
    http://mail.python.org/pipermail/python-dev/2010-November/105392.html

People broke threading a few times so you might have to search on the subject.

And ick.  The thread's more of a mess than I remembered.  Reading what Guido wrote last it seems like:

All private names should be prepended with "_" .  Imported modules are the exception to this -- they're private unless included in __all__.  Reading between the lines I think it's also saying that not all public names need to be in __all__.

So to resolve this ticket:

1) Is this the actual consensus from the end of those threads?
2) Are the three names mentioned in this ticket public or private?
3a) If private, initiate deprecation and create underscore versions of the variables.
3b) If public, documentation and adding to __all__ are good but not necessary.
msg125485 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-06 00:15
> So, MAXFD, list2cmdline, and mswindows seem to be left out.

IMO they should all be prefixed with an underscore. Greg?
msg125734 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-01-08 00:13
My understanding is much like Toshio's: ambiguous (typically, undocumented or omitted from __all__) non-underscored names should be resolved, with the three possible outcomes listed, on a case-by-case basis.
msg127020 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-01-25 17:07
On Thu, Jan 06, 2011 at 12:15:26AM +0000, Antoine Pitrou wrote:
> IMO they should all be prefixed with an underscore. Greg?
> 

+1 to this suggestion. It would make it consistent with expectations.
But yeah, I also think that all public methods should be in __all__ is
not a guarantee.
msg133567 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2011-04-12 05:52
Issue #11827 seems to be strongly related
msg240177 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-06 20:08
For now there are three non-underscored names in subprocess that are missed in __all__: SubprocessError, TimeoutExpired, and list2cmdline. SubprocessError and TimeoutExpired are documented.
msg240206 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-04-07 13:10
IMO the two documented exceptions should definitely be added to __all__.

Not so sure, but list2cmdline() should probably be left out, with a comment explaining the decision. It is not mentioned in the main documentation that I can find, but it is mentioned in the doc string of the “subprocess” module. If it is only meant to be an internal detail, it shouldn’t be mentioned by name. If it is an external API (which I doubt), it should be documented better and added to __all__.
msg240213 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-04-07 13:50
I believe it is and should remain internal.  I agree that the mention should be deleted from the docstring.  We removed it from the docs a while back but I guess we forgot the docstring.

(If there were an external API for doing what list2cmdline does, it would belong in the windows equivalent of the shlex module, something that has been discussed (and I think there is an issue in the tracker) but no one has stepped forward to write it :)
msg240236 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-07 22:58
New changeset 10b0a8076be8 by Gregory P. Smith in branch 'default':
Addresses Issue #10838: The subprocess now module includes
https://hg.python.org/cpython/rev/10b0a8076be8
msg240238 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-07 23:04
the things left to to before closing this are to rename mswindows and MAXFD as those shouldn't be exported... and to wait for the windows buildbots to tell me if i missed adding anything to the intentionally_excluded list in the unittest.
msg240239 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-07 23:11
New changeset 4c14afc3f931 by Gregory P. Smith in branch 'default':
issue10838: Rename the subprocess.mswindows internal global to _mswindows.
https://hg.python.org/cpython/rev/4c14afc3f931
msg240240 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2015-04-07 23:12
Done.  MAXFD was already gone in 3.5 (yay).
msg263549 - (view) Author: Roundup Robot (python-dev) Date: 2016-04-16 12:12
New changeset cb38866e4c13 by Martin Panter in branch '3.5':
Issue #10838: Run test__all__() everywhere, even if poll() is not available
https://hg.python.org/cpython/rev/cb38866e4c13
msg263562 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-16 14:36
See Issue 26782 for a follow-up with Windows.
History
Date User Action Args
2016-04-16 14:36:43martin.pantersetmessages: + msg263562
2016-04-16 12:12:42python-devsetmessages: + msg263549
2015-04-08 04:13:13berker.peksagsetstage: commit review -> resolved
versions: + Python 3.5, - Python 3.2
2015-04-07 23:12:49gregory.p.smithsetstatus: open -> closed

assignee: gregory.p.smith
versions: + Python 3.2, - Python 3.5
messages: + msg240240
type: behavior
resolution: fixed
stage: needs patch -> commit review
2015-04-07 23:11:45python-devsetmessages: + msg240239
2015-04-07 23:04:42gregory.p.smithsetmessages: + msg240238
2015-04-07 22:58:09python-devsetnosy: + python-dev
messages: + msg240236
2015-04-07 16:23:31serhiy.storchakalinkissue23883 dependencies
2015-04-07 13:50:26r.david.murraysetnosy: + r.david.murray
messages: + msg240213
2015-04-07 13:10:28martin.pantersetnosy: + martin.panter
messages: + msg240206
2015-04-06 20:08:37serhiy.storchakasetversions: + Python 3.5, - Python 3.2
nosy: + serhiy.storchaka

messages: + msg240177

stage: needs patch
2011-11-12 05:06:17eli.benderskysetnosy: - eli.bendersky
2011-04-12 05:52:57eli.benderskysetnosy: + eli.bendersky
messages: + msg133567
2011-01-25 17:07:33orsenthilsetnosy: + orsenthil
messages: + msg127020
2011-01-08 00:13:27terry.reedysetnosy: + terry.reedy
messages: + msg125734
2011-01-07 20:03:34eric.araujosetnosy: + eric.araujo
2011-01-06 00:15:30pitrousetnosy: georg.brandl, gregory.p.smith, pitrou, a.badger
versions: + Python 3.2, - Python 2.7
2011-01-06 00:15:25pitrousetnosy: + gregory.p.smith, pitrou
messages: + msg125485
2011-01-05 23:46:55a.badgersetmessages: + msg125482
2011-01-05 22:11:57a.badgersetmessages: + msg125476
2011-01-05 21:05:27georg.brandlsetnosy: + georg.brandl
messages: + msg125470
2011-01-05 20:52:47a.badgercreate