-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
bpo-32248: Implement importlib.abc.ResourceReader #4892
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
Changes from 4 commits
d0a167c
b4ccb2e
73416f2
4a0b916
8630e83
dba819b
4169401
ef68c33
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 |
|---|---|---|
|
|
@@ -230,6 +230,7 @@ ABC hierarchy:: | |
| | +-- MetaPathFinder | ||
| | +-- PathEntryFinder | ||
| +-- Loader | ||
| +-- ResourceReader | ||
| +-- ResourceLoader --------+ | ||
| +-- InspectLoader | | ||
| +-- ExecutionLoader --+ | ||
|
|
@@ -465,6 +466,72 @@ ABC hierarchy:: | |
| The import machinery now takes care of this automatically. | ||
|
|
||
|
|
||
| .. class:: ResourceReader | ||
|
|
||
| An :term:`abstract base class` for :term:`loaders <loader>` | ||
| representing a :term:`package` to provide the ability to read | ||
| *resources*. | ||
|
|
||
| From the perspective of this class, a *resource* is a binary | ||
|
||
| artifact that is shipped within the package that this loader | ||
| represents. Typically this is something like a data file that | ||
|
||
| lives next to the ``__init__.py`` file of the package. The | ||
| purpose of this class is to help abstract out the accessing of | ||
| such data files so that it does not matter if the package and | ||
| its data file(s) are stored in a e.g. zip file versus on the | ||
| file system. | ||
|
|
||
| For any of methods of this class, a *resource* argument is | ||
| expected to be a :term:`file-like object` which represents | ||
|
Member
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. So
Member
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. I think that term should say that a resource is a
Member
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. |
||
| conceptually just a file name. This means that no subdirectory | ||
| paths should be included in the *resource* argument. This is | ||
| because the location of the package that the loader is for acts | ||
| as the "directory". Hence the metaphor for directories and file | ||
| names is packages and resources, respectively. This is also why | ||
| instances of this class are expected to directly correlate to | ||
| a specific package (instead of potentially representing multiple | ||
| packages or a module). | ||
|
|
||
| .. versionadded:: 3.7 | ||
|
|
||
| .. abstractmethod:: open_resource(resource) | ||
|
|
||
| Returns an opened, :term:`file-like object` for binary reading | ||
| of the *resource*. | ||
|
|
||
| If the resource cannot be found, :exc:`FileNotFoundError` is | ||
| raised. | ||
|
|
||
| .. abstractmethod:: resource_path(resource) | ||
|
|
||
| Returns the file system path to the *resource*. | ||
|
|
||
| If the resource does not concretely exist on the file system, | ||
| raise :exc:`FileNotFoundError`. | ||
|
Member
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. Hmm... is the same error returned when the given resource doesn't exist, or when the given resource exists but underlying storage is not a regular directory (e.g. a zip file)?
Member
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. Yes. In either case, |
||
|
|
||
| .. abstractmethod:: is_resource(path) | ||
|
||
|
|
||
| Returns ``True`` if the named *path* is considered a resource. | ||
| :exc:`FileNotFoundError` is raised if *path* does not exist. | ||
|
|
||
| .. abstractmethod:: contents() | ||
|
|
||
| Returns an :term:`iterator` of strings over the contents of | ||
| the package. Due note that it is not required that all names | ||
|
||
| returned by the iterator be actual resources, e.g. it is | ||
| acceptable to return names for which :meth:`is_resource` would | ||
| be false. | ||
|
|
||
| Allowing non-resource names to be returned is to allow for | ||
| situations where how a package and its resources are stored | ||
| are known a priori and the non-resource names would be useful. | ||
| For instance, returning subdirectory names is allowed so that | ||
| when it is known that the package and resources are stored on | ||
| the file system then those subdirectory names can be used. | ||
|
|
||
| The abstract method returns an empty iterator. | ||
|
|
||
|
|
||
| .. class:: ResourceLoader | ||
|
|
||
| An abstract base class for a :term:`loader` which implements the optional | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -305,6 +305,45 @@ def test_get_filename(self): | |
| ) = test_util.test_both(InspectLoaderDefaultsTests) | ||
|
|
||
|
|
||
| class ResourceReader: | ||
|
|
||
| def open_resource(self, *args, **kwargs): | ||
| return super().open_resource(*args, **kwargs) | ||
|
|
||
| def resource_path(self, *args, **kwargs): | ||
| return super().resource_path(*args, **kwargs) | ||
|
|
||
| def is_resource(self, *args, **kwargs): | ||
| return super().is_resource(*args, **kwargs) | ||
|
|
||
| def contents(self, *args, **kwargs): | ||
| return super().contents(*args, **kwargs) | ||
|
|
||
|
|
||
| class ResourceReaderDefaultsTests(ABCTestHarness): | ||
|
|
||
| SPLIT = make_abc_subclasses(ResourceReader) | ||
|
Member
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. Is
Member
Author
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. Yes and I didn't name it. |
||
|
|
||
| def test_open_resource(self): | ||
| with self.assertRaises(FileNotFoundError): | ||
| self.ins.open_resource('dummy_file') | ||
|
|
||
| def test_resource_path(self): | ||
| with self.assertRaises(FileNotFoundError): | ||
| self.ins.resource_path('dummy_file') | ||
|
|
||
| def test_is_resource(self): | ||
| with self.assertRaises(FileNotFoundError): | ||
| self.ins.is_resource('dummy_file') | ||
|
|
||
| def test_contents(self): | ||
| self.assertEqual([], list(self.ins.contents())) | ||
|
|
||
| (Frozen_RRDefaultTests, | ||
| Source_RRDefaultsTests | ||
| ) = test_util.test_both(ResourceReaderDefaultsTests) | ||
|
|
||
|
|
||
| ##### MetaPathFinder concrete methods ########################################## | ||
| class MetaPathFinderFindModuleTests: | ||
|
|
||
|
|
||
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.
This reads a bit awkwardly for me. What about "An abstract base class for package loaders which provide the ability to read resources"?