Closed GoogleCodeExporter closed 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
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
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:
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
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
> 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
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
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
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
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
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
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
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
ArgumentParser(...,
option("version", ...),
option("help", ...)
);
Is it better?
Original comment by techtonik@gmail.com
on 19 Apr 2010 at 8:48
Original issue reported on code.google.com by
bruno-th...@gmx.net
on 6 Nov 2009 at 10:13