Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.4k
gh-86298: Ensure that __loader__ and __spec__.loader agree in warnings.warn_explicit()#97803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In _warnings.c, in the C equivalent of warnings.warn_explicit(), if the moduleglobals are given (and not None), the warning will attempt to get the sourceline for the issued warning. To do this, it needs the module's loader.Previously, it would only look up `__loader__` in the module globals. Inpython#86298 we want to defer to the`__spec__.loader` if available.The first step on this journey is to check that `loader` == `__spec__.loader`and issue another warning if it is not. This commit does that.Since this is a PoC, only manual testing for now.```pythonimport warningsimport barwarnings.warn_explicit( 'warning!', RuntimeWarning, 'bar.py', 2, module='bar knee', module_globals=bar.__dict__, )``````pythonimport sysimport osimport pathlib```Then running this: `./python.exe -Wdefault /tmp/foo.py`Produces:```bar.py:2: RuntimeWarning: warning! import os```Uncomment the `__loader__ = ` line in `bar.py` and try it again:```sys:1: ImportWarning: Module bar; __loader__ != __spec__.loader (<_frozen_importlib_external.SourceFileLoader object at 0x109f7dfa0> != PosixPath('.'))bar.py:2: RuntimeWarning: warning! import os```
warsaw commentedOct 3, 2022
Tagging@brettcannon |
warsaw commentedOct 4, 2022
Converting from draft since I now have tests. |
warsaw commentedOct 4, 2022
It's better to split these consistency fixes up into separate PRs, so this one only addresses the |
warsaw commentedOct 4, 2022
@brettcannon I think this branch is ready for review. It only changes the |
Uh oh!
There was an error while loading.Please reload this page.
Note however that the semantics of the helper has changed, and can't be fullon loader blessing... yet.
warsaw commentedOct 7, 2022
@brettcannon Please take another look. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Brett Cannon <brett@python.org>
Co-authored-by: Brett Cannon <brett@python.org>
Co-authored-by: Brett Cannon <brett@python.org>
miss-islington commentedOct 7, 2022
Status check is done, and it's a success ✅. |
miss-islington commentedOct 7, 2022
Sorry, I can't merge this PR. Reason: |
miss-islington commentedOct 7, 2022
Status check is done, and it's a success ✅. |
* main:pythongh-86298: Ensure that __loader__ and __spec__.loader agree in warnings.warn_explicit() (pythonGH-97803)pythongh-82874: Convert remaining importlib format uses to f-str. (python#98005) Docs: Fix backtick errors found by sphinx-lint (python#97998)pythongh-97850: Remove deprecated functions from `importlib.utils` (python#97898) Remove extra spaces in custom openSSL documentation. (python#93568)pythonGH-90985: Revert "Deprecate passing a message into cancel()" (python#97999)
…arnings.warn_explicit() (pythonGH-97803)In `_warnings.c`, in the C equivalent of `warnings.warn_explicit()`, if the module globals are given (and not None), the warning will attempt to get the source line for the issued warning. To do this, it needs the module's loader.Previously, it would only look up `__loader__` in the module globals. Inpython#86298 we want to defer to the `__spec__.loader` if available.The first step on this journey is to check that `loader == __spec__.loader` and issue another warning if it is not. This commit does that.Since this is a PoC, only manual testing for now.```python# /tmp/foo.pyimport warningsimport barwarnings.warn_explicit( 'warning!', RuntimeWarning, 'bar.py', 2, module='bar knee', module_globals=bar.__dict__, )``````python# /tmp/bar.pyimport sysimport osimport pathlib# __loader__ = pathlib.Path()```Then running this: `./python.exe -Wdefault /tmp/foo.py`Produces:```bar.py:2: RuntimeWarning: warning! import os```Uncomment the `__loader__ = ` line in `bar.py` and try it again:```sys:1: ImportWarning: Module bar; __loader__ != __spec__.loader (<_frozen_importlib_external.SourceFileLoader object at 0x109f7dfa0> != PosixPath('.'))bar.py:2: RuntimeWarning: warning! import os```Automerge-Triggered-By: GH:warsaw
Uh oh!
There was an error while loading.Please reload this page.
In
_warnings.c, in the C equivalent ofwarnings.warn_explicit(), if the module globals are given (and not None), the warning will attempt to get the source line for the issued warning. To do this, it needs the module's loader.Previously, it would only look up
__loader__in the module globals. In#86298 we want to defer to the__spec__.loaderif available.The first step on this journey is to check that
loader == __spec__.loaderand issue another warning if it is not. This commit does that.Since this is a PoC, only manual testing for now.
Then running this:
./python.exe -Wdefault /tmp/foo.pyProduces:
Uncomment the
__loader__ =line inbar.pyand try it again:Automerge-Triggered-By: GH:warsaw