[issue42129] Support resources in namespace packages

[STINNER Victor]

STINNER Victor  added the comment:

> Please see bpo-43569: "test_importlib failed on installed Python" regression 
> introduced by commit 67148254146948041a77d8a2989f41b88cdb2f99.

It's now fixed, I close again this issue.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[STINNER Victor]

STINNER Victor  added the comment:

I reopen the issue.

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[STINNER Victor]

STINNER Victor  added the comment:

Please see bpo-43569: "test_importlib failed on installed Python" regression 
introduced by commit 67148254146948041a77d8a2989f41b88cdb2f99.

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Change by Jason R. Coombs :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Change by Jason R. Coombs :


--
keywords: +patch
pull_requests: +23456
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/24670

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Brett Cannon]

Brett Cannon  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:

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.

--
nosy: +brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:

Thanks!

No rush, but ideally soon enough to be merged before the beta release of Python 
3.10 (2021-05-03).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Filipe Laíns]

Filipe Laíns  added the comment:

Yes, I will look into it. Do we have a time schedule for when this should be 
ready, so that I can organize myself?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

Jason R. Coombs  added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Barry A. Warsaw]

Change by Barry A. Warsaw :


--
nosy: +barry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Filipe Laíns]

Change by Filipe Laíns :


--
nosy: +FFY00

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42129] Support resources in namespace packages

[Jason R. Coombs]

New submission from Jason R. Coombs :

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.

--
assignee: jaraco
messages: 379451
nosy: jaraco
priority: normal
severity: normal
status: open
title: Support resources in namespace packages
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com