Add pylock #900
Add pylock #900
Conversation
| url: str | None # = None | ||
| path: str | None # = None | ||
| size: int | None # = None | ||
| upload_time: datetime | None # = None |
There was a problem hiding this comment.
In the spec, the upload_time field is not at the same position for archive and sdist / wheel.
Is that intended or something we want to tweak here and/or in the spec?
There was a problem hiding this comment.
That was intended as the idea (and expected value) comes from the index API and I don't think archives are supported there.
There was a problem hiding this comment.
But if people have uses for an upload time for archives then I suspect making it a 1.1 change wouldn't be controversial.
There was a problem hiding this comment.
Note my remark was only about the ordering of the fields, not about the presence of upload_time in archive.
My idea was to have from_dict independent of the class to cope for future evolution, but if a new version of the standard can't be implemented as a subclass of Pylock, return type of from_dict would change, so it would be a breaking change. If/when a new version of the spec arrive, it will still be time to design the API evolution, depending of the actual changes.
|
I consider this PR ready for review. I plan to add documentation after a first round of review, assuming the overall design is agreeable to packaging maintainers, of course. |
|
FYI I'm hoping to look at this post-EuroPython in hopes that it helps unblock pip so it can add installation support for |
| PYLOCK_FILE_NAME_RE = re.compile(r"^pylock\.([^.]+)\.toml$") | ||
|
|
||
|
|
||
| def is_valid_pylock_path(path: Path) -> bool: | ||
| return path.name == "pylock.toml" or bool(PYLOCK_FILE_NAME_RE.match(path.name)) |
There was a problem hiding this comment.
If the regex is not going to be private, we might as well make it work for all names.
| PYLOCK_FILE_NAME_RE = re.compile(r"^pylock\.([^.]+)\.toml$") | |
| def is_valid_pylock_path(path: Path) -> bool: | |
| return path.name == "pylock.toml" or bool(PYLOCK_FILE_NAME_RE.match(path.name)) | |
| PYLOCK_FILE_NAME_RE = re.compile(r"^pylock\.(?:([^.]+)\.)toml$") | |
| def is_valid_pylock_path(path: Path) -> bool: | |
| return bool(PYLOCK_FILE_NAME_RE.match(path.name)) |
|
|
||
|
|
||
| def _toml_value(key: str, value: Any) -> Any: | ||
| if isinstance(value, (Version, Marker, SpecifierSet)): |
There was a problem hiding this comment.
| if isinstance(value, (Version, Marker, SpecifierSet)): | |
| if isinstance(value, {Version, Marker, SpecifierSet}): |
|
|
||
|
|
||
| def _get(d: Mapping[str, Any], expected_type: type[T], key: str) -> T | None: | ||
| """Get value from dictionary and verify expected type.""" |
There was a problem hiding this comment.
| """Get value from dictionary and verify expected type.""" | |
| """Get a value from the dictionary and verify it's the expected type.""" |
| value = d.get(key) | ||
| if value is None: | ||
| return None |
There was a problem hiding this comment.
| value = d.get(key) | |
| if value is None: | |
| return None | |
| if (value := d.get(key)) is None: | |
| return None |
|
|
||
|
|
||
| def _get_required(d: Mapping[str, Any], expected_type: type[T], key: str) -> T: | ||
| """Get required value from dictionary and verify expected type.""" |
There was a problem hiding this comment.
| """Get required value from dictionary and verify expected type.""" | |
| """Get a required value from the dictionary and verify it's the expected type.""" |
|
|
||
| PEP751_EXAMPLE = dedent( | ||
| """\ | ||
| lock-version = '1.0' |
There was a problem hiding this comment.
Would it make sense to put this in its own file in a subdirectory like metadata and musllinux?
| } | ||
| with pytest.raises(PylockValidationError) as exc_info: | ||
| Pylock.from_dict(data) | ||
| assert str(exc_info.value) == "Invalid version: '2.x' in 'lock-version'" |
There was a problem hiding this comment.
Probably better to just test that "2.x" is somewhere in the message.
Same goes for any of the other tests that's doing an exact exception message comparison instead of just testing the key details are somewhere in the string.
| { | ||
| "name": "example-1.0-py3-none-any.whl", | ||
| "path": "./example-1.0-py3-none-any.whl", | ||
| # "hashes": {"sha256": "f" * 40}, |
There was a problem hiding this comment.
| # "hashes": {"sha256": "f" * 40}, | |
| # Purposefully no "hashes" key. |
| def test_toml_roundtrip() -> None: | ||
| pylock_dict = tomllib.loads(PEP751_EXAMPLE) | ||
| pylock = Pylock.from_dict(pylock_dict) | ||
| # Check that the roundrip via Pylock dataclasses produces the same toml |
There was a problem hiding this comment.
| # Check that the roundrip via Pylock dataclasses produces the same toml | |
| # Check that the roundrip via Pylock dataclasses produces the same TOML |
| run-on = 2025-03-06T12:28:57.760769 | ||
| """ # noqa: E501 | ||
| ) | ||
|
|
There was a problem hiding this comment.
Is it worth having a test to verify this example deserializes as expected?
Here is the proposal for
pylock.tomlsupport.from_dictclass method to validate and create aPylockinstance from a toml dict (obtained fromtomllib.load)Pylock.to_dictmethod to convert to a spec-compliant toml dict (assuming types were respected when populating the dataclass), preserving the recommended field orderingis_valid_pylock_pathto validate pylock file namesDocumentation is still TODO.
closes #898