From d0a167cb2820348f9410952779cd0415792ffbc6 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 8 Dec 2017 14:42:23 -0800 Subject: [PATCH 1/8] Add code for importlib.abc.ResourceReader --- Lib/importlib/abc.py | 38 +++++++++++++++++++++++++++++ Lib/test/test_importlib/test_abc.py | 36 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/Lib/importlib/abc.py b/Lib/importlib/abc.py index d7cadf2ee74b414..f53afbcf90a2ade 100644 --- a/Lib/importlib/abc.py +++ b/Lib/importlib/abc.py @@ -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, path): + """Return True if the named 'path' is consider a resource.""" + raise FileNotFoundError + + @abc.abstractmethod + def contents(self): + """Return an iterator of strings over the contents of the package.""" + raise FileNotFoundError diff --git a/Lib/test/test_importlib/test_abc.py b/Lib/test/test_importlib/test_abc.py index 54b2da6a3d233ae..6b261dd06128f07 100644 --- a/Lib/test/test_importlib/test_abc.py +++ b/Lib/test/test_importlib/test_abc.py @@ -305,6 +305,42 @@ def test_get_filename(self): ) = test_util.test_both(InspectLoaderDefaultsTests) +class ResourceReader: + + def open_resource(self, *args, **kwargs): + super().open_resource(*args, **kwargs) + + def resource_path(self, *args, **kwargs): + super().resource_path(*args, **kwargs) + + def is_resource(self, *args, **kwargs): + super().is_resource(*args, **kwargs) + + def contents(self, *args, **kwargs): + super().contents(*args, **kwargs) + + +class ResourceReaderDefaultsTests(ABCTestHarness): + + SPLIT = make_abc_subclasses(ResourceReader) + + def test_open_resource(self): + pass + + def test_resource_path(self): + pass + + def test_is_resource(self): + pass + + def test_contents(self): + pass + +(Frozen_RRDefaultTests, + Source_RRDefaultsTests + ) = test_util.test_both(ResourceReaderDefaultsTests) + + ##### MetaPathFinder concrete methods ########################################## class MetaPathFinderFindModuleTests: From b4ccb2eb709f9e2849d16dcad7659e9f2ae2662f Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 8 Dec 2017 15:23:23 -0800 Subject: [PATCH 2/8] Start of documentation for importlib.abc.ResourceReader Nothing more than a copy of the docstrings. --- Doc/library/importlib.rst | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 3d350e8d0d5149b..6de2d66b95a7310 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -230,6 +230,7 @@ ABC hierarchy:: | +-- MetaPathFinder | +-- PathEntryFinder +-- Loader + +-- ResourceReader +-- ResourceLoader --------+ +-- InspectLoader | +-- ExecutionLoader --+ @@ -465,6 +466,42 @@ ABC hierarchy:: The import machinery now takes care of this automatically. +.. class:: ResourceReader + + Abstract base class for loaders to provide resource reading + support. + + .. versionadded:: 3.7 + + .. abstractmethod:: open_resource(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. + + .. abstractmethod:: resource_path(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. + + .. abstractmethod:: is_resource(resource) + + Return True if the named 'path' is consider a resource. + + .. abstractmethod:: contents() + + Return an iterator of strings over the contents of the + package. + + .. class:: ResourceLoader An abstract base class for a :term:`loader` which implements the optional From 73416f2fe1896b82342d62f3b04a93eeb833cc1d Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 8 Dec 2017 15:53:33 -0800 Subject: [PATCH 3/8] Flesh out docs for importlib.abc.ResourceReader --- Doc/library/importlib.rst | 61 ++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 6de2d66b95a7310..c24f30cd420503f 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -468,38 +468,65 @@ ABC hierarchy:: .. class:: ResourceReader - Abstract base class for loaders to provide resource reading - support. + An :term:`abstract base class` for :term:`loaders ` + 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 + 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) - Return an opened, file-like object for binary reading. + Returns an opened, :term:`file-like object` for binary reading + of the *resource*. - 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. + If the resource cannot be found, :exc:`FileNotFoundError` is + raised. .. abstractmethod:: resource_path(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. + Returns the file system path to the *resource*. - If the resource does not exist on the file system, raise - FileNotFoundError. + If the resource does not concretely exist on the file system, + raise :exc:`FileNotFoundError`. - .. abstractmethod:: is_resource(resource) + .. abstractmethod:: is_resource(path) - Return True if the named 'path' is consider a resource. + Returns ``True`` if the named *path* is considered a resource. .. abstractmethod:: contents() - Return an iterator of strings over the contents of the - package. + Return 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. .. class:: ResourceLoader From 4a0b916e9d413ceb8de559500a038f115ad67e2a Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 13 Dec 2017 09:01:03 -0800 Subject: [PATCH 4/8] Tighten up what is returned by ResourceReader.contents() --- Doc/library/importlib.rst | 7 +++++-- Lib/importlib/abc.py | 2 +- Lib/test/test_importlib/test_abc.py | 19 +++++++++++-------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index c24f30cd420503f..db2eef06528e6ee 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -512,11 +512,12 @@ ABC hierarchy:: .. 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() - Return an :term:`iterator` of strings over the contents of the - package. Due note that it is not required that all names + 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. @@ -528,6 +529,8 @@ ABC hierarchy:: 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 diff --git a/Lib/importlib/abc.py b/Lib/importlib/abc.py index f53afbcf90a2ade..18313575396bfe4 100644 --- a/Lib/importlib/abc.py +++ b/Lib/importlib/abc.py @@ -377,4 +377,4 @@ def is_resource(self, path): @abc.abstractmethod def contents(self): """Return an iterator of strings over the contents of the package.""" - raise FileNotFoundError + return iter([]) diff --git a/Lib/test/test_importlib/test_abc.py b/Lib/test/test_importlib/test_abc.py index 6b261dd06128f07..c269edcb726cd81 100644 --- a/Lib/test/test_importlib/test_abc.py +++ b/Lib/test/test_importlib/test_abc.py @@ -308,16 +308,16 @@ def test_get_filename(self): class ResourceReader: def open_resource(self, *args, **kwargs): - super().open_resource(*args, **kwargs) + return super().open_resource(*args, **kwargs) def resource_path(self, *args, **kwargs): - super().resource_path(*args, **kwargs) + return super().resource_path(*args, **kwargs) def is_resource(self, *args, **kwargs): - super().is_resource(*args, **kwargs) + return super().is_resource(*args, **kwargs) def contents(self, *args, **kwargs): - super().contents(*args, **kwargs) + return super().contents(*args, **kwargs) class ResourceReaderDefaultsTests(ABCTestHarness): @@ -325,16 +325,19 @@ class ResourceReaderDefaultsTests(ABCTestHarness): SPLIT = make_abc_subclasses(ResourceReader) def test_open_resource(self): - pass + with self.assertRaises(FileNotFoundError): + self.ins.open_resource('dummy_file') def test_resource_path(self): - pass + with self.assertRaises(FileNotFoundError): + self.ins.resource_path('dummy_file') def test_is_resource(self): - pass + with self.assertRaises(FileNotFoundError): + self.ins.is_resource('dummy_file') def test_contents(self): - pass + self.assertEqual([], list(self.ins.contents())) (Frozen_RRDefaultTests, Source_RRDefaultsTests From 8630e8325ecc28f5c1984e3011d6ed2964aae050 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Dec 2017 15:31:04 -0800 Subject: [PATCH 5/8] Use 'name' instead of 'path' for contents() --- Doc/library/importlib.rst | 2 +- Lib/importlib/abc.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index db2eef06528e6ee..50a08da0d6b755e 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -509,7 +509,7 @@ ABC hierarchy:: If the resource does not concretely exist on the file system, raise :exc:`FileNotFoundError`. - .. abstractmethod:: is_resource(path) + .. abstractmethod:: is_resource(name) Returns ``True`` if the named *path* is considered a resource. :exc:`FileNotFoundError` is raised if *path* does not exist. diff --git a/Lib/importlib/abc.py b/Lib/importlib/abc.py index 18313575396bfe4..b772db3758cdee6 100644 --- a/Lib/importlib/abc.py +++ b/Lib/importlib/abc.py @@ -370,8 +370,8 @@ def resource_path(self, resource): raise FileNotFoundError @abc.abstractmethod - def is_resource(self, path): - """Return True if the named 'path' is consider a resource.""" + def is_resource(self, name): + """Return True if the named 'name' is consider a resource.""" raise FileNotFoundError @abc.abstractmethod From dba819bfdebb8494b60c300e54c10086839877ff Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Dec 2017 15:31:16 -0800 Subject: [PATCH 6/8] Grammar tweaks for the FLUFL --- Doc/library/importlib.rst | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 50a08da0d6b755e..75ee1f9791d914b 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -468,18 +468,17 @@ ABC hierarchy:: .. class:: ResourceReader - An :term:`abstract base class` for :term:`loaders ` - representing a :term:`package` to provide the ability to read + An :term:`abstract base class` for :term:`package` + :term:`loaders ` 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. + 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 @@ -511,13 +510,13 @@ ABC hierarchy:: .. abstractmethod:: is_resource(name) - Returns ``True`` if the named *path* is considered a resource. - :exc:`FileNotFoundError` is raised if *path* does not exist. + Returns ``True`` if the named *name* is considered a resource. + :exc:`FileNotFoundError` is raised if *name* 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 + 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. From 4169401dfc4ee665fba67268d3d7afd071a0e7c6 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Dec 2017 15:34:27 -0800 Subject: [PATCH 7/8] Add a news entry --- .../next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst diff --git a/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst b/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst new file mode 100644 index 000000000000000..6a4412f99673a00 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst @@ -0,0 +1,2 @@ +Add :cls:`importlib.abc.ResourceReader` as an ABC for loaders to provide a +unified API for reading resources contained within packages. From ef68c3367310d1216a3a54c7d6dd1954141b4390 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Dec 2017 16:10:49 -0800 Subject: [PATCH 8/8] Use the proper role name --- .../next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst b/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst index 6a4412f99673a00..f77cdb03dde60d0 100644 --- a/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst +++ b/Misc/NEWS.d/next/Library/2017-12-15-15-34-12.bpo-32248.zmO8G2.rst @@ -1,2 +1,2 @@ -Add :cls:`importlib.abc.ResourceReader` as an ABC for loaders to provide a +Add :class:`importlib.abc.ResourceReader` as an ABC for loaders to provide a unified API for reading resources contained within packages.