Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

files(...).joinpath doesn't accept variable number of arguments in Python 3.10+ #257

Closed
Jakub-CZ opened this issue Jul 12, 2022 · 6 comments · Fixed by #268 or #301
Closed

files(...).joinpath doesn't accept variable number of arguments in Python 3.10+ #257

Jakub-CZ opened this issue Jul 12, 2022 · 6 comments · Fixed by #268 or #301

Comments

@Jakub-CZ
Copy link
Contributor

Jakub-CZ commented Jul 12, 2022

This happens to me when I try to access resources in a native namespace package.

Tested on:

  • Windows, Python 3.10.4
  • Docker in WSL2, image python:3.10-slim, Python 3.10.5

The reason seems to be that importlib_resources.files() returns MultiplexedPath from stdlib (importlib.readers) rather than from importlib_resources.readers.

To recreate:

  1. Install any namespace package, e.g. sphinxcontrib-htmlhelp;

    pip install sphinxcontrib-htmlhelp
    
  2. Use files with joinpath to get the path of a resource.

    from importlib_resources import files
    
    print(root:=files('sphinxcontrib'), 'from', root.__module__)
    print(root.joinpath('htmlhelp', 'templates'))

Expected output (which I get with Pythons 3.7-3.9):

MultiplexedPath('/usr/local/lib/python3.9/site-packages/sphinxcontrib') from importlib_resources.readers
/usr/local/lib/python3.9/site-packages/sphinxcontrib/htmlhelp/templates

Bug with Pythons 3.10:

MultiplexedPath('/usr/local/lib/python3.10/site-packages/sphinxcontrib') from importlib.readers
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: MultiplexedPath.joinpath() takes 2 positional arguments but 3 were given

I also tested with the code in #254 but to no aval.

@jaraco
Copy link
Member

jaraco commented Jul 22, 2022

I see support for namespace packages was added in 8e1a218 released in importlib_resources 3.2.0 and Python 3.10. I'm able to replicate the error with this one-liner (requires pip-run):

$ pip-run -q sphinxcontrib-htmlhelp importlib_resources -- -c "import importlib_resources as resources; resources.files('sphinxcontrib').joinpath('htmlhelp', 'templates')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: MultiplexedPath.joinpath() takes 2 positional arguments but 3 were given

In that release, the MultiplexedPath.joinpath only accepted one parameter.

But that restriction was lifted in 6440323 (released in 5.8.0).

But somehow, it seems that the MultiplexedPath on Python 3.10 is being preferred even though importlib_resources 5.9 is being invoked.

@jaraco
Copy link
Member

jaraco commented Jul 22, 2022

I think I understand what's going on:

  • importlib_resources wraps the spec in order to supply a TraversableResources reader if the loader/spec doesn't already provide one.

  • Because Python 3.10 provides one, however, importlib_resources defers to that implementation, so on Python 3.10 and 3.11, the users get an experience based on objects in the stdlib. With Python 3.12, users will get the importlib_resources 5.8+ experience.

It's not obvious to me how importlib_resources could intercept these calls for later versions of Python that were previously adequate, and even less obvious that's what should happen. importlib_resources has to support loaders from a range of Python versions as well as a range of behaviors from custom loaders.

As a short-term workaround, I suggest to use .joinpath('htmlhelp').joinpath('templates'), which should work in all Python versions. It's possible that .joinpath('htmlhelp/templates') will also work, but that's also risky, because support for that was improved recently.

@jaraco
Copy link
Member

jaraco commented Jul 22, 2022

@FFY00 I'd like your opinion - do you think based on this report that importlib_resources should be more aggressive about supplying its own Readers and Path objects?

@FFY00
Copy link
Member

FFY00 commented Jul 22, 2022

I think that would be something worth exploring. That behavior seems desirable, but like you mentioned, the implementation might be a bit tricky. If we can come up with a reasonable way of doing this, it would be nice to have it, otherwise, I wouldn't worry too much.

jaraco added a commit that referenced this issue Oct 5, 2022
… have features/fixes present in this library. Fixes #257.
jaraco added a commit that referenced this issue Dec 24, 2022
… have features/fixes present in this library. Fixes #257.
@jaraco jaraco closed this as completed in 68be8b4 Feb 17, 2023
@jaraco jaraco reopened this Mar 15, 2024
@Jakub-CZ
Copy link
Contributor Author

Here's a unit test that fails in Python 3.10 (and only in that one, AFAIK):

diff --git a/importlib_resources/tests/test_files.py b/importlib_resources/tests/test_files.py
index 9e45f70..3e86ec6 100644
--- a/importlib_resources/tests/test_files.py
+++ b/importlib_resources/tests/test_files.py
@@ -34,6 +34,11 @@ class FilesTests:
     def test_traversable(self):
         assert isinstance(resources.files(self.data), Traversable)

+    def test_joinpath_with_multiple_args(self):
+        files = resources.files(self.data)
+        binfile = files.joinpath('subdirectory', 'binary.file')
+        self.assertTrue(binfile.is_file())
+
     def test_old_parameter(self):
         """
         Files used to take a 'package' parameter. Make sure anyone

@jaraco
Copy link
Member

jaraco commented Mar 16, 2024

The issue is with this logic:

# Python 3.10+
if reader.__class__.__module__.startswith('importlib.resources.'):
return

On Python 3.10, the module name for the loaders is "importlib.readers", not "importlib.resources.readers".

Thanks so much for the test, which (shame on me) should have been included in the original solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants