-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Display graph in IPython. #28
Conversation
This looks awesome! |
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.
Looks good!
I'll have to think about unit tests a bit (don't want to lose the current 100% coverage), but I think some mocks will take care of that.
# Re-wrap it for utf-8 | ||
import io | ||
f = io.TextIOWrapper(f.detach(), 'utf-8') | ||
if IS_INTERACTIVE: |
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.
Can you use elif
here instead of nesting an if
inside the else
? I think it'll also make the diff smaller (no indentation changes for the write-to-file branch).
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.
I think this can be tested by injecting get_ipython()
to __builtin__
.
Also: how would you like to be credited for this patch? |
This patch is credited to @dmitriyshashkin 's comment on #23 . |
Thank you! |
Got this working according to #23 .
Screenshot: