msg79548 - (view) |
Author: Armin Ronacher (aronacher) *  |
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) *  |
Date: 2009-01-10 15:47 |
fixed patch :)
|
msg79554 - (view) |
Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2009-01-10 16:57 |
Looks good to me assuming you add a test.
|
msg79604 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
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) *  |
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) *  |
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) *  |
Date: 2009-01-13 08:14 |
Nit: the "except" should only catch ValueError.
|
msg79730 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2009-01-13 11:52 |
Fixed in rev68571.
|
msg79738 - (view) |
Author: Guilherme Polo (gpolo) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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".
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:44 | admin | set | github: 49157 |
2010-10-08 00:52:56 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg118154
|
2009-01-17 07:33:17 | terry.reedy | set | keywords:
patch, patch nosy:
+ terry.reedy resolution: accepted messages:
+ msg80004 stage: patch review -> resolved |
2009-01-13 13:03:02 | aronacher | set | keywords:
patch, patch messages:
+ msg79740 |
2009-01-13 12:38:46 | mark.dickinson | set | keywords:
patch, patch messages:
+ msg79739 |
2009-01-13 12:30:43 | gpolo | set | keywords:
patch, patch nosy:
+ gpolo messages:
+ msg79738 |
2009-01-13 11:52:55 | aronacher | set | status: open -> closed keywords:
patch, patch messages:
+ msg79737 |
2009-01-13 11:45:17 | aronacher | set | keywords:
patch, patch messages:
+ msg79736 |
2009-01-13 11:40:45 | mark.dickinson | set | keywords:
patch, patch messages:
+ msg79735 |
2009-01-13 11:34:53 | aronacher | set | keywords:
patch, patch messages:
+ msg79733 |
2009-01-13 10:47:05 | mark.dickinson | set | keywords:
patch, patch messages:
+ msg79730 |
2009-01-13 08:14:10 | georg.brandl | set | keywords:
patch, patch nosy:
+ georg.brandl messages:
+ msg79723 |
2009-01-12 22:15:05 | aronacher | set | keywords:
patch, patch files:
+ literal-eval.patch messages:
+ msg79705 |
2009-01-12 20:42:39 | mark.dickinson | set | keywords:
patch, patch messages:
+ msg79701 |
2009-01-11 17:33:54 | mark.dickinson | set | keywords:
patch, patch nosy:
+ mark.dickinson messages:
+ msg79604 |
2009-01-10 16:57:37 | benjamin.peterson | set | keywords:
- needs review nosy:
+ benjamin.peterson messages:
+ msg79554 |
2009-01-10 15:47:05 | aronacher | set | keywords:
patch, patch, needs review files:
+ literal-eval.patch messages:
+ msg79550 |
2009-01-10 15:45:02 | aronacher | create | |