-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stubtest: error if typeshed is missing modules from the stdlib #15729
Stubtest: error if typeshed is missing modules from the stdlib #15729
Conversation
@sobolevn pointed out that #15725 wouldn't have been a good idea as it was based on some bad assumptions, so here's the updated list of allowlist entries that we'd have to add to typeshed if this PR was merged (the previous list was based on the assumption that #15725 would be merged first):
|
Friendly ping @hauntsaninja -- do you like the idea here? :) |
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.
Seems reasonable, although at this point in typeshed's life it might not be too helpful.
Fwiw, this is the sys.stdlib_module_names equivalent I use on Python 3.9:
all_stdlib = set()
importers = filter(
lambda x: isinstance(x, importlib.machinery.FileFinder), pkgutil.iter_importers()
)
for importer in importers:
modules = {m for m, _ in pkgutil.iter_importer_modules(importer)}
# Hacky, but whatever
if "json" in modules or "_json" in modules:
if False: # if you were to want submodules
all_stdlib.update(m.name for m in pkgutil.walk_packages([importer.path]))
else:
all_stdlib.update(modules)
all_stdlib.update(sys.builtin_module_names)
return all_stdlib
(Hm having posted that, it could actually be nice for this code to not actually import the modules like above, but ofc doesn't matter stubtest's purposed)
Nice! I pushed a variant of that. I've also pushed some changes so that we actually check submodules are importable before adding them to the set -- there are some which are not importable on all platforms.
I'd appreciate being able to keep track, I think -- my motivation for making this PR was that I realised we were missing several public importlib submodules that had been added in recent Python versions, and I was surprised stubtest wasn't complaining about their absence. |
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.
Nice, let's do it!
We currently flag modules missing from third-party stubs in stubtest, but don't do similarly for typeshed's stdlib stubs. This PR adds that functionality for typeshed's stdlib stubs as well.
This PR currently results in quite a few errors when running stubtest on typeshed's stdlib stubs. Some of them are true positives, and some are not. However, I think all of them (except for many
test
ortests
submodules, which would be suppressed if #15725 is merged) are things I'd prefer for us to have explicitly allowlisted in typeshed, so we can easily see what we're missing.If #15725 is merged and this is merged, we'd have to add the following allowlist entries for typeshed's stdlib stubs to get stubtest passing again: