Skip to content
This repository

promote aliases and flags, to ensure they have priority over config files #855

Closed
wants to merge 1 commit into from

2 participants

Min RK Fernando Perez
Min RK
Owner

See #849

adds 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 removes now-obsolete manual workaround for exactly this in IPClusterStart

closes gh-849

Min RK 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
d7a1059
Fernando Perez
Owner

Great, thanks! Merged with rebase to avoid recursive merge for just one commit, closing.

Fernando Perez fperez closed this October 13, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Oct 10, 2011
Min RK 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
d7a1059
This page is out of date. Refresh to see the latest.
53  IPython/config/application.py
@@ -24,6 +24,7 @@
24 24
 import re
25 25
 import sys
26 26
 from copy import deepcopy
  27
+from collections import defaultdict
27 28
 
28 29
 from IPython.config.configurable import SingletonConfigurable
29 30
 from IPython.config.loader import (
@@ -329,6 +330,51 @@ def initialize_subcommand(self, subc, argv=None):
329 330
         self.subapp = subapp.instance()
330 331
         # and initialize subapp
331 332
         self.subapp.initialize(argv)
  333
+    
  334
+    def flatten_flags(self):
  335
+        """flatten flags and aliases, so cl-args override as expected.
  336
+        
  337
+        This prevents issues such as an alias pointing to InteractiveShell,
  338
+        but a config file setting the same trait in TerminalInteraciveShell
  339
+        getting inappropriate priority over the command-line arg.
  340
+
  341
+        Only aliases with exactly one descendent in the class list
  342
+        will be promoted.
  343
+        
  344
+        """
  345
+        # build a tree of classes in our list that inherit from a particular
  346
+        # it will be a dict by parent classname of classes in our list
  347
+        # that are descendents
  348
+        mro_tree = defaultdict(list)
  349
+        for cls in self.classes:
  350
+            clsname = cls.__name__
  351
+            for parent in cls.mro()[1:-3]:
  352
+                # exclude cls itself and Configurable,HasTraits,object
  353
+                mro_tree[parent.__name__].append(clsname)
  354
+        # flatten aliases, which have the form:
  355
+        # { 'alias' : 'Class.trait' }
  356
+        aliases = {}
  357
+        for alias, cls_trait in self.aliases.iteritems():
  358
+            cls,trait = cls_trait.split('.',1)
  359
+            children = mro_tree[cls]
  360
+            if len(children) == 1:
  361
+                # exactly one descendent, promote alias
  362
+                cls = children[0]
  363
+            aliases[alias] = '.'.join([cls,trait])
  364
+        
  365
+        # flatten flags, which are of the form:
  366
+        # { 'key' : ({'Cls' : {'trait' : value}}, 'help')}
  367
+        flags = {}
  368
+        for key, (flagdict, help) in self.flags.iteritems():
  369
+            newflag = {}
  370
+            for cls, subdict in flagdict.iteritems():
  371
+                children = mro_tree[cls]
  372
+                # exactly one descendent, promote flag section
  373
+                if len(children) == 1:
  374
+                    cls = children[0]
  375
+                newflag[cls] = subdict
  376
+            flags[key] = (newflag, help)
  377
+        return flags, aliases
332 378
 
333 379
     def parse_command_line(self, argv=None):
334 380
         """Parse the command line arguments."""
@@ -350,9 +396,12 @@ def parse_command_line(self, argv=None):
350 396
         if '--version' in argv:
351 397
             self.print_version()
352 398
             self.exit(0)
  399
+        
  400
+        # flatten flags&aliases, so cl-args get appropriate priority:
  401
+        flags,aliases = self.flatten_flags()
353 402
 
354  
-        loader = KVArgParseConfigLoader(argv=argv, aliases=self.aliases,
355  
-                                        flags=self.flags)
  403
+        loader = KVArgParseConfigLoader(argv=argv, aliases=aliases,
  404
+                                        flags=flags)
356 405
         try:
357 406
             config = loader.load_config()
358 407
             self.update_config(config)
33  IPython/config/tests/test_application.py
@@ -17,9 +17,11 @@
17 17
 # Imports
18 18
 #-----------------------------------------------------------------------------
19 19
 
  20
+import logging
20 21
 from unittest import TestCase
21 22
 
22 23
 from IPython.config.configurable import Configurable
  24
+from IPython.config.loader import Config
23 25
 
24 26
 from IPython.config.application import (
25 27
     Application
@@ -60,11 +62,14 @@ class MyApp(Application):
60 62
                     'j' : 'Foo.j',
61 63
                     'name' : 'Foo.name',
62 64
                     'enabled' : 'Bar.enabled',
63  
-                    'log-level' : 'MyApp.log_level',
  65
+                    'log-level' : 'Application.log_level',
64 66
                 })
65 67
     
66 68
     flags = Dict(dict(enable=({'Bar': {'enabled' : True}}, "Set Bar.enabled to True"),
67  
-                  disable=({'Bar': {'enabled' : False}}, "Set Bar.enabled to False")))
  69
+                  disable=({'Bar': {'enabled' : False}}, "Set Bar.enabled to False"),
  70
+                  crit=({'Application' : {'log_level' : logging.CRITICAL}},
  71
+                        "set level=CRITICAL"),
  72
+            ))
68 73
     
69 74
     def init_foo(self):
70 75
         self.foo = Foo(config=self.config)
@@ -129,6 +134,30 @@ def test_flag_clobber(self):
129 134
         self.assertEquals(app.bar.enabled, True)
130 135
         self.assertEquals(app.bar.b, 10)
131 136
     
  137
+    def test_flatten_flags(self):
  138
+        cfg = Config()
  139
+        cfg.MyApp.log_level = logging.WARN
  140
+        app = MyApp()
  141
+        app.update_config(cfg)
  142
+        self.assertEquals(app.log_level, logging.WARN)
  143
+        self.assertEquals(app.config.MyApp.log_level, logging.WARN)
  144
+        app.initialize(["--crit"])
  145
+        self.assertEquals(app.log_level, logging.CRITICAL)
  146
+        # this would be app.config.Application.log_level if it failed:
  147
+        self.assertEquals(app.config.MyApp.log_level, logging.CRITICAL)
  148
+    
  149
+    def test_flatten_aliases(self):
  150
+        cfg = Config()
  151
+        cfg.MyApp.log_level = logging.WARN
  152
+        app = MyApp()
  153
+        app.update_config(cfg)
  154
+        self.assertEquals(app.log_level, logging.WARN)
  155
+        self.assertEquals(app.config.MyApp.log_level, logging.WARN)
  156
+        app.initialize(["--log-level", "CRITICAL"])
  157
+        self.assertEquals(app.log_level, logging.CRITICAL)
  158
+        # this would be app.config.Application.log_level if it failed:
  159
+        self.assertEquals(app.config.MyApp.log_level, "CRITICAL")
  160
+    
132 161
     def test_extra_args(self):
133 162
         app = MyApp()
134 163
         app.parse_command_line(["--Bar.b=5", 'extra', "--disable", 'args'])
6  IPython/parallel/apps/ipclusterapp.py
@@ -374,12 +374,6 @@ def start(self):
374 374
 ))
375 375
 start_aliases['clean-logs'] = 'IPClusterStart.clean_logs'
376 376
 
377  
-# set inherited Start keys directly, to ensure command-line args get higher priority
378  
-# than config file options.
379  
-for key,value in start_aliases.items():
380  
-    if value.startswith('IPClusterEngines'):
381  
-        start_aliases[key] = value.replace('IPClusterEngines', 'IPClusterStart')
382  
-
383 377
 class IPClusterStart(IPClusterEngines):
384 378
 
385 379
     name = u'ipcluster'
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.