Remove code from prefilter that duplicates functionality in inputsplitter #2299
Conversation
|
Beat you to it, @travisbot . |
|
Awesome! This is all pure cleanup, hence pure gain on the road to IPEP 2. I don't see any problems with it as-is, since all tests pass. The one thing that should be done before merging is adding a note to the what'snew doc in the backwards-incompatible section, indicating the removal of this functionality. I just want to make sure we don't forget later on... |
|
Thanks, I've added a note to the docs. |
|
With the doc update this is good to go, thanks!! Merging now. |
…orms Remove code from prefilter that duplicates functionality in inputsplitter This is the first step towards implementing IPEP 2 (#2293). Removed all the static transformations from prefilter, because we're relying on the equivalent functionality in inputsplitter. Note that this is a backwards-incompatible change for anyone who might have relied on the low-level details of the prefiltering machinery. Regular users of the IPython applications themselves will not see any changes in behavior.
|
@takluyver I won't probably have time to participate in the inputsplitter/prefilter discussions much, but I just wanted to say thanks for tackling this cleanup. We have been meaning to get to it for a long time. |
|
Not surprisingly, and purely FYI, this broke Sage (which transforms text to implement its own magics, like %load). |
|
I'm a little surprised - what broke? Were you subclassing the transformers On 25 August 2012 15:02, Jason Grout notifications@github.com wrote:
|
|
I'm not sure exactly what broke yet. Here is the error: So apparently our %load magic no longer works. We do various things with prefilter somethings, and I'm sure there are other hacks. Our comprehensive patch is at https://gist.github.com/3466520. The part starting at line 443 seems interesting, and lines 720-790 also seem interesting. |
|
The reason I'm confused is that we haven't removed any of the architecture
I'm finding the patch a bit confusing, because it's the changes to update |
|
I'm coming back to this now. The code (after all patches are applied) is here: https://bitbucket.org/jasongrout/sage-library/src/dcb6b8d0028d/sage/misc/interpreter.py |
|
Okay, it seems that we are making a call that looks like |
|
Also, apparently the IPython %load magic may have changed from 0.10? Instead of executing the loaded file immediately, it seems to now just paste in the file. I don't have an old version of IPython to check if this was a change in behavior---does anyone know off the top of their head? |
|
On Fri, Sep 7, 2012 at 12:41 AM, Jason Grout notifications@github.comwrote:
Jason, there was no %load magic in ipython 0.10 at all, that was something |
|
Okay, that explains at least part of the problem: it looks like when we upgraded to ipython, we punted to ipython's %load, which has different semantics than our %load. I think maybe the way to resolve this issue is to write our own %load magic for our backwards compatibility. Then we don't have to worry about the issue from this pull request either, because we won't be calling back to IPython's %load. I'm curious: Is there an IPython equivalent for loading and executing a file without pasting it in for editing? For example, I've used Sage's load to load a file across the web like a library. |
|
On Fri, Sep 7, 2012 at 1:22 AM, Jason Grout notifications@github.comwrote:
|
|
That might make sense for us to just make %load an alias for %run, and write the code to get %run up to speed... :) |
|
On Fri, Sep 7, 2012 at 1:30 AM, Jason Grout notifications@github.comwrote:
That would be great! |
…-transforms Remove code from prefilter that duplicates functionality in inputsplitter This is the first step towards implementing IPEP 2 (ipython#2293). Removed all the static transformations from prefilter, because we're relying on the equivalent functionality in inputsplitter. Note that this is a backwards-incompatible change for anyone who might have relied on the low-level details of the prefiltering machinery. Regular users of the IPython applications themselves will not see any changes in behavior.
This is the first step towards implementing IPEP 2 (#2293). I have removed all the static transformations from prefilter, because we're relying on the equivalent functionality in inputsplitter.
It turns out that the one part of the code still relying on prefilter for those transformations was the ipython-doctest machinery. For now, I've pointed that to a helper function from the magics for the terminal frontend, but I'll move that function into core later.
My intention is that this is merged before other work on IPEP 2, but if people prefer, I can just do the other bits in this branch and then merge in one go.