5
\$\begingroup\$

I am trying to parse the command line arguments using argparse, and if the user specifies a yaml file for the config-file, add those arguments to the args from argparse

import argparse
import yaml
from pprint import pprint


class CLI(object):

    def execute(self):
        self.create_parser()
        self.options = self.parse_args()
        pprint(self.options)

    def create_parser(self):
        self.parser = argparse.ArgumentParser()

        g = self.parser.add_argument_group('Device Targets')
        g.add_argument(
            '--config-file',
            dest='config_file',
            type=argparse.FileType(mode='r'))
        g.add_argument('--name', default=[], action='append')
        g.add_argument('--age', default=[], action='append')
        g.add_argument('--delay', type=int)
        g.add_argument('--stupid', dest='stupid', default=False, action='store_true')

    def parse_args(self):
        args = self.parser.parse_args()
        if args.config_file:
            data = yaml.load(args.config_file)
            delattr(args, 'config_file')
            for key, value in data.items():
                if isinstance(value, list):
                    for v in value:
                        getattr(args, key, []).append(v)
                else:
                    setattr(args, key, value)
        return args

cli = CLI()
cli.execute()

If my config-file has the following data:

name: [Jill, Bob]
age: [21, 33]
delay: 30

And I run my code like this:

python test.py --conf args.txt --name 'Mark'

I get this for the output:

Namespace(age=[21, 33], delay=30, name=['Mark', 'Jill', 'Bob'], stupid=False)

So, it works, but is it good code?

\$\endgroup\$
7
\$\begingroup\$

You don't need a class

In your current version of the code, it is hard to see what is stored in an instance and how the data flows around the different methods. If you do write a class, I find it clearer to have an init to define the different members but in your case, you could stop writing classes.

Removing the class, you get something like :

def execute():
    pprint(parse_args(create_parser()))

def create_parser():
    parser = argparse.ArgumentParser()

    g = parser.add_argument_group('Device Targets')
    g.add_argument(
        '--config-file',
        dest='config_file',
        type=argparse.FileType(mode='r'))
    g.add_argument('--name', default=[], action='append')
    g.add_argument('--age', default=[], action='append')
    g.add_argument('--delay', type=int)
    g.add_argument('--stupid', dest='stupid', default=False, action='store_true')
    return parser

def parse_args(parser):
    args = parser.parse_args()
    if args.config_file:
        data = yaml.load(args.config_file)
        delattr(args, 'config_file')
        arg_dict = args.__dict__
        for key, value in data.items():
            if isinstance(value, list):
                for v in value:
                    arg_dict[key].append(v)
            else:
                arg_dict[key] = value
    return args

execute()

The name execute probably needs to be improved for something more meaningful.

The right tool for the job

To add all the content from value to arg_dict[key], you shouldn't use àppend but extend.

        if isinstance(value, list):
            arg_dict[key].extend(value)
        else:
            arg_dict[key] = value
\$\endgroup\$
  • \$\begingroup\$ As you were posting your answer, I edited the question. I am now doing getattr(args, key, []).append(v) and setattr(args, key, value). But, I will change to using extend, didn't think about that :) Is using setattr and getattr better than reaching into the args and pulling out the dict? The class and function are actually part of a bigger project, this code was to test if it works. Thanks for your help \$\endgroup\$ – Mark Jan 29 '15 at 21:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.