cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Recommend importlib.abc.Traversable users to implement __fspath__

Open FFY00 opened this issue 4 years ago • 8 comments

BPO 44200
Nosy @brettcannon, @jaraco, @FFY00

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-05-21.00:09:16.984>
labels = ['3.10', 'docs']
title = 'Recommend importlib.abc.Traversable users to implement __fspath__'
updated_at = <Date 2021-05-23.20:12:51.059>
user = 'https://github.com/FFY00'

bugs.python.org fields:

activity = <Date 2021-05-23.20:12:51.059>
actor = 'FFY00'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2021-05-21.00:09:16.984>
creator = 'FFY00'
dependencies = []
files = []
hgrepos = []
issue_num = 44200
keywords = []
message_count = 6.0
messages = ['394083', '394084', '394211', '394213', '394219', '394223']
nosy_count = 4.0
nosy_names = ['brett.cannon', 'jaraco', 'docs@python', 'FFY00']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue44200'
versions = ['Python 3.10']

FFY00 avatar May 21 '21 00:05 FFY00

The files()/Traversable api is meant to replace ResourceReader api, but it falls short in one dimension, tying the abstraction to a file system path.

I propose to document that Traversable users *should* implement __fspath__ if the Traversable represents a file-system path.

This will essentially make os.fspath(importlib.resources.files(module) / resource) the equivalent of importlib.resources.path(module, resource), for which currently there is no replacement using files().

FFY00 avatar May 21 '21 00:05 FFY00

I am gonna wait until/if python/cpython#70460 (bpo-44196) gets merged because this would conflict with that.

FFY00 avatar May 21 '21 00:05 FFY00

The problem with the __fspath__ protocol is that it assumes a file system. The importlib.resources and Traversable protocols are trying to provide a lower-level interface that doesn't assume a file system. Fortunately, there already exists a helper function as_file (https://docs.python.org/3/library/importlib.html#importlib.resources.as_file) that's meant to provide the user experience that path once provided, but for Traversable files.

Does that satisfy the need you've identified?

jaraco avatar May 23 '21 17:05 jaraco

That's why I said it should be optional. If the object can implement it, it should. pathlib.Path certainly can, zipfile.Path can't, and that's alright. If the Traversable does not represent a path on the file-system, it does not make sense to implement __fspath__, so it shouldn't.

I think as_file should learn to behave like path, if __fspath__ is available, it will use that path, otherwise it will do what it does currently, which is creating a temporary path and writing the resource contents there.

FFY00 avatar May 23 '21 17:05 FFY00

I think as_file should learn to behave like path, if __fspath__ is available, it will use that path.

Oh, that's an interesting idea. And that's effectively what happens here, where as_file returns a native Path object if that's what it encounters. Maybe another dispatcher could accept os.PathLike and in that case, return pathlib.Path(os.fspath(path)).

Only thing is, that code would serve no purpose until there existed at least one other resource provider besides FileReader that has file-system-local resources. I'm not aware of any such provider.

jaraco avatar May 23 '21 19:05 jaraco

CompatibilityFiles would use this. We could add a dispatch for that too, but that's 2 different cases now and os.PathLike would be able to handle both.

FFY00 avatar May 23 '21 20:05 FFY00

Hey, I'm a bit confused as to why Traversable was ever implemented in the first place. Seems like Traversable is supposed to represent file system paths, however I was sure that's why we have Paths.

Whatever Traversable's added value is, I still think the class should be coupled to Path type-wise. Maybe by marking Traversable as PathLike.

At the moment, functions like importlib.resources.files specify a return type of Iterator[Traversable] but in reality return Iterator[Path]. This results in confusing type hint warnings since IDEs have no way to know that Traversable and Path are related.

I'm not very experienced with the newer versions of importlib so my assumptions might be a bit off, hoping one of could shed some light.

Thanks

moomoohk avatar Sep 16 '22 12:09 moomoohk

CompatibilityFiles would use this. We could add a dispatch for that too, but that's 2 different cases now and os.PathLike would be able to handle both.

I'm not sure I understand the two cases. Perhaps you could illustrate how this would work within CompatibilityFiles?

jaraco avatar Mar 21 '24 02:03 jaraco

CompatibilityFiles is an abstraction over ResourceReader, it implements a Traversable interface using ResourceReader.contents, ResourceReader.resource_path, etc. In this case, the resource file can exist on the filesystem, but we cannot simply return a pathlib.Path of the root of the package in files(), as the reader might customize the available resources.

Currently, if I need to access a resource as a file on the filesystem, I need to use as_file or something similar, which will write the resource contents to a temporary file. This isn't optimal, so to avoid it, we could register another dispatch method in as_file to special-case CompatibilityFiles, like we do for pathlib.Path:

https://github.com/python/importlib_resources/blob/6fa0a7aef25178430816c4459524a4945bfb7ee1/importlib_resources/_common.py#L173-L179

But I feel like a better solution would be to have the Traversable in question, which in this example is CompatibilityFiles, implement __fspath__ if it is representing a path on the filesystem. This has two benefits:

  • CompatibilityFiles would provide an interface to get the resource path (independently of as_file)
  • We could have as_file implement a dispatch for os.PathLike, avoiding the situation described above not only for CompatibilityFiles, but for any Traversable that provides the filesystem path via __fspath__

FFY00 avatar Apr 09 '24 03:04 FFY00