GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

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

@hauntsaninja

@hauntsaninja hauntsaninja commented Jan 6, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

  • Issue: Add more specific error message when file has same name as an imported module #95754

📚 Documentation preview 📚: https://cpython-previews--113769.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Eclips4 and erlend-aasland reacted with thumbs up emoji danielhollas and hroncok reacted with heart emoji
All reactions
  • 👍 2 reactions
  • ❤️ 2 reactions
@hauntsaninja
Refactor to reduce nesting
afe03da
@hauntsaninja
hauntsaninja requested review from brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners January 6, 2024 10:31
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 6, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jan 6, 2024
Add more specific error message when file has same name as an imported module #95754
Closed
@hauntsaninja
hauntsaninja force-pushed the stdlib-error branch 2 times, most recently from 9279982 to 6f14b0d Compare January 6, 2024 10:34
@hauntsaninja
hauntsaninja marked this pull request as draft January 6, 2024 10:37
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 6, 2024
@hauntsaninja
hauntsaninja force-pushed the stdlib-error branch 3 times, most recently from f69e0b5 to d4111e5 Compare January 6, 2024 11:26
@hauntsaninja hauntsaninja added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 6, 2024
@bedevere-bot

bedevere-bot commented Jan 6, 2024

Copy link
Copy Markdown

🤖 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 6, 2024
hauntsaninja added 2 commits January 6, 2024 03:42
@hauntsaninja
Check for script shadowing stdlib
ae438a4
@hauntsaninja
Add What's New
0933591
@hauntsaninja
hauntsaninja force-pushed the stdlib-error branch from d4111e5 to 0933591 Compare January 6, 2024 11:43
@hauntsaninja

hauntsaninja commented Jan 6, 2024

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hauntsaninja
hauntsaninja marked this pull request as ready for review January 7, 2024 03:39
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 7, 2024
hauntsaninja and others added 2 commits January 6, 2024 19:42
@hauntsaninja
Avoid calling into Python
7f24b99
@blurb-it @hauntsaninja
📜🤖 Added by blurb_it.
f073672
@hauntsaninja
hauntsaninja force-pushed the stdlib-error branch 2 times, most recently from 4c09d8e to 9cb7291 Compare January 7, 2024 03:56
@hauntsaninja
just use wchar everywhere
5e22968
@hauntsaninja
hauntsaninja force-pushed the stdlib-error branch from 9cb7291 to 5e22968 Compare January 7, 2024 04:00
@hauntsaninja

hauntsaninja commented Jan 7, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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!)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja added 3 commits March 8, 2024 11:39
@hauntsaninja
test non-str origin
3d36032
@hauntsaninja
add comment about module.__file__
616ced2
@hauntsaninja
test package shadowing module
afbcd40
ericsnowcurrently
ericsnowcurrently reviewed Mar 8, 2024
View reviewed changes
Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently
ericsnowcurrently requested changes Mar 8, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ericsnowcurrently

ericsnowcurrently commented Mar 8, 2024

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hauntsaninja
feedback
2890b6b
@hauntsaninja

hauntsaninja commented Mar 8, 2024

Copy link
Copy Markdown
Contributor Author

Thank you so much for the detailed review!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon
brettcannon removed their request for review March 13, 2024 19:21
@hauntsaninja

hauntsaninja commented Mar 31, 2024

Copy link
Copy Markdown
Contributor Author

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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently

ericsnowcurrently commented Apr 1, 2024

Copy link
Copy Markdown
Member

We need to be sure the other reviewers are on board, especially @pablogsal.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently

ericsnowcurrently commented Apr 1, 2024

Copy link
Copy Markdown
Member

ping @erlend-aasland, @JelleZijlstra, @AlexWaygood

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood
AlexWaygood reviewed Apr 1, 2024
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
JelleZijlstra
JelleZijlstra reviewed Apr 1, 2024
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/whatsnew/3.13.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_import/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@hauntsaninja
try another error message variation
1d65725
@hauntsaninja hauntsaninja removed the awaiting changes label Apr 20, 2024
@hauntsaninja

hauntsaninja commented Apr 20, 2024

Copy link
Copy Markdown
Contributor Author

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!

hugovk and erlend-aasland reacted with thumbs up emoji AlexWaygood and erlend-aasland reacted with heart emoji
All reactions
  • 👍 2 reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland
erlend-aasland approved these changes Apr 22, 2024
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/moduleobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added the awaiting merge label Apr 22, 2024
@hauntsaninja
style nits
300c557
@hauntsaninja

hauntsaninja commented Apr 23, 2024

Copy link
Copy Markdown
Contributor Author

Thanks Erlend!

erlend-aasland reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hauntsaninja
hauntsaninja merged commit 8e86579 into python:main Apr 23, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 23, 2024
@hauntsaninja
hauntsaninja deleted the stdlib-error branch April 23, 2024 01:24
@henryiii

henryiii commented Jun 29, 2024

Copy link
Copy Markdown
Contributor

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)
AlexWaygood reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Sep 11, 2024
gh-123930: Better error for "from imports" when script shadows module #123929
Merged
Better error message for from imports when a script shadows a module #123930
Closed
@brianschubert brianschubert mentioned this pull request Jun 29, 2025
Getting ImportError and suggestion to rename module for no valid reason #136094
Closed
This was referenced Nov 6, 2025
- LuisMauricioDL/Reto-Master#1
Closed
[hmr] Consider adding -P flag for CLI usage promplate/pyth-on-line#514
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@ericsnowcurrently ericsnowcurrently ericsnowcurrently requested changes
@hugovk hugovk hugovk left review comments
@pablogsal pablogsal pablogsal left review comments
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@AlexWaygood AlexWaygood AlexWaygood left review comments
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@hauntsaninja @bedevere-bot @pablogsal @erlend-aasland @ericsnowcurrently @henryiii @JelleZijlstra @hugovk @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.