Unverified Kaydet (Commit) 3ab9365d authored tarafından Brett Cannon's avatar Brett Cannon Kaydeden (comit) GitHub

bpo-33254: do not return an empty list when asking for the contents of a…

bpo-33254: do not return an empty list when asking for the contents of a namespace package (GH-6467)
üst 10f715d7
...@@ -530,7 +530,7 @@ ABC hierarchy:: ...@@ -530,7 +530,7 @@ ABC hierarchy::
.. abstractmethod:: contents() .. abstractmethod:: contents()
Returns an :term:`iterator` of strings over the contents of Returns an :term:`iterable` of strings over the contents of
the package. Do 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 returned by the iterator be actual resources, e.g. it is
acceptable to return names for which :meth:`is_resource` would acceptable to return names for which :meth:`is_resource` would
...@@ -544,7 +544,7 @@ ABC hierarchy:: ...@@ -544,7 +544,7 @@ ABC hierarchy::
the file system then those subdirectory names can be used the file system then those subdirectory names can be used
directly. directly.
The abstract method returns an iterator of no items. The abstract method returns an iterable of no items.
.. class:: ResourceLoader .. class:: ResourceLoader
...@@ -926,9 +926,9 @@ The following functions are available. ...@@ -926,9 +926,9 @@ The following functions are available.
.. function:: contents(package) .. function:: contents(package)
Return an iterator over the named items within the package. The iterator Return an iterable over the named items within the package. The iterable
returns :class:`str` resources (e.g. files) and non-resources returns :class:`str` resources (e.g. files) and non-resources
(e.g. directories). The iterator does not recurse into subdirectories. (e.g. directories). The iterable does not recurse into subdirectories.
*package* is either a name or a module object which conforms to the *package* is either a name or a module object which conforms to the
``Package`` requirements. ``Package`` requirements.
......
...@@ -381,8 +381,8 @@ class ResourceReader(metaclass=abc.ABCMeta): ...@@ -381,8 +381,8 @@ class ResourceReader(metaclass=abc.ABCMeta):
@abc.abstractmethod @abc.abstractmethod
def contents(self): def contents(self):
"""Return an iterator of strings over the contents of the package.""" """Return an iterable of strings over the contents of the package."""
return iter([]) return []
_register(ResourceReader, machinery.SourceFileLoader) _register(ResourceReader, machinery.SourceFileLoader)
...@@ -9,7 +9,7 @@ from importlib.abc import ResourceLoader ...@@ -9,7 +9,7 @@ from importlib.abc import ResourceLoader
from io import BytesIO, TextIOWrapper from io import BytesIO, TextIOWrapper
from pathlib import Path from pathlib import Path
from types import ModuleType from types import ModuleType
from typing import Iterator, Optional, Set, Union # noqa: F401 from typing import Iterable, Iterator, Optional, Set, Union # noqa: F401
from typing import cast from typing import cast
from typing.io import BinaryIO, TextIO from typing.io import BinaryIO, TextIO
from zipimport import ZipImportError from zipimport import ZipImportError
...@@ -44,8 +44,7 @@ def _normalize_path(path) -> str: ...@@ -44,8 +44,7 @@ def _normalize_path(path) -> str:
If the resulting string contains path separators, an exception is raised. If the resulting string contains path separators, an exception is raised.
""" """
str_path = str(path) parent, file_name = os.path.split(path)
parent, file_name = os.path.split(str_path)
if parent: if parent:
raise ValueError('{!r} must be only a file name'.format(path)) raise ValueError('{!r} must be only a file name'.format(path))
else: else:
...@@ -228,8 +227,8 @@ def is_resource(package: Package, name: str) -> bool: ...@@ -228,8 +227,8 @@ def is_resource(package: Package, name: str) -> bool:
return path.is_file() return path.is_file()
def contents(package: Package) -> Iterator[str]: def contents(package: Package) -> Iterable[str]:
"""Return the list of entries in 'package'. """Return an iterable of entries in 'package'.
Note that not all entries are resources. Specifically, directories are Note that not all entries are resources. Specifically, directories are
not considered resources. Use `is_resource()` on each entry returned here not considered resources. Use `is_resource()` on each entry returned here
...@@ -238,15 +237,15 @@ def contents(package: Package) -> Iterator[str]: ...@@ -238,15 +237,15 @@ def contents(package: Package) -> Iterator[str]:
package = _get_package(package) package = _get_package(package)
reader = _get_resource_reader(package) reader = _get_resource_reader(package)
if reader is not None: if reader is not None:
yield from reader.contents() return reader.contents()
return
# Is the package a namespace package? By definition, namespace packages # Is the package a namespace package? By definition, namespace packages
# cannot have resources. We could use _check_location() and catch the # cannot have resources. We could use _check_location() and catch the
# exception, but that's extra work, so just inline the check. # exception, but that's extra work, so just inline the check.
if package.__spec__.origin is None or not package.__spec__.has_location: elif package.__spec__.origin is None or not package.__spec__.has_location:
return [] return ()
else:
package_directory = Path(package.__spec__.origin).parent package_directory = Path(package.__spec__.origin).parent
yield from os.listdir(str(package_directory)) return os.listdir(package_directory)
# Private implementation of ResourceReader and get_resource_reader() for # Private implementation of ResourceReader and get_resource_reader() for
......
...@@ -131,10 +131,9 @@ class ResourceFromZipsTest(util.ZipSetupBase, unittest.TestCase): ...@@ -131,10 +131,9 @@ class ResourceFromZipsTest(util.ZipSetupBase, unittest.TestCase):
class NamespaceTest(unittest.TestCase): class NamespaceTest(unittest.TestCase):
def test_namespaces_cant_have_resources(self): def test_namespaces_cannot_have_resources(self):
contents = set(resources.contents( contents = resources.contents('test.test_importlib.data03.namespace')
'test.test_importlib.data03.namespace')) self.assertFalse(list(contents))
self.assertEqual(len(contents), 0)
# Even though there is a file in the namespace directory, it is not # Even though there is a file in the namespace directory, it is not
# considered a resource, since namespace packages can't have them. # considered a resource, since namespace packages can't have them.
self.assertFalse(resources.is_resource( self.assertFalse(resources.is_resource(
......
Have :func:`importlib.resources.contents` and
:meth:`importlib.abc.ResourceReader.contents` return an :term:`iterable` instead
of an :term:`iterator`.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment