Skip to content

Conversation

loic-simon
Copy link
Contributor

@loic-simon loic-simon commented Sep 30, 2025

This PR handles an edge-case when suggesting module imports in PyREPL (see this comment):

If a module is already imported, the import machinery will only get submodules from it, even if a module of the same name is higher in the path. This can happen if eg. sys.path has been updated since the module was originaly imported.

The easiest way to reproduce this issue (and how I stumbled on it) is with stdlib modules imported during interpreter startup (before site customisation, so ignoring potential shadowing):

% mkdir collections
% touch collections/__init__.py collections/foo.py
% python
Python 3.14.0rc2 [...]
>>> from collections import <TAB>
>>> from collections import foo
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    from collections import foo
ImportError: cannot import name 'foo' from 'collections' (/Users/loic/cpython/Lib/collections/__init__.py)
>>>

This PR add a special case in _pyrepl.ModuleCompleter._find_modules that

  • check if the module to complete imports from is already imported
  • if yes, search for imports from the imported module location only, instead of using the global cache

It was a little tricky to get right, I'm not sure about everything 😅 but it pass all tests I could think off!

cc @tomasr8

@loic-simon
Copy link
Contributor Author

Ok, one of the new test fails on Windows x64/32 (but pass on arm)

I just setup and built Python on a Win 11 x64 to try to reproduce, the test pass 😵‍💫 (and the fix works)
If anyone has an idea of what may interfere? (Python 3.15.0a0 (main, Sep 30 2025, 23:36:53) [MSC v.1944 64 bit (AMD64)] on win32)

@python-cla-bot
Copy link

python-cla-bot bot commented Oct 1, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@picnixz picnixz marked this pull request as draft October 3, 2025 13:21
@picnixz
Copy link
Member

picnixz commented Oct 3, 2025

I'm going to convert it into a draft until you deem this PR ready. That will also reduce the probability of someone clicking on the PR to review it (like me...) while it's not yet ready for.

@loic-simon
Copy link
Contributor Author

loic-simon commented Oct 3, 2025

I'm going to convert it into a draft until you deem this PR ready. That will also reduce the probability of someone clicking on the PR to review it (like me...) while it's not yet ready for.

Thanks, I just fixed the failing test, this should be ready for review!

Sorry again for the noise, I was dealing with what turned out to be a importer cache issue, which I couldn't reproduce outside of the CI buildbots 😓

[edit] I just noticed I can convert to draft / mark ready myself, will do it next times!

@loic-simon loic-simon marked this pull request as ready for review October 5, 2025 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants