GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-41520: codeop no longer ignores SyntaxWarning by vstinner · Pull Request #21838 · python/cpython Conversation
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @vstinner, thank you for fixing this. 🙇♀️
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@Carreau, thank you for catching this. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-21840 is a backport of this pull request to the 3.9 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <[email protected]>
|
GH-21841 is a backport of this pull request to the 3.8 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <[email protected]>
|
Thanks for the review @csabella! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <[email protected]>
(cherry picked from commit 369a1cb) Co-authored-by: Victor Stinner <[email protected]>
| # to emit SyntaxWarning at most once. | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore") | ||
| warnings.simplefilter("error", SyntaxWarning) |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ignoring all warnings was intentional. Compile also issues DeprecationWarnings. Narrowing the filter to SyntaxWarning only reintroduces the error of DeprecationWarning being issued thrice, which was fixed by the previous code. I will write more on the issue.
Followup PR fixes regression by removing SyntaxWarning.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Then maybe the catch warnings need to be put in there 3 times, to reset the warning filter on each subsequent compilations ?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| # bpo-41520: check SyntaxWarning treated as an SyntaxError | ||
| with self.assertRaises(SyntaxError): | ||
| warnings.simplefilter('error', SyntaxWarning) | ||
| compile_command('1 is 1\n', symbol='exec') |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This source should not have '\n' as single lines are passed without it. That is why to recompile with \n and \n\n added. Fixed in followup PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Even w/o the \n 3.8.5 does not raise the SyntaxError.
$ cat foo.py
import warnings
from codeop import compile_command
warnings.simplefilter('error', SyntaxWarning)
res = compile_command('1 is 1', symbol='exec')
print('Res', res)
$ python foo.py
Res None
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I expect that, but aside from your issue, the test if better if it calls compile_command the way it is intended to be used and is used by code.InteractiveInterpreter.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
The title is misleading because before this patch, codeop was not ignoring the first SyntaxWarning. The test we added in bpo-40807 ensured that. The issue, rather, is that _maybe_compile requires 3 SyntaxErrors, not just 1, to raise a SyntaxError. As I note above, the fix to a regression introduces another regression. Fixed in #21848. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.