-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
importlib.resources regression for custom ResourceReader #127337
Comments
Looks like when the legacy API was re-added in python/importlib_resources#303, it added a new But looking at the Considering the new That said, we need to be more careful when making changes to protocols, as I think this issue was the result of GH-91623 making a breaking change to the |
Thanks Filipe for the analysis. Indeed, by allowing joinpath to accept 0 or more arguments, any caller relying on being able to pass zero arguments will fail, and Going one step deeper, I traced the change of the protocol to python/importlib_resources#248, which references GH-91298. Copying @encukou for visibility. I think the proper fix at this stage is to expand the diff --git a/importlib_resources/_adapters.py b/importlib_resources/_adapters.py
index 50688fbb66..9f78bca0ff 100644
--- a/importlib_resources/_adapters.py
+++ b/importlib_resources/_adapters.py
@@ -66,10 +66,12 @@ class CompatibilityFiles:
is_dir = is_file
- def joinpath(self, other):
+ def joinpath(self, *descendants):
+ if not descendants:
+ return self
if not self._reader:
- return CompatibilityFiles.OrphanPath(other)
- return CompatibilityFiles.ChildPath(self._reader, other)
+ return CompatibilityFiles.OrphanPath(*descendants)
+ return CompatibilityFiles.ChildPath(self._reader, '/'.join(descendants))
@property
def name(self): (maybe also ChildPath needs an update) @FFY00 Are you willing to tackle this one?
I've run into this concern a few times, and it's not obvious to me how to navigate these changes. I don't have a mental model for how safely to adapt a protocol (or if it's even possible). I'm aware of the challenges; I just don't have any ready-made solutions. Looking at the issue as reported, it appears the resource reader is reliant on the from importlib.abc import Loader
from importlib.machinery import ModuleSpec
from importlib.resources.abc import ResourceReader, TraversableResources
_DATA = {"FakeModule.NAME": b"hello"}
_DATA_NAMES = set(_DATA)
class TraversableData:
def __init__(self, prefix):
self.prefix = prefix
def is_dir(self):
return self.prefix.endswith('.')
def is_file(self):
return not self.prefix.endswith('.')
def iterdir(self):
return (
TraversableData(name)
for name in _DATA if name.startswith(self.prefix)
)
def open(self):
import io
return io.BytesIO(_DATA[self.prefix])
def joinpath(self, *descendants):
suffix = '.'.join(descendants)
return TraversableData(self.prefix + suffix)
class MyReader(TraversableResources):
def __init__(self, prefix):
self.prefix = prefix
def files(self):
return TraversableData(self.prefix)
class MyLoader(Loader):
create_module = ...
exec_module = ...
def get_resource_reader(self, fullname):
return MyReader(fullname + ".")
class FakeModule:
__loader__ = MyLoader()
__name__ = "FakeModule"
__spec__ = ModuleSpec(__name__, __loader__)
from importlib.resources import contents
print(contents(FakeModule)) That change runs on Python 3.14. I tried it on Python 3.9, and even after adapting the imports, it doesn't work as I'd hoped. I'd have to spend some more time to trace the cause, but I do think it would be beneficial to explore getting off of CompatibilityFiles and providing a native TraversableResources interface (introduced in Python 3.9). For this specific issue, what is the minimum version of Python that the resource provider needs to support? |
Right, but this requires the maintainer to do work. If the automatic adaptation gets it right, then users don't have to wait. We're in the halfway position where maintainers can sometimes not have to do anything, but will only discover it for sure after all their users have updated and broken. A better position is (a) for it all to work ( 😉 ), or (b) for it to be totally deprecated and removed so that maintainers can tell their users to avoid updating Python entirely until they've gotten an update ready. (Obviously (a) is preferred, but if we start down the path of (a) then (b) is totally cut out.)
It doesn't matter. 3.13 has to be supported, and so there's now a hack in there to handle as far back as is needed (I believe 3.7 onwards still works). But we shouldn't have let it stop working in 3.13. FWIW, my new traversable implementation looks pretty much like that, though I keep the old implementation around so that older versions also work. I'd prefer to have duplicated code that works than a single implementation that doesn't, especially when we're talking about multiple single-line functions. |
Since Python 3.13 (including 3.14), my custom ResourceReader is now raising errors when used with
importlib.resources.contents
. This code is otherwise functioning fine from 3.8 onwards.Self-contained repro below (the real one gets compiled into DLLs, so you don't want to try to debug it ;) ):
(ping @jaraco)
The text was updated successfully, but these errors were encountered: