zenlotus / argparse

Automatically exported from code.google.com/p/argparse
Other
0 stars 0 forks source link

Control flow is lost when overriding error/exit #59

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When attempting to grab information from multiple sources than argparse in 
sanescript [1], I run into the problem that I cannot give argparse 
information from external sources, e.g. config files. This way, some 
required parameters are either missing at the time of argument parsing, or 
show as non-required in the help.

Since when overriding error() I am forced to either raise an exception, or 
exit control flow, there is no chance to recover the potentially bad data. 
I am not sure if this is an inherent problem (eg missing positional args 
may break everything).

Please advise on how to fix this.

[1] http://bitbucket.org/aafshar/sanescript-main/overview/

Original issue reported on code.google.com by aafs...@gmail.com on 4 Feb 2010 at 11:37

GoogleCodeExporter commented 9 years ago
I'm not clear on exactly what it is you're trying to do. Is the issue that you 
want
to read a config file midway through argument parsing and have it update the 
current
values? If that's what you're trying to do, perhaps you can combine
fromfile_prefix_chars with overriding convert_arg_line_to_args()?

http://argparse.googlecode.com/svn/trunk/doc/ArgumentParser.html#fromfile-prefix
-chars

If that's not what you're trying to do, could you elaborate a bit more?

Original comment by steven.b...@gmail.com on 4 Feb 2010 at 5:06

GoogleCodeExporter commented 9 years ago
I want to supplement the configuration values that argparse gets from the 
command 
line, with configuration values I provide from other sources.

Yes, the file is one example, in that I want to get a filename from argparse, 
eg --
config-file, then get values from that file that might be "required" by 
argparse, but 
not available on the command line.

e.g. say our script is called "grab".

and it expects something like:

grab --username USERNAME --password PASSWORD [--config-file FILE]

then if the username and password are provided in the file, but not on the 
command 
line, argparse should have a way of not raising the error, and be updated with 
external data.

Now, I have the config file already, and would have no interest in argparse 
reading 
that file for me, since I already have a way to read it. All I need is argparse 
to 
not fail with missing required parameters, but to show them correctly in the 
help.

I hope this makes more sense

Ali

Original comment by aafs...@gmail.com on 4 Feb 2010 at 5:29

GoogleCodeExporter commented 9 years ago
Yeah, so the only way of getting something like this behavior is to use 
argparse's
ability to read arguments from files:

  grab --username USERNAME --password PASSWORD @FILE

See the link I sent above - additional arguments will be read from FILE and 
treated
as if they were inserted at that location on the command line.

I'm open to making it easier to add things like @FILE if you have an idea of 
how to
do what you want. Patches welcome!

Original comment by steven.b...@gmail.com on 4 Feb 2010 at 5:42

GoogleCodeExporter commented 9 years ago
Ok, well how about a solution like this (2 elements):

1. An optional flag to parse like ignore_missing, combined with
2. A method on the parser to read arg values from a dict

This way, I could use the parser twice, once to grab values for config files, 
then to 
read them, then to update the arparser with the values from the file, then to 
parse 
the command line again, and see if anything is missing.

A bit complicated, but seems like one of the only ways I can think.

Unfortunately @file won't work, my config files are yaml.

Original comment by aafs...@gmail.com on 4 Feb 2010 at 5:57

GoogleCodeExporter commented 9 years ago
Have you looked at parse_known_args()?

http://argparse.googlecode.com/svn/trunk/doc/other-methods.html#partial-parsing

Seems like you could have a parser that looks for the config file and call
parse_known_args() to get the config file. Then you'd call set_defaults() on 
your
main parser using the dict you read from the config file, and then call 
parse_args()
on the main parser.

Would that work?

Original comment by steven.b...@gmail.com on 4 Feb 2010 at 6:25

GoogleCodeExporter commented 9 years ago
Yes, I think it might work. The problem is that the parser already knows about 
many 
more args.

Perhaps a method like "parse_only_args(arglist, args)", where only the args in 
arglist 
are parsed and the remainder are ignored, even if the parser knows about them?

Original comment by aafs...@gmail.com on 4 Feb 2010 at 6:33

GoogleCodeExporter commented 9 years ago
I would use parent parsers instead:

>>> config = argparse.ArgumentParser(add_help=False)
>>> config.add_argument('--config')
>>> main = argparse.ArgumentParser(parents=[config])
>>> main.add_argument('foo')
>>> main.add_argument('--bar', action='store_true')
>>> config_args, _ = config.parse_known_args(['X', '--config', 'Y'])
>>> # read config_args.config file into a dictionary
... config_dict = dict(bar=True)
>>> main.set_defaults(**config_dict)
>>> main.parse_args(['X', '--config', 'Y'])
Namespace(bar=True, config='Y', foo='X')

Original comment by steven.b...@gmail.com on 4 Feb 2010 at 6:43

GoogleCodeExporter commented 9 years ago
I already have parent parsers, so parse_known args ignores the children?

Original comment by aafs...@gmail.com on 4 Feb 2010 at 9:59

GoogleCodeExporter commented 9 years ago
As long as you call it on the parent, yes. Parents don't have any access to 
their
children. 

Original comment by steven.b...@gmail.com on 4 Feb 2010 at 10:22

GoogleCodeExporter commented 9 years ago
Thanks for your help Steven, I will try this today and report the results to 
you.

Original comment by aafs...@gmail.com on 5 Feb 2010 at 8:43

GoogleCodeExporter commented 9 years ago
Marking as fixed. Feel free to reopen if the combination of parent parsers and
parse_known_args did not solve your problem.

Original comment by steven.b...@gmail.com on 1 Mar 2010 at 7:14

GoogleCodeExporter commented 9 years ago
Actually, I have not managed to make it work yet. I am trying now, but the loss 
of 
control flow makes things very difficult. At the moment, parse_known_args is 
failing 
with missing child arguments.

I use something like this:

self._subparser_factory = self._parser.add_subparsers(dest='__command__')
...
child_parser = self._subparser_factory.add_parser(command.name, 
help=command.__doc__)
...
self._parser.parse_known_args()

And that is causing SystemExit on missing argument in the child parser. I 
think! 
Maybe I am confused.

Original comment by aafs...@gmail.com on 1 Mar 2010 at 1:52

GoogleCodeExporter commented 9 years ago
Seems my problem is that I am not using child parsers, but SubParserAction. So 
my plan 
is to create a parser and child parser, then add the subparseractions to the 
child 
parser. I will report.

Original comment by aafs...@gmail.com on 2 Mar 2010 at 10:46

GoogleCodeExporter commented 9 years ago
 Does  config.add_argument('--config') have to be called before the child parser is 
created? 

Original comment by aafs...@gmail.com on 2 Mar 2010 at 11:30

GoogleCodeExporter commented 9 years ago
Yes, that makes all the difference. I think that is a bug that you have to add 
the 
arguments before creating and adding the child

Original comment by aafs...@gmail.com on 2 Mar 2010 at 11:37

GoogleCodeExporter commented 9 years ago
If nothing else, it's a documentation bug. I've opened Issue 61 for this 
problem.

Original comment by steven.b...@gmail.com on 2 Mar 2010 at 7:51

GoogleCodeExporter commented 9 years ago
Ok, thanks. I still believe that although my problem is solved (it is working 
perfectly now) the original bug that argparse takes the control flow and 
prevents it 
from returning when there is a validation failure is a major one.

In my opinion validation, and control flow should be kept separate if possible. 
I know 
that is extremely hard considering poisitional options, etc. But, in the state 
it is, 
you haven't solved the major problem of optparse in the stdlib. You have added 
many 
wonderful features, but the problem remains.

Original comment by aafs...@gmail.com on 2 Mar 2010 at 8:17

GoogleCodeExporter commented 9 years ago
Hmm... "validation and control flow should be kept separate" is a pretty vague
statement. If you can make a concrete API suggestion, there might be more I 
could do.
And of course, I'm always open to patches. ;-)

Original comment by steven.b...@gmail.com on 3 Mar 2010 at 1:37

GoogleCodeExporter commented 9 years ago
Keeping them separate would mean that the parsing+validation would always 
return with 
a Namespace, maybe with parts marked as error. There would be no sys.exit, and 
no 
exception raising at that stage. The incomplete results may be useful.

Control flow would run the validation, and display the error/exit as required.

The parsing and validation should be able to be used independently of the 
control 
flow part.

concrete API is:

parser.parse_args always returns a Namespace.

Original comment by aafs...@gmail.com on 3 Mar 2010 at 6:53

GoogleCodeExporter commented 9 years ago
Changing the parser.parse_args API would be backwards incompatible, so that's 
not
possible. But it might be possible to add a parser.parse_args_ignore_errors or
something like that, where a possibly incomplete Namespace would be returned. It
would be hard to spec this out very clearly though - since actions can do 
whatever
they want, the result returned by parser.parse_args_ignore_errors would be 
almost
undefined. For example, the action could set an attribute on the Namespace and 
then
fail, or it could fail before setting the attribute. But argparse won't know the
difference, so it won't have any way of flagging this.

I'm certainly willing to review patches though (preferably with extensive tests
demonstrating the desired behavior).

Original comment by steven.b...@gmail.com on 6 Mar 2010 at 1:43

GoogleCodeExporter commented 9 years ago
Yes, I do appreciate the task is very difficult, and the "undefined" state will 
be 
prominent. And it is almost impossible to behave correctly. I have no chance 
myself of 
writing patches to make this work, probably technically, and for time reasons. 
But, it 
is my strong opinion that until this situation is fixed, you have not really 
improved 
on optparse, apart from add nice features.

My next feature was to be able to parse the command line without first defining 
the 
arguments! (that would be even harder I think)

Original comment by aafs...@gmail.com on 6 Mar 2010 at 9:08