Repository: ipython/traitlets
Pull request #289 (2016-08-24 09:03:25)

config: generate sample-config files containing all and only classes relevant to configurable traits
The motivation is the user to know the relationships between the classes
in the generated sample configuration-file, so that he can choose at
which inheritance-level to override some trait value.

ankostis: Rational: This PR ensures that the sample-config file will describe to the user AND contain all the classes on which a trait-value may be overridden:
- either on the class owning the trait,
- or on its subclasses, even if those subclasses do not define any traits themselves.

ankostis: Attaching generated configs to facilitate review.

## With this PR:
```python
#------------------------------------------------------------------------------
# Application(SingletonConfigurable) configuration
#------------------------------------------------------------------------------

# This is an application.

# The date format used by logging formatters for %(asctime)s
# c.Application.log_datefmt = '%Y-%m-%d %H:%M:%S'

# The Logging format template
# c.Application.log_format = '[%(name)s]%(highlevel)s %(message)s'

# Set the log level by value or name.
# c.Application.log_level = 30

#------------------------------------------------------------------------------
# MyApp(Application) configuration
#------------------------------------------------------------------------------

# This is an application.

# Load this config file
# c.MyApp.config_file = ''

# Is the app running?
# c.MyApp.running = False

# Should print a warning if `MyApp.warn-typo=...` command is passed
# c.MyApp.warn_tpyo = 'yes the name is wrong on purpose'

#------------------------------------------------------------------------------
# Bar(Configurable) configuration
#------------------------------------------------------------------------------

# The integer b.
# c.Bar.b = 0

# Enable bar.
# c.Bar.enabled = True

#------------------------------------------------------------------------------
# Foo(Configurable) configuration
#------------------------------------------------------------------------------

# The integer i.
# c.Foo.i = 0

# The integer j.
# c.Foo.j = 1

# First name.
# c.Foo.name = 'Brian'

#------------------------------------------------------------------------------
# NoTraits(Foo,Bar) configuration
#------------------------------------------------------------------------------
```

## Without this PR:

```python
#------------------------------------------------------------------------------
# Application configuration
#------------------------------------------------------------------------------

# This is an application.

# The date format used by logging formatters for %(asctime)s
# c.Application.log_datefmt = '%Y-%m-%d %H:%M:%S'

# The Logging format template
# c.Application.log_format = '[%(name)s]%(highlevel)s %(message)s'

# Set the log level by value or name.
# c.Application.log_level = 30

#------------------------------------------------------------------------------
# MyApp configuration
#------------------------------------------------------------------------------

# This is an application.

# Load this config file
# c.MyApp.config_file = ''

# Is the app running?
# c.MyApp.running = False

# Should print a warning if `MyApp.warn-typo=...` command is passed
# c.MyApp.warn_tpyo = 'yes the name is wrong on purpose'

#------------------------------------------------------------------------------
# Bar configuration
#------------------------------------------------------------------------------

# The integer b.
# c.Bar.b = 0

# Enable bar.
# c.Bar.enabled = True

#------------------------------------------------------------------------------
# Foo configuration
#------------------------------------------------------------------------------

# The integer i.
# c.Foo.i = 0

# The integer j.
# c.Foo.j = 1

# First name.
# c.Foo.name = 'Brian'
```
ankostis: Updated the sample above to compare the new "demarcation" commit.

Patch:
diff --git a/traitlets/config/application.py b/traitlets/config/application.py
index 3b9b571..a373124 100644
--- a/traitlets/config/application.py
+++ b/traitlets/config/application.py
@@ -12,7 +12,7 @@ import logging
 import os
 import re
 import sys
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 
 from decorator import decorator
 
@@ -597,14 +597,43 @@ class Application(SingletonConfigurable):
                 ))
 
 
+    def _classes_in_config_sample(self):
+        """
+        Yields only classes with own traits, and their subclasses.
+
+        Thus, produced sample config-file will contain all classes
+        on which a trait-value may be overridden:
+
+        - either on the class owning the trait,
+        - or on its subclasses, even if those subclasses do not define
+          any traits themselves.
+        """
+        cls_to_config = OrderedDict( (cls, bool(cls.class_own_traits(config=True)))
+                              for cls
+                              in self._classes_inc_parents())
+
+        def is_any_parent_included(cls):
+            return any(b in cls_to_config and cls_to_config[b] for b in cls.__bases__)
+
+        ## Mark "empty" classes for inclusion if their parents own-traits,
+        #  and loop until no more classes gets marked.
+        #
+        while True:
+            to_incl_orig = cls_to_config.copy()
+            cls_to_config = OrderedDict( (cls, inc_yes or is_any_parent_included(cls))
+                                  for cls, inc_yes
+                                  in cls_to_config.items())
+            if cls_to_config == to_incl_orig:
+                break
+        for cl, inc_yes in cls_to_config.items():
+            if inc_yes:
+                yield cl
+
     def generate_config_file(self):
         """generate default config file from Configurables"""
         lines = ["# Configuration file for %s." % self.name]
         lines.append('')
-        for cls in self._classes_inc_parents():
-            if not cls.class_own_traits(config=True):
-                # skip classes with no config (Singleton, etc.)
-                continue
+        for cls in self._classes_in_config_sample():
             lines.append(cls.class_config_section())
         return '\n'.join(lines)
 
diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py
index 1094800..1a3fc0e 100644
--- a/traitlets/config/configurable.py
+++ b/traitlets/config/configurable.py
@@ -66,12 +66,12 @@ class Configurable(HasTraits):
             if kwargs.get('config', None) is None:
                 kwargs['config'] = parent.config
             self.parent = parent
-        
+
         config = kwargs.pop('config', None)
-        
+
         # load kwarg traits, other than config
         super(Configurable, self).__init__(**kwargs)
-        
+
         # load config
         if config is not None:
             # We used to deepcopy, but for now we are trying to just save
@@ -85,7 +85,7 @@ class Configurable(HasTraits):
         else:
             # allow _config_default to return something
             self._load_config(self.config)
-        
+
         # Ensure explicit kwargs are applied after loading config.
         # This is usually redundant, but ensures config doesn't override
         # explicitly assigned values.
@@ -95,25 +95,25 @@ class Configurable(HasTraits):
     #-------------------------------------------------------------------------
     # Static trait notifiations
     #-------------------------------------------------------------------------
-    
+
     @classmethod
     def section_names(cls):
         """return section names as a list"""
         return  [c.__name__ for c in reversed(cls.__mro__) if
             issubclass(c, Configurable) and issubclass(cls, c)
         ]
-    
+
     def _find_my_config(self, cfg):
         """extract my config from a global Config object
-        
+
         will construct a Config object of only the config values that apply to me
         based on my mro(), as well as those of my parent(s) if they exist.
-        
+
         If I am Bar and my parent is Foo, and their parent is Tim,
         this will return merge following config sections, in this order::
-        
+
             [Bar, Foo.bar, Tim.Foo.Bar]
-        
+
         With the last item being the highest priority.
         """
         cfgs = [cfg]
@@ -127,17 +127,17 @@ class Configurable(HasTraits):
                 if c._has_section(sname):
                     my_config.merge(c[sname])
         return my_config
-    
+
     def _load_config(self, cfg, section_names=None, traits=None):
         """load traits from a Config object"""
-        
+
         if traits is None:
             traits = self.traits(config=True)
         if section_names is None:
             section_names = self.section_names()
-        
+
         my_config = self._find_my_config(cfg)
-        
+
         # hold trait notifications until after all config has been loaded
         with self.hold_trait_notifications():
             for name, config_value in my_config.items():
@@ -160,7 +160,7 @@ class Configurable(HasTraits):
                     matches = get_close_matches(name, traits)
                     msg = u"Config option `{option}` not recognized by `{klass}`.".format(
                         option=name, klass=self.__class__.__name__)
-                    
+
                     if len(matches) == 1:
                         msg += u"  Did you mean `{matches}`?".format(matches=matches[0])
                     elif len(matches) >= 1:
@@ -220,7 +220,7 @@ class Configurable(HasTraits):
     @classmethod
     def class_get_trait_help(cls, trait, inst=None):
         """Get the help string for a single trait.
-        
+
         If `inst` is given, it's current trait values will be used in place of
         the class default.
         """
@@ -261,11 +261,12 @@ class Configurable(HasTraits):
             """return a commented, wrapped block."""
             s = '\n\n'.join(wrap_paragraphs(s, 78))
 
-            return '# ' + s.replace('\n', '\n# ')
+            return '## ' + s.replace('\n', '\n#  ')
 
         # section header
         breaker = '#' + '-'*78
-        s = "# %s configuration" % cls.__name__
+        parent_classes = ','.join(p.__name__ for p in cls.__bases__)
+        s = "# %s(%s) configuration" % (cls.__name__, parent_classes)
         lines = [breaker, s, breaker, '']
         # get the description trait
         desc = cls.class_traits().get('description')
@@ -280,7 +281,7 @@ class Configurable(HasTraits):
 
         for name, trait in sorted(cls.class_own_traits(config=True).items()):
             lines.append(c(trait.help))
-            lines.append('# c.%s.%s = %s' % (cls.__name__, name, trait.default_value_repr()))
+            lines.append('#c.%s.%s = %s' % (cls.__name__, name, trait.default_value_repr()))
             lines.append('')
         return '\n'.join(lines)
 


Test patch:
diff --git a/traitlets/config/tests/test_application.py b/traitlets/config/tests/test_application.py
index 844f21c..2a1caf2 100644
--- a/traitlets/config/tests/test_application.py
+++ b/traitlets/config/tests/test_application.py
@@ -64,13 +64,13 @@ class MyApp(Application):
                     'enabled' : 'Bar.enabled',
                     '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"),
                   crit=({'Application' : {'log_level' : logging.CRITICAL}},
                         "set level=CRITICAL"),
             ))
-    
+
     def init_foo(self):
         self.foo = Foo(parent=self)
 
@@ -115,7 +115,7 @@ class TestApplication(TestCase):
         self.assertEqual(app.foo.i, 10)
         self.assertEqual(app.foo.j, 10)
         self.assertEqual(app.bar.enabled, False)
-    
+
     def test_cli_priority(self):
         """Test that loading config files does not override CLI options"""
         name = 'config.py'
@@ -185,7 +185,7 @@ class TestApplication(TestCase):
         app.parse_command_line(["--enable"])
         app.init_bar()
         self.assertEqual(app.bar.enabled, True)
-    
+
     def test_aliases(self):
         app = MyApp()
         app.parse_command_line(["--i=5", "--j=10"])
@@ -193,7 +193,7 @@ class TestApplication(TestCase):
         self.assertEqual(app.foo.i, 5)
         app.init_foo()
         self.assertEqual(app.foo.j, 10)
-    
+
     def test_flag_clobber(self):
         """test that setting flags doesn't clobber existing settings"""
         app = MyApp()
@@ -217,8 +217,8 @@ class TestApplication(TestCase):
 
         self.assertIn("warn_typo", stream.getvalue())
         self.assertIn("warn_tpyo", stream.getvalue())
-        
-    
+
+
     def test_flatten_flags(self):
         cfg = Config()
         cfg.MyApp.log_level = logging.WARN
@@ -230,7 +230,7 @@ class TestApplication(TestCase):
         self.assertEqual(app.log_level, logging.CRITICAL)
         # this would be app.config.Application.log_level if it failed:
         self.assertEqual(app.config.MyApp.log_level, logging.CRITICAL)
-    
+
     def test_flatten_aliases(self):
         cfg = Config()
         cfg.MyApp.log_level = logging.WARN
@@ -242,7 +242,7 @@ class TestApplication(TestCase):
         self.assertEqual(app.log_level, logging.CRITICAL)
         # this would be app.config.Application.log_level if it failed:
         self.assertEqual(app.config.MyApp.log_level, "CRITICAL")
-    
+
     def test_extra_args(self):
         app = MyApp()
         app.parse_command_line(["--Bar.b=5", 'extra', "--disable", 'args'])
@@ -256,7 +256,7 @@ class TestApplication(TestCase):
         self.assertEqual(app.bar.enabled, True)
         self.assertEqual(app.bar.b, 5)
         self.assertEqual(app.extra_args, ['extra', '--disable', 'args'])
-    
+
     def test_unicode_argv(self):
         app = MyApp()
         app.parse_command_line(['ünîcødé'])
@@ -268,7 +268,20 @@ class TestApplication(TestCase):
     def test_generate_config_file(self):
         app = MyApp()
         assert 'The integer b.' in app.generate_config_file()
-    
+
+    def test_generate_config_file_classes_to_include(self):
+        class NoTraits(Foo, Bar):
+            pass
+
+        app = MyApp()
+        app.classes.append(NoTraits)
+        conf_txt = app.generate_config_file()
+        self.assertIn('The integer b.', conf_txt)
+        self.assertIn('# Bar(Configurable)', conf_txt)
+        self.assertIn('# Foo(Configurable)', conf_txt)
+        self.assertNotIn('# Configurable', conf_txt)
+        self.assertIn('# NoTraits(Foo,Bar)', conf_txt)
+
     def test_multi_file(self):
         app = MyApp()
         app.log = logging.getLogger()
@@ -285,7 +298,7 @@ class TestApplication(TestCase):
                 app.load_config_file(name, path=[td1, td2])
                 app.init_bar()
                 self.assertEqual(app.bar.b, 1)
-    
+
     @mark.skipif(not hasattr(TestCase, 'assertLogs'), reason='requires TestCase.assertLogs')
     def test_log_bad_config(self):
         app = MyApp()
@@ -298,7 +311,7 @@ class TestApplication(TestCase):
                 app.load_config_file(name, path=[td])
         output = '\n'.join(captured.output)
         self.assertIn('SyntaxError', output)
-    
+
     def test_raise_on_bad_config(self):
         app = MyApp()
         app.raise_config_file_errors = True