no immediate default_value validation

[fwkoch]

Regarding validating default values, I'm curious why this is allowed at all:

This seems to go against the emphasis on validating default values in the module description.

There are also some important difference in our design:

• enthought.traits does not validate default values. We do.

I recognize that this default value is validated eventually if the traitlet is part of a HasTraits class; it just happens on instantiation rather than class definition. Of course, dynamic defaults can only be validated at this point, but it seems like static defaults could be validated sooner.

Thanks!

[rmorshea]

The motivation seems to come from TraitType._validate(obj, value) which accepts the current instance. Interestingly I don't think this needs to be the case, because traitlets only appears to use the instance for:

1. Error reporting.
2. Engaging the "cross validation lock".

However both of these uses could be avoided because:

1. We could just as easily use TraitType.this_class for error reporting.
2. The cross validation lock seems symptomatic of having
   grouped both static validation and bound validation under
   the umbrella of _validate rather than separating the tasks
   into their own methods.

While having the instance in _validate does have its uses, these can, and probably should be fulfilled by cross validators - a TraitType ought not know about a HasTraits instance to operate. I get the impression that this confusion stems from the first versions of traitlets which lacked a HasTraits cross validation scheme (i.e. _<name>_validate now @validate), so it was instead implemented in TraitType instead.

I'll have to think more on this... but my gut tells me that changes here would cause big breakages.

[ankostis]

@fwkoch wrote:

I recognize that this default value is validated eventually if the traitlet is part of a HasTraits class; ...

Actually, it is not always gets validated; for instance, if the default value is invalid, the validation of this class does not raise:

class Foo(HasTraits):
    good = Unicode('BAD')

    @validate('good')
    def _valid_prop(self, proposal):
        if proposal['value'] == 'BAD':
            raise TraitError()
        return proposal['value']

Foo()             ## Not raised!
Foo(good='BAD')   ## Still not raised.
Foo().good='BAD'  ## But this raises OK.

To make this validation fire correctly, you need to define the trait with a non-violating value, like this:

class Foo(HasTraits):
    good = Unicode('GOOD')

    @validate('good')
    ...

Foo(good='BAD')  ## Screams ok.

Traitlets version: 4.2.2

[SylvainCorlay]

The motivation seems to come from TraitType._validate(obj, value) which accepts the current instance. Interestingly I don't think this needs to be the case, because traitlets only appears to use the instance for:

It is needed for the cross validation called from _validate.

[SylvainCorlay]

@ankostis initial values are not cross-validated.

[SylvainCorlay]

@ankostis I would like to close this. If you wish to enable cross-validation of initial values default_values, can you open a separate issue?

[ankostis]

The current behavior is not that clear to me, to open a new bug.
Can we discuss it a bit more here?

I'm not pushing for cross-validation of initial values (you don't mean "default values" here?), or the opposite, for that matter.
But currently the behavior appears not to be consistent, since it depends on specific default-value.
To explain it, I'm re-writting my above example, assuming the same @validation code (not repeated, for brevity), and crafting two classes that have conflicting behavior when invalid constructor-args values are given (are these the "initial-values"?):

>>> class A1(HasTraits):
...    good = Unicode('BAD')
...    ....

>>> class A2(HasTraits):
...    good = Unicode('GOOD')
...    ...

>>> A1('good='BAD')  ## Pass
>>> A2('good='BAD')  ## Scream!
traitlets.traitlets.TraitError

So it seems that in A1, the initial-values are not validated, while in A2 they are.
Is this discrepancy by design?

[SylvainCorlay]

Yes, I meant default values.

[ankostis]

To continue my guesswork, the root cause for me it seems to be that default-values get validated against the trait-type, but not against any `@validation@ rules.
[edit: ] Now seen your repsonse, and that changes everything.

[SylvainCorlay]

Yes, that is right. If you want default values to be cross validated (by cross validated, I mean with the @validate decorator), it is not currently the case and it is a valid issue to be raised!

[ankostis]

You may close this now, thanks for your prompt replies.

[rmorshea]

@SylvainCorlay, my criticism of _validate Isn't that it accepts an instance, but rather that the tasks it performs (static-validation and cross-validation) should actually be done in separate methods. In other words the structure should be:

def _validate(self, inst, value):
    value = _static_validate(value)
    if inst._cross_validation_lock is False:
        value = self._cross_validate(inst, value)
    return value

This way _static_validate can be invoked at the time the descriptor instance is created.

In this restructure:

• TraitType.validate(inst, value) would actually be called during _cross_validate since that's what the signature of the method, which accepts the current instance, implies it should be doing.
• All current implementations of TraitType.validate in traitlets could probably be performed in _static_validate. Though doing this in a backward compatible way would require some thought.

[rmorshea]

I think this issue should be reopened. Given that HasTraits.add_traits requires TraitTypes to be instantiated outside the context of a class, Int("1") should raise to simplify the ensuing traceback (both here, and in general).