From 2137d144be9538e14938ae4ec804ecaf630931c6 Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Thu, 6 Apr 2017 17:08:45 -0700 Subject: [PATCH 1/4] call super __init__ at end because instance_init calls are made in setup_instance --- traitlets/config/configurable.py | 11 +++-------- traitlets/config/tests/test_configurable.py | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py index bb7ebad7..0eafb4c3 100644 --- a/traitlets/config/configurable.py +++ b/traitlets/config/configurable.py @@ -77,9 +77,6 @@ def __init__(self, config=None): 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 @@ -94,11 +91,8 @@ def __init__(self, config=None): # 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. - for key, value in kwargs.items(): - setattr(self, key, value) + # load kwarg traits, other than config + super(Configurable, self).__init__(**kwargs) #------------------------------------------------------------------------- # Static trait notifiations @@ -173,6 +167,7 @@ def _load_config(self, cfg, section_names=None, traits=None): msg += u" Did you mean `{matches}`?".format(matches=matches[0]) elif len(matches) >= 1: msg +=" Did you mean one of: `{matches}`?".format(matches=', '.join(sorted(matches))) + print(msg) warn(msg) @observe('config') diff --git a/traitlets/config/tests/test_configurable.py b/traitlets/config/tests/test_configurable.py index b3ccae1b..70b3d3a3 100644 --- a/traitlets/config/tests/test_configurable.py +++ b/traitlets/config/tests/test_configurable.py @@ -6,6 +6,7 @@ import logging import sys +import warnings from unittest import TestCase from pytest import mark From bdd56188f7d936a5a23585483eac79a1eeb32778 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 11 Apr 2017 16:15:26 +0200 Subject: [PATCH 2/4] test config-kwarg order, priority --- traitlets/config/tests/test_configurable.py | 28 ++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/traitlets/config/tests/test_configurable.py b/traitlets/config/tests/test_configurable.py index 70b3d3a3..c6dc47bb 100644 --- a/traitlets/config/tests/test_configurable.py +++ b/traitlets/config/tests/test_configurable.py @@ -19,7 +19,7 @@ from traitlets.traitlets import ( Integer, Float, Unicode, List, Dict, Set, - _deprecations_shown, + _deprecations_shown, validate ) from traitlets.config.loader import Config @@ -459,6 +459,32 @@ def _config_default(self): d2 = DefaultConfigurable() self.assertIs(d2.config, single.config) self.assertEqual(d2.a, 5) + + def test_kwarg_config_priority(self): + # a, c set in kwargs + # a, b set in config + # verify that: + # - kwargs are set before config + # - kwargs have priority over config + class A(Configurable): + a = Unicode('default', config=True) + b = Unicode('default', config=True) + c = Unicode('default', config=True) + c_during_config = Unicode('never') + @validate('b') + def _record_c(self, proposal): + # setting b from config records c's value at the time + self.c_during_config = self.c + return proposal.value + + cfg = Config() + cfg.A.a = 'a-config' + cfg.A.b = 'b-config' + obj = A(a='a-kwarg', c='c-kwarg', config=cfg) + assert obj.a == 'a-kwarg' + assert obj.b == 'b-config' + assert obj.c == 'c-kwarg' + assert obj.c_during_config == 'c-kwarg' class TestLogger(TestCase): From 11dc842532b756e99e76bdb269a8c5bedc0c3cba Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 11 Apr 2017 16:16:13 +0200 Subject: [PATCH 3/4] config: only re-assign traits from constructor kwargs if overridded by config avoids *unnecessary* double-validation, but will be double-validated if loaded more than once. --- traitlets/config/configurable.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py index 0eafb4c3..f2afe50e 100644 --- a/traitlets/config/configurable.py +++ b/traitlets/config/configurable.py @@ -77,6 +77,20 @@ def __init__(self, config=None): config = kwargs.pop('config', None) + # load kwarg traits, other than config + super(Configurable, self).__init__(**kwargs) + + # record traits set by config + config_override_names = set() + def notice_config_override(change): + """Record traits set by both config and kwargs. + + They will need to be overridden again after loading config. + """ + if change.name in kwargs: + config_override_names.add(change.name) + self.observe(notice_config_override) + # load config if config is not None: # We used to deepcopy, but for now we are trying to just save @@ -90,9 +104,11 @@ def __init__(self, config=None): else: # allow _config_default to return something self._load_config(self.config) + self.unobserve(notice_config_override) + + for name in config_override_names: + setattr(self, name, kwargs[name]) - # load kwarg traits, other than config - super(Configurable, self).__init__(**kwargs) #------------------------------------------------------------------------- # Static trait notifiations From a536588105d0082569f8a1c483b8829175e29006 Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 4 May 2017 10:42:09 +0200 Subject: [PATCH 4/4] remove print --- traitlets/config/configurable.py | 1 - 1 file changed, 1 deletion(-) diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py index 387456f4..be3087f5 100644 --- a/traitlets/config/configurable.py +++ b/traitlets/config/configurable.py @@ -183,7 +183,6 @@ def _load_config(self, cfg, section_names=None, traits=None): msg += u" Did you mean `{matches}`?".format(matches=matches[0]) elif len(matches) >= 1: msg +=" Did you mean one of: `{matches}`?".format(matches=', '.join(sorted(matches))) - print(msg) warn(msg) @observe('config')