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 2 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

  • Changes have tests (existing build+install and type-checking tests)
  • News fragment added in newsfragments/.
    (See documentation for details)

@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

setup.py Outdated
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

@Avasam Avasam force-pushed the generate-distuttils-stubs-on-install branch from 73befc0 to d9e413e Compare March 9, 2025 02:11
@Avasam
Copy link
Contributor Author

Avasam commented Mar 9, 2025

I think this doesn't work with tox because of PEP660 editable installs? I'll need help to get that working.

@Avasam
Copy link
Contributor Author

Avasam commented Mar 9, 2025

I very nearly got it working with a custom build backend to support PEP 660 editable installs, which also happens to leave the stubs in the source (which I can just .gitignore). Bringing all the benefits of #4704 .
But I'm currently blocked on this mypy issue: python/mypy#18775

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