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

[Draft] Recursion support for observe #1242

Closed
wants to merge 2 commits into from

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Jul 14, 2020

This is a draft PR that will be closed immediately. This is created solely for future reference and to keep a record for work that was done before.

Background: In the two proof-of-concept PRs for the observe framework (#942 and #969), recursion support was experimented with because on_trait_change does have this support. The feature was excluded for it is not clear if there is indeed a need, and the implementation isn't trivial.

The main difficulties are:

  • We need to compare two graphs containing cycles to determine if they are equivalent (graph isomorphism problem). But given our use case, we can make a few assumptions and shortcuts to make this easier.
  • We need to construct an ObserverGraph containing cycles from an expression (e.g. in the mini-language "a.[b,c]*"). This requires maintaining the identity of objects as the graph is constructed.

This implementation does the following:

  • Change the data structure of ObserverGraph so it keeps track of what are branches and what are cycles.
  • In Expression, keeps track of the ObserverGraph objects in a cache, it is similar to how deepcopy handles cycle references.

Important note:

  • Previous work in [PoC][EEP-3] User-facing expression for defining target traits to observe #969 tried to construct the graph "from inside-out", because "outside-in" did not work then. Here the construction is made from the outside-in. I realized that previously the "outside-in" approach did not work because the ObserverGraph was hashing its children graphs too early and the hash was persisted. It is important that during the process of constructing the observer graph with cycles, the hashes of the intermediate observer graphs are never used or persisted.

Checklist

  • Tests
  • Update API reference (docs/source/traits_api_reference)
  • Update User manual (docs/source/traits_user_manual)
  • Update type annotation hints in traits-stubs

@kitchoi kitchoi closed this Jul 14, 2020
@kitchoi
Copy link
Contributor Author

kitchoi commented Jul 14, 2020

The last commit is tagged experimental/recursion-observe

@kitchoi kitchoi deleted the stash-recursion-experiment branch July 14, 2020 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant