importlib: improve bytes handling#9070
Conversation
| class ResourceLoader(Loader): | ||
| @abstractmethod | ||
| def get_data(self, path: _Path) -> bytes: ... | ||
| def get_data(self, path: str) -> bytes: ... |
There was a problem hiding this comment.
The docstring says it must be a str: https://github.com/python/cpython/blob/2cfcaf5af602b297fc90086de4d8ac980c7891e2/Lib/importlib/abc.py#L182
| class ExecutionLoader(InspectLoader): | ||
| @abstractmethod | ||
| def get_filename(self, fullname: str) -> _Path: ... | ||
| def get_filename(self, fullname: str) -> str: ... |
There was a problem hiding this comment.
Docstring says "Abstract method which should return the value that file is to be set to." __file__ is always a str, not bytes. (Note that _Path = str | bytes.)
| class SourceLoader(ResourceLoader, ExecutionLoader, metaclass=ABCMeta): | ||
| def path_mtime(self, path: _Path) -> float: ... | ||
| def set_data(self, path: _Path, data: bytes) -> None: ... | ||
| def path_mtime(self, path: str) -> float: ... |
There was a problem hiding this comment.
Docstrings say the path must be str for all three methods.
| class PathEntryFinder(Finder): | ||
| def find_module(self, fullname: str) -> Loader | None: ... | ||
| def find_loader(self, fullname: str) -> tuple[Loader | None, Sequence[_Path]]: ... | ||
| def find_loader(self, fullname: str) -> tuple[Loader | None, Sequence[str]]: ... |
There was a problem hiding this comment.
The second element is ModuleSpec.submodule_search_locations which is a list[str]. https://github.com/python/cpython/blob/2cfcaf5af602b297fc90086de4d8ac980c7891e2/Lib/importlib/abc.py#L155
| def __init__(self, fullname: str, path: _Path) -> None: ... | ||
| def get_data(self, path: _Path) -> bytes: ... | ||
| def get_filename(self, name: str | None = ...) -> _Path: ... | ||
| path: str |
There was a problem hiding this comment.
This class has no implementation, but from _bootstrap_external.FileLoader it seems clear the path is not intended to be bytes: https://github.com/python/cpython/blob/2cfcaf5af602b297fc90086de4d8ac980c7891e2/Lib/importlib/_bootstrap_external.py#L1195
| @classmethod | ||
| def find_spec( | ||
| cls, fullname: str, path: Sequence[bytes | str] | None = ..., target: types.ModuleType | None = ... | ||
| cls, fullname: str, path: Sequence[str] | None = ..., target: types.ModuleType | None = ... |
There was a problem hiding this comment.
The path gets passed to _get_spec, which ignores any non-str entries in the path: https://github.com/python/cpython/blob/2cfcaf5af602b297fc90086de4d8ac980c7891e2/Lib/importlib/_bootstrap_external.py#L1525
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| def exec_module(self, module: types.ModuleType) -> None: ... | ||
| @staticmethod | ||
| def source_to_code(data: bytes | str, path: str = ...) -> types.CodeType: ... | ||
| def source_to_code(data: ReadableBuffer | str, path: str = ...) -> types.CodeType: ... |
There was a problem hiding this comment.
Could also be AST, but I don't think that's too important.
No description provided.