Skip to content

Conversation

@loic-simon
Copy link
Contributor

@loic-simonloic-simon commented Nov 1, 2025

Add the ability for _pyrepl._module_completer.ModuleCompleter to suggest attributes, in addition to modules.

If the module to import from is not imported yet, ask the user if they want to import it.
This is done by adding the notion of completion action to _pyrepl.completing_reader.CompletingReader:

  • get_completions can return a completion action, in addtion to the suggestions list
  • A completion action is a prompt to show + a callback to call if the user press TAB again
  • The callback can return a message to show instead (here, if import failed)
  • After the callback is fired, get_completions is called again, so it can propose new results (here, module attributes)
  • If the user does any other input than TAB when the prompt is shown, the action is discarded

Other changes:

  • I had to tweak a little the code used to display a message below the prompt, to handle the case when it doesn't fit on the console
  • Some unrelated tests to get the coverage rate of _module_completer to 100% (in a separate commit, I can remove it if needed) -> gh-140870: Full coverage for _pyrepl._module_completer #143244

Interactive WebAssembly demo (up to date): https://pyrepl-attributes-import-completion.pages.dev

Possible improvement: make modules printing stuff when imported not mess with the prompt ; see this Discourse message

@loic-simonloic-simon changed the title gh-14870: PyREPL auto-complete module attributes in import statements gh-140870: PyREPL auto-complete module attributes in import statements Nov 1, 2025
@tomasr8
Copy link
Member

Some unrelated tests to get the coverage rate of _module_completer to 100% (in a separate commit, I can remove it if needed)

Could you extract those into a separate PR? I think those are nice to have in any case and we can merge it quickly :)

@loic-simon
Copy link
ContributorAuthor

loic-simon commented Dec 28, 2025

Could you extract those into a separate PR? I think those are nice to have in any case and we can merge it quickly :)

Sure, #143244

@tomasr8
Copy link
Member

Quick initial question, can we disable the prompt for stdlib modules? I don't think there should be any concerns with auto-importing from the stdlib

@loic-simon
Copy link
ContributorAuthor

We could, yes. I don't think we can decide to auto-import solely on module name (to take care of potential shadowing by first/third-party modules), so we would need to look the module spec location, but since we already have self._is_stdlib_module and self._global_cache that should be easy and fast!

(I'll won't be to get my hands on the code until early January tho)

@tomasr8
Copy link
Member

Yup, we definitely need to look at the module location, not just the name

@loic-simon
Copy link
ContributorAuthor

loic-simon commented Jan 10, 2026

The new test is quite platform-dependant (I just fixed an iOS + Andoid only fail), it may be a good idea to trigger buildbots on this PR!

@StanFromIreland
Copy link
Member

Thanks!

trigger buildbots on this PR

I can run the buildbots, do you want them now?

@loic-simon
Copy link
ContributorAuthor

I can run the buildbots, do you want them now?

Yes please! I don't plan any more change on my side.

@StanFromIrelandStanFromIreland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 11, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @StanFromIreland for commit 9c80367 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140871%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 11, 2026
@loic-simon
Copy link
ContributorAuthor

loic-simon commented Jan 11, 2026

Okay that was useful, buildbot failures analysis:

  • A lot of fails on compression.ztsd not suggestingcompress attribute, which is expected since it is an optional module, I'll use a better test case 😅
  • All Reafleaks buildbots fail on collections.abc not suggesting Buffer, I don't know why but I suppose it's expected too, I'll use another test case
  • iOS ARM64 Simulator has a lot of other fails, I don't quite understand what's going on 🤔
    • maybe an importer cache issue like last time? I'll add some invalidation
Details

Missing compression.ztsd:

AMD64 Debian root PR AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR AMD64 RHEL8 LTO + PGO PR AMD64 RHEL8 LTO PR AMD64 RHEL8 PR AMD64 RHEL8 Refleaks PR AMD64 Ubuntu Shared PR ARM64 MacOS M1 NoGIL PR ARM64 MacOS M1 Refleaks NoGIL PR ARM64 Raspbian Debug PR ARM64 Raspbian PR PPC64LE RHEL8 LTO + PGO PR PPC64LE RHEL8 LTO PR PPC64LE RHEL8 PR PPC64LE RHEL8 Refleaks PR s390x Fedora Stable Clang Installed PR s390x Fedora Stable Clang PR s390x Fedora Stable LTO + PGO PR s390x Fedora Stable LTO PR s390x Fedora Stable PR s390x Fedora Stable Refleaks PR s390x RHEL8 LTO + PGO PR s390x RHEL8 LTO PR s390x RHEL8 PR s390x RHEL8 Refleaks PR s390x RHEL9 LTO + PGO PR s390x RHEL9 LTO PR s390x RHEL9 PR s390x RHEL9 Refleaks PR x86-64 MacOS Intel ASAN NoGIL PR x86-64 MacOS Intel NoGIL PR 

Missing collections.abc.Buffer:

Missing AMD64 CentOS9 NoGIL Refleaks PR AMD64 FreeBSD Refleaks PR AMD64 RHEL8 Refleaks PR ARM64 MacOS M1 Refleaks NoGIL PR PPC64LE RHEL8 Refleaks PR s390x Fedora Stable Refleaks PR s390x RHEL8 Refleaks PR s390x RHEL9 Refleaks PR 

A lot of fails:

iOS ARM64 Simulator PR 

@loic-simon
Copy link
ContributorAuthor

@StanFromIreland if you could trigger a builbots run again, that would be great! Thanks

@StanFromIrelandStanFromIreland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 24, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @StanFromIreland for commit da6cf7f 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140871%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 24, 2026
@loic-simon
Copy link
ContributorAuthor

loic-simon commented Jan 24, 2026

Ok, good news: iOS Siumlator is green!

Bad news: all Refleaks builds now fail ~all attribute completion tests 😞

Good news: I reproduced in local (python -m test pass, -m test -R: fails), on it!

@loic-simon
Copy link
ContributorAuthor

That check fail doesn't look related to this PR (Error: The template is not valid. .github/workflows/build.yml (Line: 720, Col: 24): Error reading JToken from JsonReader. Path '', line 0, position 0.)

Refleak tests pass locally, may I ask for a last run with 🔨 test-with-refleak-buildbots@StanFromIreland, just to be sure? Thanks a lot!

@StanFromIreland
Copy link
Member

That check fail doesn't look related to this PR (Error: The template is not valid. .github/workflows/build.yml (Line: 720, Col: 24): Error reading JToken from JsonReader. Path '', line 0, position 0.)

However, it is. You need to update the branch, the current version is incompatible with our CI.

@StanFromIrelandStanFromIreland added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 24, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @StanFromIreland for commit 0106165 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140871%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jan 24, 2026
@loic-simon
Copy link
ContributorAuthor

All buildbots runs have exactly the same refleaks as main (12 references on test_httplib and 6 on test_bytes), so I suppose we're good here! Thanks again Stan 😃

Sign up for freeto 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.

4 participants

@loic-simon@tomasr8@StanFromIreland@bedevere-bot