[issue17490] Improve ast.literal_eval test suite coverage

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> rejected
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests:  -21504

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Guido van Rossum]

Guido van Rossum  added the comment:

Hm, I don't think PR 22469 is relevant to this issue?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Batuhan Taskaya]

Change by Batuhan Taskaya :


--
keywords: +patch
pull_requests: +21504
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/22469

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Guido van Rossum]

Guido van Rossum  added the comment:

I agree that it's better to just close this issue, the patch is 7 years old.

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

literal_eval() is not purposed to evaluate arbitrary arithmetic expressions. It 
is only purposed to handle strings produced by repr() of some simple builtin 
objects. repr(6j+3) is '(3+6j)', not '(6j+3)' and not '(6j--3)', so it accepts 
the former form and not the latters.

If Nick no longer works on this I propose to close this issue. literal_eval() 
supports now more types of expressions, but it is more strict in other cases. I 
have doubts that any test changes are applicable to the current code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Irit Katriel]

Irit Katriel  added the comment:

I just noticed that the patch by Serhiy has unit tests for this case, expecting 
ValueError. So this is apprently a feature and not a bug.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Irit Katriel]

Irit Katriel  added the comment:

I think this is a bug in literal_eval:

>>> 3+6j
(3+6j)
>>> 6j+3
(3+6j)
>>> ast.literal_eval('3+6j')
(3+6j)
>>> ast.literal_eval('6j+3')
Traceback (most recent call last):
  File "", line 1, in 
  File "C:\Users\User\src\cpython\lib\ast.py", line 105, in literal_eval
return _convert(node_or_string)
  File "C:\Users\User\src\cpython\lib\ast.py", line 104, in _convert
return _convert_signed_num(node)
  File "C:\Users\User\src\cpython\lib\ast.py", line 78, in _convert_signed_num
return _convert_num(node)
  File "C:\Users\User\src\cpython\lib\ast.py", line 69, in _convert_num
_raise_malformed_node(node)
  File "C:\Users\User\src\cpython\lib\ast.py", line 66, in _raise_malformed_node
raise ValueError(f'malformed node or string: {node!r}')
ValueError: malformed node or string: 


literal_eval accepts complex rhs, but not lhs: 
https://github.com/python/cpython/blame/master/Lib/ast.py#L99
if isinstance(left, (int, float)) and isinstance(right, complex):

This was introduced here, I'm not sure why:
https://github.com/python/cpython/commit/d8ac4d1d5ac256ebf3d8d38c226049abec82a2a0


In Nick's aborted patch, he removed that check.

--
nosy: +iritkatriel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Batuhan Taskaya]

Change by Batuhan Taskaya :


--
nosy: +BTaskaya

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Rahul Jha]

Rahul Jha  added the comment:

Some of the test cases from Nick's patch are not passing on master:

ast.literal_eval('')  # raises SyntaxError; expected: ValueError
ast.literal_eval('6j--3')  # expected: 3+6j
ast.literal_eval('(2j+4j)+(1+2)')  # expected: 3+6j
ast.literal_eval('(2j+4j)-(1+2)')  # expected: -3+6j

I'm assuming that new changes in ast.py do not allow for these. Can anyone 
confirm that this is indeed the case, and not a bug with literal_eval?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Rahul Jha]

Rahul Jha  added the comment:

Nick, hello! I'd like to take it onwards from here.

--
nosy: +RJ722

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Nick Coghlan]

Nick Coghlan  added the comment:

Belatedly removing the issue assignment here, as I'm not actively working on 
this.

I've also marked this as an easy newcomer friendly task, as all that's involved 
is taking the `test_ast.py` changes from 
https://bugs.python.org/file29520/issue17490_ast_literal_eval_converters.diff 
and turning them into a test suite PR on GitHub.

--
assignee: ncoghlan -> 
keywords: +easy, newcomer friendly -patch
stage: patch review -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Nick Coghlan]

Change by Nick Coghlan :


--
assignee:  -> ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Nick, do you mind to create a PR for new tests?

Some additional tests were added since this issue was created.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Nick Coghlan]

Nick Coghlan added the comment:

Right, the only reason this is still open is because I thought the extra test 
cases might be interesting in their own right (just not interesting enough to 
sit down and apply).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I agreed with Raymond, the current version of the code looks clearer.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Nick Coghlan]

Nick Coghlan added the comment:

I don't know if the suggested test suite diff gives full coverage, but unless 
other improvements have been made to those tests in the meantime, it should 
give more coverage than we have right now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Julien Palard]

Julien Palard added the comment:

> I also discovered several holes in the test suite coverage while refactoring 
> it.

Are they all tested in the current diff?

--
nosy: +mdk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17490] Improve ast.literal_eval test suite coverage

[Nick Coghlan]

Nick Coghlan added the comment:

I don't think we should change the implementation, but I also discovered 
several holes in the test suite coverage while refactoring it. We should pursue 
some of the test suite changes, including switching them over to be data driven 
and take advantage of the new subtest support.

--
title: Use converter functions to implement ast.literal_eval - Improve 
ast.literal_eval test suite coverage

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17490
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com