Pylab fix #1052
Merged
Pylab fix #1052
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on#648. code used it as a dict. Updated that code to handle a dict correctly, and added tests to catch this issue in the future (also increases test coverage of pylab code).
Also, fix a few tests that the previous commit broke.
This avoids a circular import problem, and also organizes more cleanly the code that is event-loop specific.
IPython/core/pylabtools.py
Outdated
Member
There was a problem hiding this comment.
This will make ipython --pylab fail in the absence of zmq.
IPython/core/pylabtools.py
Outdated
Member
There was a problem hiding this comment.
I would just not call this function if shell is None - no need for it to conditionally do nothing if it was given inappropriate args.
Member
Author
fperez
added a commit
that referenced
this pull request
Nov 27, 2011
Fix bug in pylab support introduced in #648, and refactor the pylab/gui support to eliminate a lot of code duplication we had in a number of places. Now all duplicate code is gone, and the only real difference is how gui event loops are integrated, which is reduced to a single static method that each relevant class grabs from its specific machinery.
mattvonrocketstein
pushed a commit
to mattvonrocketstein/ipython
that referenced
this pull request
Nov 3, 2014
mattvonrocketstein
pushed a commit
to mattvonrocketstein/ipython
that referenced
this pull request
Nov 3, 2014
Fix bug in pylab support introduced in ipython#648, and refactor the pylab/gui support to eliminate a lot of code duplication we had in a number of places. Now all duplicate code is gone, and the only real difference is how gui event loops are integrated, which is reduced to a single static method that each relevant class grabs from its specific machinery.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When merging #648, we inadvertently broke pretty badly the pylab support. I added a test to catch the problem and started fixing things, but quickly saw we had a pretty messy setup in our pylab support machinery, just the product of the fast development we had during the creation of the qt console and zeromq support. The only way to fix this cleanly was to refactor things a little bit, but I think it's in decent shape now.
The main problem is that we had a ton of code duplication, with several methods implemented twice in the terminal and zmq shells, but with actually minimal differences. But the duplication meant that fixing a but required hunting down a lot of stuff around.
Now all duplicate code is gone, and the only real difference is how gui event loops are integrated, which is reduced to a single static method that each relevant class grabs from its specific machinery.
Merging this is, however, very urgent because right now master is fully broken. I'm going to see if @minrk is available on IRC and can have a quick look. If not, I'll merge it immediately, but having the pull request helps to easily see what I did in isolation, and we can do a post-merge reivew. I did verify that it fixes the problem we had, added new tests, and checked that both the qt console and the notebook work fine.