-
Notifications
You must be signed in to change notification settings - Fork 99
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
Replace dynamic on_trait_change with observe #590
Conversation
|
||
self.datasource_change_handler( | ||
TraitChangeEvent( | ||
object=new, | ||
name="color_data", | ||
old=None, | ||
new=new.color_data | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is certainly a pattern we would like to avoid. For now I just went with the most straightforward transition possible to preserve behavior, but at some point we should rethink this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM with a few suggestions
@@ -526,7 +526,7 @@ def _outline_color_changed(self): | |||
self.invalidate_draw() | |||
self.request_redraw() | |||
|
|||
def _either_metadata_changed(self): | |||
def _either_metadata_updated(self, event): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to confirm, this method was inherited?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, from BaseXYPlot
Co-authored-by: Poruri Sai Rahul <[email protected]>
If anyone sees this: A typo slipped through with this pull request. I've opened an issue for this: #914 |
This PR starts replacing the dynamic uses of on_trait_change with observe equivalents
I am going to arbitrarily stop here to keep this PR shorter to review. After these changes, there are 18 files left containing
.on_trait_change
which can be addressed in a separate PR.