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

Recommend importlib.abc.Traversable users to implement __fspath__ #88366

Open
FFY00 opened this issue May 21, 2021 · 9 comments
Open

Recommend importlib.abc.Traversable users to implement __fspath__ #88366

FFY00 opened this issue May 21, 2021 · 9 comments
Assignees
Labels
3.10 only security fixes docs Documentation in the Doc dir topic-importlib

Comments

@FFY00
Copy link
Member

FFY00 commented May 21, 2021

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
Copy link
Member Author

FFY00 commented May 21, 2021

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 FFY00 added 3.10 only security fixes labels May 21, 2021
@FFY00
Copy link
Member Author

FFY00 commented May 21, 2021

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

@FFY00 FFY00 added the docs Documentation in the Doc dir label May 21, 2021
@FFY00 FFY00 added the docs Documentation in the Doc dir label May 21, 2021
@jaraco
Copy link
Member

jaraco commented May 23, 2021

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?

@FFY00
Copy link
Member Author

FFY00 commented May 23, 2021

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.

@jaraco
Copy link
Member

jaraco commented May 23, 2021

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.

@FFY00
Copy link
Member Author

FFY00 commented May 23, 2021

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.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@moomoohk

This comment was marked as off-topic.

@jaraco
Copy link
Member

jaraco commented Mar 21, 2024

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 jaraco assigned FFY00 and unassigned jaraco Mar 21, 2024
@FFY00
Copy link
Member Author

FFY00 commented Apr 9, 2024

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__

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes docs Documentation in the Doc dir topic-importlib
Projects
None yet
Development

No branches or pull requests

4 participants