classification
Title: Argparse.parse_args exits on unrecognized option with exit_on_error=False
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: mhughes, paul.j3, rhettinger, xtreak
Priority: normal Keywords: patch

Created on 2020-07-09 08:40 by mhughes, last changed 2020-07-10 16:41 by paul.j3.

Files
File name Uploaded Description Edit
exit_on_error_tests.patch mhughes, 2020-07-09 13:59
Messages (7)
msg373382 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 08:40
>>> import argparse
    >>> parser = argparse.ArgumentParser(exit_on_error=False)
    >>> parser.parse_args(["--unknown"])
    usage: [-h]
    : error: unrecognized arguments: --unknown

The docs https://docs.python.org/3.10/library/argparse.html#exit-on-error say:

> Normally, when you pass an invalid argument list to the parse_args() method of an ArgumentParser, it will exit with error info.
> If the user would like catch errors manually, the feature can be enable by setting exit_on_error to False:

This description _appears_ to be at odds with the observed behavior.
msg373383 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2020-07-09 09:05
I guess the docs by manually mean that ArgumentError will be raised when exit_on_error is False that can be handled. By default with exit_on_error being True self.error() which raises SystemExit and catching SystemExit can mask other errors. This was added in bpo-9938 with GH-15362. There is also a typo in the docs that it should have used enabled instead of enable in "the feature can be enable by setting exit_on_error to False"
msg373385 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 09:51
> typo in the docs that it should have used enabled instead of enable

Well spotted, I'll happily fix this up.

> I guess the docs by manually mean that ArgumentError will be raised when exit_on_error is False that can be handled.

To be clear, in this case, even with exit_on_error=False, ArgumentError is _not_ being raised, but SystemExit is.
msg373405 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 13:59
I've attached a patch containing tests showing the current behavior, namely that exit_on_error does not change the behavior of argparse.ArgumentParser.parse_args in either case:

* An unrecognized option is given
* A required option is not given

Should the docs be updated to note this?
msg373437 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 03:01
I didn't pay attention to the patch that added this "exit_on_error" parameter.  So I don't know exactly what error handling it was supposed to handle or why.  But given the location of the code change, it does not handle every possible error.

Specifically it's  in parser.parse_known_args() where it calls _parse_known_args().  With this parameter True, a argparse.ArgumentError is caught and converted to parse.error() call.  That's the original behavior.

With False, there's no special handling for ArgumentError.  Catching that is left to the developer, as illustrated in the docs.

In the documented example, it's a 'type' error. 'choices' would also behave this way. 'nargs' errors also.  But not all errors are handled like this.

Inside _parse_known_args(), `self.error()` is called several times, once for 'required' arguments failure, and for a required mutually_exclusive_group error.  I count 9 self.error() calls; exit_on_error only changes one of those.

The error highlighted in this issue is called in parser.parse_args().  This calls parse_known_args(), and raises an error if there are 'extras', unrecognized strings.

So clearly the new docs is is describing this new parameter in overly broad terms.  It only changes the handling of a subset of parser.error() calls.  Off hand I can't think of clean way of refining the description without getting overly technical about the error handling.

Developers already had the documented option of changing the parse.error() and parse.exit() methods.
msg373440 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 04:39
For custom handling of unrecognized arguments, use parser_known_args().  You don't need this new parameter.
msg373470 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 16:41
The docs could change 

"catch errors manually"

to

"catch ArgumentError manually"

But while 'argparse.ArgumentError' is imported, it is not documented. We have to study the code to learn when it is raised.  

Its class def:

    def __init__(self, argument, message):

shows it's tied to a specific 'argument', an Action object.  Most commonly it is produced by reraising a ValueError, TypeError or ArgumentTypeError during the check_values step.

Unrecognized arguments, and missing required arguments errors aren't produced while processing an argument, but rather while checking  things after parsing.  So they can't raise an ArgumentError, and aren't handled by this new parameter.

I found a old issue that discusses this https://bugs.python.org/issue9938,  https://github.com/python/cpython/pull/15362

There wasn't much discussion about the scope of this change, or about the documentation wording.  My only comment was in 2013, https://bugs.python.org/msg192147

Until we iron out the wording I think this patch should be reverted.

While exploring other problems, I thought it would be a good idea of refactor parse_known_args and _parse_known_args.  Specifically I'd move the 'required' testing and self.error() calls out of _parse_known_args, allowing a developer to write their own versions of parse_known_args.  The goal was to make it easier to test for mixes of seen and unseen arguments.  

In light of the current issue, we might want to look into consolidating all (or at least most) of the calls to self.error() in one function.  Until then, the documented idea of modifying the error() method itself is the best user/developer tool, https://docs.python.org/3.10/library/argparse.html#exiting-methods
History
Date User Action Args
2020-07-10 16:41:46paul.j3setmessages: + msg373470
2020-07-10 04:39:09paul.j3setmessages: + msg373440
2020-07-10 03:01:12paul.j3setmessages: + msg373437
2020-07-09 13:59:51mhughessetfiles: + exit_on_error_tests.patch
keywords: + patch
messages: + msg373405
2020-07-09 09:51:16mhughessetmessages: + msg373385
versions: - Python 3.9
2020-07-09 09:05:01xtreaksetnosy: + rhettinger, xtreak, paul.j3

messages: + msg373383
versions: + Python 3.9
2020-07-09 08:40:33mhughescreate