python / importlib_metadata

Library to access metadata for Python packages
https://importlib-metadata.readthedocs.io
Apache License 2.0
128 stars 82 forks source link

Make `importlib_metadata._meta.SimplePath` public #494

Closed matthewhughes934 closed 3 months ago

matthewhughes934 commented 3 months ago

I see this class is documented https://importlib-metadata.readthedocs.io/en/latest/api.html#importlib_metadata._meta.SimplePath but the fact it is under the ._meta package, which looks private at a glance, makes me wonder: is is supported for me to access this, is its API as stable as any other part of the package? Additionally (if the answer is yes): could this class just be re-exported by importlib_metadata to avoid this need for access to ._meta?

My use case is: I have a class that sub-classes Distribution and need to annotate the return value for locate_file which should be one of these SimplePaths, and asking here because this appears to be the origin of the class, with it being added upstream from here: https://github.com/python/cpython/commit/667294d5b2ee812ebe0c9c1efd58e2006b61f827

jaraco commented 3 months ago

Great question. I'm fairly sure I made it private so that it wouldn't be used as an API. For type checking, however, it probably should be exposed. SimplePath was added in #315.

@FFY00 Do you have any concerns with exporting SimplePath in importlib_metadata (__init__.py)?

matthewhughes934 commented 3 months ago

:thinking: actually, since SimplePath is a protocol, exposing it feels unnecessary: why can't my code just return a concrete type that implements it

FFY00 commented 3 months ago

@FFY00 Do you have any concerns with exporting SimplePath in importlib_metadata (__init__.py)?

I don't, I think it should be exposed.

🤔 actually, since SimplePath is a protocol, exposing it feels unnecessary: why can't my code just return a concrete type that implements it

That might be fine in your particular use-case, but if you want to annotate a public API that consumes SimplePath types, it makes sense to have it exposed.

jaraco commented 3 months ago

Oh, I see it's already exposed; it's just not listed in __all__ (making it explicitly, intentionally exposed); I'll do that.