GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-32888: Improve exception message in ast.literal_eval by Rosuav · Pull Request #340 · python/cpython

Conversation

Rosuav commented Feb 27, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

When a non-literal is given to literal_eval, attempt to be more
helpful with the message, rather than calling it 'malformed'.

https://bugs.python.org/issue32888

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

the-knights-who-say-ni added the CLA signed label

Feb 27, 2017

akruis pushed a commit to stackless-dev/stackless that referenced this pull request

Nov 1, 2017


          Issue python#28556: merge 5 more typing changes from upstream (python…

83ec302

…#340, python#344, python#348, python#349, python#350)

akruis pushed a commit to stackless-dev/stackless that referenced this pull request

Nov 1, 2017


          Issue python#28556: merge 5 more typing changes from upstream (python…

043a8bc

…#340, python#344, python#348, python#349, python#350) (3.5->3.6)

akruis pushed a commit to stackless-dev/stackless that referenced this pull request

Nov 1, 2017


          Issue python#28556: merge 5 more typing changes from upstream (python…

52f2959

…#340, python#344, python#348, python#349, python#350) (3.6->3.7)

merwok commented Feb 18, 2018

Copy link

Copy Markdown

Member

This looks good, do you mind opening a bugs.python.org ticket and adding a news entry?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rosuav changed the title ~~Improve exception message in ast.literal_eval~~ bpo-32888: Improve exception message in ast.literal_eval

Feb 20, 2018


          Improve exception message in ast.literal_eval

26ba98e

When a non-literal is given to literal_eval, attempt to be more
helpful with the message, rather than calling it 'malformed'.

Rosuav force-pushed the literal_eval-exception branch from b9b6e9d to 26ba98e Compare

February 20, 2018 17:41

Rosuav commented Feb 20, 2018

Copy link

Copy Markdown

Contributor Author

Rebased onto current master, which includes shifting where the actual change happens. NEWS entry added, issue number added.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka reviewed

Feb 20, 2018

View reviewed changes

serhiy-storchaka left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add a special test in test_ast.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/ast.py

		return node.n
		elif isinstance(node, AST):
		raise ValueError('%s not allowed in literal' % type(node).__name__)
		raise ValueError('malformed node or string: ' + repr(node))

serhiy-storchaka Feb 20, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In what cases this exception is raised? Is it covered by tests?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Rosuav Feb 20, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The easiest way to trigger it is to use something that isn't permitted in literal_eval, but is valid syntax. There's already one such test - proving that f-strings aren't literal_evallable - and I guess there could be more, but since this is a whitelist, the error is basically "anything else, raise". The only difference in this patch is that the message is different for "syntactically valid but not acceptable by literal_eval" and "syntactically invalid".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Feb 20, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I meant what input triggers the "malformed node or string" error now?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Rosuav Feb 20, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Calling literal_eval with an AST node that has non-AST children in it. I don't think it's possible (post-patch) to trigger this error by calling literal_eval with a string, but I can't say for sure, so I don't want to say just "malformed node".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Misc/NEWS.d/next/Library/2018-02-21-04-39-49.bpo-32888.G5i41Q.rst

		@@ -0,0 +1,2 @@
		Improve wording of error messages from ast.literal_eval, distinguishing
		valid-but-unacceptable nodes from those that are actually malformed.

serhiy-storchaka Feb 20, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add "Patch by..."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Rosuav Feb 20, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should that be done by amending the current commit, or adding an additional commit?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Feb 20, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is better for review to add a new commit. In any case all commits are squashed before merging into the repository.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Rosuav Feb 20, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

zware added the awaiting review label

Feb 20, 2018

Rosuav added 3 commits

February 21, 2018 05:10


          Include patch author name in NEWS

08ffc03


          Distinguish different causes of error

fb83598


          Add test of different forms of exception message

cce0c6c

briancurtin requested changes

Jul 3, 2018

View reviewed changes

briancurtin left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Overall this looks good, just one suggestion on the test implementation.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_ast.py

		ast.BinOp(ast.Num(1), ast.Add(), 'oops'): "malformed node or string: 'oops'",
		}
		for test, message in tests.items():
		with self.assertRaises(ValueError) as cm:

briancurtin Jul 3, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A helpful thing to use in between lines 594 and 595 would be with self.subTest(test=test, message=message): and then indent the parts below it. That way each of these end up being tested independently rather than potentially having the items loop have a case fail and cause the rest of the cases to fail to execute.

Check out https://docs.python.org/3/library/unittest.html#distinguishing-test-iterations-using-subtests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-bot commented Jul 3, 2018

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting changes and removed awaiting review labels

Jul 3, 2018

csabella commented May 13, 2019

Copy link

Copy Markdown

Contributor

@Rosuav, please address the last change request by @briancurtin and also rebase to fix the conflict. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rosuav commented May 13, 2019

Copy link

Copy Markdown

Contributor Author

BPO cites another couple of issues, one of which is still open, as dependencies. And it's not really a high priority - it's nothing more than a nice-to-have. There are a lot of barriers to getting patches accepted and it's often not worth the effort unless you REALLY care about the patch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

merwok commented May 13, 2019

Copy link

Copy Markdown

Member

(Note that you can rebase or do a regular merge, which is often easier! All PRs are squash merged in the end @csabella)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

merwok commented May 13, 2019 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

@serhiy-storchaka Should this wait for bpo-32893 per your last message on the ticket or could it be merged soon? (when conflicts are resolved + suggestion from Brian)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

isidentical mentioned this pull request

Dec 14, 2019

bpo-38396: dump(node) instead of repr(node) on ast.literal_eval #16620

Closed

vstinner commented Dec 14, 2019

Copy link

Copy Markdown

Member

A different approach using ast.dump(): PR #16620.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Dec 14, 2019

Copy link

Copy Markdown

Member

What's the status of this PR?

@Rosuav: Are you still working on this?

A different approach using ast.dump(): PR #16620.

Oh, I just closed it since it could produce very long error message: https://bugs.python.org/issue38396#msg354132

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rosuav commented Dec 14, 2019

Copy link

Copy Markdown

Contributor Author

No, I'm not. It was a nice-to-have but between the pushback (no core dev seemed to want it to happen) and the constant need to fix merge conflicts meant that I abandoned this proposal ages ago. If someone else wants to champion this fix, then sure, but I don't have the energy to push everything, and this one wasn't worth it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Dec 15, 2019

Copy link

Copy Markdown

Member

@isidentical: Are you interested to take over this change? I'm not the good candidate to review it. Maybe @pablogsal or @serhiy-storchaka.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

isidentical commented Dec 19, 2019

Copy link

Copy Markdown

Member

Sure.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

isidentical mentioned this pull request

Dec 19, 2019

gh-77069: add context info to ast.literal_eval error messages #17662

Closed

csabella commented Jan 12, 2020

Copy link

Copy Markdown

Contributor

Closing this pull request as it is now GH-17662.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

csabella closed this

Jan 12, 2020

jaraco pushed a commit that referenced this pull request

Dec 2, 2022


          Bump celery from 4.4.3 to 4.4.4 (#340)

17deb6e

Bumps [celery](https://github.com/celery/celery) from 4.4.3 to 4.4.4.
- [Release notes](https://github.com/celery/celery/releases)
- [Changelog](https://github.com/celery/celery/blob/master/Changelog.rst)
- [Commits](celery/celery@4.4.3...v4.4.4)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

SonicField added a commit to SonicField/cpython that referenced this pull request

May 12, 2026


          docs: FRESH-EYES checklist v2.1.1 add A10.1 application-trace

a0704da

v2.1.1 single-rule extension to A10 per supervisor 01:05:42Z 2026-05-12
+ pythia python#341 independent-push-window observation. Authored under
structural escalation from pythia python#335(c) (POST-A10 empirical class-
repeat trigger): 7th feedback_fabrication_detector_verify instance
(supervisor self at 22:36Z) was the 1st POST-A10-codification instance.

A10.1: when issuing CONFIRMED-FABRICATION verdict (per A10 trigger),
verdict post MUST inline (a) verbatim git command output snippet
supporting the accusation, (b) explicit HEAD-at-claim disambiguation
distinguishing claim-time HEAD from current HEAD when they differ.
Closes the python#340(2) grep-scope-mismatch gap that produced both
POST-A10 incidents (librarian 21:26Z + supervisor 22:36Z self-violation).

Explicit non-coverage: routine verification claims (gate posts, state
reports, ABBA results, build-status, push-verify) remain governed by
feedback_no_tool_execution_citation (Alex 2026-04-29 directive). A10.1
verbatim-snippet requirement is FABRICATION-VERDICT-CLASS-ONLY, not
generic. The two rules coexist via claim-class distinction.

Adoption record updated to note v2.1.1 amendment on top of v2.1 352cc1f.

Reviewers

serhiy-storchaka

serhiy-storchaka left review comments

briancurtin

briancurtin requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Rosuav commented Feb 27, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

merwok commented Feb 18, 2018

Uh oh!

Rosuav commented Feb 20, 2018

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briancurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jul 3, 2018

Uh oh!

csabella commented May 13, 2019

Uh oh!

Rosuav commented May 13, 2019

Uh oh!

merwok commented May 13, 2019

Uh oh!

merwok commented May 13, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Dec 14, 2019

Uh oh!

vstinner commented Dec 14, 2019

Uh oh!

Rosuav commented Dec 14, 2019

Uh oh!

vstinner commented Dec 15, 2019

Uh oh!

isidentical commented Dec 19, 2019

Uh oh!

csabella commented Jan 12, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Rosuav commented Feb 27, 2017 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

merwok commented May 13, 2019 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.