traitlets icon indicating copy to clipboard operation
traitlets copied to clipboard

Cross-validation of dependent traits

Open jasongrout opened this issue 8 years ago • 14 comments

Suppose I have an object with traits 'a', 'b', and 'c', with the constraints that a>b>c and default values a=1, b=10, c=100. I'd like to write a validator that checks this constraint, such that I can assign all three values at once to get a new consistent state (either on creation, like MyObject(a=5, b=6, c=7). How would I write such a validator?

If it's not possible, would it be possible to include in the proposal argument to a validator the new values of any other traits in the same assignment, so I can compare against what the object state will be instead of what it currently is?

jasongrout avatar Apr 05 '17 21:04 jasongrout

The closest thing to "future-state" observation is this:

checked = 0

class X(HasTraits):

    a = Int(1)
    b = Int(10)
    c = Int(100)

    @observe("a", "b", "c")
    def abc_increase(self, change):
        global checked
        if not self.a < self.b < self.c:
            print(checked + 1)
            raise ValueError("%s < %s < %s is not true" % (self.a, self.b, self.c))
        checked += 1
        

x = X()

with x.hold_trait_notifications():
    x.a = 10
    x.b = 100
    x.c = 0

print(checked) # '1'

However, if no error is encountered, the change notifications are not squashed and the observer will check state for each trait assignment (i.e. it will check 3 times). @observe doesn't make it clear that there's validation happening, but the performance impact of @validate would be even worse since the state gets checked whether or not there is a true "change".

rmorshea avatar Apr 05 '17 22:04 rmorshea

@jasongrout, as for whether there could be true "future-state" observation or validation? The answer is, "No." That is, unless we're willing to fundamentally change how notifications are held, squashed and retriggered by hold_trait_notifications.

rmorshea avatar Apr 05 '17 22:04 rmorshea

Another work-around:

class X(HasTraits):

    a = Int(1)
    b = Int(10)
    c = Int(100)

    @observe("a", "b", "c")
    def abc_increase(self, change, seen=[]):
        if change.name not in seen:
            seen.append(change.name)
        if len(seen) == 3
            if not self.a < self.b < self.c:
                raise ValueError("%s < %s < %s is not true" % (self.a, self.b, self.c))
            else:
                del seen[:]

rmorshea avatar Apr 06 '17 01:04 rmorshea

Thanks for the quick response, @rmorshea! Here's what I think would be cool, with the validation handler run just once on initialization or on hold_notification

class X(HasTraits):
    a = Int(1)
    b = Int(10)
    c = Int(100)

    @validate(['a','b','c'])
    def _validate_state(self, proposal):
        if not (proposal['a'] < proposal['b'] < proposal['c']):
            raise TraitError('constraint not satisfied')
        return True

jasongrout avatar Apr 06 '17 02:04 jasongrout

The closest thing that we have is the hold_traits_notifications context manager.

SylvainCorlay avatar Apr 06 '17 06:04 SylvainCorlay

Which rolls back changes if the validation fails when the context is released.

Setting multiple values at once is not possible and validating the future state would require duplicating the object for the testing.

I think that the hold_trait_notifications is simpler to handle than the heacy lifting that would be necessary to validate the hypothetical resulting state.

SylvainCorlay avatar Apr 06 '17 06:04 SylvainCorlay

I also realized that it seems that the object initialization is special-cased - the object values are all set in each call of the validation, so you can easily do this 'future state' validation.

Thanks for your comments. I'm closing this, and we can revisit later if we want to support setting multiple values at a time better.

jasongrout avatar Apr 06 '17 15:04 jasongrout

If possible, what @jasongrout suggested above with some modifications would be really useful:

class X(HasTraits):
    a = Int(1)
    b = Int(10)
    c = Int(100)

    @validate('a','b','c', combined=True)
    def _validate_inequalities(self, proposals):
        if not (proposals['a'].value < proposals['b'].value < proposals['c'].value):
            raise TraitError('constraint not satisfied')
        return proposals

ankostis avatar Jul 09 '17 12:07 ankostis

As a "halfway" alternative, what about a way to observe and manage when hold_trait_notifications contexts are invoke? This would mean that we could notify subsets of traits from the set of all traits which were updated inside the context.

rmorshea avatar Jul 09 '17 16:07 rmorshea

...to observe and manage when hold_trait_notifications contexts are invoke?

I have a rather vague picture of the configuration sources and where hold_trait_notifications applies, so I cannot figure out how this would work; can you provide an example? (of course you can skip explanations, i don't want to wast your time). Wild guess, should a user re-apply the decorator in its own code, would that fire twice?

ankostis avatar Jul 10 '17 09:07 ankostis

In support of the decorator approach, the @jasongrout solution does not fire unless all traits have their values changed, and probably that's not what users want. I believe the common use-case is to fire after configs for all dependent configs have applied, regardless of whether some are defaults or overridden.

ankostis avatar Jul 10 '17 09:07 ankostis

@ankostis and @jasongrout, my thinking here is that in order to make this work outside the special case of object initialization, we need to have some sort of "batch notifier" that collects notifications and then triggers them later. The main problem here is in determining when to stop collecting notifications, and begin triggering them. In order to know when this transition should be made, we need a special context which, when entered, will cause batch notifiers to collect, and when left, will cause them to trigger.

  • Does such a thing as a "batch notifier" sound appealing?
  • If not, special casing a solution for configurables might be better.

Presumably this could be specified with a batched=true key work in either @validate or @observe and the context manager could be named batched_notifications. Unfortunately, for various reasons, hold_trait_notifications could not be used as the context manager.

rmorshea avatar Jul 10 '17 17:07 rmorshea

@rmoshea That sounds just right. But how would the API work? Inside a batched @validate/@observe you would get a proposal with all values, at once, on context-manager exit?

ankostis avatar Jul 18 '17 09:07 ankostis

I think that it would in fact be possible to provide a proposal with all the values. Because we register an ObserveHandler as the callback, we maintain information about what traits are part of the group it's observing. Thus, when at the end of the batched context, we have a list of changes. We can run through the observers, and pick out the ones they are interested in.

rmorshea avatar Jul 19 '17 14:07 rmorshea