traits icon indicating copy to clipboard operation
traits copied to clipboard

TraitDict should have an item_validator rather than key_validator and value_validator

Open corranwebster opened this issue 4 years ago • 7 comments

Currently the new TraitDict expects to be supplied a separate key_validator and a value_validator. We should perhaps be using a single item_validator which is given both the key and the value and is expected to return a validated key, value pair.

As motivation, Python 3.8 has the TypedDict where certain keys are expected to hold certain values. With separate validators we can’t implement something like this in Traits, because the value validator doesn’t know what key it is validating.

On the other hand, we can easily write an item validator which calls out to separate key and value validators that are supplied to it in its constructor.

An alternate approach might be to keep the separate validators, but to pass the key to the value validator and the value to the key validator. There is then a secondary issue of which to do first and whether to pass the validate value of the first to the second...

corranwebster avatar Apr 30 '20 08:04 corranwebster

:+1: This makes sense to me; I can imagine use-cases where you'd want to know what the key was when validating the value.

mdickinson avatar Apr 30 '20 08:04 mdickinson

Putting in 6.2.0, but let's discuss later today whether we might want to squeeze this into the 6.1.0 milestone.

mdickinson avatar Apr 30 '20 08:04 mdickinson

One thought: item_validator is clearly the right name in this context, but now I'm wondering whether it makes the use of item_validator for the TraitSet and TraitList confusing. Should those be renamed to something like element_validator instead?

mdickinson avatar Apr 30 '20 08:04 mdickinson

let's discuss later today

I forgot that @corranwebster wouldn't be in that meeting.

Unless anyone thinks this isn't a good idea, or thinks that more discussion is needed, let's put this into 6.1.0. I think it should be a straightforward change.

mdickinson avatar Apr 30 '20 18:04 mdickinson

This proposal may be mutually exclusive with validating keys and values on input, though: what would we do with def myobj.mydict[some_key]? Maybe we still need key_validator and value_validator, but the key should be passed as an argument to the value_validator?

mdickinson avatar May 01 '20 08:05 mdickinson

The more I think about this, the less clear it is to me either that this is the right thing to do, or the right way to do it. Let's not rush this in for 6.1; we can take time to consider it and possibly get it into 6.2.

Putting back into 6.2. Sorry for the U-turn.

mdickinson avatar May 01 '20 10:05 mdickinson

Bumping, but we really need to resolve this one way or the other before 7.0.

mdickinson avatar Oct 04 '21 13:10 mdickinson