Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

promote aliases and flags, to ensure they have priority over config f…

…iles

add Application.flatten_flags() method, which adjusts the alias
and flag dicts, such that they point to the subclass in the Application.classes list when passed to the argv parser.

This prevents TerminalInteractiveShell.colors in a config file overriding
`--colors` on the command-line, which points to InteractiveShell.colors.

Flattening is only done when the answer is unambiguous, so multiply inherited classes (e.g. Launchers in ipcluster) are not touched.

also remove now-obsolete manual workaround for this in IPClusterStart

closes gh-849
  • Loading branch information...
commit d7a105993fc639f205d474a7c6c13dc9e79834b3 1 parent 52688c2
@minrk authored
View
53 IPython/config/application.py
@@ -24,6 +24,7 @@
import re
import sys
from copy import deepcopy
+from collections import defaultdict
from IPython.config.configurable import SingletonConfigurable
from IPython.config.loader import (
@@ -329,6 +330,51 @@ def initialize_subcommand(self, subc, argv=None):
self.subapp = subapp.instance()
# and initialize subapp
self.subapp.initialize(argv)
+
+ def flatten_flags(self):
+ """flatten flags and aliases, so cl-args override as expected.
+
+ This prevents issues such as an alias pointing to InteractiveShell,
+ but a config file setting the same trait in TerminalInteraciveShell
+ getting inappropriate priority over the command-line arg.
+
+ Only aliases with exactly one descendent in the class list
+ will be promoted.
+
+ """
+ # build a tree of classes in our list that inherit from a particular
+ # it will be a dict by parent classname of classes in our list
+ # that are descendents
+ mro_tree = defaultdict(list)
+ for cls in self.classes:
+ clsname = cls.__name__
+ for parent in cls.mro()[1:-3]:
+ # exclude cls itself and Configurable,HasTraits,object
+ mro_tree[parent.__name__].append(clsname)
+ # flatten aliases, which have the form:
+ # { 'alias' : 'Class.trait' }
+ aliases = {}
+ for alias, cls_trait in self.aliases.iteritems():
+ cls,trait = cls_trait.split('.',1)
+ children = mro_tree[cls]
+ if len(children) == 1:
+ # exactly one descendent, promote alias
+ cls = children[0]
+ aliases[alias] = '.'.join([cls,trait])
+
+ # flatten flags, which are of the form:
+ # { 'key' : ({'Cls' : {'trait' : value}}, 'help')}
+ flags = {}
+ for key, (flagdict, help) in self.flags.iteritems():
+ newflag = {}
+ for cls, subdict in flagdict.iteritems():
+ children = mro_tree[cls]
+ # exactly one descendent, promote flag section
+ if len(children) == 1:
+ cls = children[0]
+ newflag[cls] = subdict
+ flags[key] = (newflag, help)
+ return flags, aliases
def parse_command_line(self, argv=None):
"""Parse the command line arguments."""
@@ -350,9 +396,12 @@ def parse_command_line(self, argv=None):
if '--version' in argv:
self.print_version()
self.exit(0)
+
+ # flatten flags&aliases, so cl-args get appropriate priority:
+ flags,aliases = self.flatten_flags()
- loader = KVArgParseConfigLoader(argv=argv, aliases=self.aliases,
- flags=self.flags)
+ loader = KVArgParseConfigLoader(argv=argv, aliases=aliases,
+ flags=flags)
try:
config = loader.load_config()
self.update_config(config)
View
33 IPython/config/tests/test_application.py
@@ -17,9 +17,11 @@
# Imports
#-----------------------------------------------------------------------------
+import logging
from unittest import TestCase
from IPython.config.configurable import Configurable
+from IPython.config.loader import Config
from IPython.config.application import (
Application
@@ -60,11 +62,14 @@ class MyApp(Application):
'j' : 'Foo.j',
'name' : 'Foo.name',
'enabled' : 'Bar.enabled',
- 'log-level' : 'MyApp.log_level',
+ 'log-level' : 'Application.log_level',
})
flags = Dict(dict(enable=({'Bar': {'enabled' : True}}, "Set Bar.enabled to True"),
- disable=({'Bar': {'enabled' : False}}, "Set Bar.enabled to False")))
+ disable=({'Bar': {'enabled' : False}}, "Set Bar.enabled to False"),
+ crit=({'Application' : {'log_level' : logging.CRITICAL}},
+ "set level=CRITICAL"),
+ ))
def init_foo(self):
self.foo = Foo(config=self.config)
@@ -129,6 +134,30 @@ def test_flag_clobber(self):
self.assertEquals(app.bar.enabled, True)
self.assertEquals(app.bar.b, 10)
+ def test_flatten_flags(self):
+ cfg = Config()
+ cfg.MyApp.log_level = logging.WARN
+ app = MyApp()
+ app.update_config(cfg)
+ self.assertEquals(app.log_level, logging.WARN)
+ self.assertEquals(app.config.MyApp.log_level, logging.WARN)
+ app.initialize(["--crit"])
+ self.assertEquals(app.log_level, logging.CRITICAL)
+ # this would be app.config.Application.log_level if it failed:
+ self.assertEquals(app.config.MyApp.log_level, logging.CRITICAL)
+
+ def test_flatten_aliases(self):
+ cfg = Config()
+ cfg.MyApp.log_level = logging.WARN
+ app = MyApp()
+ app.update_config(cfg)
+ self.assertEquals(app.log_level, logging.WARN)
+ self.assertEquals(app.config.MyApp.log_level, logging.WARN)
+ app.initialize(["--log-level", "CRITICAL"])
+ self.assertEquals(app.log_level, logging.CRITICAL)
+ # this would be app.config.Application.log_level if it failed:
+ self.assertEquals(app.config.MyApp.log_level, "CRITICAL")
+
def test_extra_args(self):
app = MyApp()
app.parse_command_line(["--Bar.b=5", 'extra', "--disable", 'args'])
View
6 IPython/parallel/apps/ipclusterapp.py
@@ -374,12 +374,6 @@ def start(self):
))
start_aliases['clean-logs'] = 'IPClusterStart.clean_logs'
-# set inherited Start keys directly, to ensure command-line args get higher priority
-# than config file options.
-for key,value in start_aliases.items():
- if value.startswith('IPClusterEngines'):
- start_aliases[key] = value.replace('IPClusterEngines', 'IPClusterStart')
-
class IPClusterStart(IPClusterEngines):
name = u'ipcluster'
Please sign in to comment.
Something went wrong with that request. Please try again.