gh-135801: Improve filtering by module in warn_explicit() without module argument #140151
Conversation
|
Since we need the code for Python and C implementations of |
serhiy-storchaka
force-pushed
the
warn_explicit-no-module
branch
from
a0aeb4f to
50be3cc
Compare
…ut module argument * Try to match the module name pattern with module names constructed starting from different parent directories of the filename. E.g., for "/path/to/package/module" try to match with "path.to.package.module", "to.package.module", "package.module" and "module". * Ignore trailing "/__init__.py". * Ignore trailing ".py" on Windows. * Keep matching with the full filename (without optional ".py" extension) for compatibility. * Only ignore the case of the ".py" extension on Windows.
serhiy-storchaka
force-pushed
the
warn_explicit-no-module
branch
from
649c884 to
f37f14c
Compare
serhiy-storchaka
requested review from
AA-Turner,
Eclips4,
brettcannon,
ericsnowcurrently,
ncoghlan,
tomasr8 and
warsaw
as code owners
|
Ready for review. |
serhiy-storchaka
force-pushed
the
warn_explicit-no-module
branch
from
5aaff03 to
bc5981e
Compare
serhiy-storchaka
force-pushed
the
warn_explicit-no-module
branch
from
ff502b6 to
c0b9a69
Compare
|
This is definitely a new feature, but it would be nice to backport it to 3.14.1, taking into account how much fuss was made by new syntax warnings in 3.14. This would mitigate impact. cc @hugovk |
|
Thank you for working on this. Before even considering this for 3.14, I'd want to see it in a 3.15 alpha first, with testing and feedback from those unhappy with the 3.14 syntax warnings that this addresses their needs. |
Lib/_py_warnings.py
Outdated
| if is_py and filename[-9:].lower() in (r'\__init__', '/__init__'): | ||
| filename = filename[:-9] | ||
| elif not is_py and filename[-4:].lower() == '.pyw': | ||
| filename = filename[:-4] |
There was a problem hiding this comment.
\__init__.pyw is supported on Windows, you can do something like:
| if is_py and filename[-9:].lower() in (r'\__init__', '/__init__'): | |
| filename = filename[:-9] | |
| elif not is_py and filename[-4:].lower() == '.pyw': | |
| filename = filename[:-4] | |
| if not is_py and filename[-4:].lower() == '.pyw': | |
| filename = filename[:-4] | |
| is_py = True | |
| if is_py and filename[-9:].lower() in (r'\__init__', '/__init__'): | |
| filename = filename[:-9] |
There was a problem hiding this comment.
Hmm, I was too lazy to check and mistakenly supposed that .pyw is not supported for __init__. But I just checked -- indeed, it works.
| filename = support.findfile('test_import/data/syntax_warnings.py') | ||
| with open(filename, 'rb') as f: | ||
| source = f.read() | ||
| with warnings.catch_warnings(record=True) as wlog: |
There was a problem hiding this comment.
You may use a loop since the code is basically duplicated. Something like:
for exec_globals in ...:
There was a problem hiding this comment.
Most of tests (except test_warnings) is actually a ground for other PR. They will not be duplicates in https://github.com/python/cpython/pull/139652/files#diff-f23235c1f5dea0e2d79b33a58e00025ad3c440f96bb76013a1a37868e70a776b
Misc/NEWS.d/next/Library/2025-10-16-17-17-20.gh-issue-135801.faH3fa.rst
Outdated
Show resolved
Hide resolved
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Thank you for your review. You noticed 👀 some details that I missed.
Lib/_py_warnings.py
Outdated
| if is_py and filename[-9:].lower() in (r'\__init__', '/__init__'): | ||
| filename = filename[:-9] | ||
| elif not is_py and filename[-4:].lower() == '.pyw': | ||
| filename = filename[:-4] |
There was a problem hiding this comment.
Hmm, I was too lazy to check and mistakenly supposed that .pyw is not supported for __init__. But I just checked -- indeed, it works.
| filename = support.findfile('test_import/data/syntax_warnings.py') | ||
| with open(filename, 'rb') as f: | ||
| source = f.read() | ||
| with warnings.catch_warnings(record=True) as wlog: |
There was a problem hiding this comment.
Most of tests (except test_warnings) is actually a ground for other PR. They will not be duplicates in https://github.com/python/cpython/pull/139652/files#diff-f23235c1f5dea0e2d79b33a58e00025ad3c440f96bb76013a1a37868e70a776b
|
|
The Android failure was probably caused by low disk space, as in #138649 (comment). This time there was no CoreSimulator.prev.log, so I had to delete the live CoreSimulator.log (over 20 GB), and then kill
It restarted automatically on the next iOS buildbot run. @freakboy3742: is there any way we can stop this log file from getting so large? /Users/buildbot/Library/Logs/CoreSimulator also contains 7,000 smaller log files, which is a number that might start causing its own problems (my terminal wasn't very happy when I tried to list them). |
So - I've taken a look at the log file, and it looks like the simulator itself might have gotten into a weird state. The log was filled with errors that indicated problems with missing SDKs and the like. I've updated the SDK on the buildbot machine, and purged all the simulators except for the one that we're - and now it's not even creating a CoreSimulator.log... so... success? I'll keep an eye on this, but maybe it was just a configuration issue.
This looks like it might have been an artefact of the Xcode setup that existed before August. It was creating a cloned simulator every time it ran the test suite; evidently it wasn't cleaning up those clones. I've purged them all, and now there's a single simulator image in that directory. |
6 participants
/__init__.py".