Skip to content
This repository

timeit with string ending in space gives "ValueError: No closing quotation" #1109

Closed
jonovik opened this Issue December 06, 2011 · 7 comments

3 participants

Jon Olav Vik Min RK Fernando Perez
Jon Olav Vik

%timeit gives a spurious "No closing quotation" error if the statement to be timed contains a parenthesis with a string ending in space.

Microsoft Windows [Versjon 6.1.7601]
Python 2.7.2 |EPD 7.1-1 (32-bit)| (default, Jul  3 2011, 15:13:59) [MSC v.1500 32 bit (Intel)]
IPython 0.11 -- An enhanced Interactive Python.

In [1]: timeit " "
100000000 loops, best of 3: 17.8 ns per loop

In [2]: timeit "a "
100000000 loops, best of 3: 17.7 ns per loop

In [3]: timeit "a"
100000000 loops, best of 3: 17.7 ns per loop

In [4]: timeit ("a")
100000000 loops, best of 3: 18.9 ns per loop

In [5]: timeit ("a ")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
C:\Users\jonvi\<ipython-input-5-a5c40ee123a6> in <module>()
----> 1 get_ipython().magic(u'timeit ("a ")')

C:\Python27\lib\site-packages\IPython\core\interactiveshell.pyc in magic(self, arg_s, next_input)
   1892                 self._magic_locals = sys._getframe(1).f_locals
   1893             with self.builtin_trap:
-> 1894                 result = fn(magic_args)
   1895             # Ensure we're not keeping object references around:

   1896             self._magic_locals = {}

C:\Python27\lib\site-packages\IPython\core\magic.pyc in magic_timeit(self, parameter_s)
   1826
   1827         opts, stmt = self.parse_options(parameter_s,'n:r:tcp:',
-> 1828                                         posix=False)
   1829         if stmt == "":
   1830             return

C:\Python27\lib\site-packages\IPython\core\magic.pyc in parse_options(self, arg_str, opt_str, *long_opts, **kw)
    267             # If the list of inputs only has 0 or 1 thing in it, there's no

    268             # need to look for options

--> 269             argv = arg_split(arg_str,posix)
    270             # Do regular option processing

    271             try:

C:\Python27\lib\site-packages\IPython\utils\process.pyc in arg_split(s, posix)
    121     lex = shlex.shlex(s, posix=posix)
    122     lex.whitespace_split = True
--> 123     tokens = list(lex)
    124     if is_unicode:
    125         # Convert the tokens back to unicode.


C:\Python27\lib\shlex.pyc in next(self)
    267
    268     def next(self):
--> 269         token = self.get_token()
    270         if token == self.eof:
    271             raise StopIteration

C:\Python27\lib\shlex.pyc in get_token(self)
     94             return tok
     95         # No pushback.  Get a token.

---> 96         raw = self.read_token()
     97         # Handle inclusions

     98         if self.source is not None:

C:\Python27\lib\shlex.pyc in read_token(self)
    170                         print "shlex: I see EOF in quotes state"
    171                     # XXX what error should be raised here?

--> 172                     raise ValueError, "No closing quotation"
    173                 if nextchar == self.state:
    174                     if not self.posix:

ValueError: No closing quotation
Fernando Perez
Owner

Unfortunately this seems to be a bug in python itself, not ours:

In [18]: import shlex

In [19]: lex = shlex.shlex(' ("a ")', posix=False)

In [20]: lex.whitespace_split = True

In [21]: list(lex)
# Produces the same error here.

Since this is a fairly odd edge case (timeit is not meant to be called with parens anyway, since it's a magic function), I'd rather see the bug fixed in Python than add clunky workarounds in ipython. We do add workarounds for Python bugs when they really cause major problems for all users, but in scenarios as this it's better to report the bug where it can be fixed upstream, so everyone ultimately benefits and we don't end up carrying a bunch of ugly hacks for a long time.

So I'd suggest reporting it to Python itself with the above snippet, where someone can hopefully work on it and fix it for good.

Jon Olav Vik

Thanks for the analysis. I have reported it as http://bugs.python.org/issue13543.

Regarding the assertion that

this is a fairly odd edge case (timeit is not meant to be called with parens anyway, since it's a magic function)

please consider timeit "this is a bug".count(" ") which is similar to my original use case (in reference to a question on Stackoverflow). In fact, most of my uses of timeit involve function calls. This bug precludes the use of " " as a function argument when using timeit. A workaround is to define space = " ", then e.g. timeit "this is a bug".count(space).

Jon Olav Vik

Fernando, could you please comment on http://bugs.python.org/issue13543#msg148942, which asks "Why do you consider this to be a bug?" I'm not familiar enough with the shlex module to answer that.

Jon Olav Vik

I tried to look into the use of the shlex module, but do not understand its options. It may be that it is indeed working as documented, and that it is simply tricky to parse statements correctly for %timeit. Consider the following examples:

In [1]: %doctest_mode
Exception reporting mode: Plain
Doctest mode is: ON
>>> def f(*args, **kwargs):
...     print args, kwargs
...
>>> f(1, " ")
(1, ' ') {}
>>> f(" ", 1)
(' ', 1) {}
>>> timeit -r1 -n1 f(1, " ")
(1, ' ') {}
1 loops, best of 1: 1.16 ms per loop
>>> timeit -r1 -n1 f(" ", 1)
Traceback (most recent call last):
ValueError: No closing quotation

>>> timeit -r1 -n1 f(1, " ", 2)
(1, ' ', 2) {}
1 loops, best of 1: 1.32 ms per loop
>>> timeit -r1 -n1 f(1, " ", 2, " ")
(1, ' ', 2, ' ') {}
1 loops, best of 1: 1.08 ms per loop
>>> timeit -r1 -n1 f(" ", 1, " ", 2, " ")
Traceback (most recent call last):
ValueError: No closing quotation
>>> timeit -r1 -n1 ("a " + "b")
1 loops, best of 1: 1.28 us per loop
>>> timeit -r1 -n1 f("a " + "b")
('a  b',) {}
1 loops, best of 1: 1.87 ms per loop
>>> timeit -r1 -n1 f("a " + "b ")
Traceback (most recent call last):
ValueError: No closing quotation
Min RK
Owner

The problem appears to be that Python source obviously doesn't sit well with non-posix whitespace-split shlex.

quotation marks can connect blobs, but only if they start a block:

lex = shlex.shlex('blob f(" ")', posix=False)
lex.whitespace_split=True
for s in lex:
    print repr(s)
'blob'
'f("'
ValueError

The remaining blob is "), an unterminated quotation.

However, if we protect the quotes, so they are always led by whitespace:

lex = shlex.shlex('blob f( " ")', posix=False)
lex.whitespace_split=True
for s in lex:
    print repr(s)
'blob'
'f('
'" "'
')'

Then the whole thing is split safely.

So if you lead all your open-quotes with whitespace, you should be fine (works for all given examples, at least).

I have no idea whether this is a Python bug or not, since I don't know what the reference standard is, but this is definitely an IPython bug. We should not be trying to use shlex to parse Python code as if it were command-line arguments.

FWIW, here is a real shlex bug (as far as I know), related to #1123:

shlex.split("'it\\'s' valid")

Apparently shlex can't handle escaped quotes.

Min RK
Owner

hm, turns out I was wrong about the shlex bug - in posix, you can't escape quotes inside a single-quoted string, but you can escape them in double-quoted strings. I always forget that the world outside Python doesn't treat single and double quotes the same.

Min RK minrk referenced this issue from a commit December 08, 2011
Commit has since been removed from the repository and is no longer available.
Min RK
Owner

PR #1130 should fix this particular issue, though we should consider a cleaner fix in the long term, where we aren't trying to parse Python code with shlex.

Min RK minrk referenced this issue from a commit in minrk/ipython December 08, 2011
Min RK add strict flag to arg_split, to optionally ignore shlex parse errors
Sometimes we pass things that aren't really command-line args to arg_split, e.g:

    %timeit python_code(" ")

This commit adds a `strict` flag, which defaults to the same raising behavior
as before.

Currently magic_timeit is the *only* place, we use strict=False, but it should
also be done in completions (PR #1116).

closes #1109
4f1e79b
Min RK minrk closed this issue from a commit December 12, 2011
Min RK Merge shlex PRs (#1130, #1116)
* arg_split now takes optional strict flag, to ignore ValueErrors in
  shlex parsing
* %timeit uses strict=False, to avoid errors parsing python code
* %run completer uses arg_split(strict=False) for its unicode behavior, instead
  of custom shlex derivative, which is now redundant.

closes #1109
closes #1115
closes #1116
closes #1130
790cb14
Min RK minrk closed this in 790cb14 December 12, 2011
Brian E. Granger ellisonbg referenced this issue from a commit January 10, 2012
Commit has since been removed from the repository and is no longer available.
Brian E. Granger ellisonbg referenced this issue from a commit January 10, 2012
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.