Skip to content

added a key_trait argument to Dict #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
minrk merged 6 commits into from
Nov 28, 2016
Merged

added a key_trait argument to Dict #306

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
90ff8dc
added a key_trait argument to Dict
burnpanck Sep 6, 2016
b0b3c7d
fixed ignoring of Dict.key_trait when no other sub-traits are set
burnpanck Nov 16, 2016
8b75d38
Merge branch 'master' into add-Dict-key-validation
burnpanck Nov 16, 2016
d9f1e6d
cleanup Dict.validate_elements
burnpanck Nov 16, 2016
92e3323
restored backwards compatiblity for argument order in Dict, and depre…
burnpanck Nov 24, 2016
3dca190
added missing calls to `class_init` and `instance_init` for `key_trait`
burnpanck Nov 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions traitlets/tests/test_traitlets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1598,39 +1598,55 @@ def test_dict_assignment():
assert c.value is d


class UniformlyValidatedDictTrait(HasTraits):
class UniformlyValueValidatedDictTrait(HasTraits):

value = Dict(trait=Unicode(),
default_value={'foo': '1'})


class TestInstanceUniformlyValidatedDict(TraitTestBase):
class TestInstanceUniformlyValueValidatedDict(TraitTestBase):

obj = UniformlyValidatedDictTrait()
obj = UniformlyValueValidatedDictTrait()

_default_value = {'foo': '1'}
_good_values = [{'foo': '0', 'bar': '1'}]
_bad_values = [{'foo': 0, 'bar': '1'}]


class KeyValidatedDictTrait(HasTraits):
class NonuniformlyValueValidatedDictTrait(HasTraits):

value = Dict(traits={'foo': Int()},
default_value={'foo': 1})


class TestInstanceKeyValidatedDict(TraitTestBase):
class TestInstanceNonuniformlyValueValidatedDict(TraitTestBase):

obj = KeyValidatedDictTrait()
obj = NonuniformlyValueValidatedDictTrait()

_default_value = {'foo': 1}
_good_values = [{'foo': 0, 'bar': '1'}, {'foo': 0, 'bar': 1}]
_bad_values = [{'foo': '0', 'bar': '1'}]


class KeyValidatedDictTrait(HasTraits):

value = Dict(key_trait=Unicode(),
default_value={'foo': '1'})


class TestInstanceKeyValidatedDict(TraitTestBase):

obj = KeyValidatedDictTrait()

_default_value = {'foo': '1'}
_good_values = [{'foo': '0', 'bar': '1'}]
_bad_values = [{'foo': '0', 0: '1'}]


class FullyValidatedDictTrait(HasTraits):

value = Dict(trait=Unicode(),
key_trait=Unicode(),
traits={'foo': Int()},
default_value={'foo': 1})

Expand All @@ -1641,7 +1657,7 @@ class TestInstanceFullyValidatedDict(TraitTestBase):

_default_value = {'foo': 1}
_good_values = [{'foo': 0, 'bar': '1'}, {'foo': 1, 'bar': '2'}]
_bad_values = [{'foo': 0, 'bar': 1}, {'foo': '0', 'bar': '1'}]
_bad_values = [{'foo': 0, 'bar': 1}, {'foo': '0', 'bar': '1'}, {'foo': 0, 0: '1'}]


def test_dict_default_value():
Expand Down
56 changes: 37 additions & 19 deletions traitlets/traitlets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2477,8 +2477,9 @@ def instance_init(self, obj):
class Dict(Instance):
"""An instance of a Python dict."""
_trait = None
_key_trait = None

def __init__(self, trait=None, traits=None, default_value=Undefined,
def __init__(self, trait=None, key_trait=None, traits=None, default_value=Undefined,
Copy link
Member

@minrk minrk Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid backward-incompatibility with positional args, can you add key_trait to the end?

Copy link
Contributor Author

@burnpanck burnpanck Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I think this schema feature accessible via the traits argument doesn't belong into Dict at all. I'd rather not have it block access to key_trait. What about a compromise: Keep key_trait before traits and when an instance of dict (the python class, not the trait) is encountered in key_trait, raise a warning and map it to traits?

Copy link
Member

@minrk minrk Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be for all of these after the first to be keyword-only arguments, but Python 2 doesn't make that very easy other than **kwargs. I would add it to the end and treat it as if it were keyword-only.

Copy link
Contributor Author

@burnpanck burnpanck Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me the keys of a dict are at least as important as the values if not more. I have the feeling everyone here seems to implicitly assume unicode keys for their dicts, but that is not validated in the current implementation. I'd put the key trait even before the value trait if not for backwards compatibility.

Copy link
Member

@minrk minrk Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned more with clarity than priority. I don't view the order as being significant (to me, there is only one positional arg for any TraitType, but Python 2 doesn't make this easy). If you'd like to make value_trait and key_trait, that would be fine by me, as long as it's done in a backward-compatible way (i.e. positional args retain their order). I wouldn't expect anyone to pass key_trait positionally ever, and I'm AOK encouraging people to pass value_trait by keyword-arg, as well, since that would be clearest.

Copy link
Contributor Author

@burnpanck burnpanck Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how there is a general one positional arg that would have a well defined meaning for all TraitType's, so the positional args could equally well be trait specific. But then, I guess backwards compatibility shouldn't be broken lightly, so it's anyway too late. I'll shut up now and will just fix backwards compatibility.

**kwargs):
"""Create a dict trait type from a Python dict.

Expand All @@ -2492,6 +2493,10 @@ def __init__(self, trait=None, traits=None, default_value=Undefined,
The specified trait type to check and use to restrict contents of
the Container. If unspecified, trait types are not checked.

key_trait : TraitType [ optional ]
The type for restricting the keys of the container. If
unspecified, the types of the keys are not checked.

traits : Dictionary of trait types [ optional ]
A Python dictionary containing the types that are valid for
restricting the content of the Dict Container for certain keys.
Expand Down Expand Up @@ -2525,16 +2530,27 @@ def __init__(self, trait=None, traits=None, default_value=Undefined,
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)
self._trait = trait() if isinstance(trait, type) else trait
trait = trait()
self._trait = trait
elif trait is not None:
raise TypeError("`trait` must be a Trait or None, got %s" % repr_type(trait))

if is_trait(key_trait):
if isinstance(key_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)
key_trait = key_trait()
self._key_trait = key_trait
elif key_trait is not None:
raise TypeError("`key_trait` must be a Trait or None, got %s" % repr_type(key_trait))

self._traits = traits

super(Dict, self).__init__(klass=dict, args=args, **kwargs)

def element_error(self, obj, element, validator):
e = "Element of the '%s' trait of %s instance must be %s, but a value of %s was specified." \
def element_error(self, obj, element, validator, side='Values'):
e = side + " of the '%s' trait of %s instance must be %s, but a value of %s was specified." \
% (self.name, class_of(obj), validator.info(), repr_type(element))
raise TraitError(e)

Expand All @@ -2546,25 +2562,27 @@ def validate(self, obj, value):
return value

def validate_elements(self, obj, value):
use_dict = bool(self._traits)
default_to = (self._trait or Any())
if not use_dict and isinstance(default_to, Any):
per_key_override = self._traits or {}
key_trait = self._key_trait
value_trait = self._trait
if not (key_trait or value_trait or per_key_override):
return value

validated = {}
for key in value:
if use_dict and key in self._traits:
validate_with = self._traits[key]
else:
validate_with = default_to
try:
v = value[key]
if not isinstance(validate_with, Any):
v = validate_with._validate(obj, v)
except TraitError:
self.element_error(obj, v, validate_with)
else:
validated[key] = v
v = value[key]
if key_trait:
try:
key = key_trait._validate(obj, key)
except TraitError:
self.element_error(obj, key, key_trait, 'Keys')
active_value_trait = per_key_override.get(key, value_trait)
if active_value_trait:
try:
v = active_value_trait._validate(obj, v)
except TraitError:
self.element_error(obj, v, active_value_trait, 'Values')
validated[key] = v

return self.klass(validated)

Expand Down