Add json_metadata property to BaseDistribution#11095
Conversation
|
How important is it that |
|
@pfmoore In this case, it was convenient to have it since So from this unique sample, I say this is useful. In other places where I've had to handle package metadata, email.message.Message is also involved, so it is certainly a convenient intermediate representation. That said, exposing functions that handle metadata file encodings properly in |
|
Sounds good. I can expose |
| return field.lower().replace("-", "_") | ||
|
|
||
|
|
||
| def msg_to_json(msg: Message) -> Dict[str, Any]: |
There was a problem hiding this comment.
This should probably be msg_to_dict instead since the output is not actually JSON (which should be a str), but a dict decoded from JSON metadata.
| ] | ||
|
|
||
|
|
||
| def json_name(field: str) -> str: |
There was a problem hiding this comment.
Let’s make this private so we don’t need to worry about json_name being much too vague…
There was a problem hiding this comment.
It's not in __all__ in the original implementation, but agreed a better name might be good.
Would you care to review https://github.com/pfmoore/pkg_metadata? I'd be happy to incorporate any other feedback you might have in the original library.
src/pip/_internal/metadata/base.py
Outdated
| raise NotImplementedError() | ||
|
|
||
| @property | ||
| def json_metadata(self) -> Dict[str, Any]: |
There was a problem hiding this comment.
Same for this property name; json feels misleading 😟
|
Hi @uranusjr, thanks for the review! Actually this json conversion function is copied verbatim from @pfmoore 's pkg_metadata, as the goal is to vendor it when Paul says it is good enough for that. That is also the reason why there are no tests for it here. See also comment The I felt a property was appropriate since there was already a I agree |
|
I’ll probably change the name to |
e3db108 to
70d0336
Compare
To make it explicit that it is a private implementation detail.
5ffcb7f to
ae67371
Compare
| :raises NoneMetadataError: If the metadata file is available, but does | ||
| not contain valid metadata. | ||
| """ | ||
| return msg_to_json(self.metadata) |
There was a problem hiding this comment.
This could be cached, too. There's no real reason to recalculate it every time. Although it's probably not a performance bottleneck, so it's not a big deal either way.
As suggested by @pfmoore in #10771 (comment), this copies the
msg_to_jsonfunction from hispkg_metadataproject.This function will be used in
pip install --report(and possiblypip list --format=json).Towards #53 and #10771.