| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
🤖 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.
|
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.
|
Should I disable this if sys.flags.safe_path? Also this won't currently work for python nested_folder/script.py where nested_folder contains the shadowing module... I could base this around Py_GetPath()[0] instead. Oh hmm, Py_GetPath is deprecated, so I guess I have to get sys.path[0] (and assume it hasn't changed). This means we have to call back into Python and risk the bad recursion again. See the code from before 7f24b99 , curious if people have suggestions that are more bulletproof than module name checks. (edit from later: I discovered PySys_GetObject, so now I can safely do a bunch of stuff!) |
Sorry, something went wrong.
There was a problem hiding this comment.
A couple more minor comments.
Sorry, something went wrong.
|
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.
|
Thank you so much for the detailed review! |
Sorry, something went wrong.
|
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.
|
We need to be sure the other reviewers are on board, especially @pablogsal. |
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
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 a problem hiding this comment.
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.
|
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 a problem hiding this comment.
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.
|
Thanks Erlend! |
Sorry, something went wrong.
|
This is great, but I'm greedy now: what about the other form? This works for AttributeError import x; x.y but not for the from x import y ImportError form. $ 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.
| Back | FazBrowse Home | New Git URL |
This is a very common mistake for beginners. This PR tries to detect the most common case: where a file in the current directory ends up on sys.path and shadows a standard library module. (My previous PR #112577 was only marginally helpful and wouldn't help with many such errors)
The first commit in this PR is just refactoring.
📚 Documentation preview 📚: https://cpython-previews--113769.org.readthedocs.build/