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

Generate distutils-stubs on install #4861

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Mar 4, 2025

Summary of changes

3rd iteration to close #4689 (doesn't solve everything mentioned in that issue, but it would greatly reduce the scope so I'd prefer re-opening as a new issue)

This is built off of #4704 where the stubs are automatically generated, but now they're done on install instead of existing in the source code.

Further Advantages over #4704:

  • Doesn't pollute the source or history

Further Disadvantages over #4704:

  • Contributors must install the local setuptools repo in their environment (pip install .) to get proper type-checking in their editor on Python 3.12+ when it comes to pypa/distutils derived types. (they already had to install types-setuptools, this replaces that)

Pull Request Checklist

@Avasam
Copy link
Contributor Author

Avasam commented Mar 4, 2025

Aaaah, right, I forgot that *-stubs will never override the stdlib types from typeshed. So this only affects Python 3.12+
For end users: This is already the case with typeshed's own setuptools stubs, so that's fine. (This is true even of #4689)
For setuptools contributors: This means we can't use this to get correct types in our own code, when pypa differs from stdlib on Python < 3.12

Overall still an improvement (and typeshed won't have to special case this stub anymore) and a necessary step for #2345

Comment on lines +51 to +53
module = "setuptools._distutils." + str(relative_path.with_suffix("")).replace(
os.sep, "."
).removesuffix(".__init__")
Copy link
Contributor Author

@Avasam Avasam Mar 4, 2025

Choose a reason for hiding this comment

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

Alternative code for readability (but less "correct" because replace is run on an unnecessary extra bit of str):

Suggested change
module = "setuptools._distutils." + str(relative_path.with_suffix("")).replace(
os.sep, "."
).removesuffix(".__init__")
module = (
("setuptools._distutils." + str(relative_path.with_suffix("")))
.replace(os.sep, ".")
.removesuffix(".__init__")
)

Playing with it some more (also changed with_suffix for removesuffix, just to see):

Suggested change
module = "setuptools._distutils." + str(relative_path.with_suffix("")).replace(
os.sep, "."
).removesuffix(".__init__")
module = (
"setuptools._distutils." # (don't unwrap)
+ str(relative_path)
.removesuffix(".py")
.replace(os.sep, ".")
.removesuffix(".__init__")
)

@Avasam Avasam mentioned this pull request Mar 4, 2025
2 tasks
@Avasam
Copy link
Contributor Author

Avasam commented Mar 4, 2025

@jaraco I think that this version has minimal (if any) downsides, solves real issues (addresses the user-facing concerns of #4689 and part of the setuptools internal typing ones), is literally as good as typeshed's and works towards #2345

(PS: Please squash merge if I don't rebase this)

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.

[FR] Vendor distutils stubs
1 participant