Make Tuple use a dynamic default when necessary #1521
Conversation
| # Dynamic default, used when at least one of the child traits requires | ||
| # a dynamic default. | ||
| return tuple( | ||
| inner_trait.default_value_for(object, "<inner_trait>") |
There was a problem hiding this comment.
This part's annoying: we have to invent a name because default_value_for expects one, but the DefaultValue.callable default type doesn't pass any name along. So I chose something that should at least be non-ambiguous and can easily be searched for in the Traits source.
I couldn't find any way to actually get the name to show up in validation error messages. For example with List child traits, at the point where this dynamic default is used, it's then validated and at the validation step the TraitList objects are recreated with name matching the name of the actual trait; those recreated TraitList objects are what are actually used in the trait value.
| @@ -2283,16 +2283,40 @@ def __init__(self, *types, **metadata): | |||
| if len(types) == 0: | |||
| types = [Trait(element) for element in default_value] | |||
|
|
|||
| self.types = tuple([trait_from(type) for type in types]) | |||
| self.types = tuple(trait_from(type) for type in types) | |||
There was a problem hiding this comment.
Gratuitous drive-by style cleanup.
| self.init_fast_validate(ValidateTrait.tuple, self.types) | ||
|
|
||
| if default_value is None: | ||
| default_value = tuple( | ||
| [type.default_value()[1] for type in self.types] | ||
| # Optimisation: if all child traits have a constant default value, |
There was a problem hiding this comment.
Hmm. This isn't actually purely optimisation: we don't validate defaults in the static case, so in current Traits we can have for example:
>>> from traits.api import *
>>> class A(HasTraits):
... foo = Tuple(Int("some string"), Str(5))
...
>>> a = A()
>>> a.foo
('some string', 5)
The "optimisation" here ensures that this behaviour (or arguably misbehaviour) persists.
| child_defaults = [] | ||
| child_default_types = [] | ||
| for child_trait in self.types: | ||
| child_default_type, child_default = child_trait.default_value() | ||
|
|
||
| child_default_types.append(child_default_type) | ||
| child_defaults.append(child_default) | ||
|
|
||
| constant_default = all( | ||
| dvt == DefaultValue.constant for dvt in child_default_types | ||
| ) |
There was a problem hiding this comment.
Short-circuit the loop if you hit a non-constant default
| child_defaults = [] | |
| child_default_types = [] | |
| for child_trait in self.types: | |
| child_default_type, child_default = child_trait.default_value() | |
| child_default_types.append(child_default_type) | |
| child_defaults.append(child_default) | |
| constant_default = all( | |
| dvt == DefaultValue.constant for dvt in child_default_types | |
| ) | |
| child_defaults = [] | |
| constant_default = True | |
| for child_trait in self.types: | |
| child_default_type, child_default = child_trait.default_value() | |
| if child_default_type != DefaultValue.constant: | |
| constant_default = False | |
| break | |
| child_defaults.append(child_default) |
or something like this?
There was a problem hiding this comment.
I actually already did this, and then changed to the current implementation on the basis that it was clearer and that the loop seemed unlikely to be a bottleneck.
There was a problem hiding this comment.
(Off-topic, but turning a list of pairs into a pair of lists seems unnecessarily awkward in Python - it feels as though it ought to be a one-liner.)
This PR fixes the
Tupletrait type so that it only uses a constant default value when all of its component traits can use a constant default value. This includes simple cases likeTuple(Int(), Str())as well as nested cases likeTuple(Int(), Tuple(Int(), Str())).Fixes #1520.