Magic %paste error #1258

Closed
wesm opened this Issue Jan 12, 2012 · 17 comments

Comments

Projects
None yet
6 participants

wesm commented Jan 12, 2012

off git master


In [2]: paste
from StringIO import StringIO

text = """a,b,c,d
one,1,2,3
one,1,2,3
,1,2,3
one,1,2,3
,1,2,3
,1,2,3
one,1,2,3
two,1,2,3"""

data = StringIO(text)
## -- End pasted text --
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/home/wesm/code/pandas/<ipython-input-2-84f91d8a6054> in <module>()
----> 1 get_ipython().magic(u'paste ')

/home/wesm/code/repos/ipython/IPython/core/interactiveshell.pyc in magic(self, arg_s, next_input)
   1996                 self._magic_locals = sys._getframe(1).f_locals
   1997             with self.builtin_trap:
-> 1998                 result = fn(magic_args)
   1999             # Ensure we're not keeping object references around:

   2000             self._magic_locals = {}

/home/wesm/code/repos/ipython/IPython/frontend/terminal/interactiveshell.pyc in magic_paste(self, parameter_s)
    651             write("## -- End pasted text --\n")
    652 
--> 653         store_or_execute(self.shell, block, name)
    654 
    655     # Class-level: add a '%cls' magic only on Windows


/home/wesm/code/repos/ipython/IPython/frontend/terminal/interactiveshell.pyc in store_or_execute(shell, block, name)
     93     # Dedent and prefilter so what we store matches what is executed by

     94     # run_cell.

---> 95     b = shell.prefilter(textwrap.dedent(block))
     96 
     97     if name:

/home/wesm/code/repos/ipython/IPython/core/prefilter.pyc in prefilter_lines(self, lines, continue_prompt)
    355         if len(llines) > 1:
    356             out = '\n'.join([self.prefilter_line(line, lnum>0)
--> 357                              for lnum, line in enumerate(llines) ])
    358         else:
    359             out = self.prefilter_line(llines[0], continue_prompt)

/home/wesm/code/repos/ipython/IPython/core/prefilter.pyc in prefilter_line(self, line, continue_prompt)
    332             return normal_handler.handle(line_info)
    333 
--> 334         prefiltered = self.prefilter_line_info(line_info)
    335         # print "prefiltered line: %r" % prefiltered

    336         return prefiltered

/home/wesm/code/repos/ipython/IPython/core/prefilter.pyc in prefilter_line_info(self, line_info)
    272         # print "prefilter_line_info: ", line_info

    273         handler = self.find_handler(line_info)
--> 274         return handler.handle(line_info)
    275 
    276     def find_handler(self, line_info):

/home/wesm/code/repos/ipython/IPython/core/prefilter.pyc in handle(self, line_info)
    801         esc     = line_info.esc
    802         continue_prompt = line_info.continue_prompt
--> 803         obj = line_info.ofind(self)['obj']
    804         #print 'pre <%s> ifun <%s> rest <%s>' % (pre,ifun,the_rest)  # dbg

    805 

KeyError: 'obj'
Owner

fperez commented Jan 12, 2012

Yup, confirmed. Swamped right now, but we definitely need this one fixed, along with an added test... Thanks for the report!

Contributor

rkern commented Jan 12, 2012

The line that is triggering the problem is the ,1,2,3 line. The initial , is IPython syntax for applying autocalling. ,f x y gets translated to f('x','y'). I'm not entirely sure why store_or_execute() is trying to transform that line independently. Typing that code at that IPython prompt manually does not cause any problem. Executing ,1,2,3 by itself at the command line "correctly" transforms it to 1(",2,3") and fails as expected.

Owner

takluyver commented Jan 12, 2012

Normally we only run prefilter when the input is a single line. Is there a good reason we use it on multiline code for %paste?

Owner

fperez commented Jan 12, 2012

On Thu, Jan 12, 2012 at 1:53 PM, Thomas
reply@reply.github.com
wrote:

Normally we only run prefilter when the input is a single line. Is there a good reason we use it on multiline code for %paste?

Well, we still want magics and such to work in pasted blocks.

Owner

takluyver commented Jan 13, 2012

Well, we still want magics and such to work in pasted blocks.

Of course. But we could use inputsplitter, as we do for multiline cells. It just means automagics (paste without the %) don't work.

Owner

fperez commented Jan 13, 2012

On Thu, Jan 12, 2012 at 4:14 PM, Thomas
reply@reply.github.com
wrote:

Of course. But we could use inputsplitter, as we do for multiline cells. It just means automagics (paste without the %) don't work.

Ah, we most certainly should be doing that! It's probably then that
this code never got updated. It's OK for unqualified magics to not
work here, just like they don't work in multiline cells either.

Owner

takluyver commented Jan 13, 2012

For that matter, we should probably unify inputsplitter and prefilter, as they now do largely equivalent things (since inputsplitter is no longer responsible for actually splitting input).

Owner

fperez commented Jan 13, 2012

On Thu, Jan 12, 2012 at 5:18 PM, Thomas
reply@reply.github.com
wrote:

For that matter, we should probably unify inputsplitter and prefilter, as they now do largely equivalent things (since inputsplitter is no longer responsible for actually splitting input).

Yes, a cleanup on this front would be great.

It's important that we maintain an entry point for users to provide
their own custom parsing of user input, as this is used in practice by
projects such as Sage. But a simplification of this machinery would
be most welcome.

Owner

fperez commented Apr 15, 2012

@takluyver, do you want to tackle this one? I'm kind of triaging what's realistic for 0.13, and this one is indeed a bit of a nasty bug, but there's only so much we'll be able to chew before release...

Owner

takluyver commented Apr 15, 2012

I'll have a look, but it may be that it needs to wait for a proper refactoring of prefilter & inputsplitter, which I guess would be post 0.13.

Owner

fperez commented Apr 15, 2012

OK, thanks. I'll leave it to you then: if you decide not to tackle it soon, then go ahead and remove the milestone marker on it so that it doesn't appear for 0.13. It would be great to have this cleaned up, bug given how going deep into that machinery is potentially very disruptive, the wiser course of action may just be to wait so we don't break something critical just prior to release.

Contributor

bfroehle commented Jun 14, 2012

Another case where this can bite you is the % magic syntax:

In [3]: cpaste
Pasting code; enter '--' alone on the line to stop or use Ctrl-D.
:    print('My favorite number is %d'
:          % 5)
:
:<EOF>
  File "<ipython-input-3-d00b86fe925c>", line 2
    get_ipython().magic(u'5 )')
Owner

fperez commented Jun 23, 2012

BTW, I'm working on this one... Just pinging here so nobody starts at the same time and we don't repeat work.

@fperez fperez added a commit to fperez/ipython that referenced this issue Jun 23, 2012

@fperez fperez Add failing test when pasting multiline strings with leading commas.
See #1258.
656079a
Owner

fperez commented Jun 23, 2012

As indicated by @jseabold on the mailing list:

I am able to reproduce this with the latest master. Can anyone
reproduce? Known issue? If I have a function with a line that ends in
a commented out question mark I get an error when using paste magic.

Ie., this code

def func():
   if True: #am i true?
       a = 3

if copied and pasted into the IPython interpreter via paste looks like this

[39]: paste
def func():
   if True: #am i true?
       a = 3
## -- End pasted text --
Object `if` not found.

I need to add a test for this as well...

Owner

fperez commented Jun 26, 2012

OK folks, I'd greatly appreciate a review of #2015 now. I'm not willing to admit in public how long it took me to code this, it proved to be quite tricky and I kept stumbling on odd edge cases. But the code is now well tested and does fix a pretty broken %paste/%cpaste (their logic was very messed up), so I'd like to see this go into 0.13.

Hence my plea for review :) @wesm, let me know if #2015 indeed fixes all the problem cases you'd found...

Contributor

jseabold commented Jun 27, 2012

FWIW, this fixes the issues I was seeing. Thanks.

Owner

fperez commented Jun 27, 2012

Great, glad to hear that @jseabold, thanks for the feedback! I'll leave it in case @takluyver has a chance to review it tomorrow, but if he can't make it we'll merge it for sure; your data point helps cement that decision.

fperez closed this in 28ad69d Jun 28, 2012

@jenshnielsen jenshnielsen pushed a commit to jenshnielsen/ipython that referenced this issue Jun 28, 2012

@fperez fperez Merge pull request #2015 from fperez/paste_1258
Fixes for %paste with special transformations.

Fix the fact that we use prefilter too aggressively in `%paste`, which results in applying single-line transformations inside of strings and function definitions.

Closes #1258.
11b77df

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014

@fperez fperez Add failing test when pasting multiline strings with leading commas.
See #1258.
bbaa66a

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014

@fperez fperez Properly clean up input for interactive pasting.
This requires applying email quote removal, input splitting and
prefiltering of all single-line inputs in a fairly careful manner.
New method `clean_input` was added to the terminal magics to implement
this logic in an isolated and easy to test place.

Closes #1258.
0a6ef89

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014

@fperez fperez Merge pull request #2015 from fperez/paste_1258
Fixes for %paste with special transformations.

Fix the fact that we use prefilter too aggressively in `%paste`, which results in applying single-line transformations inside of strings and function definitions.

Closes #1258.
e936549
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment