Skip to content

Use protocols instead of importlib.abc.Loader/MetaPathFinder/PathEntryFinder #11890

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

Merged
merged 9 commits into from
May 12, 2024
34 changes: 34 additions & 0 deletions stdlib/@tests/test_cases/check_importlib.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
from __future__ import annotations

import importlib.abc
import importlib.util
import pathlib
import sys
import zipfile
from collections.abc import Sequence
from importlib.machinery import ModuleSpec
from types import ModuleType
from typing_extensions import Self

# Assert that some Path classes are Traversable.
if sys.version_info >= (3, 9):
Expand All @@ -11,3 +18,30 @@ def traverse(t: importlib.abc.Traversable) -> None:

traverse(pathlib.Path())
traverse(zipfile.Path(""))


class MetaFinder:
@classmethod
def find_spec(cls, fullname: str, path: Sequence[str] | None, target: ModuleType | None = None) -> ModuleSpec | None:
return None # simplified mock for demonstration purposes only


class PathFinder:
@classmethod
def path_hook(cls, path_entry: str) -> type[Self]:
return cls # simplified mock for demonstration purposes only

@classmethod
def find_spec(cls, fullname: str, target: ModuleType | None = None) -> ModuleSpec | None:
return None # simplified mock for demonstration purposes only


class Loader:
@classmethod
def load_module(cls, fullname: str) -> ModuleType:
return ModuleType(fullname)


sys.meta_path.append(MetaFinder)
sys.path_hooks.append(PathFinder.path_hook)
importlib.util.spec_from_loader("xxxx42xxxx", Loader)
18 changes: 18 additions & 0 deletions stdlib/_typeshed/importlib.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Implicit protocols used in importlib.
# We intentionally omit deprecated and optional methods.

from collections.abc import Sequence
from importlib.machinery import ModuleSpec
from types import ModuleType
from typing import Protocol

__all__ = ["LoaderProtocol", "MetaPathFinderProtocol", "PathEntryFinderProtocol"]

class LoaderProtocol(Protocol):
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to replace types._LoaderProtocol with this as well.

Copy link
Contributor Author

@abravalheri abravalheri May 10, 2024

Choose a reason for hiding this comment

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

Replacing that in types.pyi would create an import loop between types.pyi and _typeshed/importlib.pyi. Is that OK to do for type stubs?

I can do the replacement in the pkg_resources stub without problems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Loops are not a problem in stubs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the clarification, I introduced this change in 7075e5e.

def load_module(self, fullname: str, /) -> ModuleType: ...

class MetaPathFinderProtocol(Protocol):
def find_spec(self, fullname: str, path: Sequence[str] | None, target: ModuleType | None = ..., /) -> ModuleSpec | None: ...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure about making it positional only arguments... but I copied the definitions from stdlib/sys/__init__.pyi, stdlib/importlib/abc.pyi, stdlib/types.pyi.

Maybe it is better just to remove the /?

Copy link
Member

@JelleZijlstra JelleZijlstra May 10, 2024

Choose a reason for hiding this comment

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

Protocols should usually have positional-only parameters, because otherwise implementations need to use the exact same parameter names. Use positional-or-keyword parameters only if consumers of the protocol pass keyword arguments.

Copy link
Contributor Author

@abravalheri abravalheri May 10, 2024

Choose a reason for hiding this comment

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

Oh, I see, that makes a lot of sense! Thank you very much for the clarification.

In accordance to that, I also introduced 6cddd52 to make the arguments in PathEntryFinderProtocol positional only.


class PathEntryFinderProtocol(Protocol):
def find_spec(self, fullname: str, target: ModuleType | None = ...) -> ModuleSpec | None: ...
4 changes: 2 additions & 2 deletions stdlib/importlib/abc.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SourceLoader(ResourceLoader, ExecutionLoader, metaclass=ABCMeta):

# The base classes differ starting in 3.10:
if sys.version_info >= (3, 10):
# Please keep in sync with sys._MetaPathFinder
# Please keep in sync with _typeshed.MetaPathFinderProtocol
class MetaPathFinder(metaclass=ABCMeta):
if sys.version_info < (3, 12):
def find_module(self, fullname: str, path: Sequence[str] | None) -> Loader | None: ...
Expand All @@ -85,7 +85,7 @@ if sys.version_info >= (3, 10):
def find_spec(self, fullname: str, target: types.ModuleType | None = ...) -> ModuleSpec | None: ...

else:
# Please keep in sync with sys._MetaPathFinder
# Please keep in sync with _typeshed.MetaPathFinderProtocol
class MetaPathFinder(Finder):
def find_module(self, fullname: str, path: Sequence[str] | None) -> Loader | None: ...
def invalidate_caches(self) -> None: ...
Expand Down
5 changes: 3 additions & 2 deletions stdlib/importlib/util.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import importlib.machinery
import sys
import types
from _typeshed import ReadableBuffer, StrOrBytesPath
from _typeshed.importlib import LoaderProtocol
from collections.abc import Callable
from typing import Any
from typing_extensions import ParamSpec
Expand All @@ -23,13 +24,13 @@ def source_from_cache(path: str) -> str: ...
def decode_source(source_bytes: ReadableBuffer) -> str: ...
def find_spec(name: str, package: str | None = None) -> importlib.machinery.ModuleSpec | None: ...
def spec_from_loader(
name: str, loader: importlib.abc.Loader | None, *, origin: str | None = None, is_package: bool | None = None
name: str, loader: LoaderProtocol | None, *, origin: str | None = None, is_package: bool | None = None
) -> importlib.machinery.ModuleSpec | None: ...
def spec_from_file_location(
name: str,
location: StrOrBytesPath | None = None,
*,
loader: importlib.abc.Loader | None = None,
loader: LoaderProtocol | None = None,
submodule_search_locations: list[str] | None = ...,
) -> importlib.machinery.ModuleSpec | None: ...
def module_from_spec(spec: importlib.machinery.ModuleSpec) -> types.ModuleType: ...
Expand Down
12 changes: 6 additions & 6 deletions stdlib/pkgutil.pyi
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import sys
from _typeshed import SupportsRead
from _typeshed.importlib import LoaderProtocol, MetaPathFinderProtocol, PathEntryFinderProtocol
from collections.abc import Callable, Iterable, Iterator
from importlib.abc import Loader, MetaPathFinder, PathEntryFinder
from typing import IO, Any, NamedTuple, TypeVar
from typing_extensions import deprecated

Expand All @@ -23,7 +23,7 @@ if sys.version_info < (3, 12):
_PathT = TypeVar("_PathT", bound=Iterable[str])

class ModuleInfo(NamedTuple):
module_finder: MetaPathFinder | PathEntryFinder
module_finder: MetaPathFinderProtocol | PathEntryFinderProtocol
name: str
ispkg: bool

Expand All @@ -37,11 +37,11 @@ if sys.version_info < (3, 12):
def __init__(self, fullname: str, file: IO[str], filename: str, etc: tuple[str, str, int]) -> None: ...

@deprecated("Use importlib.util.find_spec() instead. Will be removed in Python 3.14.")
def find_loader(fullname: str) -> Loader | None: ...
def get_importer(path_item: str) -> PathEntryFinder | None: ...
def find_loader(fullname: str) -> LoaderProtocol | None: ...
def get_importer(path_item: str) -> MetaPathFinderProtocol | PathEntryFinderProtocol | None: ...
@deprecated("Use importlib.util.find_spec() instead. Will be removed in Python 3.14.")
def get_loader(module_or_name: str) -> Loader | None: ...
def iter_importers(fullname: str = "") -> Iterator[MetaPathFinder | PathEntryFinder]: ...
def get_loader(module_or_name: str) -> LoaderProtocol | None: ...
def iter_importers(fullname: str = "") -> Iterator[MetaPathFinderProtocol | PathEntryFinderProtocol]: ...
def iter_modules(path: Iterable[str] | None = None, prefix: str = "") -> Iterator[ModuleInfo]: ...
def read_code(stream: SupportsRead[bytes]) -> Any: ... # undocumented
def walk_packages(
Expand Down
13 changes: 4 additions & 9 deletions stdlib/sys/__init__.pyi
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import sys
from _typeshed import OptExcInfo, ProfileFunction, TraceFunction, structseq
from _typeshed.importlib import MetaPathFinderProtocol, PathEntryFinderProtocol
from builtins import object as _object
from collections.abc import AsyncGenerator, Callable, Sequence
from importlib.abc import PathEntryFinder
from importlib.machinery import ModuleSpec
from io import TextIOWrapper
from types import FrameType, ModuleType, TracebackType
from typing import Any, Final, Literal, NoReturn, Protocol, TextIO, TypeVar, final
Expand All @@ -15,10 +14,6 @@ _T = TypeVar("_T")
_ExitCode: TypeAlias = str | int | None
_OptExcInfo: TypeAlias = OptExcInfo # noqa: Y047 # TODO: obsolete, remove fall 2022 or later

# Intentionally omits one deprecated and one optional method of `importlib.abc.MetaPathFinder`
class _MetaPathFinder(Protocol):
def find_spec(self, fullname: str, path: Sequence[str] | None, target: ModuleType | None = ..., /) -> ModuleSpec | None: ...

# ----- sys variables -----
if sys.platform != "win32":
abiflags: str
Expand All @@ -44,13 +39,13 @@ if sys.version_info >= (3, 12):
last_exc: BaseException # or undefined.
maxsize: int
maxunicode: int
meta_path: list[_MetaPathFinder]
meta_path: list[MetaPathFinderProtocol]
modules: dict[str, ModuleType]
if sys.version_info >= (3, 10):
orig_argv: list[str]
path: list[str]
path_hooks: list[Callable[[str], PathEntryFinder]]
path_importer_cache: dict[str, PathEntryFinder | None]
path_hooks: list[Callable[[str], PathEntryFinderProtocol]]
path_importer_cache: dict[str, PathEntryFinderProtocol | None]
platform: str
if sys.version_info >= (3, 9):
platlibdir: str
Expand Down