ipython / traitlets

A lightweight Traits like module
https://traitlets.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
617 stars 201 forks source link

`Dict` trait is lacking a way to constrain all keys with a single trait #304

Closed burnpanck closed 7 years ago

burnpanck commented 8 years ago

Coming from the original traits, I am used to tr.Dict(TraitAppliedToKeys,TraitAppliedToValues). This functionality is lacking in traitlets. When describing a mathematical map, the two trait specifications would specify the domain and image of the map. Obviously, this trait instead offers the functionality of something that I'd call lightweight object specification, where the keys are essentially fixed values. To me, that describes less of a mapping, but really more of a set of (typically named?) properties. Maaybe we should have two aliases Mapping and PropertySet, both describing instances of dict?

SylvainCorlay commented 8 years ago

@burnpanck you can already specify

t = Dict(trait=Unicode(), 
         traits={
            'a': CaselessStrEnum(['foo', 'bar']),
            'b': CaselessStrEnum(['baz', 'bat']),
         })

traits is a dict of trait types specifying trait types of values for specific keys trait is a trait type specifying the trait type of all other values

At the moment we don't have such a functionality for keys. The reason is that in the context where we needed the constraint (widgets), dictionaries have to be json-serializable which already forces the keys to be of type string.

A similar trait keyword argument exists for List.

burnpanck commented 8 years ago

But wouldn't serializability to json be a prime candiate exactly for a keytrait=Unicode()? My reading of the current implementation's code reveals nothing that actually constrains the keys to be strings implicitly (or did I miss something?). I anyway wouldn't want such an implicit constraint on a trait named Dict (possibly if named JSONDict or similar).

minrk commented 8 years ago

But wouldn't serializability to json be a prime candiate exactly for a keytrait=Unicode()?

Yes, I think so.

SylvainCorlay commented 8 years ago

But wouldn't serializability to json be a prime candiate exactly for a keytrait=Unicode()?

Yes, I think so.

Linking to an earlier discussion with @Minrk on this: https://github.com/ipython/traitlets/pull/10

burnpanck commented 8 years ago

Ok, sorry, didn't research enough. I'll continue over there.

SylvainCorlay commented 8 years ago

Here is fine! This was just for reference.

burnpanck commented 8 years ago

Ok, shall I go ahead and do a PR for key_trait?

SylvainCorlay commented 8 years ago
Dict(traits={
    'configuration': Dict(trait=String()),
    'flag': Bool()
})

although I think that for deeply nested things, we should not do this.

In the context of jsonable values, the cleanest way to do this probably to have a custom validator registered with @validate, which checks the proposed value with a proper scheme validator like jsonschema. This would be better than reinventing the wheel.

import jsonschema

foo_schema = {
     "type" : "object",
     "properties" : {
         "price" : {"type" : "number"},
         "name" : {"type" : "string"},
     },
 }

@validate('foo')
def _check_schema(self, proposal):
    try:
        jsonschema.validate(proposal['value'],  self.foo_schema)
    except ValidationError as e:
        raise TraitError(e)
    return proposal['value']
SylvainCorlay commented 8 years ago

Just thinking that an example with some nested Dict and a jsonschema validation could be added to the docs.

rmorshea commented 7 years ago

@minrk, I think we could better refine this API before the 5.0 release:

Dict(trait=None, trait_mapping=None, default_value=Undefined, **kwargs)
trait
A TraitType or a tuple of TraitTypes with length 2. In the case of a TraitType, the trait will validate a dict's values. In the case of a tuple the first element validates keys and the second validates values - Dict(traits=(Unicode(), Int()).
trait_mapping
A dict of traits keyed on values (identical to per_key_traits). Though trait_mapping is more descriptive, we could keep traits and make this a completely backward compatible change.