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

Replace dynamic on_trait_change with observe #590

Merged
merged 22 commits into from
Apr 2, 2021

Conversation

aaronayres35
Copy link
Contributor

@aaronayres35 aaronayres35 commented Mar 31, 2021

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.

Comment on lines +112 to +120

self.datasource_change_handler(
TraitChangeEvent(
object=new,
name="color_data",
old=None,
new=new.color_data
)
)
Copy link
Contributor Author

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

@aaronayres35 aaronayres35 changed the title [WIP] Replace dynamic on_trait_change with observe Replace dynamic on_trait_change with observe Mar 31, 2021
Copy link
Contributor

@rahulporuri rahulporuri left a 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

chaco/axis.py Outdated Show resolved Hide resolved
chaco/base_1d_mapper.py Outdated Show resolved Hide resolved
chaco/base_2d_plot.py Outdated Show resolved Hide resolved
chaco/base_2d_plot.py Outdated Show resolved Hide resolved
chaco/base_2d_plot.py Outdated Show resolved Hide resolved
chaco/overlays/databox.py Outdated Show resolved Hide resolved
chaco/overlays/databox.py Outdated Show resolved Hide resolved
chaco/plot.py Outdated Show resolved Hide resolved
chaco/plotscrollbar.py Outdated Show resolved Hide resolved
@@ -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):
Copy link
Contributor

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?

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, from BaseXYPlot

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
@aaronayres35 aaronayres35 merged commit 25da527 into master Apr 2, 2021
@aaronayres35 aaronayres35 deleted the replace-dynamic-on_trait_change branch April 2, 2021 12:53
@markasbach
Copy link

If anyone sees this: A typo slipped through with this pull request. I've opened an issue for this: #914

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.

3 participants