Default values are not cross-validated with `@validation`

[ankostis]

While the defaults values are validated against the type of their trait, they are not cross-validated against any @validate rules.

This may lead to conflicting behavior in HasTraits subclasses when they contain invalid defaults, as exemplified in #263, and copied here for clarity:

class A1(HasTraits):
   virtue = Unicode('BAD')

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

class A2(HasTraits):
   virtue = Unicode('GOOD')

   @validate('virtue')
   def _no_evil(self, proposal):
       if proposal['value'] == 'BAD':
           raise TraitError()

>>> A1('virtue='BAD')  ## Pass
>>> A2('virtue='BAD')  ## Screams!
traitlets.traitlets.TraitError

[SylvainCorlay]

Thanks for reposting here. There were some unrelated discussions on the other thread.

[SylvainCorlay]

I just wanted to point out that it is not a trivial problem and not doing it was a deliberate choice. Basically, it breaks the possibility of a delayed initialization for a value if it is accessed at the cross-validation stage.

[rmorshea]

It turns out that the underlying issue here is quite complicated.

Summary:

If we choose not to guarantee validated defaults, then the bug is actually that hold_trait_notifications operates under the assumption that all prior trait values have been validated. However that assumption fails if the prior value was a default value.

Working Example:

from traitlets import *

class A(HasTraits):
    virtue = Unicode()

    @default('virtue')
    def _bad_default(self):
        return "BAD"

    @validate('virtue')
    def _no_evil(self, proposal):
        # small change so "EVIL" can raise too
        if proposal['value'] != 'GOOD':
            raise TraitError("value must be good")
        return proposal['value']

a1 = A()

try:
    a1.virtue
except:
    print('default generation raises')

a2 = A()

with a2.hold_trait_notifications():
    a2.virtue
    print("default generation passes")

    # also note that if I hadn't called a2.virtue, we'd
    # encounter another bug where the default value
    # used would be A.virtue.default_value, and not
    # the dynamically generated one! (however this
    # would also mean that "EVIL" would raise)

    a2.virtue = "EVIL" # pass
    a2.virtue = "BAD" # raise

Detail:

1. Setting via HasTraits(virtue='BAD') invokes a hold_trait_notifications context.
2. Cross validation never occurs inside this context => validation of default values cannot occur here.
   • In other words dynamic defaults created in the context never get validated. So even if virtue had a dynamic default of "BAD" via @default (which is normally validated) the problem noted above would still persist.
3. The fact that invalid default values are allowed to generate inside hold_trait_notifications (dynamic or otherwise) means that they will be used to determine whether change notifications should occur in TraitType.set.
   • This implies that the failure of A(virtue=invalid) to raise is actually an edge case where invalid happens to equal A.virtue.default_value and a change notification is never sent to HasTraits.notify_change. That failure to notify means that hold_trait_notifications can't retrace the change cache to trigger normal cross validation.

Notes

There is an unrelated bug in the above example where TraitType.set fails to use the defined dynamic defaults in change notifications_

We've stumbled upon a pretty serious string of bugs here... 😳

cc: @SylvainCorlay

[rmorshea]

@minrk fixing the above issues may require some backward incompatible changes!

[rmorshea]

Some of this issue is addressed by #332, however, the bug in hold_trait_notifications still persists.

[rmorshea]

@ankostis, see jupyter-widgets/ipywidgets#1409 for a reevaluation of traitlets default validation heuristics. If you have any thoughts or opinions, I'd express them there.

[ankostis]

Indeed, you may close this one to signify the discussion has moved.
Thanks.

[rmorshea]

@ankostis, even with default cross-validation, you presented an edge case here that is still broken because of how hold_trait_notifications works to suppress cross-validation. This comment details that problem. Once I review that bug, I'll close this and open a new issue for it.

[rmorshea]

The regression that was caused by cross-validating defaults was deeply problematic. For that reason I am closing this issue.

Given that it is in fact possible to recreate cross validation by accessing the trait during @default and calling the requisite methods (mention me here if those instructions are too vague). To my knowledge this does not require you to use any private methods of the trait, so while it may look messy, it's also not a hack.