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

Supporting context manager usage of files() -- similar to the old path() handling. #291

Closed
eli-schwartz opened this issue Nov 23, 2023 · 4 comments
Assignees

Comments

@eli-schwartz
Copy link

Migrated from mesonbuild/meson#12401 (comment)

I've never loved the fact that one needs to files(module) / 'filename' for the most basic usage. I often find myself using files(module).joinpath('filename') in order to have an expression on which I can invoke a method (e.g. files(module).joinpath('filename').read_text().

What I'd really like to see is that in python 3.13 I could do this:

The main reason it wasn't implemented this way was because for the common case where packages are installed into the file system, it was nice for files() to return a pathlib.Path object, which may have (or develop) its own behavior when entered as a context manager, but more importantly, would need to be wrapped in order to supply behavior that's not intrinsic to a path.

I suppose what you're proposing could be possible, but I'm not sure the ergonomics benefits would outweigh the complication that would ensue from wrapping pathlib.Path and replacing as_file with an integrated context manager.

It would require updating this protocol and then probably wrap the return value from files in an adapter that provides the context manager. The real challenge is going to be writing an adapter that's resilient across traversal.

And now that I think about it, it won't work to change Traversable. Traversable is what the providers are meant to supply. We'll have to change the files return type from a Traversable to this new enterable type.

Would you be willing to draft an implementation and tell me if after experimenting with the above approach or perhaps an alternative if you think the approach would be worth the trouble?

If so, I think the next best step would be to file a bug with importlib_resources.

Originally posted by @jaraco in mesonbuild/meson#12401 (comment)

I have take a stab at this. main...eli-schwartz:importlib_resources:files-supports-with

It's really more of a quick shim than anything else, to demonstrate feasibility. In summary:

  • create subclasses of pathlib.Path / zipfile.Path, which add __enter__() / __exit__()
  • change files() to check which type of return value it got from from_package, and immediately replace it with the derived class
@eli-schwartz
Copy link
Author

The question I guess is what to do for MultiplexedPath, which from a very hasty look did not much seem to like pkg_resources style packages, those being the only type that support zipfiles anyway.

@eli-schwartz
Copy link
Author

ping?

@jaraco
Copy link
Member

jaraco commented Dec 21, 2023

The question I guess is what to do for MultiplexedPath

The question is more general than that, since any package provider is able to implement a TraversableResources on the finder to provide resources. The solution should ideally support arbitrary Traversables.

I'll take the work you started and see if I can't evolve that into something that supports an arbitrary Traversable.

@jaraco
Copy link
Member

jaraco commented Mar 5, 2024

After attempting to adopt this concept to account for the concern above in #292, I ultimately concluded that generalizing adapting an arbitrary Traversable to something Enterable can't be done reliably. I'm happy to continue to brainstorm on it, but for now I'm closing this as infeasible.

@jaraco jaraco closed this as completed Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants