naming and inheritance for importlib by tungol · Pull Request #12775 · python/typeshed

tungol · 2024-10-10T20:05:46Z

This MR breaks out _frozen_importlib_external (which is the same thing as importlib._bootstrap_external) and _frozen_importlib (which is the same thing as importlib._bootstrap).

related to #3968

I waffled on just adding importlib._bootstrap and importlib._bootstrap_external because the _frozen* modules are unusual even by private module standards, but decided to go ahead with _frozen* since that's the names that show up at runtime.

This MR breaks out _frozen_importlib_external (which is the same thing as importlib._bootstrap_external) and _frozen_importlib (which is the same thing as importlib._bootstrap). related to python#3968

tungol · 2024-10-10T20:10:43Z

huh, stubtest didn't do that on my local machine. Looking into it.

Well, forgetting to git add some files didn't help matters.

tungol · 2024-10-10T20:50:23Z

I'll investigate it more later, but it looks like at least some of the mypy primer stuff is because of the circular inheritance issue

tungol · 2024-10-14T19:47:13Z

Key diffs for the things moved between files:

__import__ moved from importlib.__init__ moved to _frozen_importlib: No diff.
spec_from_loader and module_from_spec moved from importlib.util to _frozen_importlib: No diff.
ModuleSpec, BuiltinImporter, and FrozenImporter moved from importlib.machinery to _frozen_importlib: No diff.
MAGIC_NUMBER, cache_from_source, source_from_cache, decode_source, and spec_from_file_location moved from importlib.util to _frozen_importlib_external: No diff.

The block of WindowsRegistryFinder through NamspaceLoader, moved from importlib.machinery to _frozen_importlib_external:

--- importlib.machinery (old)
+++ _frozen_importlib_external (new)
@@ -8,7 +8,7 @@
         cls, fullname: str, path: Sequence[str] | None = None, target: types.ModuleType | None = None
     ) -> ModuleSpec | None: ...
 
-class PathFinder:
+class PathFinder(importlib.abc.MetaPathFinder):
     if sys.version_info >= (3, 10):
         @staticmethod
         def invalidate_caches() -> None: ...
@@ -46,12 +44,47 @@
         cls, *loader_details: tuple[type[importlib.abc.Loader], list[str]]
     ) -> Callable[[str], importlib.abc.PathEntryFinder]: ...
 
-class SourceFileLoader(importlib.abc.FileLoader, importlib.abc.SourceLoader):
+class _LoaderBasics:
+    def is_package(self, fullname: str) -> bool: ...
+    def create_module(self, spec: ModuleSpec) -> types.ModuleType | None: ...
+    def exec_module(self, module: types.ModuleType) -> None: ...
+    def load_module(self, fullname: str) -> types.ModuleType: ...
+
+class SourceLoader(_LoaderBasics):
+    def path_mtime(self, path: str) -> float: ...
+    def set_data(self, path: str, data: bytes) -> None: ...
+    def get_source(self, fullname: str) -> str | None: ...
+    def path_stats(self, path: str) -> Mapping[str, Any]: ...
+    def source_to_code(
+        self, data: ReadableBuffer | str | _ast.Module | _ast.Expression | _ast.Interactive, path: ReadableBuffer | StrPath
+    ) -> types.CodeType: ...
+    def get_code(self, fullname: str) -> types.CodeType | None: ...
+
+class FileLoader:
+    name: str
+    path: str
+    def __init__(self, fullname: str, path: str) -> None: ...
+    def get_data(self, path: str) -> bytes: ...
+    def get_filename(self, name: str | None = None) -> str: ...
+    def load_module(self, name: str | None = None) -> types.ModuleType: ...
+    if sys.version_info >= (3, 10):
+        def get_resource_reader(self, module: types.ModuleType) -> importlib.readers.FileReader: ...
+    else:
+        def get_resource_reader(self, module: types.ModuleType) -> Self | None: ...
+        def open_resource(self, resource: str) -> _io.FileIO: ...
+        def resource_path(self, resource: str) -> str: ...
+        def is_resource(self, name: str) -> bool: ...
+        def contents(self) -> Iterator[str]: ...
+
+class SourceFileLoader(importlib.abc.FileLoader, FileLoader, importlib.abc.SourceLoader, SourceLoader):  # type: ignore[misc]  # incompatible method arguments in base classes
     def set_data(self, path: str, data: ReadableBuffer, *, _mode: int = 0o666) -> None: ...
+    def path_stats(self, path: str) -> Mapping[str, Any]: ...
 
-class SourcelessFileLoader(importlib.abc.FileLoader, importlib.abc.SourceLoader): ...
+class SourcelessFileLoader(importlib.abc.FileLoader, FileLoader, _LoaderBasics):
+    def get_code(self, fullname: str) -> types.CodeType | None: ...
+    def get_source(self, fullname: str) -> None: ...
 
-class ExtensionFileLoader(importlib.abc.ExecutionLoader):
+class ExtensionFileLoader(FileLoader, _LoaderBasics, importlib.abc.ExecutionLoader):
     def __init__(self, name: str, path: str) -> None: ...
     def get_filename(self, name: str | None = None) -> str: ...
     def get_source(self, fullname: str) -> None: ...
@@ -62,8 +95,6 @@
     def __hash__(self) -> int: ...
 
 if sys.version_info >= (3, 11):
-    import importlib.readers
-
     class NamespaceLoader(importlib.abc.InspectLoader):
         def __init__(
             self, name: str, path: MutableSequence[str], path_finder: Callable[[str, tuple[str, ...]], ModuleSpec]
@@ -80,3 +111,29 @@
             @staticmethod
             @deprecated("module_repr() is deprecated, and has been removed in Python 3.12")
             def module_repr(module: types.ModuleType) -> str: ...
+
+    _NamespaceLoader = NamespaceLoader
+else:
+    class _NamespaceLoader:
+        def __init__(
+            self, name: str, path: MutableSequence[str], path_finder: Callable[[str, tuple[str, ...]], ModuleSpec]
+        ) -> None: ...
+        def is_package(self, fullname: str) -> Literal[True]: ...
+        def get_source(self, fullname: str) -> Literal[""]: ...
+        def get_code(self, fullname: str) -> types.CodeType: ...
+        def create_module(self, spec: ModuleSpec) -> None: ...
+        def exec_module(self, module: types.ModuleType) -> None: ...
+        @deprecated("load_module() is deprecated; use exec_module() instead")
+        def load_module(self, fullname: str) -> types.ModuleType: ...
+        if sys.version_info >= (3, 10):
+            @staticmethod
+            @deprecated("module_repr() is deprecated, and has been removed in Python 3.12")
+            def module_repr(module: types.ModuleType) -> str: ...
+            def get_resource_reader(self, module: types.ModuleType) -> importlib.readers.NamespaceReader: ...
+        else:
+            @classmethod
+            @deprecated("module_repr() is deprecated, and has been removed in Python 3.12")
+            def module_repr(cls, module: types.ModuleType) -> str: ...
+
+if sys.version_info >= (3, 13):
+    class AppleFrameworkLoader(ExtensionFileLoader, importlib.abc.ExecutionLoader): ...

In that diff, _LoaderBasics is new, and SourceLoader and FileLoader are roughly copies of the versions in importlib.abc, which have the following diffs respectively:

--- importlib.abc
+++ _frozen_importlib_external
@@ -1,5 +1,9 @@
-class SourceLoader(ResourceLoader, ExecutionLoader, metaclass=ABCMeta):
+class SourceLoader(_LoaderBasics):
     def path_mtime(self, path: str) -> float: ...
     def set_data(self, path: str, data: bytes) -> None: ...
     def get_source(self, fullname: str) -> str | None: ...
     def path_stats(self, path: str) -> Mapping[str, Any]: ...
+    def source_to_code(
+        self, data: ReadableBuffer | str | _ast.Module | _ast.Expression | _ast.Interactive, path: ReadableBuffer | StrPath
+    ) -> types.CodeType: ...
+    def get_code(self, fullname: str) -> types.CodeType | None: ...

--- importlib.abc
+++ _frozen_importlib_external
@@ -1,7 +1,15 @@
-class FileLoader(ResourceLoader, ExecutionLoader, metaclass=ABCMeta):
+class FileLoader:
     name: str
     path: str
     def __init__(self, fullname: str, path: str) -> None: ...
     def get_data(self, path: str) -> bytes: ...
     def get_filename(self, name: str | None = None) -> str: ...
     def load_module(self, name: str | None = None) -> types.ModuleType: ...
+    if sys.version_info >= (3, 10):
+        def get_resource_reader(self, module: types.ModuleType) -> importlib.readers.FileReader: ...
+    else:
+        def get_resource_reader(self, module: types.ModuleType) -> Self | None: ...
+        def open_resource(self, resource: str) -> _io.FileIO: ...
+        def resource_path(self, resource: str) -> str: ...
+        def is_resource(self, name: str) -> bool: ...
+        def contents(self) -> Iterator[str]: ...

There's also the new _NamespaceLoader class, a version of NamespaceLoader for 3.10 and below. Here's a diff from the existing NamespaceLoader to the new copy:

--- NamespaceLoader (3.11+)
+++ _NamespaceLoader (3.10-)
@@ -1,4 +1,4 @@
-    class NamespaceLoader(importlib.abc.InspectLoader):
+    class _NamespaceLoader:
         def __init__(
             self, name: str, path: MutableSequence[str], path_finder: Callable[[str, tuple[str, ...]], ModuleSpec]
         ) -> None: ...
@@ -9,8 +9,12 @@
         def exec_module(self, module: types.ModuleType) -> None: ...
         @deprecated("load_module() is deprecated; use exec_module() instead")
         def load_module(self, fullname: str) -> types.ModuleType: ...
-        def get_resource_reader(self, module: types.ModuleType) -> importlib.readers.NamespaceReader: ...
-        if sys.version_info < (3, 12):
+        if sys.version_info >= (3, 10):
             @staticmethod
             @deprecated("module_repr() is deprecated, and has been removed in Python 3.12")
             def module_repr(module: types.ModuleType) -> str: ...
+            def get_resource_reader(self, module: types.ModuleType) -> importlib.readers.NamespaceReader: ...
+        else:
+            @classmethod
+            @deprecated("module_repr() is deprecated, and has been removed in Python 3.12")
+            def module_repr(cls, module: types.ModuleType) -> str: ...

tungol · 2024-10-14T19:53:37Z

Final notes: I added the private _init_module_attrs and _NamespaceLoader because both showed up in mypy-primer. I investigated the remaining mypy-primer result from pytest, and it seems correct: they're using an undocumented class directly, giving it arguments of the wrong type, but then never using it in a way where that matters.

github-actions · 2024-10-14T20:05:49Z

Diff from mypy_primer, showing the effect of this PR on open source code:

beartype (https://github.com/beartype/beartype)
+ beartype/claw/_importlib/_clawimpload.py:28: error: Unused "type: ignore" comment  [unused-ignore]

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/pathlib.py:704: error: Argument 2 to "_NamespaceLoader" has incompatible type "Path"; expected "MutableSequence[str]"  [arg-type]
+ src/_pytest/pathlib.py:704: error: Argument 3 to "_NamespaceLoader" has incompatible type "PathFinder"; expected "Callable[[str, Tuple[str, ...]], ModuleSpec]"  [arg-type]

srittau · 2024-10-18T10:26:44Z

stdlib/@tests/stubtest_allowlists/common.txt

+_frozen_importlib_external.ExtensionFileLoader.get_filename  # Wrapped with _check_name decorator which changes runtime signature
+_frozen_importlib_external.FileLoader.get_filename  # Wrapped with _check_name decorator which changes runtime signature
+_frozen_importlib_external.FileLoader.get_resource_reader  # Wrapped with _check_name decorator which changes runtime signature
+_frozen_importlib_external.FileLoader.load_module  # Wrapped with _check_name decorator which changes runtime signature


You put that in the "should be fixed" section, but if I understand your comments correctly, this isn't fixable?

These are duplicates of allowlist lines already present in this section, they're just showing up in other modules. Neither the mismatch or the comment is new to this MR.

I believe they could be fixed; right now the stubs match the functions as written directly instead of their modified signature and I'm not totally sure why it was done that way, but I left it alone.

srittau · 2024-10-18T10:26:54Z

stdlib/@tests/stubtest_allowlists/common.txt

+importlib._bootstrap_external.ExtensionFileLoader.get_filename  # Wrapped with _check_name decorator which changes runtime signature
+importlib._bootstrap_external.FileLoader.get_filename  # Wrapped with _check_name decorator which changes runtime signature
+importlib._bootstrap_external.FileLoader.get_resource_reader  # Wrapped with _check_name decorator which changes runtime signature
+importlib._bootstrap_external.FileLoader.load_module  # Wrapped with _check_name decorator which changes runtime signature


See the other reply and also the lines just below here.

srittau

I didn't compare this with the implementation, but your reasoning sounds good and CI likes this PR, so I'm going to merge this as is.

naming and inheritance for importlib

defdf88

This MR breaks out _frozen_importlib_external (which is the same thing as importlib._bootstrap_external) and _frozen_importlib (which is the same thing as importlib._bootstrap). related to python#3968

tungol marked this pull request as draft October 10, 2024 20:07