classification
Title: Undocumented subprocess functions on Windows
Type: behavior Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.2, Python 3.1, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: akuchling, benjamin.peterson, brian.curtin, georg.brandl, python-dev
Priority: normal Keywords: easy, needs review, patch

Created on 2010-02-02 17:00 by brian.curtin, last changed 2011-04-29 21:29 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue7838_v2.diff brian.curtin, 2010-02-03 15:34
issue7838_v3.diff brian.curtin, 2010-02-22 14:56
Messages (11)
msg98749 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-02 17:00
On Windows, the subprocess module makes use of functions publicly exposed by PC/_subprocess.c to interact with Win32 API functions. However, no documentation exists for these functions, neither in the online docs nor in docstrings.
msg98756 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-02 21:16
Attached is a patch which adds docstrings to the functions in PC/_subprocess.c, documents the functions in Doc/library/subprocess.rst, and removes a chunk of unneeded import code from Lib/subprocess.py
msg98757 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-02-02 21:22
This is likely because these functions are meant to be undocumented implementation details.
msg98758 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-02 21:35
True. Is there value in eventually privatizing these functions?

It feels weird having them exposed but not documented at all...maybe just keep the docstrings around?
msg98771 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2010-02-03 02:48
Actually, I consider them private now, given that they aren't in __all__ or documented. I'd prefer if the code was changed to qualify their use with _subprocess.X actually. Docstrings are fine, I suppose.
msg98780 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-02-03 08:49
I agree.  Docstrings never hurt.
msg98787 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-03 15:34
Here's a patch which keeps docstrings, and explicitly qualifies the Windows specific functions from _subprocess rather than using import * (which causes a couple of lines just over 79 chars). Now the functions are more hidden than before.

Ran the tests to be sure I got each change, nothing seems to have slipped by.
msg99712 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2010-02-22 04:22
I know nothing about Windows, but the patch looks sensible.

Typos in docstrings: 'associatedwith' missing space;
'a duplicate a handle object' (remove first 'a')?; 
'return value is tuple' ('is a tuple').
msg99740 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-02-22 14:56
Here's a patch which makes those changes. Thanks for taking a look and catching that stuff.
msg104097 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-04-24 16:34
Fixed with r80439-r80442.
msg134813 - (view) Author: Roundup Robot (python-dev) Date: 2011-04-29 21:29
New changeset 80971f71b0d9 by Brian Curtin in branch '3.1':
Further fix #7838. CREATE_NEW_CONSOLE was exposed, but none of the
http://hg.python.org/cpython/rev/80971f71b0d9
History
Date User Action Args
2011-04-29 21:29:17python-devsetnosy: + python-dev
messages: + msg134813
2010-04-24 16:34:36brian.curtinsetstatus: open -> closed

components: + Extension Modules, - Documentation
assignee: georg.brandl -> brian.curtin
nosy: akuchling, georg.brandl, benjamin.peterson, brian.curtin
messages: + msg104097
resolution: fixed
stage: patch review -> resolved
2010-02-22 14:56:35brian.curtinsetfiles: + issue7838_v3.diff

messages: + msg99740
2010-02-22 14:55:35brian.curtinsetfiles: - issue7838.diff
2010-02-22 04:22:06akuchlingsetnosy: + akuchling
messages: + msg99712
2010-02-03 15:34:05brian.curtinsetfiles: + issue7838_v2.diff

messages: + msg98787
2010-02-03 08:49:29georg.brandlsetmessages: + msg98780
2010-02-03 02:48:47benjamin.petersonsetstatus: pending -> open

messages: + msg98771
2010-02-02 21:35:50brian.curtinsetstatus: open -> pending

messages: + msg98758
2010-02-02 21:22:39benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg98757
2010-02-02 21:17:17brian.curtinsetfiles: + issue7838.diff
2010-02-02 21:17:05brian.curtinsetfiles: - issue7838.diff
2010-02-02 21:16:06brian.curtinsetkeywords: + patch, needs review
files: + issue7838.diff
messages: + msg98756

stage: needs patch -> patch review
2010-02-02 17:00:23brian.curtincreate