pombreda / argparse

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

--help and --version methods; change defaults #43

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

>>> import argparse
>>> parser = argparse.ArgumentParser(version='0.1')
>>> parser.add_argument('--verbose', '-v', action='store_true')
>>>

(I have used optparse for quite a while and use -v/--verbose all the time)

What is the expected output? What do you see instead?

I expect:
_StoreTrueAction(option_strings=['--verbose', '-v'], dest='verbose',
nargs=0, const=True, default=False, type=None, choices=None, help=None,
metavar=None)

I get:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "argparse.py", line 1287, in add_argument
    return self._add_action(action)
  File "argparse.py", line 1659, in _add_action
    self._optionals._add_action(action)
  File "argparse.py", line 1496, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "argparse.py", line 1301, in _add_action
    self._check_conflict(action)
  File "argparse.py", line 1448, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "argparse.py", line 1455, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument -v/--version: conflicting option string(s): -v

What version of the product are you using? On what operating system?
1.1a1 (os independent)

As of argparse 1.1a1, the --help (unless suppressed) and --version
arguments are created during initialization of the ArgumentParser by calls
to the all-purpose method add_argument (directly in the __init__ method).
This way it is not easily possible to override the default arguments in a
subclass of ArgumentParser; help and version info creation must be
suppressed and done explicitly. This is inconvenient.

"-v" is not commonly used as a --version shorthand:
cp:       --version (-v is --verbose)
find:     --version (no -v or -V argument)
grep: -V, --version
less: -V, --version (no -v argument)
ls:       --version (no -v argument)
sed:  -V, --version
wc:       --version (no -v argument)

InfoZip's zip uses -v for verbose operation *and* version info, but this is
not a serious option...

Some programs even use -h for something else than the help: e.g. for the
(quite important) host argument of database commandline clients, and grep
uses -h/-H for the enforcement/suppression of filenames.

cp:      --help
dirname: --help
find:    --help (no -h or -? argument)
grep:    --help
less:    --help, -? ("Number is required after -h")
ls:      --help (no -h or -? argument)
sed:     --help, -h, -?
unzip:   --help, -h, -?
zip:     --help, -h, -?

As long as the default help and version arguments are created first and
block usage of -h and -v for other options, IMO the default option strings
must be reduced to:

--help (always help, if present)
-?     (dto.)
--version (the only version option which is *always* implemented)

If the creation of these options would be delayed (just before parsing),
-h, -V and *perhaps* -v could be tried as well, but this is another proposal.

For now, I propose to
- change the default option strings (blocking -v is a defect IMO)
- put the calls to add_argument in dedicated methods --
  e.g. add_help_argument, add_version_argument -- which can be easily
  overridden or monkey-patched.

Original issue reported on code.google.com by bruno-th...@gmx.net on 6 Nov 2009 at 10:13

GoogleCodeExporter commented 9 years ago
Just found issue 22 (I have seeked before, but I finally found it when viewing 
all
issues, omitting any search expressions).

I don't like the solution because "version='%(prog)s 2.0'" should not be an 
attribute
of the argument but of the ArgumentParser; and it is a keyword argument for one
special case only, where a suitable argument is already present for the 
constructor.

And '-v' is a bad (and unnecessary!) choice for the --version option, like 
stated
before. If it would be removed (or replaced by '-V'), everyone could have a 
--version
argument without blocking -v, just by specifying the version.

Original comment by bruno-th...@gmx.net on 6 Nov 2009 at 11:21

GoogleCodeExporter commented 9 years ago
There are two things missing from your proposal that would be crucial for 
changing
the "-h" and "-v" defaults:

(1) Evidence from the community of argparse users that the "-h" and "-v" 
defaults are
inappropriate for most people.

(2) A clear deprecation and upgrade strategy for people who are relying on the
current behavior and will need to move to your new suggested behavior.

I'm open to changing things if you can provide both of these items.

Original comment by steven.b...@gmail.com on 7 Nov 2009 at 4:29

GoogleCodeExporter commented 9 years ago
This is not about the argparse's users community only; you want argpase to be a
standard module, and as such it should match the expectations of the whole 
Python
community.

Let's face the fact that the "-v" --version switch is not common anywhere else, 
and
an argument parsing library shouldn't invent new standards here but support 
people in
writing programs which behave like they are commonly expected to behave.

optparse uses '--version' only; adding '-v' to the list makes it harder to 
replace
optparse by argparse.

Simply removing '-v' can't possibly break any Python script using argparse.
Replacing it by '-V' would IMHO be unlikely to do so; however, since you are so
concerned about it:  the attached patch keeps the '-v' string; it will be 
available
unless the '-v' is used in another option.

The attached patch
- moves the creation of the version and help options to their own public
  methods add_help_argument and add_version_argument
  (could be named add_help_option etc. as well, since these *are* options)
- postpones the creation of these options. This way, the really interesting
  options are printed first, just below the usage info (and, if present, the
  required arguments); the --help option is printed last, where it is spotted
  easily.
- introduces the new conflict_handler 'modest' which drops option strings for
  the *new* (postponed) action but raises an ArgumentError when none are left
  (which will happen only when someone has used e.g. the '--version' string
  for some other option)

This way, *all* existing scripts will continue to work;
everyone can use -v, -V and -h to his heart's content;
and the --version and --help options are moved to the bottom.

I noticed that many tests fail now because they check the exact help output;  I 
could
try to tackle this problem with vim and a few regular expression substitutions.

Original comment by bruno-th...@gmx.net on 7 Nov 2009 at 4:19

Attachments:

GoogleCodeExporter commented 9 years ago
Just to be clear, I'm not opposed to the idea of changing some defaults - we 
just
need to go through the standard process of making a change to an existing 
library.
That means either a deprecation process (which requires community consent) or a
change which is fully backwards compatible.

> This is not about the argparse's users community only; ... it should match 
the 
> expectations of the whole Python community.

I'm happy to have the poll over the whole Python community. That would be even 
better.

> Simply removing '-v' can't possibly break any Python script using argparse.

This is just not true. If someone has written a user interface with argparse, 
and we
make this change with no deprecation process, then users of that interface will 
have
their interface broken -- if they try to use '-v' to print the version, it will 
no
longer work.

> The attached patch...

Thanks for providing a patch. I'm nervous about adding actions inside of 
parse_args.
All arguments should be added before parse_args is called.

Correct me if I'm wrong, but your primary goals are to be able to customize 
which
flags are used by the help and version actions, right? What's wrong with this:

    parser = argparse.ArgumentParser(add_help=True)
    parser.add_argument('--my-help', action='help')
    parser.add_argument('--my-version', action='version', version='My Version')

Note that if you take this approach, and you want your help argument displayed 
last,
it's easy enough for you to put it there.

Original comment by steven.b...@gmail.com on 7 Nov 2009 at 7:39

GoogleCodeExporter commented 9 years ago
I guess I should also say that any patch that breaks existing tests (yes, 
including
those that test exact help output) is not an acceptable patch. Changes that 
break
tests are backwards incompatible changes, and they need to be added following 
the
usual deprecation process, which means that for the current version, no tests 
should
fail.

Original comment by steven.b...@gmail.com on 7 Nov 2009 at 7:42

GoogleCodeExporter commented 9 years ago
> Just to be clear, I'm not opposed to the idea of changing some defaults
> - we just need to go through the standard process of making a change to
> an existing library.
> That means either a deprecation process (which requires community
> consent) or a change which is fully backwards compatible.

With the tests updated (see below), my patch *does* provide full backwards 
compatibility.

>> This is not about the argparse's users community only; ... it should
>> match the expectations of the whole Python community.
> 
> I'm happy to have the poll over the whole Python community. That would
> be even better.

Yes of course. Many will know about (and have used) optparse and will prefer a
solution which makes it easy to migrate.

>> Simply removing '-v' can't possibly break any Python script using
>> argparse.
> 
> This is just not true. If someone has written a user interface with
> argparse, and we make this change with no deprecation process, then
> users of that interface will have their interface broken -- if they
> try to use '-v' to print the version, it will no longer work.

Not true when using my patch.  Unless the '-v' is used in another option, it is
finally used for '--version'.

>> The attached patch...
> 
> Thanks for providing a patch. I'm nervous about adding actions inside of
> parse_args.
> All arguments should be added before parse_args is called.

Why?
Is there any postprocessing done to the arguments outside the add_argument 
calls?
Is there another hook available which would be called before parsing?
What would be *your* suggestion how to make sure the available option strings 
are
finally used? (you are the one who considers this crucial)

> Correct me if I'm wrong, but your primary goals are to be able to
> customize which
> flags are used by the help and version actions, right?

Wrong. I consider it important that argparse makes it as easy as possible to 
create
scripts which follow the de-facto-standards of commandline programs; this 
includes
-V/--version (but *not* -v) for the version information. It should not cause
confusion by introducing an aberrant "standard" for Pythons scripts which use 
argparse.

Currently, it is easier to create non-conforming scripts with argparse; but 
every
aberration should need to be done on purpose (= with a particular reason, e.g.
mimicking a certain program's options).

> What's wrong with this:
> 
>     parser = argparse.ArgumentParser(add_help=True)
>     parser.add_argument('--my-help', action='help')
>     parser.add_argument('--my-version', action='version', version='My
> Version')

(add_help should the False here, of course, and the option strings differ...)

> Note that if you take this approach, and you want your help argument
> displayed last,
> it's easy enough for you to put it there.

I tell you what's wrong: it must be done explicitly, although it could be done
automatically.

I was always irritated when getting the least interesting options first:  when
calling the help, one usually is more interested in the options which are 
specific
for the program.

Consider a long help output for an important program -- so important that you 
want a
printout.  It the printout wouldn't fit on one page, you'd surely prefer the 
--help
and --version help to be on the 2nd page, rather than some more important 
option.

My patch simply kills three birds with one stone:
* it makes -v and -h available for e.g. --verbose and --host uses
* it retains them for --version and --help, if not used otherwise
* it moves the least interesting options to the end

Of course it would be possible (and better than nothing) to finally /insert/ the
postponed arguments; but this would seem to me like putting on the pants with 
pliers.

> I guess I should also say that any patch that breaks existing tests
> (yes, including those that test exact help output) is not an
> acceptable patch. (...)

*Of course* I didn't expect you to commit the patch immediately; but you could 
have
tried it and see that it works nicely.

As I already wrote ("I could try to tackle this problem with vim and a few 
regular
expression substitutions."), I'm working on the tests issue.
I managed to move the lines using regular expressions, but still got many 
errors, and
the differences aren't easy to spot. In the Roundup issue-tracker project, we 
have
nice diffs; my next patch will enable these for test_argparse, too.

When providing a tests patch, I'll add a commented sed script which creates it; 
this
will help you in reviewing the changes.

Original comment by bruno-th...@gmx.net on 9 Nov 2009 at 7:45

GoogleCodeExporter commented 9 years ago
I think I wasn't clear about the tests. Changing tests to make a patch work is
unacceptable. The tests should pass as they are. Changes to argparse that 
change the
current behavior need to follow the standard processes for changing behavior in 
software:

Release 1. Add new behavior as optional. All existing tests pass with no 
changes. New
tests check for optional behavior.

Release 2. Add deprecation warning for original behavior. All existing tests 
pass
with no changes. New tests check for optional behavior.

Release 3. Change optional behavior to default behavior. Changes to old tests 
are
acceptable in this release.

Is that clearer? So right now, a patch that requires changes to any existing 
tests is
not an acceptable patch. Your patch *must* pass with no changes to the test 
suite,
though it is fully acceptable to add tests for your new optional behavior. 
Changes to
the test suite will not be acceptable until your new behavior has been around 
for at
least two releases.

Original comment by steven.b...@gmail.com on 9 Nov 2009 at 11:23

GoogleCodeExporter commented 9 years ago
As a voice from Python community I dislike implicit default "-V" and "-v" 
shortcuts for 
--version. The proposed patch to treat "-v" implicitly as version if explicit 
"-v" is 
not defined also makes thing more complicated than they need to be.

If version could be displayed along with program name in usage by default then 
you 
won't need any "shortcuts" for frequent calls to --version.

Original comment by techtonik@gmail.com on 8 Dec 2009 at 2:14

GoogleCodeExporter commented 9 years ago
I wonder if the real solution here is to deprecate the version= keyword argument
entirely, since everyone seems to have a different thing they want it to do.

Original comment by steven.b...@gmail.com on 8 Dec 2009 at 5:36

GoogleCodeExporter commented 9 years ago
The real solution would be to put available variants to voting. Any gadgets for 
that 
issue on the front page?

http://www.google.com/ig/directory?
type=gadgets&url=www.gebweb.net/gadgets/VoteMate/VoteMate.xml
http://www.google.com/ig/directory?url=echo3.net/poll/poll.xml

Original comment by techtonik@gmail.com on 12 Dec 2009 at 1:53

GoogleCodeExporter commented 9 years ago
Done. I wasn't able to get either of the links you sent to work, but I did get 
a Vizu
poll working. It's on the front page now: http://code.google.com/p/argparse/. 
We'll
see if we get any votes.

Original comment by steven.b...@gmail.com on 13 Dec 2009 at 9:44

GoogleCodeExporter commented 9 years ago
So we've had a bunch of votes on this now, and the two main contenders are to 
just
drop the "-v", and to drop the version= argument entirely. The latter has a much
better migration strategy, so that's the route I'm going to go.

Thus, the version= argument to ArgumentParser is henceforth deprecated as of 
r83.
Anyone currently using this should switch to .add_argument(..., 
action="version", ...).

Original comment by steven.b...@gmail.com on 28 Feb 2010 at 6:49

GoogleCodeExporter commented 9 years ago
Sorry I fell silent for a while; I had a password database problem.

The only problem with the version keyword was the '-v' default short argument; 
now
there is one more.

Deprecating the version argument is the 2nd worst solution I could ever have 
thought
of (the very worst would have been to remove it entirely).  Every single script 
using
it will yield a deprecation warning, no matter if anyone anyone parses the 
output or
not.  And '-v' is still used by default.

Now people have to bother about how to switch off the warning, and they are 
urged to
replace a simple keyword argument by an additional method call.  Very bad, 
really.

Original comment by bruno-th...@gmx.net on 18 Apr 2010 at 2:25

GoogleCodeExporter commented 9 years ago
ArgumentParser(...,
   option("version", ...),
   option("help", ...)
);

Is it better?

Original comment by techtonik@gmail.com on 19 Apr 2010 at 8:48