classification
Title: ast.literal_eval does not properly handled complex numbers
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: aronacher, benjamin.peterson, georg.brandl, gpolo, mark.dickinson, rhettinger, terry.reedy
Priority: normal Keywords: patch

Created on 2009-01-10 15:45 by aronacher, last changed 2010-10-08 00:52 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
literal-eval.patch aronacher, 2009-01-10 15:45 broken patch
literal-eval.patch aronacher, 2009-01-10 15:47
literal-eval.patch aronacher, 2009-01-12 22:15 final patch with unittests
Messages (17)
msg79548 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-10 15:45
ast.literal_eval does not properly handle complex numbers:

>>> ast.literal_eval("1j")
1j
>>> ast.literal_eval("2+1j")
Traceback (most recent call last):
  ...
ValueError: malformed string
>>> ast.literal_eval("(2+1j)")
Traceback (most recent call last):
  ...
ValueError: malformed string

Expected result:

>>> ast.literal_eval("1j")
1j
>>> ast.literal_eval("2+1j")
(2+1j)
>>> ast.literal_eval("(2+1j)")
(2+1j)

I attached a patch that fixes this problem.
msg79550 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-10 15:47
fixed patch :)
msg79554 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-01-10 16:57
Looks good to me assuming you add a test.
msg79604 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-11 17:33
I'm not sure that this is desirable behaviour.  There's no such thing as a 
complex literal---only imaginary literals.  Why allow evaluation of 2+1j
but not of 2 + 1, or 2*1j.

In any case, I'm not sure that the patch behaves as intended.  For 
example,

>>> ast.literal_eval('2 + (3 + 4j)')
(5+4j)
msg79701 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-12 20:42
BTW, both those "I'm not sure"s should be taken literally:  I'm not a user 
of the ast module, I don't know who the typical users are, and I don't 
know what the typical uses for the literal_eval function are.  The patch 
just struck me as odd, so I thought I'd comment.

IOW, take my comments with a large pinch of salt.
msg79705 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-12 22:15
Here a patch with unittests to correctly handle complex numbers.  This
does not allow the user of arbitrary add/sub expressions on complex numbers.
msg79723 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-01-13 08:14
Nit: the "except" should only catch ValueError.
msg79730 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-13 10:47
Nice fix!

Exactly which complex strings should be accepted here?
The patched version of literal-eval still accepts some
things that would be rejected as inputs to complex():

>>> ast.literal_eval('-1+-3j')
(-1-3j)
>>> complex('-1+-3j')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: complex() arg is a malformed string

and it produces different results from the complex
constructor in some other cases:

>>> complex('-0.0+0.0j').real
-0.0
>>> ast.literal_eval('-0.0+0.0j').real
0.0

But since I don't really know what ast_literal
is used for, I don't know whether this matters.

It still seems odd to me to be doing just one
special case of expression evaluation in
ast_literal, but maybe that's just me.
msg79733 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-13 11:34
literal_eval has eval() semantics and not complex() constructor
semantics.  It accepts what eval() accepts just without arithmetic and
unsafe features.

For exmaple "(2 + 4j)" is perfectly fine even though the complex call
only supports "2+4j" (no parentheses and whitespace).

I commit the fix with the ValueError except Georg suggested.
msg79735 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-13 11:40
So why accept (4+2j) but not (2j+4)?

(BTW, I'm fairly sure that the complex constructor does
accept parentheses;  you're right about the whitespace, though.)
msg79736 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-13 11:45
Indeed, it accepts parentheses in 2.6 now, but not in 2.5 or earlier.

Why not the other way round?  Somewhere there has to be a limit.  And if
you write down complex numbers you usually have the imaginary part after
the real part.

But let's try no to make this a bikeshed discussion.  If you say that
literal_eval can safely evaluate the repr() of builtins (with the
notable exception of reprs that eval can't evaluate either [like nan,
inf etc.]) and probably a bit more it should be fine :)
msg79737 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-13 11:52
Fixed in rev68571.
msg79738 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2009-01-13 12:30
Why didn't you use assertRaises in place of that try/except for a test ?

I was somewhat following this issue and just saw it being commited, but
the change was being discussed. Aren't you supposed to commit these kind
of changes only after entering in agreement with others ?
msg79739 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-01-13 12:38
> If you say that
> literal_eval can safely evaluate the repr() of builtins

Sorry, yes, that makes perfect sense.  (And now I see that that's what
distinguishes 4+2j from 2j+4---finally the light dawns.) Apologies for
being obtuse.
msg79740 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2009-01-13 13:03
> Why didn't you use assertRaises in place of that try/except for a test ?
Could be changed.

> I was somewhat following this issue and just saw it being commited,
> but the change was being discussed. Aren't you supposed to commit
> these kind of changes only after entering in agreement with others ?
The "needs review" keyowrd was removed, I was under the impression I can
commit now :)
msg80004 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2009-01-17 07:33
I assume from the discussion that the patch was accepted/committed and
changed the resolution and stage field to match.

FWIW, list displays, for instance, are not literals either but are
successfully evaluated, so doing same for complex 'displays' seems
sensible to me too, and in line with the purpose of the method.
msg118154 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2010-10-08 00:52
Fixed handling on unary minus in r85314.  In so doing, it also liberalized what literal_eval() accepts (3j+4 for example). This simplified the implementation and removed an unnecessary restriction which wasn't needed for "safety".
History
Date User Action Args
2010-10-08 00:52:56rhettingersetnosy: + rhettinger
messages: + msg118154
2009-01-17 07:33:17terry.reedysetkeywords: patch, patch
nosy: + terry.reedy
resolution: accepted
messages: + msg80004
stage: patch review -> resolved
2009-01-13 13:03:02aronachersetkeywords: patch, patch
messages: + msg79740
2009-01-13 12:38:46mark.dickinsonsetkeywords: patch, patch
messages: + msg79739
2009-01-13 12:30:43gpolosetkeywords: patch, patch
nosy: + gpolo
messages: + msg79738
2009-01-13 11:52:55aronachersetstatus: open -> closed
keywords: patch, patch
messages: + msg79737
2009-01-13 11:45:17aronachersetkeywords: patch, patch
messages: + msg79736
2009-01-13 11:40:45mark.dickinsonsetkeywords: patch, patch
messages: + msg79735
2009-01-13 11:34:53aronachersetkeywords: patch, patch
messages: + msg79733
2009-01-13 10:47:05mark.dickinsonsetkeywords: patch, patch
messages: + msg79730
2009-01-13 08:14:10georg.brandlsetkeywords: patch, patch
nosy: + georg.brandl
messages: + msg79723
2009-01-12 22:15:05aronachersetkeywords: patch, patch
files: + literal-eval.patch
messages: + msg79705
2009-01-12 20:42:39mark.dickinsonsetkeywords: patch, patch
messages: + msg79701
2009-01-11 17:33:54mark.dickinsonsetkeywords: patch, patch
nosy: + mark.dickinson
messages: + msg79604
2009-01-10 16:57:37benjamin.petersonsetkeywords: - needs review
nosy: + benjamin.peterson
messages: + msg79554
2009-01-10 15:47:05aronachersetkeywords: patch, patch, needs review
files: + literal-eval.patch
messages: + msg79550
2009-01-10 15:45:02aronachercreate