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

Add tests to demonstrate behaviour when mutating extension point directly #346

Merged
merged 2 commits into from
Oct 27, 2020

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Oct 26, 2020

This PR add tests to demonstrate the contradiction / challenge for solving #291.

In summary:

  • ExtensionPoint is like a property. Its value is recomputed on every access of the attribute.
  • Mutating the extension point list directly has no effects on the registry, nor will it fire events for the x_items listener
  • Being able to listen to x_items is misleading.

Details:
When someone defines an ExtensionPoint in their class like this:

class MyApplication(Application):
    x = ExtensionPoint(List, id="my.extension")

One can listen to changes to x_items, like this:

    @on_trait_change("x_items")
    def handle(self):
        ....

This makes developers believe that x is like a persisted list and they can listen to its mutation.
But that is in fact not true. The value of the extension point is computed every time when the attribute is accessed. It is more like a Property. For it is ephemeral, one cannot listen to the mutation on the list that is refreshed on every access.

In fact, when one listens to x_items in the context of extension points, what they are listening to is changes from the contributors of that extension point, and what they want is the specific details on what have been removed or added to the extension point. Envisage could well have called that x_contribution_changed, or x_whatever_event. The naming of x_items is unfortunate as it clashes with the meaning of mutating a persisted list in on_trait_change mini-language.

Listening to "x_items" in the on_trait_change mini-language
means listening to mutation of a traits list/dict. However the
"x_items" here is actually just another trait, and the extension
point value is in fact a ephemeral property that gets
recomputed every time. Attempt to mutate it does nothing meaningful,
which I believe is intended, otherwise the extension point value
and the registry internal state will become inconsistent.
@kitchoi kitchoi changed the title Add tests to demonstrate and protect behaviour when mutating extension point directly Add tests to demonstrate behaviour when mutating extension point directly Oct 26, 2020
Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests LGTM! I believe I still need to a do a bit of study on the issues/PRs you have opened and the code itself to fully understand what/why things are happening in this way.

The tests are very useful though at demonstrating the behavior! thank you!


# when
f = Foo()
f.x.append(42)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test LGTM, behavior is still a litte confusing to me. So effectively this line does nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Technically, this line does append to a list, but that list is not kept anywhere and is thrown away.

@kitchoi kitchoi merged commit b0657de into master Oct 27, 2020
@kitchoi kitchoi deleted the tst-extension-point-items-change branch October 27, 2020 13:54
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.

2 participants