gh-69605: Check for already imported modules in PyREPL module completion #139461

loic-simon · 2025-09-30T20:00:49Z

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

Issue: Readline completion of module names in import statements #69605

loic-simon · 2025-09-30T21:48:02Z

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)

Lib/_pyrepl/_module_completer.py

python-cla-bot · 2025-10-01T23:50:18Z

All commit authors signed the Contributor License Agreement.

…dule

…mported-modules

picnixz · 2025-10-03T13:21:42Z

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 · 2025-10-03T14:20:54Z

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!

…ules' of https://github.com/loic-simon/cpython into pyrepl-module-completion-check-for-already-imported-modules

…ready-imported-modules

…mported-modules

tomasr8 · 2025-12-30T23:31:16Z

Lib/_pyrepl/_module_completer.py

+            if os.path.basename(imported_path) == "__init__.py":  # package
+                imported_path = os.path.dirname(imported_path)
+            import_location = os.path.dirname(imported_path)
+            modules = list(pkgutil.iter_modules([import_location]))


I'm not sure if this is correct, it's going to find submodules but we actually want this to be the top-level modules

I believe it's OK, because os.path.dirname make us search in the folder containing the module origin, not on the origin itself (that's why we need to do it twice for a package):

>>> import os, pkgutil, typing, concurrent >>> >>> typing.__spec__.origin # single-file module '<venv>/lib/python3.14/typing.py' >>> concurrent.__spec__.origin # package '<venv>/lib/python3.14/concurrent/__init__.py' >>> >>> loc = os.path.dirname(typing.__spec__.origin) # or do it twice for concurrent >>> [mod for mod in pkgutil.iter_modules([loc]) if mod.name in ("typing", "concurrent")] [ModuleInfo(module_finder=FileFinder('<venv>/lib/python3.14'), name='concurrent', ispkg=True), ModuleInfo(module_finder=FileFinder('<venv>/lib/python3.14'), name='typing', ispkg=False)]

While refactoring this into a separate function I ended up rewriting the whole thing, it should be more explicit now (and it checks module.__package__ instead of the os.path.basename(imported_path) == "__init__.py" hack)

tomasr8 · 2025-12-30T23:32:12Z

Lib/_pyrepl/_module_completer.py

+                imported_path = os.path.dirname(imported_path)
+            import_location = os.path.dirname(imported_path)


Might be safer to check __spec__.has_location before treating the origin as a path.

Oh, I didn't saw this attribute, will do!

That made me realize completion didn't work anymore for already imported frozen modules (for which has_location=False and origin='frozen'), quite a niche issue but fixed it!

That made me realize completion didn't work anymore for already imported frozen modules (for which has_location=False and origin='frozen'), quite a niche issue but fixed it!

Might want to add a test for that as well :)

I added one yes (plus one in the non-imported case), coverage is still 100% on this branch 👌

…mported-modules

loic-simon

I also realized we will need the same logic for my other PR about attributes completion, so I factored the logic into a private method, I suggest we merge this one first so I can ~~rebase~~ merge main into the other and call it!

EDIT: ok that's not true, since attributes completion checks directly the module object, I forgot 😅 but the refactor is a good thing anyway I think

loic-simon · 2026-01-01T15:59:56Z

Lib/_pyrepl/_module_completer.py

+                imported_path = os.path.dirname(imported_path)
+            import_location = os.path.dirname(imported_path)


Oh, I didn't saw this attribute, will do!

loic-simon · 2026-01-01T16:17:18Z

Lib/_pyrepl/_module_completer.py

+            if os.path.basename(imported_path) == "__init__.py":  # package
+                imported_path = os.path.dirname(imported_path)
+            import_location = os.path.dirname(imported_path)
+            modules = list(pkgutil.iter_modules([import_location]))


I believe it's OK, because os.path.dirname make us search in the folder containing the module origin, not on the origin itself (that's why we need to do it twice for a package):

>>> import os, pkgutil, typing, concurrent >>> >>> typing.__spec__.origin # single-file module '<venv>/lib/python3.14/typing.py' >>> concurrent.__spec__.origin # package '<venv>/lib/python3.14/concurrent/__init__.py' >>> >>> loc = os.path.dirname(typing.__spec__.origin) # or do it twice for concurrent >>> [mod for mod in pkgutil.iter_modules([loc]) if mod.name in ("typing", "concurrent")] [ModuleInfo(module_finder=FileFinder('<venv>/lib/python3.14'), name='concurrent', ispkg=True), ModuleInfo(module_finder=FileFinder('<venv>/lib/python3.14'), name='typing', ispkg=False)]

While refactoring this into a separate function I ended up rewriting the whole thing, it should be more explicit now (and it checks module.__package__ instead of the os.path.basename(imported_path) == "__init__.py" hack)

loic-simon · 2026-01-01T16:27:58Z

Lib/_pyrepl/_module_completer.py

+                imported_path = os.path.dirname(imported_path)
+            import_location = os.path.dirname(imported_path)


That made me realize completion didn't work anymore for already imported frozen modules (for which has_location=False and origin='frozen'), quite a niche issue but fixed it!

tomasr8 · 2026-01-01T22:20:35Z

Lib/_pyrepl/_module_completer.py

+            return None
+        if not spec.has_location:
+            if spec.origin == "frozen":  # See Tools/build/freeze_modules.py
+                return os.path.join(self._stdlib_path, f"{spec.name}.py")


This is not always going to be a real path right? e.g. _frozen_importlib.. in any case it does not seem that useful to create the full path when we strip it away with os.path.dirname right after.

Perhaps we can have a smaller diff if instead of reverse-engineering the location, we simply look for the right ModuleInfo in the global cache. After all if the module has already been imported, it should be visible to pkgutil.iter_modules. Something like this:

modules: Iterable[pkgutil.ModuleInfo] = self.global_cache imported_module = sys.modules.get(path.split('.')[0]) if imported_module is not None: # Filter modules to those whose name and spec matches the imported module modules = ...

We can compare the module name and origin (maybe with something like mod_info.module_finder.find_spec(mod_info.name)) to make sure we have the right one.

loic-simon added 2 commits September 30, 2025 21:38

PyREPL module completion: check for already imported modules

6ed2776

Add blurb

48fd43f

loic-simon requested review from ambv, lysnikolaou and pablogsal as code owners September 30, 2025 20:00

bedevere-app bot added the awaiting review label Sep 30, 2025

bedevere-app bot mentioned this pull request Sep 30, 2025

Readline completion of module names in import statements #69605

Open

loic-simon mentioned this pull request Sep 30, 2025

gh-69605: Hardcode some stdlib submodules in PyREPL module completion (os.path, collections.abc...) #138268

Merged

ashm-dev suggested changes Oct 1, 2025

View reviewed changes

Lib/_pyrepl/_module_completer.py Outdated Show resolved Hide resolved

bedevere-app bot added awaiting core review and removed awaiting review labels Oct 1, 2025

loic-simon added 2 commits October 2, 2025 01:48

Better convey intent

7ac428e

[TEMP] debug tests on windows using modern technology (print statements)

6515e2f

loic-simon added 7 commits October 2, 2025 23:07

[TEMP] More debugging, where is my module??

7dbb906

[TEMP] More debugging, where is my module?? (bis)

ac3065a

[TEMP] Day 57, deep into debugging, I still don't know where is my mo…

75a33da

…dule

[TEMP] Moar logs

ce124b1

Merge branch 'main' into pyrepl-module-completion-check-for-already-i…

ee7047f

…mported-modules

[TEMP] Is it a FileFinder cache issue??

3f362cd

[TEMP] Looks like a cache issue indeed

ed8ce73

picnixz marked this pull request as draft October 3, 2025 13:21

bedevere-app bot removed the awaiting core review label Oct 3, 2025

loic-simon added 2 commits October 3, 2025 15:41

Tests: clean FileFinder cache

19c49bb

Remove all debugging junk

16e44af

loic-simon marked this pull request as ready for review October 5, 2025 13:38

bedevere-app bot added the awaiting review label Oct 5, 2025

loic-simon and others added 5 commits October 5, 2025 15:42

Small if refactor

14f6175

Merge branch 'pyrepl-module-completion-check-for-already-imported-mod…

bdd7bdf

…ules' of https://github.com/loic-simon/cpython into pyrepl-module-completion-check-for-already-imported-modules

Full test coverage for new code

78e4737

Merge branch 'python:main' into pyrepl-module-completion-check-for-al…

e3f1ddb

…ready-imported-modules

Merge branch 'main' into pyrepl-module-completion-check-for-already-i…

2644400

…mported-modules

tomasr8 self-requested a review December 28, 2025 15:29

tomasr8 reviewed Dec 30, 2025

View reviewed changes

loic-simon added 2 commits January 1, 2026 18:09

Merge branch 'main' into pyrepl-module-completion-check-for-already-i…

5fa70cf

…mported-modules

Check __spec__.has_location + refactor

d332e14

loic-simon commented Jan 1, 2026

View reviewed changes

Rename private helper

1a5327c

loic-simon requested a review from tomasr8 January 1, 2026 18:14

tomasr8 reviewed Jan 1, 2026

View reviewed changes

		imported_path = os.path.dirname(imported_path)
		import_location = os.path.dirname(imported_path)

Uh oh!

gh-69605: Check for already imported modules in PyREPL module completion #139461

Are you sure you want to change the base?

gh-69605: Check for already imported modules in PyREPL module completion #139461

Conversation

loic-simon commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loic-simon commented Sep 30, 2025

Uh oh!

Uh oh!

python-cla-bot bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz commented Oct 3, 2025

Uh oh!

loic-simon commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomasr8 Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasr8 Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasr8 Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

loic-simon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-simon Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasr8 Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

loic-simon commented Sep 30, 2025 •

edited

Loading

python-cla-bot bot commented Oct 1, 2025 •

edited

Loading

loic-simon commented Oct 3, 2025 •

edited

Loading

loic-simon Jan 1, 2026 •

edited

Loading

loic-simon Jan 1, 2026 •

edited

Loading

loic-simon left a comment •

edited

Loading

loic-simon Jan 1, 2026 •

edited

Loading

loic-simon Jan 1, 2026 •

edited

Loading