classification
Title: lib.argparse._check_value() using repr instead of str
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: avdwoude, paul.j3, rhettinger
Priority: normal Keywords:

Created on 2020-10-29 07:45 by avdwoude, last changed 2020-10-29 16:53 by paul.j3.

Messages (3)
msg379856 - (view) Author: Aschwin (avdwoude) Date: 2020-10-29 07:45
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))}
msg379880 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-10-29 16:39
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.
msg379881 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-10-29 16:53
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 issue42191.py -f a -g a
    Namespace(f=[1, 2, 3], g='a')
    [1, 2, 3] [1, 2, 3]

help:

    0945:~/mypy$ python3 issue42191.py -h
    usage: issue42191.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 issue42191.py -f x
    usage: issue42191.py [-h] [-f {[1, 2, 3],astring}] [-g {a,b}]
    issue42191.py: error: argument -f: invalid choice: 'x' (choose from 
    [1, 2, 3], 'astring')

    0945:~/mypy$ python3 issue42191.py -g x
    usage: issue42191.py [-h] [-f {[1, 2, 3],astring}] [-g {a,b}]
    issue42191.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.
History
Date User Action Args
2020-10-29 16:53:11paul.j3setmessages: + msg379881
2020-10-29 16:39:31paul.j3setmessages: + msg379880
2020-10-29 08:35:00avdwoudesettitle: lib.argparse._check_value() using repre instead of str -> lib.argparse._check_value() using repr instead of str
2020-10-29 07:56:23xtreaksetnosy: + rhettinger, paul.j3
2020-10-29 07:45:26avdwoudecreate