-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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 all 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,71 @@ ABC hierarchy:: | |
The import machinery now takes care of this automatically. | ||
|
||
|
||
.. class:: ResourceReader | ||
|
||
An :term:`abstract base class` for :term:`package` | ||
:term:`loaders <loader>` to provide the ability to read | ||
*resources*. | ||
|
||
From the perspective of this ABC, a *resource* is a binary | ||
artifact that is shipped within a package. 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 | ||
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`. | ||
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)? 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(name) | ||
|
||
Returns ``True`` if the named *name* is considered a resource. | ||
:exc:`FileNotFoundError` is raised if *name* does not exist. | ||
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. And False is returned if...? 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. ...it isn't. :) Do you think we need to clarify that? 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. if it was me I would actually have the method return |
||
|
||
.. abstractmethod:: contents() | ||
|
||
Returns an :term:`iterator` of strings over the contents of | ||
the package. Do 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) | ||
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 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: | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Add :class:`importlib.abc.ResourceReader` as an ABC for loaders to provide a | ||
unified API for reading resources contained within packages. |
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.
So
open_resource
takes a file object and returns a file object? I may be misunderstanding here...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.
I think that term should say that a resource is a
path-like object
. I'll fix that in my follow up branch.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.
#4911