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.

Sign up for GitHub

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

Jump to bottom

Fix usage of hasattr for kc #87

Closed
wants to merge 1 commit into from
Closed

Fix usage of hasattr for kc #87

wants to merge 1 commit into from

Conversation

JoostvDoorn
Copy link

Usage of getattr was wrong here, replaced with hasattr

MSeal
MSeal approved these changes Jul 1, 2020
View reviewed changes
Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

Thanks. I think the combination of two lines had left the needed dunder check without changing to a hasattr call.

@MSeal
Copy link
Contributor

MSeal commented Jul 1, 2020

I believe the test failures are related to the socket cleanup fixes in jupyter_client and nbclient tests not properly respecting cleanups. I'll look into it later and make sure it's that and not related to this PR later.

@JoostvDoorn
Copy link
Author

JoostvDoorn commented Jul 1, 2020 •
edited
Loading

On second inspection I mistakenly thought this issue was not yet fixed, it has already been fixed. In #83 self.kc = None was added in the __init__ method.

@JoostvDoorn JoostvDoorn closed this Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MSeal MSeal MSeal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JoostvDoorn @MSeal