-
-
Couldn't load subscription status.
- Fork 33.2k
gh-121190: A better error message from importlib.resources.files() when module spec is None
#138531
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
078c801
c79a7cc
5ed2652
847d8df
e25491e
06b8233
7dc7e46
9e98cfc
d3ec82a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,17 @@ | |
|
|
||
| from . import abc | ||
|
|
||
| TYPE_CHECKING = False | ||
| if TYPE_CHECKING: | ||
| from ..machinery import ModuleSpec | ||
|
|
||
|
|
||
| class SpecLoaderAdapter: | ||
| """ | ||
| Adapt a package spec to adapt the underlying loader. | ||
| """ | ||
|
|
||
| def __init__(self, spec, adapter=lambda spec: spec.loader): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was the default adapter removed? Isn't this a breaking change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the simplicity sake, because it seems not to be used anywhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it does not pertain to the issue that's addressed in this PR I'd recommend reverting this change anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, got it 👍 |
||
| def __init__(self, spec: ModuleSpec, adapter): | ||
| self.spec = spec | ||
| self.loader = adapter(spec) | ||
|
|
||
|
|
@@ -160,9 +164,9 @@ def files(self): | |
| return CompatibilityFiles.SpecPath(self.spec, self._reader) | ||
|
|
||
|
|
||
| def wrap_spec(package): | ||
| def wrap_spec(spec: ModuleSpec): | ||
| """ | ||
| Construct a package spec with traversable compatibility | ||
| on the spec/loader/reader. | ||
| """ | ||
| return SpecLoaderAdapter(package.__spec__, TraversableResourcesLoader) | ||
| return SpecLoaderAdapter(spec, TraversableResourcesLoader) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,7 +113,10 @@ def from_package(package: types.ModuleType): | |
| # deferred for performance (python/cpython#109829) | ||
| from ._adapters import wrap_spec | ||
|
|
||
| spec = wrap_spec(package) | ||
| if package.__spec__ is None: | ||
| raise TypeError(f"Can't access resources on a module with no spec: {package}") | ||
|
||
|
|
||
| spec = wrap_spec(package.__spec__) | ||
| reader = spec.loader.get_resource_reader(spec.name) | ||
| return reader.files() | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ``importlib.resources.files(package)`` raises an error with a clearer message when ``package.__spec__`` is ``None`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python stdlib is not typed (that's what typeshed is for) so you'll likely be asked to revert these changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please revert all type annotation changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks updated!