-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Implement PEP 643 (Dynamic
field for core metadata)
#4698
base: main
Are you sure you want to change the base?
Conversation
94a60ef
to
2ea79ce
Compare
@jaraco would this kind of approach be acceptable in setuptools? |
2ea79ce
to
f7b42e4
Compare
…ad of _static.{Tuple,Mapping} for better compatibility
f7b42e4
to
38d6b05
Compare
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.
I haven't gone deep into the details, but at a high level, this change seems to honor the intentions of PEP 643.
It's not obvious to me what changes from a user's perspective. What projects are affected by this change? Does it simplify or complicate their experience? I think the answer is that projects largely don't have to do anything, but this change gives more fidelity to the metadata (at the cost of the added logic maintenance). If that's right, that seems like a fair tradeoff. Happy to give it a try. Let me know if there's a type of project you think I should run it against.
Thank you very much @jaraco. The motivation of the PEP is purely to catch up with the standards, I don't think it changes much from a user's perspective. However it may be used by frontends/installer for optimisations and therefore improve installation speed.
This week I will not have the time, but my plan is to build some projects and try to upload them to |
return _CONVERSIONS.get(type(value), noop)(value) # type: ignore[call-overload] | ||
|
||
|
||
def is_static(value: Any) -> bool: |
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.
An object parameter should match anything. You should be able to avoid using Any
here.
def is_static(value: Any) -> bool: | |
def is_static(value: object) -> bool: |
You can also make this function a type-guard with:
def is_static(value: Any) -> bool: | |
if TYPE_CHECKING: | |
from typing_extensions import TypeGuard # Added to typing in Python 3.10 | |
def is_static(value: object) -> TypeGuard[Static]: |
IMPORTANT: #4701 needs to be merged first, then this PR needs to be rebased (probably there will be conflicts). It might also be a good idea to wait until the next version of wheel is released (v0.45).
Summary of changes
This PR attempts to implement the 2nd approach suggested in #2685:
Values given via
setup.py
are considered dynamic for the sake of simplicity.The "trick" implemented in this PR to avoid the problems with plugins/customisations described in #4629 (comment), is to mark static values instead of dynamic ones.
This marking is a bit "unconventional" (via subclasses of buit-in types), but it simplifies the tracking of changes and is backward compatible with the existing distutils/setuptools code base.
OPEN PROBLEM:
wheel.metadata.pkginfo_to_metadata
hardcodesMetadata-Version: 2.1
.PKG_INFO
file in setuptools is good enough and obviate usage ofpkginfo_to_metadata
=> Preserve originalPKG-INFO
contents when creating wheel (instead of callingwheel.metadata.pkginfo_to_metadata
) #4701.Closes #2685
Pull Request Checklist
newsfragments/
.(See documentation for details)