Skip to content
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.

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

Add optional keyword argument exit_on_error to argparse.ArgumentParser #54147

Closed
jayt mannequin opened this issue Sep 24, 2010 · 21 comments
Closed

Add optional keyword argument exit_on_error to argparse.ArgumentParser #54147

jayt mannequin opened this issue Sep 24, 2010 · 21 comments
Labels
3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@jayt
Copy link
Mannequin

jayt mannequin commented Sep 24, 2010

BPO 9938
Nosy @rhettinger, @ezio-melotti, @merwok, @bitdancer, @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll, @matrixise, @wm75, @zvyn, @miss-islington, @shihai1991
PRs
  • bpo-9938: Add optional keyword argument exit_on_error to argparse.ArgumentParser #15362
  • Files
  • issue9938.patch
  • issue9938_with_test.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:

    assignee = None
    closed_at = <Date 2019-09-12.10:57:30.263>
    created_at = <Date 2010-09-24.14:31:29.200>
    labels = ['easy', 'type-feature', 'library', '3.9']
    title = 'Add optional keyword argument exit_on_error to argparse.ArgumentParser'
    updated_at = <Date 2019-09-12.11:21:54.018>
    user = 'https://bugs.python.org/jayt'

    bugs.python.org fields:

    activity = <Date 2019-09-12.11:21:54.018>
    actor = 'shihai1991'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2019-09-12.10:57:30.263>
    closer = 'matrixise'
    components = ['Library (Lib)']
    creation = <Date 2010-09-24.14:31:29.200>
    creator = 'jayt'
    dependencies = []
    files = ['21793', '22172']
    hgrepos = []
    issue_num = 9938
    keywords = ['patch', 'easy']
    message_count = 21.0
    messages = ['117287', '121531', '124116', '124128', '124219', '134549', '136082', '137184', '149545', '191973', '191974', '191975', '191978', '191997', '192147', '294384', '347804', '350092', '352103', '352104', '352118']
    nosy_count = 17.0
    nosy_names = ['rhettinger', 'bethard', 'ezio.melotti', 'eric.araujo', 'r.david.murray', 'docs@python', 'jayt', 'xuanji', 'tanqazx', 'paul.j3', 'a_b', 'matrixise', 'wolma', 'zvyn', 'Sourav Singh', 'miss-islington', 'shihai1991']
    pr_nums = ['15362']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue9938'
    versions = ['Python 3.9']

    @jayt
    Copy link
    Mannequin Author

    jayt mannequin commented Sep 24, 2010

    I want to create a custom interactive shell where I continually do
    parse_args. Like the following:
    parser = argparse.ArgumentParser()
    command = raw_input()
    while(True):
    args = parser.parse_args(shlex.split(command))
    # Do some magic stuff
    command = raw_input()
    The problem is that if I give it invalid input, it errors and exits
    with a help message.

    I learned from argparse-users group that you can override the exit method like the following:

    class MyParser(ArgumentParser): 
      def exit(self, status=0, message=None): 
        # do whatever you want here 

    I would be nice to have this usage documented perhaps along with best practices for doing help messages in this scenario.

    @jayt jayt mannequin added the docs Documentation in the Doc dir label Sep 24, 2010
    @jayt jayt mannequin assigned docspython Sep 24, 2010
    @jayt jayt mannequin added the type-feature A feature request or enhancement label Sep 24, 2010
    @merwok
    Copy link
    Member

    merwok commented Nov 19, 2010

    Do you want to work on a patch?

    (Aside: you may want to learn about the cmd and shlex modules for read-eval-print-loop programs :)

    @merwok merwok added the easy label Nov 19, 2010
    @tanqazx
    Copy link
    Mannequin

    tanqazx mannequin commented Dec 16, 2010

    I am also trying to use argparse interactively, but in this case by combining it with the cmd module. So I'm doing something like below:

    class MyCmd(cmd.Cmd):
    
        parser = argparse.ArgumentParser(prog='addobject')
        parser.add_argument('attribute1')
        parser.add_argument('attribute2')
        parser.add_argument('attribute3')
    
        def do_addobject(self, line):
            args = MyCmd.parser.parse_args(line.split())
            newobject = object(args.attribute1, args.attribute2, args.attribute3)
            myobjects.append(newobject)

    I'm faced with the same problem that when given invalid input, parse_args exits the program completely, instead of exiting just to the Cmd shell.

    I have the feeling that this use case is sufficiently common such that it would be good if people did not have to override the exit method themselves, and instead an alternative to parse_args was provided that only raises exceptions for the surrounding code to handle rather than exiting the program entirely.

    @merwok
    Copy link
    Member

    merwok commented Dec 16, 2010

    You can always catch SystemExit.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Dec 17, 2010

    In the short term, just catch the SystemExit.

    In the slightly longer term, we could certainly provide a subclass, say, ErrorRaisingArgumentParser, that overrides .exit and .error to do nothing but raise an exception with the message they would have printed. We'd probably have to introduce a new Exception subclass though, maybe ArgumentParserExit or something like that.

    Anyway if you're interested in this, please file a new ticket (preferably with a patch). Regardless of whether we ever provide the subclass, we certainly need to patch the documentation to tell people how to override error and exit.

    @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll
    Copy link
    Mannequin

    I don't think it's best to create a new subclass to throw an ArgumentParserExit exception; if I read the stack trace I'd see that an ArgumentError was thrown, then caught, then an ArgumentParserExit was thrown, which IMHO is confusing. In the current design, parse_known_errors catches an ArgumentError and then exits. I propose that the user be optionally allowed to turn off the handling of ArgumentError and to handle it himself instead through an exit_on_argument_error flag.

    Attached patch does this. Also I think this issue falls under component 'Lib' too.

    @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll IIIIllllIIIIllllIIIIllllIIIIllllIIIIll mannequin added the stdlib Python modules in the Lib dir label Apr 27, 2011
    @ezio-melotti
    Copy link
    Member

    FWIW unittest had a similar issue and it's been solved adding an 'exit' argument to unittest.main() 0.

    I think using an attribute here might be fine.
    The patch contains some trailing whitespace that should be removed, also it might be enough to name the attribute "exit_on_error".
    It should also include tests to check that the attribute is set with the correct default value and that it doesn't raise SystemExit when the attribute is False.

    @IIIIllllIIIIllllIIIIllllIIIIllllIIIIll
    Copy link
    Mannequin

    Updated previous patch with test cases and renamed exit_on_argument_error flag to exit_on_error.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Dec 15, 2011

    Looks good to me.

    @ab
    Copy link
    Mannequin

    ab mannequin commented Jun 28, 2013

    What is the status of this? If the patch looks good, then will it be pushed into 3.4?

    @bitdancer
    Copy link
    Member

    It's great that this patch was provided. Xuanji, can you submit a contributor agreement, please?

    The patch is missing an update to the documentation.

    (Really the patch should have been in a separate issue, as requested, since this one is about improving the documentation for the existing released versions. I guess we'll have to open a new issue for updating the docs in the existing versions).

    @ab
    Copy link
    Mannequin

    ab mannequin commented Jun 28, 2013

    The patch doesn't work for 3.3 (I think it's just because the line numbers are different), but looking over what the patch does, it looks like parse_known_args will return a value for args if there is an unrecognized argument, which will cause parse_args to call error() (it should raise ArgumentError instead).

    @ethanfurman
    Copy link
    Member

    It doesn't look like xuanji has signed a CLA.

    Should we create a new issue, and have someone else create a new patch, and let this issue just be about the docs?

    @bitdancer
    Copy link
    Member

    Yes, I think opening a new issue at this point might be a good idea. The reason is that there are a changes either in place or pending in other issues that involve the parse_know_args code, so a new patch is probably required regardless.

    I wish I had time to review and commit all the argparse patches, but so far I haven't gotten to them. They are on my todo list somewhere, though :)

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 1, 2013

    The exit and error methods are mentioned in the 3.4 documentation, but there are no examples of modifying them.

    16.4.5.9. Exiting methods
    ArgumentParser.exit(status=0, message=None)
    ArgumentParser.error(message)
    

    test_argparse.py has a subclass that redefines these methods, though I think it is more complex than necessary.

        class ErrorRaisingArgumentParser(argparse.ArgumentParser):

    In http://bugs.python.org/file30204/test_intermixed.py , part of http://bugs.python.org/issue14191 , which creates a parser mode that is closer to optparse in style, I simply use:

        def error(self, message):
            usage = self.format_usage()
            raise Exception('%s%s'%(usage, message))
        ArgumentParser.error = error

    to catch errors.

    https://github.com/nodeca/argparse a Javascript port of argparse, adds a 'debug' option to the ArgumentParser, that effectively redefines this error method. They use that extensively in testing.

    Another approach is to trap the sysexit. Ipython does that when argparse is run interactively.

    Even the simple try block works, though the SystemExit 2 has no information about the error.

    try:
        args = parser.parse_args('X'.split())
    except SystemExit as e:
        print(e)
    

    Finally, plac ( https://pypi.python.org/pypi/plac ) is a pypi package that is built on argparse. It has a well developed interactive mode, and integrates threads and multiprocessing.

    @SouravSingh
    Copy link
    Mannequin

    SouravSingh mannequin commented May 24, 2017

    I would like to send a patch for the issue. How do I start

    @zvyn
    Copy link
    Mannequin

    zvyn mannequin commented Jul 13, 2019

    This issue is a duplicate of bpo-9112 which was resolved by commit 9375492

    @shihai1991
    Copy link
    Member

    It is a good idea. So I update this title and add PR 15362.

    I am not sure there have a problem of xuanli's CLA or not~

    @shihai1991 shihai1991 added 3.9 only security fixes and removed docs Documentation in the Doc dir labels Aug 21, 2019
    @shihai1991 shihai1991 changed the title Documentation for argparse interactive use Improving interactive use of argparse Aug 21, 2019
    @shihai1991 shihai1991 changed the title Improving interactive use of argparse Add optional kwargs to argparse Aug 23, 2019
    @matrixise matrixise changed the title Add optional kwargs to argparse Add optional keyword argument exit_on_error to argparse.ArgumentParser Sep 11, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset f545638 by Miss Islington (bot) (Hai Shi) in branch 'master':
    bpo-9938: Add optional keyword argument exit_on_error to argparse.ArgumentParser (GH-15362)
    f545638

    @matrixise
    Copy link
    Member

    Thank you for your PR and for your time, I have merged the PR into master.

    @shihai1991
    Copy link
    Member

    Stéphane, thanks for your good comment. Some argparse's bpo is too old ;)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants