Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
miss-islington merged 17 commits intopython:mainfromwarsaw:issue-86298
Oct 7, 2022

Conversation

warsaw
Copy link
Member

@warsawwarsaw commentedOct 3, 2022
edited by miss-islington
Loading

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__.loader if available.

The first step on this journey is to check thatloader == __spec__.loader and issue another warning if it is not. This commit does that.

Since this is a PoC, only manual testing for now.

# /tmp/foo.pyimportwarningsimportbarwarnings.warn_explicit('warning!',RuntimeWarning,'bar.py',2,module='bar knee',module_globals=bar.__dict__,    )
# /tmp/bar.pyimportsysimportosimportpathlib# __loader__ = pathlib.Path()

Then running this:./python.exe -Wdefault /tmp/foo.py

Produces:

bar.py:2: RuntimeWarning: warning!  import os

Uncomment the__loader__ = line inbar.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

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
Copy link
MemberAuthor

Tagging@brettcannon

@warsawwarsaw changed the titlePoC for checking both __loader__ and __spec__.loaderissue-86298: PoC for checking both __loader__ and __spec__.loaderOct 3, 2022
@warsawwarsaw changed the titleissue-86298: PoC for checking both __loader__ and __spec__.loader#86298: PoC for checking both __loader__ and __spec__.loaderOct 3, 2022
@warsawwarsaw changed the title#86298: PoC for checking both __loader__ and __spec__.loadergh-86298: PoC for checking both __loader__ and __spec__.loaderOct 3, 2022
@warsawwarsaw marked this pull request as draftOctober 3, 2022 22:33
@warsawwarsaw self-assigned thisOct 3, 2022
@warsawwarsaw marked this pull request as ready for reviewOctober 4, 2022 00:03
@warsaw
Copy link
MemberAuthor

Converting from draft since I now have tests.

@warsaw
Copy link
MemberAuthor

It's better to split these consistency fixes up into separate PRs, so this one only addresses the_warnings.c part of the issue.

@warsaw
Copy link
MemberAuthor

@brettcannon I think this branch is ready for review. It only changes the_warnings.c case, adds tests, and updates some documentation. We'll split any related changes outside of this file into separate PRs.

@warsawwarsaw changed the titlegh-86298: PoC for checking both __loader__ and __spec__.loadergh-86298: Ensure that __loader__ and __spec__.loader agree in warnings.warn_explicit()Oct 4, 2022
Note however that the semantics of the helper has changed, and can't be fullon loader blessing... yet.
@warsaw
Copy link
MemberAuthor

@brettcannon Please take another look.

warsawand others added4 commitsOctober 6, 2022 17:51
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
Copy link
Contributor

Status check is done, and it's a success ✅.

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason:Head branch was modified. Review and try the merge again..

@miss-islington
Copy link
Contributor

Status check is done, and it's a success ✅.

@miss-islingtonmiss-islington merged commit13d4489 intopython:mainOct 7, 2022
@warsawwarsaw deleted the issue-86298 branchOctober 7, 2022 02:50
carljm added a commit to carljm/cpython that referenced this pull requestOct 8, 2022
* 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)
mpage pushed a commit to mpage/cpython that referenced this pull requestOct 11, 2022
…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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@brettcannonbrettcannonbrettcannon approved these changes

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

Assignees

@warsawwarsaw

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@warsaw@miss-islington@brettcannon@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp