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

importlib.metadata vs importlib-metadata #10157

Closed
brechtm opened this issue Feb 2, 2022 · 7 comments
Closed

importlib.metadata vs importlib-metadata #10157

brechtm opened this issue Feb 2, 2022 · 7 comments
Labels

Comments

@brechtm
Copy link
Contributor

brechtm commented Feb 2, 2022

Describe the bug

Commit 6c5c66b by @takluyver removed the dependency on pkg_resources. It prefers the third-party importlib-metadata package over the stdlib importlib.metadata module, even if the Python version provides the latter (which is the case since 3.8). The Sphinx setup.py specifies importlib-metadata as a dependency for Python 3.10.

This poses a problem when using Sphinx 4.4.0 with the rinoh builder. rinohtype only falls back to using importlib-metadata if importlib.metadata is not available. So, when using Python 3.8 or 3.9, rinohtype will create a DynamicRinohDistribution instance which inherits from importlib.metadata.Distribution, while Sphinx will be using importlib-metadata, which expects Distribution instances to have a _normalized_name attribute and this crashes on encountering the DynamicRinohDistribution (test log).

Is there any particular reason Sphinx does not prefer to use the importlib.metadata included with Python? If that is the case, can you suggest an approach to fix this issue. Personally, I don't see one yet.

Thanks!

How to Reproduce

$ git clone https://github.com/brechtm/rinohtype.git
$ cd rinohtype
$ poetry install
$ nox -s "unit_sphinx(sphinx='4.4.0')"

Expected behavior

No response

Your project

https://github.com/brechtm/rinohtype

Screenshots

No response

OS

any

Python version

3.9.9

Sphinx version

4.4.0

Sphinx extensions

rinohtype

Extra tools

No response

Additional context

No response

@astrojuanlu
Copy link
Contributor

If I understand correctly, the solution would be to switch

try: # Python < 3.10 (backport)
from importlib_metadata import entry_points
except ImportError:
from importlib.metadata import entry_points

for

try:
    from importlib.metadata import entry_points
except ImportError:  # Python < 3.10 (backport)
    from importlib_metadata import entry_points

?

@brechtm
Copy link
Contributor Author

brechtm commented Feb 2, 2022

@astrojuanlu Yes, that would be a solution. I feel it makes sense to use the Python-provided module if it's available and only use importlib-metadata as a fallback. However, perhaps @takluyver had a good reason to not do that.

@takluyver
Copy link
Contributor

The importlib.metadata API changed in 3.10 (see compatibility note in the docs). I used the new API, because it seems fairly clear from the docs and the deprecation warnings that the intent is to get rid of the old way in a couple of Python versions. But using the new API means the backport has to be used for Python 3.8 & 3.9.

So if you want to change the preference around in Sphinx, you'll also have to port the code loading the entry points back to the deprecated API, and then at some point in a couple of years someone will have to switch it back again. It's pretty trivial to actually make either change, though.

@brechtm
Copy link
Contributor Author

brechtm commented Feb 2, 2022

@takluyver That makes sense as well, of course. I'd prefer not to make the Sphinx code less future-proof of course.

I hope you don't mind me pinging you, @jaraco. But perhaps you have an idea on how to best handle this issue:

  • one package defaults to using importlib.metadata, and subclasses its Distribution class (which doesn't yet have the _normalized_name attribute)
  • another package defaults to using a recent importlib-metadata which expects a _normalized_name attribute in the Distribution instance

I know it's not a proper solution, but I'm considering just adding the _normalized_name property myself as a quick fix.

EDIT: I guess what matters here is whether importlib.metadata and importlib-metadata are backwards or forwards compatible. If older versions are happy with Distribution instances from newer versions (the other way around is obviously not working), perhaps I can use just importlib-metadata (when available)?

@brechtm
Copy link
Contributor Author

brechtm commented Feb 11, 2022

My first basic tests with importing importlib-metadata if it's available seem to indicate that it's Distribution class is backwards compatible. i.e. the instance of my subclass is happily accepted by Python 3.9's importlib.metadata's distributions and entry_points.

I'll change rinohtype to import importlib-metadata if it's available.

brechtm added a commit to brechtm/rinohtype that referenced this issue Feb 11, 2022
@jaraco
Copy link

jaraco commented Feb 11, 2022

If you have a choice between using the older entry points API and the new one, I'd definitely recommend going with the new one where possible. I'm okay with a custom Distribution supplying its own _normalized_name to satisfy the interface for forward compatibility. I don't know if rinohtype could consider unconditionally requiring importlib_metadata; python_version<"3.9", but if so that should most simply provide compatibility. It sounds like you have pretty good handle on the issue and have good options moving forward. Sorry for the inconvenience.

@brechtm
Copy link
Contributor Author

brechtm commented Feb 22, 2022

@jaraco Supplying my own _normalized_name turned out not to be an option because other things were missing as well. The current approach of making use of importlib-metadata when it's available seems to working well so far.

The importlib-metadata docs currently say "Users are encouraged to use the Python standard library where suitable and fall back to this library for future compatibility". My experience seems to suggest that this might not be the best approach. I couldn't find any other information about backwards/forwards compatibility. Please consider adding some information regarding this.

@brechtm brechtm closed this as completed Feb 22, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants