8000 [doc] subprocess module: Clarify kwarg handing for convenience APIs · Issue #74605 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

[doc] subprocess module: Clarify kwarg handing for convenience APIs #74605

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.

Sign up for GitHub

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

Jump to bottom
Open
ncoghlan opened this issue May 21, 2017 · 7 comments
Open

[doc] subprocess module: Clarify kwarg handing for convenience APIs #74605

ncoghlan opened this issue May 21, 2017 · 7 comments
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir topic-subprocess Subprocess issues. type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor
BPO 30420
Nosy @ncoghlan, @vadmium, @Mariatta, @AlexWaygood
PRs
  • bpo-30420: Document the cwd argument to check_output and check_call #1685
  • [3.6] bpo-30420: List cwd parameter in subprocess convenience APIs (G… #2253
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = None
created_at = <Date 2017-05-21.03:29:11.095>
labels = ['type-feature', '3.7', 'docs']
title = '[doc] subprocess module: Clarify kwarg handing for convenience APIs'
updated_at = <Date 2021-12-29.11:59:30.400>
user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2021-12-29.11:59:30.400>
actor = 'AlexWaygood'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2017-05-21.03:29:11.095>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 30420
keywords = []
message_count = 7.0
messages = ['294070', '294073', '294075', '294523', '294526', '296396', '409253']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'docs@python', 'martin.panter', 'Mariatta', 'AlexWaygood']
pr_nums = ['1685', '2253']
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30420'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']
    @ncoghlan
    Copy link
    Contributor Author

    As per the discussion in #1685, the current nominal signatures of the convenience APIs in subprocess is confusing, as they don't make it clear that all Popen keyword arguments are supported, with only the most common ones being listed in the signature.

    Proposed fixes:

    • add cwd to the list of frequently used arguments, and list it in the nominal signatures for run(), call(), check_call(), and check_output()
    • append **popenargs to the nominal signatures of run(), call(), and check_call()
    • append **runargs to the nominal signature of check_output()
    • amend the prose descriptions for these functions accordingly

    Background:

    The status quo is that while this behaviour is documented, it's documented solely as text in the bodies of the API definitions, using paragraphs like the following:

    run(): """The arguments shown above are merely the most common ones, described below in Frequently Used Arguments (hence the use of keyword-only notation in the abbreviated signature). The full function signature is largely the same as that of the Popen constructor - apart from timeout, input and check, all the arguments to this function are passed through to that interface."""

    call(), check_call(): """The arguments shown above are merely the most common ones. The full function signature is largely the same as that of the Popen constructor - this function passes all supplied arguments other than timeout directly through to that interface."""

    check_output(): """The arguments shown above are merely the most common ones. The full function signature is largely the same as that of run() - most arguments are passed directly through to that interface."""

    While it's technically anecdotal, we still have pretty decent evidence that this isn't adequate as:

    • I worked on the original patch splitting out the "Frequently Used Arguments" section, and initially missed not only that the run() docs had retained that paragraph, but also missed the revised paragraphs in the relocated docs for the older API
    • as mentioned in the linked PR, Paul Kehrer 8000 had missed that check_call() and check_output() both supported the Popen cwd parameter

    The working theory behind adding the appropriately named **kwargs parameters to the end of the nominal signatures is that it will give readers a clearer indication that we're not explicitly listing all the parameters, and a hint as to which API to go look at to find out more about the unlisted ones.

    @ncoghlan ncoghlan added the 3.7 (EOL) end of life label May 21, 2017
    @ncoghlan ncoghlan added docs Documentation in the Doc dir type-feature A feature request or enhancement labels May 21, 2017
    @vadmium
    Copy link
    Member
    vadmium commented May 21, 2017

    For the curious, Nick added the “frequently used arguments” in bpo-13237. Before that the signatures were like <https://docs.python.org/release/3.2.2/library/subprocess.html#convenience-functions\>:

    subprocess.call(*popenargs, **kwargs)

    @vadmium
    Copy link
    Member
    vadmium commented May 21, 2017

    If you add “cwd” to Frequently Use Arguments, please try to keep the details in one place. Otherwise you encourage a fix for bpo-15533 (cwd platform specifics) to repeat the problem of bpo-20344 (args vs shell platform specifics), where some details are only found in one section and contradict the other section.

    @ncoghlan
    Copy link
    Contributor Author

    New changeset 368cf1d by Nick Coghlan (Alex Gaynor) in branch 'master':
    bpo-30420: List cwd parameter in subprocess convenience APIs (GH-1685)
    368cf1d

    @ncoghlan
    Copy link
    Contributor Author

    The just merged PR covers the "list cwd in the nominal signatures for run(), call(), check_call(), and check_output()" part of the proposal.

    The rest of the proposed fixes are still pending a patch (and keeping in mind Martin's caveat on minimising duplication between the "Frequently Used Arguments" section and the full Popen documentation)

    @Mariatta
    Copy link
    Member

    New changeset 0a4fe1d by Mariatta in branch '3.6':
    [3.6] bpo-30420: List cwd parameter in subprocess convenience APIs (GH-1685) (GH-2253)
    0a4fe1d

    @AlexWaygood
    Copy link
    Member

    The modern docs for these functions seem to:

    • Document the cwd argument for these functions, following PR 1685 & PR 2253.
    • Include an **other_popen_kwargs parameter for all these functions.

    Nick, is there anything left to do here, or can this issue be closed now?

    @AlexWaygood AlexWaygood changed the title Clarify kwarg handing for subprocess convenience APIs [doc] subprocess module: Clarify kwarg handing for convenience APIs Dec 29, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @vstinner vstinner added the topic-subprocess Subprocess issues. label Jul 8, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life docs Documentation in the Doc dir topic-subprocess Subprocess issues. type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @vstinner @vadmium @ncoghlan @Mariatta @AlexWaygood
    0