Implement the logic for observing a named trait #1069
Conversation
…oduce indirection. Removed.
| target=target, | ||
| dispatcher=dispatcher, | ||
| remove=False, | ||
| ) |
There was a problem hiding this comment.
Functions in these modules are refactored as they will be reused by FilteredTraitObserver.
| target=target, | ||
| dispatcher=dispatcher, | ||
| event_factory=trait_event_factory, | ||
| prevent_event=lambda event: event.old is Uninitialized, |
There was a problem hiding this comment.
This is consistent with the current behaviour of on_trait_change.
|
I moved the |
| ObserverGraph( | ||
| node=NamedTraitObserver(name="bar", notify=True), | ||
| ), | ||
| create_observer(name="bar"), |
There was a problem hiding this comment.
Shouldn't this be ObserverGraph(node=create_observer(name="bar"))? (same below)
There was a problem hiding this comment.
Oops, yes you are right, thank you!
| @@ -43,12 +60,153 @@ def notify(self): | |||
| """ | |||
| return self._notify | |||
|
|
|||
| @property | |||
| def optional(self): | |||
There was a problem hiding this comment.
Thought: should this class be a subclass of named tuple? This would get you a lot of the infrastructure for free (you wouldn't need properties and shadow attributes, __hash__ will just work, and __eq__ may also just work or need minimal wrapping.
|
Thank you both for your review! On namedtuple: I would want to use it if it does not also throw in |
This PR implements item 7 in #977 for observing a named trait.
AddEdited: See Add TraitChangeEvent object for wrapping CTrait change #1070TraitChangeEventfor wrapping(object, name, old, new). This is the object to be received by the user handler.optionalargument in order to support"items"special keyword in the mini-language. The"items"keyword is ambiguous about whether it means a trait named "items" or items in a container. We will use this named trait observer but set optional to true for that usage.NamedTraitObserverto implementIObserverinterface:- return the instance Ctrait for adding/removing notifiers (including "maintainer")
- return the value for the next observer, if it is defined. Care is taken to avoid evaluating default values and passing on values such as
Undefined.- return a notifier wrapping the user's change handler so that the handler receives
TraitChangeEvent- return a notifier (the "maintainer") for maintaining downstream observers when the trait change.
When an observer is registered, it should not trigger default initializers to be evaluated. That is consistent with the current behaviour of
on_trait_change. However the implementation inon_trait_changefor avoiding evaluating defaults caused other issues (e.g. #275) which aren't easy to fix. The implementation here will not cause the same issue as seen in #275 and will avoid the side effect of evaluating default at the same time.Not in this PR:
trait_addedevent is not included here. That is in item 11 of Implement new framework to observe and notify for trait changes #977. (I also don't want this PR to be any bigger).Note that in #977 there will also be a "FilteredTraitObserver" which can be used for observing traits with an arbitrary filter function. While
NamedTraitObserver(..., optional=True)could have been implemented usingFilteredTraitObserver,NamedTraitObserver(..., optional=False)cannot. Raising an error when a requested trait is not found will allow us to avoid issues like #447, it will also be useful for catching human errors or for migrating fromon_trait_changetoobserver, as the mini-language syntax has changed.Checklist
docs/source/traits_api_reference): Will conflict with Add user-facing Expression for composing a graph of observers #1067 but the conflicts are trivial.Update User manual (docs/source/traits_user_manual)Update type annotation hints intraits-stubs