Conversation
src/packaging/pylock_select.py
Outdated
| f"Invalid wheel filename {package_wheel.name!r} for " | ||
| f"package {package.name!r} at index {package_index}" | ||
| ) from e | ||
| if not package_wheel_tags.isdisjoint(tags): |
There was a problem hiding this comment.
Is this the correct way to determine that a wheel is compatible with tags?
There was a problem hiding this comment.
Conceptually, the way to select it to iterate through the compatible tags to look for any wheel that's compatible as the compatibility tags should be in priority order:
# Pseudocode
for tag in tags:
for wheel in wheels:
if tag in tags_from_wheel_name(wheel):
return wheel
else:
raise Exception("no compatible wheel found")It might be more efficient to build a dict of tag: priority once for the compatible tags and go through the wheels looking for the highest-priority compatible tag. My assumption is that there are way more compatibility tags than tags from wheels, to upfront processing it worth it.
There was a problem hiding this comment.
@brettcannon that's interesting. As a side note the isdisjoint approach is the one used by pip.
Should we create some select_compatible_wheels(tags: Sequence[Tag], wheel_filenames: Collection[str]) -> Ierator[str] function?
edit: I'd make the second argument Collection[NamedWheel | str] where NamedWheel is a Protocol with a wheel_name() -> str property.
Is there existing code somewhere that could serve as a test suite for such a function?
There was a problem hiding this comment.
As a side note the
isdisjointapproach is the one used by pip.
It's probably right most of the time thanks to wheel tag name sorting.
Should we create some
select_compatible_wheels(tags: Sequence[Tag], wheel_filenames: Collection[str]) -> Ierator[str]function?
We could. I have thought about it, but I'm always worried the perf would suck without some state in case some preprocessing was worth it. Maybe a closure that returned a filter function?
def create_wheel_filter(tags: Sequence[Tag]) -> Callable[[Collection[str]], Iterator[str]:
...I also don't know if unparsed wheel file names will be the most common input or ones already parsed?
There was a problem hiding this comment.
Would it be ok to land this PR with not isdisjoint and defer a more advanced wheel selection mechanism to later?
|
/cc @brettcannon |
9a2a299 to
699543d
Compare
699543d to
77ce285
Compare
5ed55d5 to
f52e9cf
Compare
f52e9cf to
9112d47
Compare
| continue | ||
|
|
||
| # #. If :ref:`pylock-packages-requires-python` is specified, check if it is | ||
| # satisfied; an error MUST be raised if it isn't. |
There was a problem hiding this comment.
@brettcannon why is it an error if a package requires-python is not satisfied? Should it not be skipped in that case?
There was a problem hiding this comment.
That's for when https://packaging.python.org/en/latest/specifications/pylock-toml/#requires-python has been satisfied but somehow a specific package claims otherwise. When I wrote that I'm sure my brain was assuming requires-python would be set if packages.requires-python was set.
src/packaging/pylock.py
Outdated
| # a lack of source for the project. | ||
| for package_wheel in package.wheels: | ||
| try: | ||
| assert package_wheel.name # XXX get name from path or url |
This will be handled by validation.
This is WIP. Main missing part is handling of dependency groups and extras. And tests of course.
Comments are from PEP 751 Installation section.
closes #1087