msg252288 - (view) |
Author: Julien Baley (Julien Baley) * |
Date: 2015-10-04 21:04 |
The documentation of the action `store_const` states that it defaults to None. In turn, the documentation of `store_true` and `store_false` states that "[t]hese are special cases of 'store_const'", suggesting that they would also default to None.
Thankfully, `store_true` defaults to False and `store_false` to True, but this is not documented. That would be useful, as I keep seeing people writing `action='store_true' default=False` and vice-versa.
|
msg252290 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2015-10-04 22:11 |
There is another issue to fix the documentation for store_const, which does not in fact default to anything. The 'default" in this case is the default for the value stored when the option is specified. So if store_true also implies 'default=False', that should indeed be documented.
|
msg252291 - (view) |
Author: Julien Baley (Julien Baley) * |
Date: 2015-10-04 22:28 |
That's true, store_const has no default and will throw an exception if const is not provided. Updated the patch consequently.
|
msg252427 - (view) |
Author: paul j3 (paul.j3) *  |
Date: 2015-10-06 20:16 |
I have seen questions on StackOverflow with 'store_true' and explicit `default=False` parameters. I don't recall any where this was a problem. Usually it's as harmless as users specifying other defaults like `action='store'`, or a dest that echos a long option, etc.
But if we are going to fix the wrong default statement in `store_const`, we might as well clarify this case as well.
I wonder if this is the time and place to add a note that specifying 'type' is not necessary (and even wrong) - re. this closed issue: http://bugs.python.org/issue24754
Is the line 'These are special cases of ``'store_const'``' useful? The statement echos the class definition
class _StoreTrueAction(_StoreConstAction)
but I'm not sure it adds much to the user's understanding. 'store_true' and 'store_false' are used frequently; 'store_const' is much less common (based on SO questions).
|
msg259020 - (view) |
Author: Julien Baley (Julien Baley) * |
Date: 2016-01-27 13:25 |
This issue has now been open for nearly three months. I think my patch is an improvement over the current documentation. If people want to improve the documentation further, they can probably submit a patch for that.
What can I do to get this accepted?
|
msg259094 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2016-01-28 06:26 |
Hi Julien, thanks for your patch and sorry for the lack of reviews. I find the new wording in the second hunk confusing:
``'store_true'`` and ``'store_false'`` - These store the values ``False`` respectively (Note that these default to ``False`` and ``True`` respectively). These are special cases of ``'store_const'``.
On first read, It seems to contradict itself. I know what is meant, but I think it should be expanded a bit.
Also, the part about ``store_const`` should still mention the default for its value (not for the const). I suppose it's ``None``?
|
msg259109 - (view) |
Author: Julien Baley (Julien Baley) * |
Date: 2016-01-28 09:00 |
Hi Georg,
thanks for your answer. I think maybe you're missing a bit in there?
"``'store_true'`` and ``'store_false'`` - These store the values ``True`` and ``False`` respectively (Note that these default to ``False`` and ``True`` respectively). These are special cases of ``'store_const'``." (added "``True`` andd")
But if I'm correct, you're talking of the parenthesis which would be confusing? Can we come up with a better wording?
"(and default to False and True if the action is not triggered)" ?
As for `store_const`, R. David Murray says it has no default. It is technically correct: store_const behaves like any other action in that it defaults to whatever the argument `default` in `add_argument` is set to.
The fact that `default` defaults to None is indicated in 16.4.3.5. default:
"The default keyword argument of add_argument(), whose value defaults to None,"
Therefore, I believe it is more correct the way R. David Murray suggested.
What do you think?
|
msg259158 - (view) |
Author: paul j3 (paul.j3) *  |
Date: 2016-01-28 20:54 |
How about:
These store the values True and False respectively (and default to False and True if absent).
The reference to `store_const` is confusing, since almost no one uses that action.
The `store_const` paragraph has:
(Note that the const keyword argument defaults to the rather unhelpful None.)
But the most common `store` action also defaults to the default default None. In fact most action types do that.
With 'store' action, None is a useful default, since its impossible to provide in the commandline. Thus
if args.foo is None:
is a clear test that 'foo' was not present in the commandline. I can imagine using the same test on a 'store_const' argument (though I've never had the need to use it or recommend it).
I'm almost of the opinion that we should remove that parenthetical remark.
The 'const' parameter is used most often with 'store' and "nargs='?'", as illustrated in that 'nargs' subsection.
To complicate things, if I provide a "argument_default='boo'" parameter in the parser definition, it overrides all of these default defaults, including for 'store_true'. In that case an explicit 'default=False' is *required*, not superfluous.
To further complicate things, 'parser.set_defaults' can override all of these.
We've almost given users too many ways of setting 'default'. :) Fortunately in practice they don't use most of them.
|
msg259164 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-01-28 22:06 |
I definitely agree with removing the remark about the “const” value with store_const. People here seem to be overloading the terms “default” and “argument”. When using store_const, it seems the programmer must also specify a value for the “const” parameter to add_argument(). It is incorrect to say it is None if not explicitly set.
On the other hand, I think if the end user omits a CLI argument configured with store_const, then the argparse module will substitute None, or the value of the “default” parameter to add_argument().
For the original report about store_true/false, perhapse it would be sufficient to port revision 49677cc6d83a to Python 3. Although there is a stray “using” that should probably be fixed.
|
msg259166 - (view) |
Author: Julien Baley (Julien Baley) * |
Date: 2016-01-28 22:34 |
I like paul.j3's suggestion, but it would probably make more sense to have it consistent with python2 and port the change Martin pointed to.
How does one do that?
|
msg262560 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-03-28 06:17 |
New changeset 566fe3564684 by Raymond Hettinger in branch '3.5':
Issue #25314: store_true and store_false also create appropriate defaults.
https://hg.python.org/cpython/rev/566fe3564684
New changeset 9fdeca5fdbf0 by Martin Panter in branch 'default':
Issue #25314: Merge argparse doc from 3.5
https://hg.python.org/cpython/rev/9fdeca5fdbf0
|
msg262563 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-03-28 06:36 |
For the record, porting the change was rather easy (hg graft + tweak + hg commit --amend). With that applied, I think it eliminates the need for the store_true/false half of Julien’s patch.
Is the store_const half of Julien’s patch ready to commit?
|
msg263060 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-04-09 04:08 |
Patch with the outstanding change for const, plus an extra fix under the main description.
|
msg263535 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-04-16 00:22 |
New changeset c42a9233af04 by Martin Panter in branch '2.7':
Issue #25314: Remove confused statement about const argument
https://hg.python.org/cpython/rev/c42a9233af04
New changeset 59b8db278e3c by Martin Panter in branch '3.5':
Issue #25314: Remove confused statement about const argument
https://hg.python.org/cpython/rev/59b8db278e3c
New changeset 7d61a991f405 by Martin Panter in branch 'default':
Issue #25314: Merge argparse doc from 3.5
https://hg.python.org/cpython/rev/7d61a991f405
|