Skip to content
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

Use sys.stdlib_module_names to complement known stdlib modules #2295

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

devdanzin
Copy link

This PR attempts to fix #2250 by adding the names from sys.stdlib_module_names (or sys.builtin_module_names for versions below 3.10) to the known stdlib modules lists (and as such is an alternative to #2279).

It still uses Sphinx to download known module names, adding the names from sys to the downloaded lists. Some of the new names (antigravity, this) are excluded, please let me know if any others should be too.

A test for some of the new known modules is included.

Happy to apply any suggestions to the code.

…or versions below 3.10) to the known stdlib modules lists.
@DanielNoord
Copy link
Member

With #2306 we will be using the stdlibs package.

However, as they say themselves in the README on newer versions this shouldn't be necessary. It would be good to rebase this after we merge the linked PR and replace where possible. If all is well that shouldn't create a diff in the actual stdlib list but only in the generating code.

Copy link
Collaborator

@staticdev staticdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devdanzin thanks a lot by your contribution. Unfortunately this one was a bit forgoten and got outdated. Do you mind updating it and if possible also adding 3.13?

@devdanzin
Copy link
Author

Unfortunately this one was a bit forgoten and got outdated. Do you mind updating it and if possible also adding 3.13?

Sure, I'll do it if we decide it's worth it, but I see main is using the stdlibs package to gather module names since #2306, that might be as good as or even better than using sys.stdlib_module_names. It's up to you whether we keep it like that or use the internal list of module names from sys.

Also, I see that the new code skips "private" stdlib modules that start with an underscore, while my original patch included them, because #2250 is about such a module. So we should decide whether to keep it without "private" modules, or add them back (stdlibs has all of them). I think having all possible module names listed wouldn't hurt and might avoid further bug reports for incorrect sorting of "private" modules.

@devdanzin
Copy link
Author

Aha, and my patch is incorrect anyway: it uses sys.stdlib_module_names of the running Python version instead of the target version. It'd be possible to make a dance to launch the target versions and fetch the module names from them, but using the stdlibs package seems cleaner.

With that, this PR should only focus on whether to include private modules and whether to remove some entries, like antigravity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isort does not recognize _collections_abc as part of the standard library
3 participants