gh-146401: suggest a module in sys.builtin_module_names#146421
gh-146401: suggest a module in sys.builtin_module_names#146421Locked-chess-official wants to merge 1 commit intopython:mainfrom
Conversation
Locked-chess-official
requested review from
FFY00,
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
Lib/importlib/_bootstrap.py
Outdated
| @classmethod | ||
| def discover(cls, spec=None): | ||
| if spec is not None: # assume that built-in modules have no submodule | ||
| return | ||
| for i in sys.builtin_module_names: | ||
| yield cls.find_spec(i) |
There was a problem hiding this comment.
I don't understand why we need this specific change here. It feels wrong to change importlib.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
If you want to improve suggestions for built-in names, it should be at the REPL level. Not in |
|
Re-opening as we will make the change elsewhere. |
|
@Locked-chess-official I'll make it a draft. Once you're done with your changes, ping me and I'll remove the reviewers as they won't be relevant anymore. |
Locked-chess-official
force-pushed
the
suggest_builtin_module
branch
from
3bdbfa1 to
c01925d
Compare
picnixz
removed request for
FFY00,
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
| else: | ||
| parent = None | ||
| d = [] | ||
| d = list(sys.builtin_module_names) |
There was a problem hiding this comment.
Ah now, I see what happens. I'm really sorry. Maybe the change in the finder is ok? cc @brettcannon
There was a problem hiding this comment.
I don't know what "finder" you're referring to as this it in the traceback module that's not a finder.
There was a problem hiding this comment.
Sorry for the lack of context. We use "finders" in sys.meta_path to know about the "known" modules via the discover() method but BuiltinImporter (I think it's this one) doesn't have a discover() method. Should we add one to this importer?
There was a problem hiding this comment.
More precisely, we previously had this addition in the importer:
@classmethod
def discover(cls, spec=None):
if spec is not None: # assume that built-in modules have no submodule
return
for i in sys.builtin_module_names:
yield cls.find_spec(i)
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
Can you add a test on this change?
4 participants
Change to use
@classmethodto yield specs.