Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new framework to observe and notify for trait changes #977

Closed
19 tasks done
kitchoi opened this issue Apr 1, 2020 · 5 comments
Closed
19 tasks done

Implement new framework to observe and notify for trait changes #977

kitchoi opened this issue Apr 1, 2020 · 5 comments
Assignees
Labels
topic: traits listener rework Issues related to reworking listener infrastructure; see also EEP2, EEP3 type: enhancement
Milestone

Comments

@kitchoi
Copy link
Contributor

kitchoi commented Apr 1, 2020

This is an issue for implementing a new, simpler and cleaner, parallel notification system to the current trait listeners, with the eventual intent of replacing the existing listeners in new code, and eventually deprecating for old code.

Targeting EEP 3.

Specific "end" goals for this issue:

  • @observe decorator analogous to the @on_trait_change

More specific, smaller issues will be opened referencing this one.

Updated

Specific tasks:

@kitchoi kitchoi added the topic: traits listener rework Issues related to reworking listener infrastructure; see also EEP2, EEP3 label Apr 1, 2020
@kitchoi kitchoi added this to the 6.1.0 release milestone Apr 1, 2020
@kitchoi
Copy link
Contributor Author

kitchoi commented Apr 1, 2020

Assuming the PoC in #942 has more or less get the framework right, I will try to break down components in the order of implementation:

  1. ObserverPath (pending PR Implement ObserverGraph with NamedTraitObserver #976). (See also ListenerPath in the poc)
  2. Notifier wrapper for the user defined change handler. (See TraitObserverNotifier in the poc)
  3. Notifier wrapper for unhooking and rehooking downstream notifiers when an upstream object is changed. See ListenerChangeNotifier in the poc.
  4. Implement observe function for a specific type of observer: e.g. A trait with a specific name. This should implement adding and removing notifiers, putting together the components above.
  5. Implement a decorator for observer, to be used like on_trait_change.

After point 4, the following can happen independently of the rest. I would consider them orthogonal to this issue, although not necessarily excluded from the 6.1.0 release (to be confirmed):

  • Observer for list items (see ListItemListener in the poc)
  • Observer for dict items
  • Observer for dict keys
  • Observer for dict values
  • Observer for set items
  • Observer for filtering traits using metadata
  • Observer for an arbitrary filter on traits
    ...

@kitchoi
Copy link
Contributor Author

kitchoi commented Apr 21, 2020

I have updated the main comment with more specific tasks and their dependencies. One can open separate issues when we come closer to being able to work on them.

@kitchoi
Copy link
Contributor Author

kitchoi commented May 20, 2020

Implement the print method on Expression (bonus)

I initially added this item in the list because I had implemented this feature in my test branch. The print method does something like this:

>>> parse("a.items.c").print()                                                                                                                                                                                                                                                                                                                                        
Observes a trait named 'a' with notifications,
    then optionally observes a trait named 'items' with notifications,
         then observes a trait named 'c' with notifications.
    then optionally observes items in a list with notifications,
         then observes a trait named 'c' with notifications.
    then optionally observes items in a dict with notifications,
         then observes a trait named 'c' with notifications.
    then optionally observes items in a set with notifications,
         then observes a trait named 'c' with notifications.

It was useful for inspection (i.e. for me while tackling recursion) and would be useful for users learning the mini-language, this is not needed for another purposes.

While self-documenting components are preferred to written documentation and the feature is valuable for educational purposes, it still requires code, and with code, maintenance.

For most use cases, I believe the mini-language expression is intuitive enough such that this feature for inspection may not be necessary. All-in-all, this is a nice-to-have, not a must have.

With that, I am planning to exclude this feature from the list and call this issue feature-complete.

@kitchoi
Copy link
Contributor Author

kitchoi commented May 21, 2020

Status update:

This is the last pending item

Documentation

API documentation is completed as part of the PRs introducing new code.
#1060 will expand the user manual.

@kitchoi
Copy link
Contributor Author

kitchoi commented May 22, 2020

Closing via #976, #1000, #1007, #1065, #1023, #1066, #1070, #1069, #1067, #1080, #1082, #1079, #1071, #1072, #1075, #1085, #1089, #1078, #1093, #1086, #1077, #1095, #1102, #1108, #1110, #1112, #1117, #1118, #1123, #1125, #1126, #1128, #1129, #1135, #1060

@kitchoi kitchoi closed this as completed May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: traits listener rework Issues related to reworking listener infrastructure; see also EEP2, EEP3 type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants