bpo-14074: argparse doesn't allow metavar to be a tuple by cykerway · Pull Request #10847 · python/cpython

cykerway · 2018-12-01T13:47:02Z

Currently argparse allows nargs>1 for positional arguments but doesn't
allow metavar to be a tuple:

import argparse
parser = argparse.ArgumentParser()
parser.add_argument('foo', nargs=2, metavar=('bar', 'baz'))
args = parser.parse_args()

The above code will raise exception both with and without '--help'.

This patch fixed some functions about metavar processing so the above
code will pass.

https://bugs.python.org/issue14074

Issue: argparse allows nargs>1 for positional arguments but doesn't allow metavar to be a tuple #58282

the-knights-who-say-ni · 2018-12-01T13:47:07Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

embray · 2019-04-07T14:02:27Z

Thanks for the PR. I can confirm that this fixes this (very old) bug...

rhettinger · 2020-01-28T07:55:27Z

Consider restricting this to the case where nargs is an integer. It doesn't make much sense with * or REMAINDER.

Lib/argparse.py

Misc/NEWS.d/next/Library/2018-12-04-07-36-27.bpo-14074.fMLKCu.rst

jacobtylerwalls · 2021-06-15T13:30:55Z

Lib/argparse.py

+        if isinstance(argument.nargs, int) and \
+           isinstance(argument.metavar, tuple):
+            return ' '.join(argument.metavar)


CPython follows PEP8, so I believe the style is to prefer open parentheses over \.

There it is: c0204fd

Nothing else changed.

Currently argparse allows nargs>1 for positional arguments but doesn't allow metavar to be a tuple: import argparse parser = argparse.ArgumentParser() parser.add_argument('foo', nargs=2, metavar=('bar', 'baz')) args = parser.parse_args() The above code will raise exception both with and without '--help'. This patch fixed some functions about metavar processing so the above code will pass.

rhettinger · 2022-05-10T21:41:49Z

For:

 parser.add_argument('foo', nargs=2, metavar=('bar', 'baz'))

Wouldn't it be better to specify two separate arguments?

 parser.add_argument('foo')
 parser.add_argument('baz')

Having two different meta variables implies that the arguments have different meanings. Collapsing them into a single argument seems like an anti-pattern to me. Both warrant their own error messages, possible type conversions, and possible default values.

cykerway · 2022-05-12T17:08:14Z

Having two different meta variables implies that the arguments have different meanings. Collapsing them into a single argument seems like an anti-pattern to me. Both warrant their own error messages, possible type conversions, and possible default values.

That's the matter of nargs. nargs=N will not exist if each action consumes exactly one command-line argument.

nargs=N comes in handy when an argument is inherently a tuple. For example, RGB color components, monthly salaries. The tuple items often have the same type and just need a display name for each. The tuple items may never be used individually and probably do not worth a variable for each. nargs=N can create the tuple, saving application code.

If we assume nargs=N should exist, then having a tuple metavar looks better than repeating a string metavar.

serhiy-storchaka

I wondered why TestHelpTupleMetavarPositional tests only argument with numerical value of nartgs, while TestHelpTupleMetavar tests with all nargs values. So I tried to change the test. It failed, but a simple change in _format_action_invocation() can fix this. I am nor sure that this is a best representation for nargs='*' and nargs='+', but this at least fixes failure.

The change in _get_action_name() is not tested. We need a test for it, and it should be generalized to support other nargs values.

serhiy-storchaka · 2024-09-23T16:40:35Z

Lib/argparse.py

+        if isinstance(
+            argument.nargs, int) and isinstance(argument.metavar, tuple):
+            return ' '.join(argument.metavar)


This change is not covered by tests.

serhiy-storchaka · 2024-09-23T16:41:50Z

Lib/test/test_argparse.py

+
+    parser_signature = Sig(prog='PROG')
+    argument_signatures = [
+        Sig('foo', help='foo help', nargs=2, metavar=('bar', 'baz'))


Suggested change

Sig('foo', help='foo help', nargs=2, metavar=('bar', 'baz'))

Sig('w', help='w help', nargs='+', metavar=('W1', 'W2')),

Sig('x', help='x help', nargs='*', metavar=('X1', 'X2')),

Sig('y', help='y help', nargs=3, metavar=('Y1', 'Y2', 'Y3')),

Sig('z', help='z help', nargs='?', metavar=('Z1', )),

serhiy-storchaka · 2024-09-23T16:42:35Z

Lib/test/test_argparse.py

+    ]
+    argument_group_signatures = []
+    usage = '''\
+        usage: PROG [-h] bar baz


Suggested change

usage: PROG [-h] bar baz

usage: PROG [-h] W1 [W2 ...] [X1 [X2 ...]] Y1 Y2 Y3 [Z1]

serhiy-storchaka · 2024-09-23T16:42:49Z

Lib/test/test_argparse.py

+    help = usage + '''\
+
+        positional arguments:
+          bar baz     foo help


Suggested change

bar baz foo help

W1 W2 w help

X1 X2 x help

Y1 Y2 Y3 y help

Z1 z help

serhiy-storchaka · 2024-09-23T16:43:36Z

Lib/argparse.py

+            if isinstance(action.nargs, int):
+                return ' '.join(self._metavar_formatter(action, default)(1))
            metavar, = self._metavar_formatter(action, default)(1)
            return metavar


Suggested change

if isinstance(action.nargs, int):

return ' '.join(self._metavar_formatter(action, default)(1))

metavar, = self._metavar_formatter(action, default)(1)

return metavar

return ' '.join(self._metavar_formatter(action, default)(1))

serhiy-storchaka · 2024-09-30T08:36:49Z

Thank you for your contribution @cykerway and sorry for a lack of response.
#124782 generalizes this PR to support other nargs values.

the-knights-who-say-ni added the CLA not signed label Dec 1, 2018

bedevere-bot added the awaiting review label Dec 1, 2018

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Dec 3, 2018

csabella requested a review from rhettinger January 28, 2020 02:28

rhettinger self-assigned this Jan 28, 2020

ZackerySpytz reviewed Dec 10, 2020

View reviewed changes

Lib/argparse.py Outdated Show resolved Hide resolved

Misc/NEWS.d/next/Library/2018-12-04-07-36-27.bpo-14074.fMLKCu.rst Outdated Show resolved Hide resolved

cykerway force-pushed the bpo-14074 branch from 42f14dc to 50c844f Compare March 8, 2021 14:17

rhettinger removed their assignment Dec 29, 2021

cykerway force-pushed the bpo-14074 branch from 50c844f to 5b47645 Compare December 29, 2021 16:25

jacobtylerwalls reviewed Dec 29, 2021

View reviewed changes

cykerway force-pushed the bpo-14074 branch from 5b47645 to c0204fd Compare December 29, 2021 19:44

cykerway force-pushed the bpo-14074 branch from c0204fd to e95217a Compare January 1, 2022 22:17

rhettinger removed their request for review May 3, 2022 06:22

tshepang mannequin mentioned this pull request Apr 10, 2022

argparse allows nargs>1 for positional arguments but doesn't allow metavar to be a tuple #58282

Closed

ezio-melotti removed the CLA signed label Jul 13, 2022

Merge branch 'main' into bpo-14074

94c78b2

serhiy-storchaka reviewed Sep 23, 2024

View reviewed changes

serhiy-storchaka closed this Sep 30, 2024

-        Sig('foo', help='foo help', nargs=2, metavar=('bar', 'baz'))
+        Sig('w', help='w help', nargs='+', metavar=('W1', 'W2')),
+        Sig('x', help='x help', nargs='*', metavar=('X1', 'X2')),
+        Sig('y', help='y help', nargs=3, metavar=('Y1', 'Y2', 'Y3')),
+        Sig('z', help='z help', nargs='?', metavar=('Z1', )),

	usage: PROG [-h] bar baz
	usage: PROG [-h] W1 [W2 ...] [X1 [X2 ...]] Y1 Y2 Y3 [Z1]

-          bar baz     foo help
+          W1 W2       w help
+          X1 X2       x help
+          Y1 Y2 Y3    y help
+          Z1          z help

Uh oh!

Conversation

cykerway commented Dec 1, 2018 • edited by serhiy-storchaka Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Dec 1, 2018

Uh oh!

embray commented Apr 7, 2019

Uh oh!

rhettinger commented Jan 28, 2020

Uh oh!

Uh oh!

Uh oh!

jacobtylerwalls Jun 15, 2021

Choose a reason for hiding this comment

Uh oh!

cykerway Dec 29, 2021

Choose a reason for hiding this comment

Uh oh!

rhettinger commented May 10, 2022

Uh oh!

cykerway commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Sep 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

cykerway commented Dec 1, 2018 •

edited by serhiy-storchaka

Loading

cykerway commented May 12, 2022 •

edited

Loading