Replace DegenerateFiles with CompatibilityFiles

[FFY00]

When reimplementing the lagacy api with files() in preparation for the legacy API deprecation, as discussed in #80, I uncovered that it does not have proper support for the legacy ResourceReader API.

We implement DegenerateFiles, which takes a module spec and implements a degenerate object to be used as a fallback if no better TraversableReader is available.

importlib_resources/importlib_resources/_adapters.py

Lines 31 to 74 in a024adf

	class DegenerateFiles:
	"""
	Adapter for an existing or non-existant resource reader
	to provide a degenerate .files().
	"""
	class Path(abc.Traversable):
	def iterdir(self):
	return iter(())
	def is_dir(self):
	return False
	is_file = exists = is_dir # type: ignore
	def joinpath(self, other):
	return DegenerateFiles.Path()
	def name(self):
	return ''
	def open(self):
	raise ValueError()
	def __init__(self, spec):
	self.spec = spec
	@property
	def _reader(self):
	with suppress(AttributeError):
	return self.spec.loader.get_resource_reader(self.spec.name)
	def _native(self):
	"""
	Return the native reader if it supports files().
	"""
	reader = self._reader
	return reader if hasattr(reader, 'files') else self
	def __getattr__(self, attr):
	return getattr(self._reader, attr)
	def files(self):
	return DegenerateFiles.Path()

importlib_resources/importlib_resources/_compat.py

Lines 64 to 77 in a024adf

	return (
	# native reader if it supplies 'files'
	_native_reader(self.spec)
	or
	# local ZipReader if a zip module
	_zip_reader(self.spec)
	or
	# local NamespaceReader if a namespace module
	_namespace_reader(self.spec)
	or
	# local FileReader
	_file_reader(self.spec)
	or _adapters.DegenerateFiles(self.spec)
	)

In some of the cases where it is used, we actually have a valid ResourceReader that we can use. We have one example in the tests.

importlib_resources/importlib_resources/tests/util.py

Lines 54 to 61 in a024adf

	def create_package(file, path, is_package=True, contents=()):
	name = 'testingpackage'
	module = types.ModuleType(name)
	loader = Reader(file=file, path=path, _contents=contents)
	spec = ModuleSpec(name, loader, origin='does-not-exist', is_package=is_package)
	module.__spec__ = spec
	module.__loader__ = loader
	return module

The files() api won't work here, even though the legacy API does. If we deprecate the legacy api, users in these situations won't have any good alternative. I expect some of them to be in situations where they are not the ones that provide the reader, and so they are not able to fix it by adopting TraversableReader.

To solve this I propose replacing DegenerateFiles with a new implementation, CompatibilityFiles. It would adapt the ResourceReader api to provide a TraversableReader, essentially the oposite of TraversableResources, which implements a ResourceReader from a TraversableReader.

I have a working implementation in #221. Though, when changing the leagcy api to use files(), in order to make sure it supports all the use cases in the legacy api tests, I ran into some failures.

https://github.com/FFY00/importlib_resources/blob/7cb935e1defc995c24a63e8a08453e9f55d0fef2/importlib_resources/_adapters.py#L32-L113

---

@brettcannon you wrote the two tests that are now failing when you added support for the ResourceReader api, so I was hoping maybe you would be able to clarify some things.

test_resource_path does not set contents in create_package and then tries to open the file. This worked in the legacy api, but raises FileNotFoundError in files(). I would guess that it was simply ommited because it worked, but that might not be necessarily true. Do you remember?

I have set contents so that it includes utf-8.file in my PR, which fixes the issue. I think it is reasonable to fail reading a resource if that resource is not listed by ResourceReader.contents, that's actually what I would expect.

importlib_resources/importlib_resources/tests/util.py

Lines 121 to 126 in a024adf

	def test_resource_path(self):
	bytes_data = io.BytesIO(b'Hello, world!')
	path = __file__
	package = create_package(file=bytes_data, path=path)
	self.execute(package, 'utf-8.file')
	self.assertEqual(package.__loader__._path, 'utf-8.file')

The other test, which is the one that is giving me trouble, is test_resource_opener. As far as I can tell, it essentially tests if we can still read a resource when resource_path, is_resource and contents (from ResourceReader) raise an exception, which does not seem very correct to me.

Those methods are writen to raise an exception if Reader.path is an exception. This test specifically triggers that and then atempts to read the file.

@brettcannon do you have any more context on this? Does it seems right? Or should it be removed?

importlib_resources/importlib_resources/tests/util.py

Lines 115 to 119 in a024adf

	def test_resource_opener(self):
	bytes_data = io.BytesIO(b'Hello, world!')
	package = create_package(file=bytes_data, path=FileNotFoundError())
	self.execute(package, 'utf-8.file')
	self.assertEqual(package.__loader__._path, 'utf-8.file')

importlib_resources/importlib_resources/tests/util.py

Lines 17 to 51 in a024adf

	class Reader(ResourceReader):
	def __init__(self, **kwargs):
	vars(self).update(kwargs)
	def get_resource_reader(self, package):
	return self
	def open_resource(self, path):
	self._path = path
	if isinstance(self.file, Exception):
	raise self.file
	else:
	return self.file
	def resource_path(self, path_):
	self._path = path_
	if isinstance(self.path, Exception):
	raise self.path
	else:
	return self.path
	def is_resource(self, path_):
	self._path = path_
	if isinstance(self.path, Exception):
	raise self.path
	for entry in self._contents:
	parts = entry.split('/')
	if len(parts) == 1 and parts[0] == path_:
	return True
	return False
	def contents(self):
	if isinstance(self.path, Exception):
	raise self.path
	yield from self._contents

Hopefully I have provided here all the context needed to understand the issue without having to go dig through the code, but let me know if there is still something I could clarify.

[jaraco]

In e1a0b90, I added some docstrings to those two tests to help capture what I believe their purpose is.

As it is currently, neither of those tests exercise contents(). Can CompatibilityFiles continue to support read/open/path on a legacy resource reader without invoking contents?

[FFY00]

I have updated the implementation to allow building nonexistent paths, like pathlib.Path, only erroring out when we try to do any unsupported operation. The whole test suite passes now without any changes, with the exception of test_resource.py::test_contents, but the fail is related to the content ordering, which is not relevant.

[jaraco]

I believe this was addressed in #221.