Add PackageSpec.from_wheel #18
Conversation
pyodide_lock/spec.py
Outdated
| raise RuntimeError(f"Could not parse wheel metadata from {path.name}") | ||
|
|
||
| return PackageSpec( | ||
| name=_normalized_name(metadata.name), |
There was a problem hiding this comment.
I'm not sure it's correct to use normalized names here and in dependencies. I think it should be the package names found in the METADATA file in the wheel?
There was a problem hiding this comment.
Welp... working on various tools, I've had a lot of issues using anything other than the normalized names without having to to assume they need to be re-normalize every single time a comparison/lookup is done.
The wheel artifact name is absolute, of course, and shouldn't be messed with... even though that has now been "standardized" to lowercased _, the vast bulk of them out there aren't.
This APIs on PyPI have long been lowerecase- normalized.
But due to the above baggage, the contents of Requires-Dist in wheels also don't have normalized names, such that in using them verbatim would make it possible to have two packages in the same lockfile that specify the same effective dependency with different cases.
I think as a bespoke lockfile format, normalizing everything on ingest is the right play, and indeed, might consider further tightening the schema with a JSON Schema compatible regex. pip and friends which do have to support all the millions of wheels are already going to have to do normalizing, so even if a lockfile is the input to another process that builds HTML, downstreams will be able to handle pyyaml vs PyYAML just fine.
There was a problem hiding this comment.
I've actually gone a bit further, using the already-used packaging.utils.canonicalize_name all over. The tests added in b01b33b show how this might change expectations.
There was a problem hiding this comment.
Indeed, I feel like this should go even further, and hoist the various regexen to pattern on name and depends so they make it all the way to #3. pydantic 1.x and 2.x both have constr, but sadly require regex or pattern, respectively, while the likely preferable typing.Annotated route probably doesn't work in 1.x. So probably picking a major version to support would clear things up.
There was a problem hiding this comment.
OK, sounds good to me. Thanks for the detailed explanation! Also given #20 (comment) let's go forward with it.
|
Cheers, thanks. |
References
Changes
PackageSpec.from_wheel[wheel]extra withpackagingandpkginfoTYPE_CHECKINGblocks in coverage reportingQuestions