Using argparse with parameters defined in config file

Question

I understand that and why using eval is generally considered bad practice in most cases (see e.g. here). Related questions on config files and argparse don't use types. (1, 2)

I needed to parse a number of command line arguments and decided to store the arguments, defaults and help strings in a .json file. I read the .json and add every argument to an argparse.ArgumentParser. To specify a type I need to pass a callable like float and not the string "float".

To get from the string to the callable I thought of

• using eval
• using a dict to map from string to callable
• using a if/else or switch

and decided to use to use eval to avoid hard coding all types.

I have no security concerns because the argument file is supplied by the user and the program that uses this code is targeted at scientists that will realistically want to change the file to add parameters or change defaults. (Also, it is a university project which will never be run except for grading. I handed in a version using eval.)

Is there a smart solution avoiding hardcoding all types and avoiding eval, or did I find a place where eval is a sensible choice? I was only allowed to use the standard library.

Minimal args.json:

{
    "dt": {
        "type": "float",
        "help": "Time step size",
        "default": 0.4},
    "T": {
        "type": "int",
        "help": "Time steps between outputs",
        "default": 50},
    "f": {
        "type": "str",
        "help": "Landscape file name",
        "default": "small.dat"}
}

Runnable code, put args.json above in same directory to run:

import json
import argparse
import pprint

def setup_parser(arguments, title):

    parser = argparse.ArgumentParser(description=title,
                                     formatter_class=argparse.ArgumentDefaultsHelpFormatter)

    for key, val in arguments.items():
        parser.add_argument('-%s' % key,
                            type=eval(val["type"]),
                            help=val["help"],
                            default=val["default"])

    return parser


def read_params(parser):

    parameters = vars(parser.parse_args())
    return parameters

def get_parameters(title=None):

    with open("args.json") as data_file:
        data = json.load(data_file)
    parser = setup_parser(data, title)
    parameters = read_params(parser)

    return parameters

if __name__ == "__main__":
    params = get_parameters()
    pprint.pprint(params)

Answer 1

I don't like the eval here anywhere for the usual reasons. I think you have two better options here:

Find the right type within all builtin types

See https://stackoverflow.com/a/3222774/620382 (use builtins for python3)

Use the type of the default value from json.

Json types are properly translated to python. type(val['default']) will provide this type. You can check with __name__ == val['type'] if you want to keep the redundancy. Of course you then should make sure to use "default": 0.0 if the value should be float.

Answer 2

The use of a json file/expression as a source for defining argparse arguments is clever, and I think sound. ipython defines many of its commandline arguments from values in a config file.

I commented that we need to be careful about what type does. It's a frequent point of confusion. It is a callable, usually a function, that takes a string, and converts it to the kind of value you want in the args namespace. Its relation to builtin types and the the Python type function is only tangential.

int and float work because those functions take a string and return a desired value (or raise an error). bool does not because it does not take string and return a True/False (try bool('False').

argparse has an undocumentated registries mechanism that maps from strings to callables. Mostly it is used for the action parameter, but can also be used with type. Basically it's a dictionary.

Here's a quick session to illustrate its use.

In [2]: parser=argparse.ArgumentParser()

parser.register is an undocumented method, used internally, but available to the user (no _):

In [3]: parser.register?
Type:       instancemethod
...
File:       /usr/local/lib/python2.7/argparse.py
Definition: parser.register(self, registry_name, value, object)
Docstring:  <no docstring>

It sets an entry in a dictionary that the parser owns. Notice all the familiar action values. This maps from those strings to the Action subclasses. There is only one type entry, the default case that does nothing (leaves a string unchanged).

In [4]: parser._registries
Out[4]: 
{'action': {None: argparse._StoreAction,
  'append': argparse._AppendAction,
  'append_const': argparse._AppendConstAction,
  'count': argparse._CountAction,
  'help': argparse._HelpAction,
  ...
 'type': {None: <function argparse.identity>}}

So let's add some entries to the register:

In [5]: parser.register('type','float',float)
In [6]: parser.register('type','int',int)
In [7]: parser.register('type','str',None)

and create some Actions to use them:

In [8]: parser.add_argument('-i',type='int')
....
In [9]: parser.add_argument('-f',type='float')
...
In [10]: parser.add_argument('-s',type='str')
... 
ValueError: None is not callable

Oops, it didn't like my None. I was intending the None to map on to the existing None mapping in the registries, but it does not do that kind of double mapping.

I think the str() function will work. It takes a string and returns it unchanged. The argparse identity is more like a do nothing lambda x:x.

In [12]: str('astirng')
Out[12]: 'astirng'

In [13]: parser.register('type','str',str)

In [14]: parser.add_argument('-s',type='str')
Out[14]: _StoreAction(option_strings=['-s'], dest='s', nargs=None, const=None, default=None, type='str', choices=None, help=None, metavar=None)

Now let's test:

In [15]: parser.parse_args('-i 1 -f 23 -s 23'.split())
Out[15]: Namespace(f=23.0, i=1, s='23')

If you don't want to use register, I'd recommend a dictionary over eval. As another answer notes there aren't that many types, and you have more control over the translation.

A recent SO question suggests on potential difficulty with your json-argparse mapping. Not all Actions take a type parameter. Your set of parameters is fine for the defaultstoreaction. But astore_trueaction raises an error if you give it atype` parameter.

https://stackoverflow.com/questions/33574270/typeerror-init-got-an-unexpected-keyword-argument-type-in-argparse

---

I'd prefer calling add_argument with:

parser.add_argument(*args, **kwargs)

and build up args as a list, and kwargs as a dictionary. For example instead of :

for key, val in arguments.items():
    parser.add_argument('-%s' % key,
                        type=eval(val["type"]),
                        help=val["help"],
                        default=val["default"])

use something like:

for key, val in arguments.items():
    args = ['-%s'%key]
    kwargs = {<some defaults>}
    kwargs.update(val)
    parser.add_argument(*args, **kwargs)

This would allow me to use json strings like

'foo': {'action': 'store_true', 'help': 'True/False values'}, 'bar': {'type': 'int', 'default': 2}, 'baz': {'nargs': '+', 'help': 'multiple values'}

I haven't tested this yet, but if I've got it right, it would give more flexibility. Internally argparse uses **kwargs a lot to handle the large numbers of parameters that its methods take.

Answer 3

So as it seems that you want to get rid of eval, or as some people call it evil.

One way to do that is to use the builtins. The __builtins__ contains all the defalt types / function / classes in a normal Python script.
For example, if you fear that a function has been overwritten you can use __builtins__ to use the standard one.

>>> max = 'max'
>>> max
'max'
>>> __builtins__.max(1, 2)
2

However how can we use this to make your program safer?

First I checked the type of object str was. This way we know a way to filter the results.

However a lot of __builtins__ were an instance of type. And so I done a check on if things were new-style classes, which means that the inherit from object.

However it would also return every exception. And so you need to filter against if it is a sub-class of BaseException

>>> type(str)
<class 'type'>
>>> isinstance(str, type)
True
>>> issubclass(str, object)
True
>>> issubclass(str, BaseException)
False
>>> issubclass(BaseException, object)
True

And so to have safer types, you can make a dictionary comprehension. That filters by 2 things. The first to short circuit the second check.

allowed = {
    key: value
    for key, value in __builtins__.__dict__.items()
    if isinstance(value, type) and not issubclass(value, BaseException)
}

This allows some 'non-types' through, but it's pretty concise.
Some non-type:

• __loader__
• classmethod
• enumerate
• filter
• super

---

However just using __builtins__ has the drawback, that you can't use custom types. But there is a way to get passed that!

>>> class MyClass:pass
... 
>>> {
...     key: value
...     for key, value in globals().items()
...     if isinstance(value, type) and not issubclass(value, BaseException)
... }
{'MyClass': <class '__main__.MyClass'>, '__loader__': <class '_frozen_importlib.BuiltinImporter'>}

Note: __loader__ is not part of globals in files.

And so to get a way safer version of evil you can:

_globals = {}
_globals.update(globals())
_globals.update(__builtins__.__dict__)
allowed = {
    key: value
    for key, value in _globals.items()
    if isinstance(value, type) and not issubclass(value, BaseException)
}

---

Now onto the rest of your code.

You should limit the amount of characters per line. As here your argparse is going out of the screen. One way to ammend this is:

parser = argparse.ArgumentParser(
    description=title,
    formatter_class=argparse.ArgumentDefaultsHelpFormatter
)

You can use more characters, val is short for value, but the latter is easier to understand. It's not that big a deal, but it's a bit nicer.

You use the old style % format. This is advised against, due to bugs, and edge-cases. Instead use str.format. E.g. '-{}'.format(key)

You have too much white-space. You don't need a newline after every function definition, and you don't need one after every 'block'.
I would write set_parser as:

def setup_parser(arguments, title):
    parser = argparse.ArgumentParser(
        description=title,
        formatter_class=argparse.ArgumentDefaultsHelpFormatter
    )
    for key, value in arguments.items():
        parser.add_argument(
            '-%s' % key,
            type=allowed[value["type"],
            help=value["help"],
            default=value["default"]
        )
    return parser

Otherwise your code's pretty good.

Answer 4

Looks fine, except that you don't need to define a "Type"; we can pass type=type(value["default"]) instead, it will determine the type based on the default value:

def setup_parser(arguments, title):
parser = argparse.ArgumentParser(
    description=title,
    formatter_class=argparse.ArgumentDefaultsHelpFormatter
)
for key, value in arguments.items():
    parser.add_argument(
        '-%s' % key,
        type=type(value["default"]),
        help=value["help"],
        default=value["default"]
    )
return parser

Answer 5

If you truly don't have any security concerns, why not store the data in a python dict in a .py file that gets imported. Then "type" can be the actual type instead of a string--no need to map strings to types.

args.py:

config = {
       "dt": {
           "type": float,
           "help": "Time step size",
           "default": 0.4},
       "T": {
           "type": int,
           "help": "Time steps between outputs",
           "default": 50},
       "f": {
           "type": str,
           "help": "Landscape file name",
           "default": "small.dat"}
   }

main code:

import argparse
import pprint

import args          


def setup_parser(arguments, title):

    for key, val in arg.config.items():  
        parser.add_argument('-' + key, **val)

    return parser


def get_parameters(title=None):
    parser = setup_parser(data, title)
    parameters = vars(parser.parse_args())

    return parameters