Skip to content
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

Problem with PackageMetadata protocol #374

Closed
jaraco opened this issue Mar 12, 2022 · 6 comments · Fixed by #375
Closed

Problem with PackageMetadata protocol #374

jaraco opened this issue Mar 12, 2022 · 6 comments · Fixed by #375
Labels
question Further information is requested

Comments

@jaraco
Copy link
Member

jaraco commented Mar 12, 2022

In #371, @saaketp reported:

The second problem is not clear whether it was intentional or a mistake. The actual type returned by the metadata function is a subclass of email.message.Message which supports "dict-like" operations like .get and .items(). I have seen some scripts rely on this dict-like behavior and I only noticed these methods are not present in PackageMetadata protocol because of pyright complaining.

Are those methods intentionally left out of the protocol or should they be added?

@jaraco jaraco added the question Further information is requested label Mar 12, 2022
@jaraco
Copy link
Member Author

jaraco commented Mar 12, 2022

Those methods were intentionally left out of the protocol. Only the protocol interface is supported. Let me know if you have follow-up concerns or advice.

@jaraco jaraco closed this as completed Mar 12, 2022
@saaketp
Copy link
Contributor

saaketp commented Mar 12, 2022

Thanks @jaraco for the response. My only concern is that it is not easy to figure out what is supported or not unless you are using a type checker, which most people don't.

The standard library docs don't mention this properly. The only section that mentions PackageMetadata is https://docs.python.org/3/library/importlib.metadata.html#distribution-metadata. It would be good to have a section about PackageMetadata itself with the list of methods it has, like it is done for all other standard library classes.

Without that I think people just look at the methods available on the object at runtime, which contains a lot of extra stuff which is not supported, and the fact that it behaves almost like a dict makes it more likely to be confusing.

I see that the docs in readthedocs have a API reference which kinda has this information (though it would be good to list the dunder methods supported too) https://importlib-metadata.readthedocs.io/en/latest/api.html#importlib_metadata.PackageMetadata but this is not included in the stdlib docs (unless I missed it somehow). I only started using this after it landed in the standard library, so I first looked at the docs in standard library and only found these docs later.

@jaraco jaraco reopened this Mar 13, 2022
@jaraco
Copy link
Member Author

jaraco commented Mar 19, 2022

These docs are synced to the standard library, except some features available here don't have an analog in CPython (such as automodule). That's why the docs here are better than the docs there. To get to these same docs in CPython, you have to follow the link to the source.

image

It would be good to have a section about PackageMetadata itself with the list of methods it has, like it is done for all other standard library classes.

I'd welcome contributions to supply this. I'm personally disinclined to write documentation that is merely a copy of what the implementation already documents and the dependency and maintenance burden that creates. Better would be for the CPython docs to support automodule or something similar to render docs in the code to the manual. Until then, the best users have is to fall back to the importlib_metadata docs.

I have seen some scripts rely on this dict-like behavior.

Perhaps importlib metadata should mask these interfaces so that others aren't tempted to rely on them... or at least advertise them as unsupported.

@saaketp
Copy link
Contributor

saaketp commented Mar 19, 2022

Maybe we can just have a "See also:" box that links to https://importlib_metadata.readthedocs.io/?
Similar to how https://docs.python.org/3/library/sqlite3.html links to https://www.sqlite.org

And then another link to https://importlib-metadata.readthedocs.io/en/latest/api.html#importlib_metadata.PackageMetadata after the first mention of PackageMetadata to advertise what is supported.

@jaraco
Copy link
Member Author

jaraco commented Mar 19, 2022

That sounds good to me. Are you willing to draft that change? If you propose it in this project under the using.rst doc, and it will get merged upstream into CPython.

@saaketp
Copy link
Contributor

saaketp commented Mar 20, 2022

Okay I created a PR for that change #375

I wasn't too sure about how rst works, so I just copy pasted the directives I saw being used in cpython docs, hopefully they work here too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
2 participants