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

Unfeature: disallow "*" in a nonterminal position in the observe mini-language #1525

Merged
merged 3 commits into from
Sep 16, 2021

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Sep 15, 2021

PR #1496 introduced "*" to mean "any trait" in the observe mini-language. But a "*" in anything other than a terminal position (for example, as in my_obj.observe(handler, "*:updated") is hard to use, thanks to a combination of two things:

  • it picks up all traits, including weird things like trait_added and trait_modified, and so typically includes at least some traits for which the following expression won't apply well - trait_added is an Event trait, and its value doesn't have an updated trait
  • the observe framework is (intentionally) nitpicky about invalid trait names: on_trait_change won't complain about trying to set up a listener for the updated trait on an object that doesn't have an updated trait (and possibly isn't even a HasTraits instance); observe will.

There may be a better meaning for "*" in a non-terminal position - for example, we might consider that after a "*", the following expression is treated permissively (using something like the existing optional=True).

For now, we make a "*" in a non-terminal position illegal, so that we have the freedom to re-add it later with better behaviour. (Note that the "*" feature has not yet appeared in any released version of Traits, so there are no backwards compatibility issues here.)

Closes #1505
Related: #1503

Copy link
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

Looks good.

Are there changes to documentation that are needed? Or are you planning those in a separate PR?

Also could you add an issue about exploring * in non-terminal positions - particularly the suggestion for having the exist flag false in that situation - with a link to this PR so we have a place to capture any discussion and context.

@mdickinson
Copy link
Member Author

Are there changes to documentation that are needed?

Good point; done. All of the examples added to the docs in #1496 are still valid, so there's no change needed there.

Also could you add an issue about exploring * in non-terminal positions

Done: #1526

@mdickinson mdickinson merged commit 6ebef90 into main Sep 16, 2021
@mdickinson mdickinson deleted the unfeature/disallow-anytrait-in-nonterminal-position branch September 16, 2021 12:22
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.

Observe mini-language: only allow "*" in a terminal position
2 participants