classification
Title: Support resources in namespace packages
Type: Stage: resolved
Components: Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: jaraco Nosy List: FFY00, barry, brett.cannon, jaraco, vstinner
Priority: normal Keywords: patch

Created on 2020-10-23 16:03 by jaraco, last changed 2021-03-22 09:01 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 24670 merged jaraco, 2021-02-28 19:31
Messages (12)
msg379451 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2020-10-23 16:03
In [importlib_resources#68](https://github.com/python/importlib_resources/issues/68), the project identified a deficiency with respect to pkg_resources for resources in namespace packages.

The project has since merged support for these packages, slated to go out with the importlib_resources 3.2 release.

This issue is to track the incorporation of those changes into importlib.resources.
msg384770 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-01-10 17:47
In [this commit](https://github.com/python/importlib_resources/commit/bd20d893f11f387d285c666bc99fd2d4a7c33ef8), I've reconciled and merged the changes from importlib_resources 3.2-5.0, mainly the namespace package support (https://importlib-resources.readthedocs.io/en/latest/history.html#v5-0-0).

Applying these changes to cpython and running the tests, there appear to be two emergent failure modes:

```
cpython master $ http --follow https://github.com/python/importlib_resources/archive/cpython.zip | bsdtar --strip-components 1 -x
$ ./python.exe Tools/scripts/run_tests.py test.test_importlib 2>&1 | gist -Pc
https://gist.github.com/dde7d5a951d92726380dced21503f843
```

In my estimation, the first two failures are due to the change in the abc.ResourceReader for contents to raise FileNotFoundError.

And the latter failures are seemingly due to the built-in NamespaceLoader not having a resource reader. I suspect the fix here is to add a .get_resource_reader() method to _NamespaceLoader to return a NamespaceReader. (hint: when updating _bootstrap_external.py, you'll need to run `make regen-importlib` or `make regen-all` and recompile).

Filipe, would you be interested in exploring these issues and propose some fixes and prepare the PR that applies the changes to CPython?
msg384776 - (view) Author: Filipe LaĆ­ns (FFY00) * Date: 2021-01-10 20:13
Yes, I will look into it. Do we have a time schedule for when this should be ready, so that I can organize myself?
msg384781 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-01-10 21:45
Thanks!

No rush, but ideally soon enough to be merged before the beta release of Python 3.10 (2021-05-03).
msg386958 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-02-14 18:01
For the first two errors, the issue seems to be that CPython includes tests for the ResourceReader ABC and asserts that .contents() returns an empty list as the default degenerate behavior (https://github.com/python/cpython/blob/1b57426e3a7842b4e6f9fc13ffb657c78e5443d4/Lib/test/test_importlib/test_abc.py#L340-L341). It appears that the ABC in importlib_resources has for a [very long time raised FileNotFound](https://github.com/python/importlib_resources/commit/e82b5675b9fef7dd971b796136687f915f7a39ca).

Oh, this is interesting - these ABCs have diverged since their inception; here's where [both the empty list behavior and test were introduced in cpython](https://github.com/python/cpython/commit/4ac5150e068a3a795ef00465f6dff51747b62b91) and [later amended to return an iterable instead of iterator](https://github.com/python/cpython/commit/3ab9365dca8438f89b2060cd3eebe00606133dc4).

Brett, is this a divergence worth maintaining? Do you recall if there was a reason for changing the ABC during the port to CPython? There's no mention of 'contents' in issue32248.

My temptation is to go with the implementation as found in importlib_resources as it presents a more consistent interface for ResourceReaders (and explicitly defaults to raising FileNotFoundError when no implementation for contents is present) and thus to change the test to:

```
cpython master $ git diff Lib/test/test_importlib/test_abc.py
diff --git a/Lib/test/test_importlib/test_abc.py b/Lib/test/test_importlib/test_abc.py
index d8b9fc89f2..d1c89c183b 100644
--- a/Lib/test/test_importlib/test_abc.py
+++ b/Lib/test/test_importlib/test_abc.py
@@ -338,7 +338,9 @@ def test_is_resource(self):
             self.ins.is_resource('dummy_file')
 
     def test_contents(self):
-        self.assertEqual([], list(self.ins.contents()))
+        with self.assertRaises(FileNotFoundError):
+            self.ins.contents()
+
 
 (Frozen_RRDefaultTests,
  Source_RRDefaultsTests
```

Brett, let me know if you have concern about harmonizing the two implementations around this behavior and if you have a strong preference for harmonizing toward the CPython implementation instead. Thanks.
msg387197 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2021-02-18 00:24
I would harmonize towards what the concrete implementations are doing since people who don't implement the defaults will get the appropriate results regardless. An entry in What's New will cover any potential notification of the change since you can't exactly deprecate this sort of thing.
msg387805 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-02-28 09:18
> I would harmonize towards what the concrete implementations...

For FileReader and ZipReader, both rely on TraversableResources to implement contents, which rely on `files().iterdir()`, which could raise FileNotFoundError and definitely don't return lists. Most importantly, this method is an abstract method, so subclasses can't rely on it (must override it). Searching Github, I couldn't find any other classes subclassing this ABC.
msg387808 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-02-28 10:11
I've pushed [this branch](https://github.com/python/cpython/tree/feature/42129-resources-namespace-packages), which includes fixes for the above two identified issues. Still one issue remains:

ERROR: test_package_has_no_reader_fallback (test.test_importlib.test_resource.ResourceCornerCaseTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jaraco/code/public/cpython/Lib/test/test_importlib/test_resource.py", line 98, in test_package_has_no_reader_fallback
    self.assertFalse(resources.is_resource(module, 'A'))
  File "/Users/jaraco/code/public/cpython/Lib/importlib/resources.py", line 157, in is_resource
    package_contents = set(contents(package))
  File "/Users/jaraco/code/public/cpython/Lib/importlib/resources.py", line 174, in contents
    transversable = _common.from_package(package)
  File "/Users/jaraco/code/public/cpython/Lib/importlib/_common.py", line 75, in from_package
    reader = spec.loader.get_resource_reader(spec.name)
AttributeError: 'object' object has no attribute 'get_resource_reader'

This same test passes on importlib_resources, and the difference seems to be rooted in how [from_package resolves the package spec using a compatibility wrapper](https://github.com/python/importlib_resources/blob/1401cc48e5077088036aa7e729c8995ffbbb9e88/importlib_resources/_common.py#L76). On the backport, this causes the package without a resource reader to have a resource reader and return a degenerate value:

```
> /Users/jaraco/code/public/importlib_resources/importlib_resources/_py3.py(139)is_resource()
-> package_contents = set(contents(package))
(Pdb) _common.from_package(package)
PosixPath('/path/which/shall/not/be')
(Pdb) from . import _compat
(Pdb) _compat.package_spec(package).loader.get_resource_reader('any').files().is_dir()
False
```

This means that the compatibility shim in from_package is masking test failure in the backport, and probably the best course of action will be to unmask that failure in the backport and figure out the best behavior there.
msg388121 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2021-03-04 18:43
New changeset 67148254146948041a77d8a2989f41b88cdb2f99 by Jason R. Coombs in branch 'master':
bpo-42129: Add support for resources in namespaces (GH-24670)
https://github.com/python/cpython/commit/67148254146948041a77d8a2989f41b88cdb2f99
msg389163 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-20 14:59
Please see bpo-43569: "test_importlib failed on installed Python" regression introduced by commit 67148254146948041a77d8a2989f41b88cdb2f99.
msg389164 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-20 14:59
I reopen the issue.
msg389288 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-03-22 09:01
> Please see bpo-43569: "test_importlib failed on installed Python" regression introduced by commit 67148254146948041a77d8a2989f41b88cdb2f99.

It's now fixed, I close again this issue.
History
Date User Action Args
2021-03-22 09:01:32vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg389288
2021-03-20 14:59:16vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg389164
2021-03-20 14:59:06vstinnersetnosy: + vstinner
messages: + msg389163
2021-03-04 18:43:43jaracosetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-03-04 18:43:09jaracosetmessages: + msg388121
2021-02-28 19:31:24jaracosetkeywords: + patch
stage: patch review
pull_requests: + pull_request23456
2021-02-28 10:11:19jaracosetmessages: + msg387808
2021-02-28 09:18:18jaracosetmessages: + msg387805
2021-02-18 00:24:11brett.cannonsetmessages: + msg387197
2021-02-14 18:01:24jaracosetnosy: + brett.cannon
messages: + msg386958
2021-01-10 21:45:24jaracosetmessages: + msg384781
2021-01-10 20:13:17FFY00setmessages: + msg384776
2021-01-10 17:47:48jaracosetmessages: + msg384770
2020-10-23 17:09:06barrysetnosy: + barry
2020-10-23 16:10:25FFY00setnosy: + FFY00
2020-10-23 16:03:26jaracocreate