Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib.argparse._check_value() using repr instead of str #86357

Closed
avdwoude mannequin opened this issue Oct 29, 2020 · 4 comments
Closed

lib.argparse._check_value() using repr instead of str #86357

avdwoude mannequin opened this issue Oct 29, 2020 · 4 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@avdwoude
Copy link
Mannequin

avdwoude mannequin commented Oct 29, 2020

BPO 42191
Nosy @rhettinger

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-10-29.07:45:26.786>
labels = ['type-feature', 'library', '3.10']
title = 'lib.argparse._check_value() using repr instead of str'
updated_at = <Date 2020-10-29.16:53:11.044>
user = 'https://bugs.python.org/avdwoude'

bugs.python.org fields:

activity = <Date 2020-10-29.16:53:11.044>
actor = 'paul.j3'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-10-29.07:45:26.786>
creator = 'avdwoude'
dependencies = []
files = []
hgrepos = []
issue_num = 42191
keywords = []
message_count = 3.0
messages = ['379856', '379880', '379881']
nosy_count = 3.0
nosy_names = ['rhettinger', 'paul.j3', 'avdwoude']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42191'
versions = ['Python 3.10']

Linked PRs

@avdwoude
Copy link
Mannequin Author

avdwoude mannequin commented Oct 29, 2020

When using a custom type in add_argument(), the error message on an incorrect value uses repr instead of str, which looks ugly.

Example code:

login2account = {
    c.account.login: c.account for c in self.admin.get_accounts()
}

action_add_parser.add_argument(
"--user",
dest="user",
type=lambda x: login2account.get(x, x),
choices=list(login2account.values()),
help="Login name of the user",
)

When using an unknown user, the output is something alike:
action add: error: argument --assignee: invalid choice: karen (choose from <Account(login=john)>, <Account(login=peter)>)

The culprit is the following line in lib.argparse._check_value():

    args = {'value': value,
            'choices': ', '.join(map(repr, action.choices))}

Which should be the following for prettier output:

    args = {'value': value,
            'choices': ', '.join(map(str, action.choices))}

@avdwoude avdwoude mannequin added 3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 29, 2020
@avdwoude avdwoude mannequin changed the title lib.argparse._check_value() using repre instead of str lib.argparse._check_value() using repr instead of str Oct 29, 2020
@avdwoude avdwoude mannequin changed the title lib.argparse._check_value() using repre instead of str lib.argparse._check_value() using repr instead of str Oct 29, 2020
@paulj3
Copy link
Mannequin

paulj3 mannequin commented Oct 29, 2020

In the Help formatting,

    choice_strs = [str(choice) for choice in action.choices]

The use of repr in this check_value() error message was, I think, deliberate. For simple choices like strings and numbers it doesn't matter. But for other cases, it may help to know more about the identify of the choice objects, such as their class.

The actual parsing test is just a 'in' test.

value not in action.choices

There is the added complication that the _get_value() function has already applied the type function the argument string, converting it to a compatible object (for testing).

Should argparse be changed, or should it be up to the developer to use a type/choices pairing that gives the desired help and error messages?

To improve the discussion, I think your 'self.admin' example should be replaced by a self contained type and class. In the spirit of a StackOverFlow question I'd like concrete minimal example that I can test and play with. I don't want to write a pseudo-account class.

@paulj3
Copy link
Mannequin

paulj3 mannequin commented Oct 29, 2020

Do you realize that choices can be a dictionary? And since you want the user to supply a key, not a value, you might not want to use a type that does the conversion.

To illustrate consider two ways of using a simple dictionary.

import argparse

adict = {'a': [1,2,3], 'b': 'astring'}

parser = argparse.ArgumentParser()
parser.add_argument('-f', 
    type = lambda x: adict.get(x,x),
    choices = list(adict.values())
    )
parser.add_argument('-g', choices = adict)

args =  parser.parse_args()
print(args)
print(args.f, adict[args.g])

sample runs:

valid key:

0942:~/mypy$ python3 bpo-42191.py -f a -g a
Namespace(f=[1, 2, 3], g='a')
[1, 2, 3] [1, 2, 3]

help:

0945:~/mypy$ python3 bpo-42191.py -h
usage: bpo-42191.py [-h] [-f {[1, 2, 3],astring}] [-g {a,b}]

optional arguments:
    -h, --help            show this help message and exit
    -f {[1, 2, 3],astring}
    -g {a,b}

Error cases:

0945:~/mypy$ python3 bpo-42191.py -f x
usage: bpo-42191.py [-h] [-f {[1, 2, 3],astring}] [-g {a,b}]
bpo-42191.py: error: argument -f: invalid choice: 'x' (choose from 
[1, 2, 3], 'astring')

0945:~/mypy$ python3 bpo-42191.py -g x
usage: bpo-42191.py [-h] [-f {[1, 2, 3],astring}] [-g {a,b}]
bpo-42191.py: error: argument -g: invalid choice: 'x' (choose from 
'a', 'b')

With -g, we have to perform the dictionary lookup after parsing, but choices, {a,b}, are clear in both the help and error.

With -f, both the help (which uses str) and the error, give wrong user choices, the dictionary values rather than the keys.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@erlend-aasland erlend-aasland moved this to Features in Argparse issues May 19, 2022
rindeal added a commit to rindeal/python--cpython--dev-fork that referenced this issue Sep 28, 2024
Fixes: python#86357

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
rindeal added a commit to rindeal/python--cpython--dev-fork that referenced this issue Sep 28, 2024
Fixes: python#86357

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
rindeal added a commit to rindeal/python--cpython--dev-fork that referenced this issue Sep 28, 2024
Fixes: python#86357

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
rindeal added a commit to rindeal/python--cpython--dev-fork that referenced this issue Oct 14, 2024
Fixes: python#86357

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
serhiy-storchaka pushed a commit that referenced this issue Oct 14, 2024
…oices (GH-117766)

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this issue Oct 14, 2024
…y to print choices (pythonGH-117766)

(cherry picked from commit 66b3922)

Co-authored-by: rindeal <dev.rindeal@gmail.com>
Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this issue Oct 14, 2024
…y to print choices (pythonGH-117766)

(cherry picked from commit 66b3922)

Co-authored-by: rindeal <dev.rindeal@gmail.com>
Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
@serhiy-storchaka serhiy-storchaka added 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes and removed 3.10 only security fixes labels Oct 14, 2024
@github-project-automation github-project-automation bot moved this from Features to Doc issues in Argparse issues Oct 14, 2024
@serhiy-storchaka
Copy link
Member

This is a borderline between a bug and new feature. I decided to backport this change.

serhiy-storchaka added a commit that referenced this issue Oct 14, 2024
…rint choices (GH-117766) (GH-125432)

(cherry picked from commit 66b3922)

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
Co-authored-by: rindeal <dev.rindeal@gmail.com>
serhiy-storchaka added a commit that referenced this issue Oct 14, 2024
…rint choices (GH-117766) (GH-125431)

(cherry picked from commit 66b3922)

Signed-off-by: Jan Chren ~rindeal <dev.rindeal@gmail.com>
Co-authored-by: rindeal <dev.rindeal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
Status: Doc issues
Development

No branches or pull requests

1 participant