python / cpython

The Python programming language
https://www.python.org
Other
63.38k stars 30.35k forks source link

argparse: add extend_const action #87326

Closed 52fcf438-5535-4986-8b54-d571dc818751 closed 2 weeks ago

52fcf438-5535-4986-8b54-d571dc818751 commented 3 years ago
BPO 43160
Nosy @rhettinger, @roganartu
PRs
  • python/cpython#24478
  • 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: ```python assignee = None closed_at = None created_at = labels = ['type-feature', 'library', '3.10'] title = 'argparse: add extend_const action' updated_at = user = 'https://github.com/roganartu' ``` bugs.python.org fields: ```python activity = actor = 'roganartu' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'roganartu' dependencies = [] files = [] hgrepos = [] issue_num = 43160 keywords = ['patch'] message_count = 4.0 messages = ['386614', '386690', '386696', '386697'] nosy_count = 3.0 nosy_names = ['rhettinger', 'paul.j3', 'roganartu'] pr_nums = ['24478'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue43160' versions = ['Python 3.10'] ```

    52fcf438-5535-4986-8b54-d571dc818751 commented 3 years ago

    I submitted this to the python-ideas mailing list early last year: https://mail.python.org/archives/list/python-ideas@python.org/thread/7ZHY7HFFQHIX3YWWCIJTNB4DRG2NQDOV/. Recently I had some time to implement it (it actually turned out to be pretty trivial), so thought I'd put forward a PR.

    Here's the summary from the mailing list submission:

    I have found myself a few times in a position where I have a repeated argument that uses the append action, along with some convenience arguments that append a specific const to that same dest (eg: --filter-x being made equivalent to --filter x via append_const). This is particularly useful in cli apps that expose some kind of powerful-but-verbose filtering capability, while also providing shorter aliases for common invocations. I'm sure there are other use cases, but this is the one I'm most familiar with.

    The natural extension to this filtering idea are convenience args that set two const values (eg: --filter x --filter y being equivalent to --filter-x-y), but there is no extend_const action to enable this.

    While this is possible (and rather straight forward) to add via a custom action, I feel like this should be a built-in action instead. append has append_const, it seems intuitive and reasonable to expect extend to have extend_const too (my anecdotal experience the first time I came across this need was that I simply tried using extend_const without checking the docs, assuming it already existed).

    Here's an excerpt from the docs I drafted for this addition that hopefully convey the intent and use case clearly.

    +* ``'extend_const'`` - This stores a list, and extends each argument value to the list.
    +  The ``'extend_const'`` action is typically useful when you want to provide an alias
    +  that is the combination of multiple other arguments. For example::
    +
    +    >>> parser = argparse.ArgumentParser()
    +    >>> parser.add_argument('--str', dest='types', action='append_const', const=str)
    +    >>> parser.add_argument('--int', dest='types', action='append_const', const=int)
    +    >>> parser.add_argument('--both', dest='types', action='extend_const', const=(str, int))
    +    >>> parser.parse_args('--str --int'.split())
    +    Namespace(types=[<class 'str'>, <class 'int'>])
    +    >>> parser.parse_args('--both'.split())
    +    Namespace(types=[<class 'str'>, <class 'int'>])
    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 3 years ago

    It's not clear what your patch does that the existing 'store_const' doesn't. Even 'append_const' does the same except for a extra level of list/tuple nesting.

    But I'll admit that I didn't much need for 'extend' addition, but others, including the original developer liked it.

    I suspect users of your addition will get a surprise if they aren't careful to provide a list or tuple 'const':

        const='foobar'
        const=str
    52fcf438-5535-4986-8b54-d571dc818751 commented 3 years ago

    Perhaps the example I added to the docs isn't clear enough and should be changed because you're right, that specific one can be served by store_const. Turns out coming up with examples that are minimal but not too contrived is hard! Let me try again with a longer example that hopefully shows more clearly how the existing action's behaviours differ from my patch.

        parser = argparse.ArgumentParser()
        parser.add_argument("--foo", action="append", default=[])
        parser.add_argument("--append", action="append_const", dest="foo", const=["a", "b"])
        parser.add_argument("--store", action="store_const", dest="foo", const=["a", "b"])

    When run on master the following behaviour is observed:

    --foo a --foo b --foo c
            Namespace(foo=['a', 'b', 'c'])
    --foo c --append
            Namespace(foo=['c', ['a', 'b']])
    --foo c --store
            Namespace(foo=['a', 'b'])
    --store --foo a
            Namespace(foo=['a', 'b', 'c'])

    If we then add the following:

        parser.add_argument("--extend", action="extend_const", dest="foo", const=["a", "b"])

    and then run it with my patch the following can be observed:

    --foo c --extend
            Namespace(foo=['c', 'a', 'b'])
    --extend --foo c
            Namespace(foo=['a', 'b', 'c'])

    store_const is actually a pretty close fit, but the way it makes order significant (specifically in that it will silently drop prev values) seems like it'd be rather surprising to users and makes it a big enough footgun for this use case that I don't think it's a satisfactory alternative.

    I suspect users of your addition will get a surprise if they aren't careful to provide a list or tuple 'const'

    I did consider that, but I don't think they'd get any more of a surprise than for doing the same with list.extend vs list.append.

    52fcf438-5535-4986-8b54-d571dc818751 commented 3 years ago

    Sorry, there's a typo in my last comment.

    --store --foo a
            Namespace(foo=['a', 'b', 'c'])

    from the first set of examples should have been

    --store --foo c
            Namespace(foo=['a', 'b', 'c'])
    serhiy-storchaka commented 1 month ago

    This looks like a too niche feature.

    You can create a custom Action. If do not make it general, it can be simpler.

    You can use append_const with a special value, and then, after parsing arguments, replace this value with the required sequence.