This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: subprocess.CompletedProcess: Add boolean value
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: ThatXliner, gregory.p.smith, rhettinger
Priority: normal Keywords:

Created on 2020-11-26 01:34 by ThatXliner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Messages (5)
msg381871 - (view) Author: ThatXliner (ThatXliner) Date: 2020-11-26 01:34
I propose making subprocess.CompletedProcess a truthy value if the returncode is 0. False, otherwise.
msg381872 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-26 02:28
Greg, do you have an opinion here?
msg381874 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-11-26 03:48
Here's a before and after example from my code.

==========================================================================

class GraphvizResult(NamedTuple):
    svg: str
    err: str

def create_svg(dot: str) -> GraphvizResult:
    'Convert a string in the "dot" format to an "svg" string using Graphviz'
    cp = run(['dot', '-Tsvg'], input=dot, capture_output=True, text=True)
    result = cp.stdout
    if not cp.returncode:
        i = result.index('<svg ')
        result = result[i:]
    return GraphvizResult(result, cp.stderr)

def create_svg(dot: str) -> GraphvizResult:
    'Convert a string in the "dot" format to an "svg" string using Graphviz'
    cp = run(['dot', '-Tsvg'], input=dot, capture_output=True, text=True)
    result = cp.stdout
    if cp:
        i = result.index('<svg ')
        result = result[i:]
    return GraphvizResult(result, cp.stderr)
msg381877 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-11-26 06:57
My concern with this as an API change is if anyone is returning or passing a CompletedProcess instance around it may well be used in a context where None is a potential value.  Existing code in that situation would ordinarily be doing a bare `if cp:` test on it to check for None.

I don't ordinarily see code that passes a CompletedProcess instance around... but it seems like an annoying potentially breaking change to make for a very small added value.

`if cp.returncode:` is very explicit about its intent when read or written.  

`if cp:` is not, and could even be misread by someone unaware of the API to assume that cp is a number most likely the returncode itself and misunderstand that as "the returncode was non-zero" when it really means the opposite.

If we had done this on day one of the run() -> CompletedProcess API this would've been a fine choice.  But changing it now doesn't seem like a good idea to me.
msg381878 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2020-11-26 07:00
Thanks for the suggestion though ThatXliner.  It is good for us to keep these things in mind for future API designs.  That __bool__ exists and could be meaningful in some contexts is often overlooked.
History
Date User Action Args
2022-04-11 14:59:38adminsetgithub: 86634
2020-11-26 07:00:37gregory.p.smithsetstatus: open -> closed
resolution: rejected
messages: + msg381878

stage: resolved
2020-11-26 06:57:20gregory.p.smithsetmessages: + msg381877
versions: - Python 3.9
2020-11-26 05:44:56gvanrossumsetnosy: - gvanrossum
2020-11-26 03:48:40rhettingersetnosy: + rhettinger
messages: + msg381874
2020-11-26 02:42:33gregory.p.smithsetassignee: gregory.p.smith
2020-11-26 02:28:28gvanrossumsetnosy: + gregory.p.smith
messages: + msg381872
2020-11-26 01:53:19rhettingersetnosy: + gvanrossum
2020-11-26 01:34:45ThatXlinersettype: enhancement
2020-11-26 01:34:38ThatXlinercreate