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

Only depend on importlib-metadata under Python 3.10 #2959

Merged
merged 4 commits into from
Sep 1, 2022

Conversation

pawamoy
Copy link
Sponsor Contributor

@pawamoy pawamoy commented Sep 1, 2022

Fixes #2958

We always try to import importlib_metadata first.
I can switch to a condition on the current Python version, instead of a try/except, if you think that's more robust.

Without the type: ignore comments I was getting this:

mkdocs/utils/__init__.py:30: error: Incompatible import of "entry_points" (imported name has type "Callable[[], Dict[str, List[EntryPoint]]]", local name has type "Callable[[KwArg(Any)], Union[EntryPoints, SelectableGroups]]")  [misc]
mkdocs/plugins.py:22: error: Incompatible import of "entry_points" (imported name has type "Callable[[], Dict[str, List[EntryPoint]]]", local name has type "Callable[[KwArg(Any)], Union[EntryPoints, SelectableGroups]]")  [misc]
Found 2 errors in 2 files (checked 58 source files)

EDIT: might need to squash to reword the commit: below instead of under (pardon my french).

@oprypin
Copy link
Contributor

oprypin commented Sep 1, 2022

Thanks for the research.

Usually the try-except imports apply nicely to stdlib modules that have 3rd party shims and you want to prioritize stdlib if it exists. And you do have to try stdlib first. The reason is that otherwise the behavior will be affected by the external state of whether this package is installed: Confusing error if it's not installed when it should be and just arbitrarily using the (hopefully) equivalent implementation when it happens to be installed but was not requested.

Here, the stdlib module existing is not enough, so that approach can't be used. So I think we indeed have to use an explicit version check.

if sys.version_info >= (3, 10):
    from importlib.metadata import entry_points
else:
    from importlib_metadata import entry_points  # type: ignore

@pawamoy
Copy link
Sponsor Contributor Author

pawamoy commented Sep 1, 2022

Thanks, I've applied your suggestion.

Copy link
Contributor

@oprypin oprypin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is good.

And yes, I'm seeing (3.10 locally and 3.9 in CI) that now apparently # type: ignore is not needed.

mkdocs/plugins.py Show resolved Hide resolved
@oprypin oprypin merged commit 4ada758 into mkdocs:master Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No python version marker on importlib-metadata
2 participants