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 — gh-95754: Better error when script shadows a standard library or third party module by hauntsaninja · Pull Request #113769 · python/cpython Conversation
|
|
🤖 New build scheduled with the buildbot fleet by @hauntsaninja for commit d4111e5 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
d4111e5 to
0933591
Compare
|
Oh actually looks like I can use stdlib_module_names.h do get sys.stdlib_module_names in C. And I can of course do the path munging in C as well. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
4c09d8e to
9cb7291
Compare
9cb7291 to
5e22968
Compare
There was an error while loading. Please reload this page.
|
Should I disable this if Oh hmm, (edit from later: I discovered |
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 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.
A couple more minor comments.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
FYI, I think we're at the tail end of my review and I don't expect to have much more feedback to offer. Thanks for working on this. You'll still want to double-check with @pablogsal and anyone else with a vested interest. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thank you so much for the detailed review! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Does anyone have any more comments? If not, I'd love a green check mark to unblock the merge. (And of course I'll be happy to address any post-merge follow-ups) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We need to be sure the other reviewers are on board, especially @pablogsal. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 previously gave feedback on the error message, which looks great to me now! Not confident enough to give other kinds of feedback on this sort of 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.
No need to wait for me before merging, as other reviewers know a lot more about the import system. However, a few minor points.
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 an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
This has been quiet for a while and I'd love to get it in ahead of beta1, so I will probably go ahead and merge soon. Please let me know if you have additional feedback and I'll follow up in additional PRs. Thanks to everyone who left reviews, in particular to ericsnowcurrently! |
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.
Looks good to me. I added some minor suggestions (C idioms and a PEP 7 nit); feel free to ignore them.
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 an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks Erlend! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This is great, but I'm greedy now: what about the other form? This works for AttributeError $ touch pathlib.py $ python3.13 -c "import pathlib; pathlib.Path" Traceback (most recent call last): File "<string>", line 1, in <module> import pathlib; pathlib.Path ^^^^^^^^^^^^ AttributeError: module 'pathlib' has no attribute 'Path' (consider renaming '/Users/henryschreiner/git/presentations/python313/pathlib.py' since it has the same name as the standard library module named 'pathlib' and the import system gives it precedence) $ python3.13 -c "from pathlib import Path" Traceback (most recent call last): File "<string>", line 1, in <module> from pathlib import Path ImportError: cannot import name 'Path' from 'pathlib' (/Users/henryschreiner/git/presentations/python313/pathlib.py) |
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.