python / cpython

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

Add "necessarily inclusive" groups to argparse #55797

Open d58029f1-f25f-4b41-baa0-910306bf7a77 opened 13 years ago

d58029f1-f25f-4b41-baa0-910306bf7a77 commented 13 years ago
BPO 11588
Nosy @cjmayo, @ikelos, @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll, @iforapsy
Files
  • patch_w_mxg2.diff
  • example1.py
  • issue11588_4.py
  • usagegroup.py
  • usage_2.patch
  • 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'] title = 'Add "necessarily inclusive" groups to argparse' updated_at = user = 'https://bugs.python.org/JohnDidion' ``` bugs.python.org fields: ```python activity = actor = 'iforapsy' assignee = 'none' closed = False closed_date = None closer = None components = [] creation = creator = 'John.Didion' dependencies = [] files = ['34176', '34193', '34285', '35608', '36553'] hgrepos = [] issue_num = 11588 keywords = ['patch'] message_count = 17.0 messages = ['131262', '132241', '134560', '211240', '211261', '211878', '211947', '212228', '212243', '212733', '215777', '220419', '226324', '226469', '265734', '286582', '294569'] nosy_count = 9.0 nosy_names = ['bethard', 'cjmayo', 'ikelos', 'xuanji', 'John.Didion', 'manveru', 'paul.j3', 'pgacv2', 'iforapsy'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue11588' versions = ['Python 3.3'] ```

    d58029f1-f25f-4b41-baa0-910306bf7a77 commented 13 years ago

    Just as some options are mutually exclusive, there are others that are "necessarily inclusive," i.e. all or nothing. I propose the addition of ArgumentParser.add_necessarily_inclusive_group(required=True).

    This also means that argparse will need to support nested groups. For example, if I want to set up options such that the user has to provide an output file OR (an output directory AND (an output file pattern OR an output file extension)):

    output_group = parser.add_mutually_exclusive_group(required=True)
    output_group.add_argument("-o", "--outfile")
    outdir_group = output_group.add_necessarily_inclusive_group()
    outdir_group.add_argument("-O", "--outdir")
    outfile_group = outdir_group.add_mutually_exclusive_group(required=True)
    outfile_group.add_argument("-p", "--outpattern")
    outfile_group.add_argument("-s", "--outsuffix")

    The usage should then look like:

    (-o FILE | (-O DIR & (-p PATTERN | -s SUFFIX))

    8955c213-fd54-471c-9758-9cc5f49074db commented 13 years ago

    I think this is a great suggestion. Care to work on a patch?

    546d008d-2f0c-4602-ba29-b4b209d655c0 commented 13 years ago

    I am subscribing to this idea as I've just fall into such use case where I need it. I would like to submit a patch, but I still have difficulties to understand argparse code not much spare time to spent on this.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    The suggestion in this issue is to add a 'mutually_inclusive_group' mechanism, one that would let users specify that certain sets of arguments must occur together. Furthermore there was mention of allowing some sort of nesting.

    Modeling it on the mutually_exclusive_group would be straight forward. But should it affect the usage and help display?mutually_exclusive_groups add a messy layer to the usage formatting.

    The only place such a group would act would be at the end of '_parse_known_args', where the current code checks for things like required actions (and mxgroups). A test at this point could use 'namespace', 'seen_actions' and 'seen_non_default_actions' to check whether the required group actions were seen.

    But the only thing that the argument_group contributes to this test is a list of argument names ('dest'?). Why not provide this list directly? And what if the user wants A to occur together with either B or C, but not both? Or make the inclusivity conditional on the value of A?

    Currently users can define argument interactions in a couple of ways. They can define custom Actions. In test_argparse.py there's a custom Actions test that does something like this (using '--spam' and 'badger'). But tests in Actions depend on the order in which arguments are given.

    An alternative is to test for interactions of arguments after parse_args. However the only information that the user has at this point is the args namespace. Reliably distinguishing between non-occurrence of arguments and default values can be difficult.

    I am proposing 'cross_test' mechanism that would give the user access to the 'seen_actions' and 'seen_non_default_actions' sets that 'mutually_exclusive_groups' use. Specifically an optional function can be called at the end of '_parse_known_args' that has access to these sets as well as the parser and the namespace.

    The core of the change would be adding

        cross_test = getattr(self, 'cross_test', None)
        if cross_test:
            cross_test(self, namespace, extras, seen_actions, seen_non_default_actions)

    at the end of 'parser._parse_known_args'. In addition 'crosstest' (or some other name) could be added to the 'ArgumentParser.\_init__' arguments.

    The feature could be used by defining such a 'cross_test' function and adding it to the parser (either instance or subclass)

        def foobar(self, namespace, extras, seen_actions, seen_non_default_actions):
            ...
            (raise self.error(...))
    
        parser.cross_test = foobar

    The patch proposed http://bugs.python.org/issue18943 should be included with any changes here since it refines the setting of 'seen_non_default_actions'.

    I am working on tests and examples of such functionality.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    Regarding a usage line like:

    (-o FILE | (-O DIR & (-p PATTERN | -s SUFFIX))

    The simplest option is to just a custom written 'usage' parameter.

    With the existing HelpFormatter, a nested grouping like this is next to impossible. It formats the arguments (e.g.'-O DIR'), interleaves the group symbols, and then trims out the excess spaces and symbols.

    http://bugs.python.org/issue10984 is a request to allow overlapping mutually_exclusive_groups. It loops on the groups, formatting each. It would be easier with that to format several different types of groups, and to handle nested ones.

    What would it take to convert a usage string like that into a logical expression that tests for the proper occurrence (or non-occurrence) of the various arguments. It might, for example be converted to

        exc(file, inc(dir, exc(pattern, suffix)))

    where 'exc' and 'inc' are exclusive and inclusive tests, and 'file','dir' etc are booleans. And what would be the error message(s) if this expression fails?

    I can imagine a factory function that would take usage line (or other expression of groupings), and produce a function that would implement a cross_test (as outlined in my previous post). It would be, in effect, a micro-language compiler.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    This patch uses:

        tests = self._registries['cross_tests'].values()

    to get a list of functions to run at the end of '_parse_known_args'.

    I replaced all of the mutually_exclusive_group tests (3 places) in the ArgumentParser with a static function defined in class _MutuallyExclusiveGroup, and registered this function. This refactoring should make it easier to add other specialized groups (e.g. inclusive) in the future.

    I'm using the _registries because they are already being shared among groups.

    A user can also register a custom testing function. For example:

        def inclusive_test(parser, seen, *args):
            # require an argument from one of the 2 groups, g0 and g1
            g0seen = seen.intersection(g0._group_actions)
            g1seen = seen.intersection(g1._group_actions)
            if len(g0seen.union(g1seen))==0:
                parser.error('one of the 2 groups is required')
        parser.register('cross_tests','inclusive', inclusive_test)

    This patched 'argparse.py' runs 'test_argparse.py' without error.

    This patch does not include the bpo-18943 changes, which make setting 'seen_non_default_actions' more reliable.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    This is an example of using 'patch_w_mxg2.diff' to handle the inclusive group case proposed by the OP.

    Since 'seen_non_default_actions' (and 'seen_actions') is a set of 'Actions', it is convenient to use 'set' methods with pointers to the actions that a collected during setup. Tests could also be done with the 'dest' or other action attributes.

    In this example I wrote 3 simple tests corresponding to the 3 proposed groups, but they could also have been written as one test.

        a_file= parser.add_argument("-o", "--outfile", metavar='FILE')
        a_dir = parser.add_argument("-O", "--outdir", metavar='DIR')
        a_pat = parser.add_argument("-p", "--outpattern", metavar='PATTERN')
        a_suf = parser.add_argument("-s", "--outsuffix", metavar='SUFFIX')
        ...
        def dir_inclusive(parser, seen_actions, *args):
            if a_dir in seen_actions:
                if 0==len(seen_actions.intersection([a_pat, a_suf])):
                    parser.error('DIR requires PATTERN or SUFFIX')
        parser.register('cross_tests', 'dir_inclusive', dir_inclusive)
        ...

    In theory tests like this could be generated from groups as proposed by the OP. There is one case in 'test_argparse.py' where a mutually_exclusive_group is nested in an argument_group. But the current groups do not implement nesting. A (plain) argument_group does not share its '_group_actions' list with its 'container'. A mutually_exclusive_group shares its '_group_actions' but the result is a flat list (no nesting).

    For now I think it is more useful to give users tools to write custom 'cross_tests' than to generalize the 'group' classes.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    http://stackoverflow.com/questions/11455218 python, argparse: enable input parameter when another one has been specified

        $ python myScript.py --parameter1 value1
        $ python myScript.py --parameter1 value1 --parameter2 value2
        $ python myScript.py --parameter2 value2  # error

    This is an example where a 'mutually inclusive group' wouldn't quite do the job.

    The proposed answers mostly use a custom Action. I'd lean toward an after-the-parse test of the namespace.

    With the patch I proposed this could be implemented with:

        a1 = parser.add_argument("--parameter1")
        a2 = parser.add_argument("--parameter2")
        def test(parser, seen_actions, *args):
            if a2 in seen_actions and a1 not in seen_actions:
                parser.error('parameter2 requires parameter1')
        parser.register('cross_tests', 'test', test)

    One poster on that thread claimed that the use of 'a1 = parser.add_argument...' is using an undocumented feature. The fact that add_argument returns an action object, should be illustrated in the documentation, and may be explicitly noted. I'll have to review the documentation to see if this is the case.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    The addition of a simple decorator to the 'ArgumentParser' class, would simplify registering the tests:

        def crosstest(self, func):
            # decorator to facilitate adding these functions
            name = func.__name__
            self.register('cross_tests', name, func)

    which would be used as:

        @parser.crosstest
        def pat_or_suf(parser, seen_actions, *args):
            if 2==len(seen_actions.intersection([a_pat, a_suf])):
                parser.error('only one of PATTERN and SUFFIX allowed')
    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    A couple more thoughts on an expanded argument testing mechanism:

    'seen_actions' is used only to test whether all required actions have been seen. These 2 sets differ in how positionals with '?' are categorized. Positionals like this are always 'seen', even if they just get the default value. But they are not required (the case of a '\' positional without default needs to be revisited.)

    For example the decorator could wrap the 'seen_non_default_actions' argument in a 'seen' function. Such a function could accept either an Action or a 'dest' string, it could accept a single Action, or a list of them, etc. There could be other functions like 'count', 'unique', 'mutually_exclusive', 'inclusive', etc.

        def testwfnc(func):
            # decorator to register function and provide 'seen'
            name = func.__name__
            def wrapped(parser, seen_actions, *args):
                def seen(*args):
                    actions = seen_actions
                    if isinstance(args[0], str):
                        actions = [a.dest for a in actions]
                    if len(args)>1:
                        return [a in actions for a in args]
                    else:
                        return args[0] in actions
                return func(parser, seen)
            parser.register('cross_tests', name, wrapped)
            return wrapped
    
        #@testwfnc
        def test(parser, seen, *args):
            if seen(a_file):
                print(seen(a_dir, a_pat, a_suf))
                cnt = sum(seen(a_dir, a_pat, a_suf))
            if cnt>0:
                parser.error('FILE cannot have DIR, PATTERN or SUFFIX')
            ...

    The attached script experiments with several versions of decorators. Some sort of testing Class is probably the way to go if we want to provide many convenience methods.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    http://stackoverflow.com/questions/22929087 A question that could be addressed with this patch(es)

    In a subparser:

    I have 4 arguments: -g, -wid, -w1, and -w2.
    
    -w1 and -w2 always appear together
    
    -wid and (-w1 -w2) are mutually exclusive, but one or the other is required
    
    -g is optional; if it is not specified only (-w1 -w2) can appear, but not -wid

    The g case does not fit either the inclusive or exclusive group patterns.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    I have developed a UsageGroup class that can implement nested 'inclusive' tests. Using this, the original example in this issue could be coded as 3 groups, 2 mutually_exclusive and inclusive one.

        parser = ArgumentParser(prog='PROG', formatter_class=UsageGroupHelpFormatter)
    
        g1 = parser.add_usage_group(dest='FILE or DIR', kind='mxg', required=True)
        a_file= g1.add_argument("-o", "--outfile", metavar='FILE')
    
        g2 = g1.add_usage_group(dest='DIR and PS', kind='inc')
        a_dir = g2.add_argument("-O", "--outdir", metavar='DIR')
    
        g3 = g2.add_usage_group(dest='P or S', kind='mxg')
        a_pat = g3.add_argument("-p", "--outpattern", metavar='PATTERN')
        a_suf = g3.add_argument("-s", "--outsuffix", metavar='SUFFIX')
        # usage: PROG [-h] (-o FILE | (-O DIR & (-p PATTERN | -s SUFFIX)))

    UsageGroup is like MutuallyExclusiveGroup, except that:

    The attached 'usagegroup.py' file has the core code for this change. The full working code is at

    https://github.com/hpaulj/argparse_issues/tree/nested

    It incorporates too many other changes (from other bug issues) to post here as a simple patch file (yet). I have a number of test scripts, but haven't cast those as unittests. Same goes for documentation.

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    http://stackoverflow.com/questions/25626109/python-argparse-conditionally-required-arguments

    asks about implementing a 'conditionally-required-arguments' case in argparse. The post-parsing test is simple enough:

        if args.argument and (args.a is None or args.b is None):
            # raise argparse error here

    I believe the clearest and shortest expression using Groups is:

        p = ArgumentParser(formatter_class=UsageGroupHelpFormatter)
        g1 = p.add_usage_group(kind='nand', dest='nand1')
        g1.add_argument('--arg', metavar='C')
        g11 = g1.add_usage_group(kind='nand', dest='nand2')
        g11.add_argument('-a')
        g11.add_argument('-b')

    The usage is (using !() to mark a 'nand' test):

    usage: issue25626109.py [-h] !(--arg C & !(-a A & -b B))

    This uses a 'nand' group, with a 'not-all' test (False if all its actions are present, True otherwise).

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 10 years ago

    Attached is a patch for 3.5.0 dev that adds UsageGroups. Now different 'kinds' are implemented as subclasses, which are accessed via the registry:

     _XorUsageGroup - replicate action of MutuallyExclusiveGroups
     _AndUsageGroup - an inclusive group
     _OrUsageGroup -  an 'any' group
     _NorUsageGroup - NotOr - also works as Not
     _NandUsageGroup - NotAnd

    Open issues:

    - what difference should the 'required' parameter make?
    - how should 'errors' in nested groups propagate?
    - formatting of error messages
    - formatting the usage line with these groups
    - any additional 'help' display
    - framework for custom test and/or subclasses
    - documentation, sample scripts, and formal testing
    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 8 years ago

    So far I've proposed adding a 'hook' at the end of '_parse_known_args', that would give the user access to the 'seen_non_default_actions' variable. This function could perform an almost arbitrarily complex set of logical co-occurrence tests on this set (or list) of Actions.

    The rest of my proposed patches (nested groups, etc) are user interface components that attempt make this testing more user-friendly, both in specification and usage display.

    It just occurred to me that an alternate stop-gap fix is to make 'seen_non_default_actions' available to the user for his own testing after parsing. Adding it to the method return is not backward compatible. But it could be added as an attribute to parser.

         self._seen_actions = seen_non_default_actions

    It would be the first case of giving the parser a memory of past parsing actions, but I don't think that's a problem.

    Another possibility is to conditionally add it to the 'namespace'.

         if hasattr(namespace, 'seen_actions'):
            setattr(namespace, 'seen_actions', seen_non_default_actions)

    The user could initial this attribute with a custom 'Namespace' object or with a 'set_defaults' call.

    (I'm proposing to save 'seen_non_default_actions' because in my earlier tests that seemed to be more useful than 'seen_actions'. It's the one used by mutually_exclusive_group testing.)

    7a064fe6-c535-4d80-a11f-a04ed39056c5 commented 7 years ago

    Another issue requesting a 'mutually dependent group' http://bugs.python.org/issue23298

    a6982a76-b293-41eb-bd2e-00129268a441 commented 7 years ago

    Just voicing my support for this. I was also looking for a solution on StackOverflow and ended up here.

    rkachach commented 2 years ago

    +1 It would be nice to have such feature.

    Eutropios commented 1 year ago

    Expressing support for this option. Would be a major improvement.

    legion49f commented 1 year ago

    I would like this as an option

    amazhangwinz commented 1 year ago

    Would love to have this feature!

    mixtah commented 1 year ago

    +1 would have used now if available

    alexjch commented 1 year ago

    +1 useful to have

    m-miedema commented 1 year ago

    +1 would be interested in using this!

    mosegui commented 1 year ago

    +1 would have used now if available

    DSantiagoBC commented 1 year ago

    I also got here by a post in Stackoverflow, when looking for this exact feature. Are there any plans to implement this?

    bjeffrey92 commented 1 year ago

    I also got here by a post in Stackoverflow, when looking for this exact feature. Are there any plans to implement this?

    ditto (although the fact that this issue has been open since 2011 doesn't give me much hope ...)

    RSWilson1 commented 11 months ago

    I would love to see this feature added!

    Re4zOon commented 8 months ago

    +1 would have used now if available

    abulgher commented 8 months ago

    +1! I also believe it would be great to have it!

    kristianhargeaxelspace commented 7 months ago

    +1

    carlosmds commented 6 months ago

    +1

    hegga commented 6 months ago

    +1 would be very useful now

    cuppajoeman commented 5 months ago

    I would also appreciate this feature, doing it manually for now.

    haytechy commented 4 days ago

    Would also love this feature <3