multimeric / ArgparsePrompt

Wrapper for the built-in Argparse, allowing missing command-line arguments to be filled in by the user via interactive prompts
GNU General Public License v3.0
11 stars 4 forks source link

Implicit Conversion of Optional Flags #6

Closed Radagaisus closed 4 years ago

Radagaisus commented 4 years ago

It’s fairly common to use flags both to check whether a given action should be done and use the flag values as data for the action. The implicit type conversion in ArgparsePrompt messes with this pattern. For example:

parser.add_argument('--population_snapshot', type=str, default=None)

Using this definition and using the default value in the prompt will convert None to 'None'.

parser.add_argument('--seed', type=int, default=None)

The same happens here, only this time an exception will be thrown (Argument "seed" was given a value not of type <class 'int'>).

multimeric commented 4 years ago

Thanks for this report. I'll try to follow what argparse does here, but I suspect it doesn't apply the type function to the default argument, which is the behaviour you also want.

In the meantime you can use a custom function that accepts either, e.g. type=lambda x: x is None ? None else int(x).

multimeric commented 4 years ago

This should now be fixed in 0.0.5, and it's included as a test now. I've also published a new release on PyPI. The new behaviour is designed to mirror what argparse does:

If the default value is a string, the parser parses the value as if it were a command-line argument. In particular, the parser applies any type conversion argument, if provided, before setting the attribute on the Namespace return value. Otherwise, the parser uses the value as is

Radagaisus commented 4 years ago

Looks good. Thanks!

Radagaisus commented 4 years ago

With the new changes, default=None will not appear in the prompt.

seed: Seed for fixed random number generation
> 
population_size: Maximum number of agents in each generation
> (100) 

Probably better if it’s visible:

seed: Seed for fixed random number generation
> (None)
multimeric commented 4 years ago

Hmm, yes it's a bit unhelpful that I convert None to an empty string. I'll fix this eventually, but PRs welcome (with a test).

multimeric commented 4 years ago

Fixed in 0.0.6.