Skip to content

bpo-32888: add context info to ast.literal_eval error messages - #17662

Open
isidentical wants to merge 1 commit into
python:mainfrom
isidentical:bpo-32888
Open

bpo-32888: add context info to ast.literal_eval error messages#17662
isidentical wants to merge 1 commit into
python:mainfrom
isidentical:bpo-32888

Conversation

@isidentical

@isidenticalisidentical commented Dec 19, 2019

Copy link
Copy Markdown
Member

Comment threadLib/test/test_fstring.py Outdated
@isidenticalisidentical changed the title bpo-32888: improve wording for ast.literal_eval error messagesbpo-32888: enhance ast.literal_eval error messagess with context informationMay 13, 2020
Comment threadLib/ast.py
def _convert_num(node):
def _raise_malformed_node(node, context = None):
message = "malformed node or string"
if context is not None and isinstance(node, AST):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to check here that node is an AST and not just that context is not None?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if 'context' is overloaded. Maybe it's better not to add it here to the string passed in, and let the context arg be complete, like "literal expression" or "unary expression arg".

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to check here that node is an AST and not just that context is not None?

For malformed nodes (manually constructed), it might not be an AST type, example (already presented in tests);

ast.BinOp(
ast.Constant(1), ast.Add(), "oops"
): "malformed node or string: 'oops'",

I wonder if 'context' is overloaded. Maybe it's better not to add it here to the string passed in, and let the context arg be complete, like "literal expression" or "unary expression arg".

I am not a native speaker, but from the point of messages, it satisfies the expectations and gives the precise location. (+a => ValueError: malformed node or string in binary operation context: <ast.Name object at 0x7f02f058d5f0>). What would you prefer instead of the message above? By the way thanks for your bump on this old PR! I completely forgot it existed since there was no activity :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I see about the first point.
  • On the wording - would it work to simply have "malformed binary operation" in this case?
  • My pleasure 👍

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026
@merwokmerwok changed the title bpo-32888: enhance ast.literal_eval error messagess with context informationbpo-32888: add context info to ast.literal_eval error messagesApr 7, 2026
@github-actionsgithub-actionsBot removed the stale Stale PR or inactive for long period of time. label May 10, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@isidentical@merwok@iritkatriel@the-knights-who-say-ni@ezio-melotti@bedevere-bot