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

ZMQ displayhook #527

Merged
merged 3 commits into from Jun 22, 2011
Merged

ZMQ displayhook #527

merged 3 commits into from Jun 22, 2011

Conversation

takluyver
Copy link
Member

Move displayhook for ZMQ shell to zmq.displayhook, and rename to make the difference clearer.

Straightforward tidying up. Because I went looking for the displayhook used by ZMQInteractiveShell, and spent a while changing the wrong one before I worked out that it was in zmqshell.py. This puts them both in zmq.displayhook, and attempts to clarify the difference.

See #512

@minrk
Copy link
Member

minrk commented Jun 20, 2011

This makes sense.

But you did move _encode_png, and it's still used in zmqshell but no longer available:
https://github.com/takluyver/ipython/blob/48c2330913f40fea5b69d278ac949149ff50da8f/IPython/zmq/zmqshell.py#L68

@takluyver
Copy link
Member Author

Thanks, Min. I've added an import there.

@takluyver
Copy link
Member Author

Rebased now that newapp is merged.

@minrk
Copy link
Member

minrk commented Jun 22, 2011

This makes sense, and I don't see a problem. Go ahead and merge, then.

@takluyver takluyver merged commit c598e48 into ipython:master Jun 22, 2011
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.

None yet

2 participants