From 00050dfbaba7dac149576233e4d23d9eac05e46b 2010-01-30 18:17:12 From: Brian Granger Date: 2010-01-30 18:17:12 Subject: [PATCH] Work on the config system. * Removed ``NoConfigDefault`` from our config logic entirely. It turns out that argparse has a nice ``argparse.SUPPRESS`` object that handles this nicely. * Completely cleared the previous config when :meth:`load_config` is called to allow it to be called more than once. * Added some additional tests and docs for the config system. --- diff --git a/IPython/config/loader.py b/IPython/config/loader.py index 0b521a9..98e13e7 100644 --- a/IPython/config/loader.py +++ b/IPython/config/loader.py @@ -40,6 +40,7 @@ class ConfigLoaderError(ConfigError): #----------------------------------------------------------------------------- # Argparse fix #----------------------------------------------------------------------------- + # Unfortunately argparse by default prints help messages to stderr instead of # stdout. This makes it annoying to capture long help screens at the command # line, since one must know how to pipe stderr, which many users don't know how @@ -200,10 +201,13 @@ class ConfigLoader(object): self.config = Config() def load_config(self): - """Load a config from somewhere, return a Struct. + """Load a config from somewhere, return a :class:`Config` instance. Usually, this will cause self.config to be set and then returned. + However, in most cases, :meth:`ConfigLoader.clear` should be called + to erase any previous state. """ + self.clear() return self.config @@ -242,6 +246,7 @@ class PyFileConfigLoader(FileConfigLoader): def load_config(self): """Load the config from a file and return it as a Struct.""" + self.clear() self._find_file() self._read_file_as_dict() self._convert_to_config() @@ -292,20 +297,10 @@ class CommandLineConfigLoader(ConfigLoader): """ -class __NoConfigDefault(object): pass -NoConfigDefault = __NoConfigDefault() - - class ArgParseConfigLoader(CommandLineConfigLoader): - #: Global default for arguments (see argparse docs for details) - argument_default = NoConfigDefault - - def __init__(self, argv=None, arguments=(), *args, **kw): - """Create a config loader for use with argparse. - With the exception of ``argv`` and ``arguments``, other args and kwargs - arguments here are passed onto the constructor of - :class:`argparse.ArgumentParser`. + def __init__(self, argv=None, arguments=(), *parser_args, **parser_kw): + """Create a config loader for use with argparse. Parameters ---------- @@ -315,18 +310,27 @@ class ArgParseConfigLoader(CommandLineConfigLoader): sys.argv[1:] is used. arguments : optional, tuple - Description of valid command-line arguments, to be called in sequence - with parser.add_argument() to configure the parser. + A tuple of two element tuples each having the form (args, kwargs). + Each such pair is passed to parser.add_argument(*args, **kwargs) + in sequence to configure the parser. + + parser_args : tuple + A tuple of positional arguments that will be passed to the + constructor of :class:`argparse.ArgumentParser`. + + parser_kw : dict + A tuple of keyword arguments that will be passed to the + constructor of :class:`argparse.ArgumentParser`. """ super(CommandLineConfigLoader, self).__init__() if argv == None: argv = sys.argv[1:] self.argv = argv self.arguments = arguments - self.args = args - kwargs = dict(argument_default=self.argument_default) - kwargs.update(kw) - self.kw = kwargs + self.parser_args = parser_args + kwargs = dict(argument_default=argparse.SUPPRESS) + kwargs.update(parser_kw) + self.parser_kw = kwargs def load_config(self, args=None): """Parse command line arguments and return as a Struct. @@ -335,10 +339,10 @@ class ArgParseConfigLoader(CommandLineConfigLoader): ---------- args : optional, list - If given, a list with the structure of sys.argv[1:] to parse arguments - from. If not given, the instance's self.argv attribute (given at - construction time) is used.""" - + If given, a list with the structure of sys.argv[1:] to parse + arguments from. If not given, the instance's self.argv attribute + (given at construction time) is used.""" + self.clear() if args is None: args = self.argv self._create_parser() @@ -353,13 +357,17 @@ class ArgParseConfigLoader(CommandLineConfigLoader): return [] def _create_parser(self): - self.parser = ArgumentParser(*self.args, **self.kw) + self.parser = ArgumentParser(*self.parser_args, **self.parser_kw) self._add_arguments() self._add_other_arguments() def _add_arguments(self): for argument in self.arguments: - self.parser.add_argument(*argument[0],**argument[1]) + # Remove any defaults in case people add them. We can't have + # command line default because all default are determined by + # traited class attributes. + argument[1].pop('default', None) + self.parser.add_argument(*argument[0], **argument[1]) def _add_other_arguments(self): """Meant for subclasses to add their own arguments.""" @@ -372,6 +380,7 @@ class ArgParseConfigLoader(CommandLineConfigLoader): def _convert_to_config(self): """self.parsed_data->self.config""" for k, v in vars(self.parsed_data).items(): - if v is not NoConfigDefault: - exec_str = 'self.config.' + k + '= v' - exec exec_str in locals(), globals() + exec_str = 'self.config.' + k + '= v' + exec exec_str in locals(), globals() + + diff --git a/IPython/config/tests/test_loader.py b/IPython/config/tests/test_loader.py index 487db8b..1276eb5 100755 --- a/IPython/config/tests/test_loader.py +++ b/IPython/config/tests/test_loader.py @@ -62,22 +62,26 @@ class TestPyFileCL(TestCase): self.assertEquals(config.D.C.value, 'hi there') +arguments = ( + (('-f','--foo'), dict(dest='Global.foo', type=str)), + (('-b',), dict(dest='MyClass.bar', type=int)), + (('-n',), dict(dest='n', action='store_true')), + (('Global.bam',), dict(type=str)) + ) + class TestArgParseCL(TestCase): def test_basic(self): - - arguments = ( - (('-f','--foo'), dict(dest='Global.foo', type=str)), - (('-b',), dict(dest='MyClass.bar', type=int)), - (('-n',), dict(dest='n', action='store_true')), - (('Global.bam',), dict(type=str)) - ) cl = ArgParseConfigLoader(arguments=arguments) config = cl.load_config('-f hi -b 10 -n wow'.split()) self.assertEquals(config.Global.foo, 'hi') self.assertEquals(config.MyClass.bar, 10) self.assertEquals(config.n, True) self.assertEquals(config.Global.bam, 'wow') + config = cl.load_config(['wow']) + self.assertEquals(config.keys(), ['Global']) + self.assertEquals(config.Global.keys(), ['bam']) + self.assertEquals(config.Global.bam, 'wow') def test_add_arguments(self): @@ -97,6 +101,18 @@ class TestArgParseCL(TestCase): self.assertEquals(config.subparser_name, '1') self.assertEquals(config.Global.x, 'frobble') + def test_argv(self): + cl = ArgParseConfigLoader( + argv='-f hi -b 10 -n wow'.split(), + arguments=arguments + ) + config = cl.load_config() + self.assertEquals(config.Global.foo, 'hi') + self.assertEquals(config.MyClass.bar, 10) + self.assertEquals(config.n, True) + self.assertEquals(config.Global.bam, 'wow') + + class TestConfig(TestCase): def test_setget(self): diff --git a/IPython/core/ipapp.py b/IPython/core/ipapp.py index ce0e9db..01cd58f 100755 --- a/IPython/core/ipapp.py +++ b/IPython/core/ipapp.py @@ -32,8 +32,7 @@ from IPython.core.application import Application from IPython.core.iplib import InteractiveShell from IPython.config.loader import ( Config, - PyFileConfigLoader, -# NoConfigDefault, + PyFileConfigLoader ) from IPython.lib import inputhook from IPython.utils.path import filefind, get_ipython_dir diff --git a/IPython/kernel/ipclusterapp.py b/IPython/kernel/ipclusterapp.py index c7c22c5..c19bcc0 100755 --- a/IPython/kernel/ipclusterapp.py +++ b/IPython/kernel/ipclusterapp.py @@ -23,8 +23,8 @@ if os.name=='posix': from twisted.scripts._twistd_unix import daemonize from IPython.core import release -from IPython.external.argparse import ArgumentParser -from IPython.config.loader import ArgParseConfigLoader, NoConfigDefault +from IPython.external.argparse import ArgumentParser, SUPPRESS +from IPython.config.loader import ArgParseConfigLoader from IPython.utils.importstring import import_item from IPython.kernel.clusterdir import ( @@ -56,7 +56,7 @@ class IPClusterCLLoader(ArgParseConfigLoader): def _add_other_arguments(self): # This has all the common options that all subcommands use parent_parser1 = ArgumentParser(add_help=False, - argument_default=NoConfigDefault) + argument_default=SUPPRESS) parent_parser1.add_argument('--ipython-dir', dest='Global.ipython_dir',type=unicode, help='Set to override default location of Global.ipython_dir.', @@ -68,7 +68,7 @@ class IPClusterCLLoader(ArgParseConfigLoader): # This has all the common options that other subcommands use parent_parser2 = ArgumentParser(add_help=False, - argument_default=NoConfigDefault) + argument_default=SUPPRESS) parent_parser2.add_argument('-p','--profile', dest='Global.profile',type=unicode, help='The string name of the profile to be used. This determines ' @@ -112,7 +112,6 @@ class IPClusterCLLoader(ArgParseConfigLoader): parser_create.add_argument( '--reset-config', dest='Global.reset_config', action='store_true', - default=NoConfigDefault, help='Recopy the default config files to the cluster directory. ' 'You will loose any modifications you have made to these files.' )