push cell magic to the head of the transformer line

[minrk]

I'm not 100% sure this is the right fix (ping @takluyver for review), since it is unclear to me what the physical/logical distinction is.

but this does allow the cell magic transform to preempt all other transforms.

Possible alternative: put cell magic at the end of the physical line, which would enable a single use case: copy/paste of cell magic to the terminal with prompts.

closes #3604

[jasongrout]

physical line transformers are run before "assemble_logical_lines", whereas logical line transformers are after: https://github.com/ipython/ipython/blob/master/IPython/core/inputsplitter.py#L633

assemble_logical_lines just joins lines according to explicit line continuations: https://github.com/ipython/ipython/blob/master/IPython/core/inputtransformer.py#L173

[minrk]

Thanks for the clarification. That is precisely the line that cell magic needed to cross, so it seems that I have done this right.

[jasongrout]

I guess you might be breaking something like:

%%r some options \
  some more options

r code

[minrk]

That's true. If there is a sensible way to only allow the continuation on the first line, that would be desirable. Otherwise, I think not touching the cell contents is much more important than allowing the first line to be wrapped.

[ellisonbg]

I agree that not touching cell magic contents is extremely important - it
has to be an invariant that is obeyed 100% of the time.

On Wed, Jul 10, 2013 at 9:07 PM, Min RK notifications@github.com wrote:

That's true. If there is a sensible way to only allow the continuation on
the first line, that would be desirable. Otherwise, I think not touching
the cell contents is much more important than allowing the first line to be
wrapped.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3606#issuecomment-20789284
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

[jasongrout]

By the way, +1 from me as well for not touching the cell contents at all.

I suppose the cell transformer itself could respect \ line continuations on the settings line.

[jasongrout]

Can you make sure that something like this doesn't trigger the cell magic?

s = """
%%r
some r code
"""

[takluyver]

Yes, it will fall foul of the case @jasongrout mentions. There are two differences between physical and logical line transformations:

• Logical lines are assembled with explicit \ line continuations.
• Logical line transformers are not applied when we're within a Python string, so that e.g. this will work:

>>> a = """
... %foo
... """

(The prompts are removed by physical line transformers, but magic transformation is done by a logical line transformer, so it doesn't see %foo. a is set to '\n%foo\n')

I put cell magics in logical transformations, because I think explicit line continuations are common to many languages. If we move them to physical transformations, we'll need to tweak the transformer so it only acts if the %%foo is the first line of a cell.

I might also put it after the prompt cleaners, so we can do something like this:

In [1]: %%timeit setup_foo()
   ...: [a**2 for a in foo]
   ...: 

[minrk]

Is there logic to know what line in the cell you are in? Because starting a cell magic mid-cell also shouldn't work

[takluyver]

The transformer gets a series of lines pushed to it followed by a None to reset it. So if the first line isn't a cell magic line, it should go into a no-op state until it gets a None, then go back to the top of its loop. The prompt transformers do the same thing with this code:

else:
    # Prompts not in input - wait for reset
    while line is not None:
        line = (yield line)

[minrk]

@takluyver another peek at this one? I put it behind the prompt transformers, and it shouldn't fire unless it starts on the first line.

[takluyver]

It seems like quite a bit of extra complexity just to get an error message when you try to start a cell magic that's not at the beginning of a cell, but I can't see a better way to do it at the moment. There should be a test for that case, though. Other than that, I think it looks good.

[takluyver]

Showing a traceback is probably unnecessary clutter for users who don't want to learn about IPython's internals. Maybe we should just print an error message?

[minrk]

I call showtraceback because that's where we handle the shortening of UsageError (no traceback, just ERROR: message).

If you have a better idea for how to show an error and abort, I would love to hear it. This did seem a bit wonky as I wrote it.

[takluyver]

Oh, I thought it was handled in ip.magic(). Maybe we should have a separate showusageerror() method.

[takluyver]

Also, check what happens with that UsageError in the terminal, because the lines are fed to the InputSplitter as they're entered, before we get to run_cell, so the error will appear somewhere else.

[minrk]

in terminal, I get this:

In [1]: %paste
def foo():
    print 'hi'

%%cellmagic
in_a_cell()
## -- End pasted text --
UsageError: Cannot call cell magics (%%cellmagic) mid-cell

[minrk]

I added Shell.show_usage_error, so the usage error case is clearer.

[takluyver]

I mean, what if you actually type something in and try to start a cell magic at a ... : prompt? %paste doesn't go through that same pathway.

[minrk]

Ah, no - it gets ERROR: cell magic not found. How and why is that code different? How do I get it to behave the same?

[minrk]

The UsageError is starting to look like a bad choice - should I just go back to letting it raise a SyntaxError?

[minrk]

I undid the UsageError stuff, it was starting to get messy. Now you just get a regular SyntaxError:

In [2]: def foo():
   ...:     pass
   ...: %%cellmagic
  File "<ipython-input-2-675fda08d994>", line 3
    %%cellmagic
    ^
SyntaxError: invalid syntax

[takluyver]

Yes, I think that the simpler option is better here.

[minrk]

I agree, good to merge then?

[takluyver]

I might still add a test that a cell magic isn't transformed mid cell. Other than that, it looks good to me.

[minrk]

mid-cell cell magic test added

[takluyver]

And Travis is happy, so in it goes.