From 872b77a04eada6237d7c116b3eac6756d2872c4a Mon Sep 17 00:00:00 2001 From: Min RK Date: Thu, 15 Oct 2020 11:35:21 +0200 Subject: [PATCH] improve backward-compatibility for Container subclasses with default_value=None --- traitlets/tests/test_traitlets.py | 20 ++++++++++ traitlets/traitlets.py | 63 +++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 11 deletions(-) diff --git a/traitlets/tests/test_traitlets.py b/traitlets/tests/test_traitlets.py index 0680ab86..595f75b0 100644 --- a/traitlets/tests/test_traitlets.py +++ b/traitlets/tests/test_traitlets.py @@ -1686,6 +1686,26 @@ class C(HasTraits): assert c.t == default_value +@pytest.mark.parametrize( + "Trait, default_value", + ((List, []), (Tuple, ()), (Set, set())), +) +def test_subclass_default_value(Trait, default_value): + """Test deprecated default_value=None behavior for Container subclass traits""" + + class SubclassTrait(Trait): + def __init__(self, default_value=None): + super().__init__(default_value=default_value) + + class C(HasTraits): + t = SubclassTrait() + + # test default value + c = C() + assert type(c.t) is type(default_value) + assert c.t == default_value + + class CRegExpTrait(HasTraits): value = CRegExp(r'') diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py index 2cd14530..20355904 100644 --- a/traitlets/traitlets.py +++ b/traitlets/traitlets.py @@ -2407,6 +2407,7 @@ class Container(Instance): To be subclassed by overriding klass. """ + klass = None _cast_types = () _valid_defaults = SequenceTypes @@ -2441,11 +2442,25 @@ def __init__(self, trait=None, default_value=Undefined, **kwargs): further keys for extensions to the Trait (e.g. config) """ + # allow List([values]): if trait is not None and default_value is Undefined and not is_trait(trait): default_value = trait trait = None + if default_value is None and not kwargs.get("allow_none", False): + # improve backward-compatibility for possible subclasses + # specifying default_value=None as default, + # keeping 'unspecified' behavior (i.e. empty container) + warn( + f"Specifying {self.__class__.__name__}(default_value=None)" + " for no default is deprecated in traitlets 5.0.5." + " Use default_value=Undefined", + DeprecationWarning, + stacklevel=2, + ) + default_value = Undefined + if default_value is Undefined: args = () elif default_value is None: @@ -2455,16 +2470,23 @@ def __init__(self, trait=None, default_value=Undefined, **kwargs): elif isinstance(default_value, self._valid_defaults): args = (default_value,) else: - raise TypeError('default value of %s was %s' %(self.__class__.__name__, default_value)) + raise TypeError( + "default value of %s was %s" % (self.__class__.__name__, default_value) + ) if is_trait(trait): if isinstance(trait, type): - warn("Traits should be given as instances, not types (for example, `Int()`, not `Int`)." - " Passing types is deprecated in traitlets 4.1.", - DeprecationWarning, stacklevel=3) + warn( + "Traits should be given as instances, not types (for example, `Int()`, not `Int`)." + " Passing types is deprecated in traitlets 4.1.", + DeprecationWarning, + stacklevel=3, + ) self._trait = trait() if isinstance(trait, type) else trait elif trait is not None: - raise TypeError("`trait` must be a Trait or None, got %s" % repr_type(trait)) + raise TypeError( + "`trait` must be a Trait or None, got %s" % repr_type(trait) + ) super(Container, self).__init__(klass=self.klass, args=args, **kwargs) @@ -2671,6 +2693,7 @@ def default_value_repr(self): class Tuple(Container): """An instance of a Python tuple.""" + klass = tuple _cast_types = (list,) @@ -2702,12 +2725,25 @@ def __init__(self, *traits, **kwargs): will be cast to a tuple. If ``traits`` are specified, ``default_value`` must conform to the shape and type they specify. """ - default_value = kwargs.pop('default_value', Undefined) + default_value = kwargs.pop("default_value", Undefined) # allow Tuple((values,)): if len(traits) == 1 and default_value is Undefined and not is_trait(traits[0]): default_value = traits[0] traits = () + if default_value is None and not kwargs.get("allow_none", False): + # improve backward-compatibility for possible subclasses + # specifying default_value=None as default, + # keeping 'unspecified' behavior (i.e. empty container) + warn( + f"Specifying {self.__class__.__name__}(default_value=None)" + " for no default is deprecated in traitlets 5.0.5." + " Use default_value=Undefined", + DeprecationWarning, + stacklevel=2, + ) + default_value = Undefined + if default_value is Undefined: args = () elif default_value is None: @@ -2717,18 +2753,23 @@ def __init__(self, *traits, **kwargs): elif isinstance(default_value, self._valid_defaults): args = (default_value,) else: - raise TypeError('default value of %s was %s' %(self.__class__.__name__, default_value)) + raise TypeError( + "default value of %s was %s" % (self.__class__.__name__, default_value) + ) self._traits = [] for trait in traits: if isinstance(trait, type): - warn("Traits should be given as instances, not types (for example, `Int()`, not `Int`)" - " Passing types is deprecated in traitlets 4.1.", - DeprecationWarning, stacklevel=2) + warn( + "Traits should be given as instances, not types (for example, `Int()`, not `Int`)" + " Passing types is deprecated in traitlets 4.1.", + DeprecationWarning, + stacklevel=2, + ) trait = trait() self._traits.append(trait) - if self._traits and default_value is None: + if self._traits and (default_value is None or default_value is Undefined): # don't allow default to be an empty container if length is specified args = None super(Container, self).__init__(klass=self.klass, args=args, **kwargs)