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 part 2 #591

Merged
merged 20 commits into from
Apr 2, 2021

Conversation

aaronayres35
Copy link
Contributor

This PR continues replacing the dynamic uses of on_trait_change with observe equivalents on top of #590

old + "_mapper", remove=True)
if new is not None:
self.plot.on_trait_change(self.__mapper_changed,
old + "_mapper", remove=True)
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 previous code seems like it was a copy paste error...
Butt the updated code is still not right ad we have a case in the test suite where old is non None, but its listener never was hooked up initially

@rahulporuri rahulporuri self-requested a review April 2, 2021 08:35
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.

LGTM with a couple of small comments.

There are a bunch of indentation errors that i'm ignoring because we'll run black on chaco at somepoint this month, which should fix all of those issues.

Comment on lines 42 to 43
# using a series of trait change handlers (defined at the end of the
# class)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is tangential but i want to dig a little and find the source of this comment - it looks like there was going to be some fancy traits feature which would remove unnecessary code.

@aaronayres35 aaronayres35 merged commit ef72277 into master Apr 2, 2021
@rahulporuri rahulporuri deleted the replace-dynamic-on_trait_change2 branch April 7, 2021 05:24
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