Skip to content
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

bpo-32248: Implement importlib.abc.ResourceReader #4892

Merged
merged 8 commits into from
Dec 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions Doc/library/importlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ ABC hierarchy::
| +-- MetaPathFinder
| +-- PathEntryFinder
+-- Loader
+-- ResourceReader
+-- ResourceLoader --------+
+-- InspectLoader |
+-- ExecutionLoader --+
Expand Down Expand Up @@ -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
Copy link
Member

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...

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The 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`.
Copy link
Member

Choose a reason for hiding this comment

The 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)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In either case, importlib.resources will know what to do with that (e.g. extract it to a temporary file in the case where it exists in the loader's view of the world, but not on the physical file system).


.. abstractmethod:: is_resource(name)

Returns ``True`` if the named *name* is considered a resource.
:exc:`FileNotFoundError` is raised if *name* does not exist.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And False is returned if...?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...it isn't. :) Do you think we need to clarify that?

Copy link
Contributor

@AraHaan AraHaan Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it was me I would actually have the method return False instead of an exception if name does not exist. That is how I do it in my own python code. At least I would think getters and setters and checks like this that returns bools should have an no throw garentee.


.. 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
Expand Down
38 changes: 38 additions & 0 deletions Lib/importlib/abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,3 +340,41 @@ def set_data(self, path, data):
"""

_register(SourceLoader, machinery.SourceFileLoader)


class ResourceReader(Loader):

"""Abstract base class for loaders to provide resource reading support."""

@abc.abstractmethod
def open_resource(self, resource):
"""Return an opened, file-like object for binary reading.

The 'resource' argument is expected to represent only a file name
and thus not contain any subdirectory components.

If the resource cannot be found, FileNotFoundError is raised.
"""
raise FileNotFoundError

@abc.abstractmethod
def resource_path(self, resource):
"""Return the file system path to the specified resource.

The 'resource' argument is expected to represent only a file name
and thus not contain any subdirectory components.

If the resource does not exist on the file system, raise
FileNotFoundError.
"""
raise FileNotFoundError

@abc.abstractmethod
def is_resource(self, name):
"""Return True if the named 'name' is consider a resource."""
raise FileNotFoundError

@abc.abstractmethod
def contents(self):
"""Return an iterator of strings over the contents of the package."""
return iter([])
39 changes: 39 additions & 0 deletions Lib/test/test_importlib/test_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is SPLIT required by ABCTestHarness? It seems an odd choice of terms.

Copy link
Member Author

Choose a reason for hiding this comment

The 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:

Expand Down
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.