-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-87646: Make tempfile.NamedTemporaryFile and TemporaryDirectory path-like
#114765
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
base: main
Are you sure you want to change the base?
Conversation
…ctory` path-like. Add `__fspath__()` methods to `tempfile.NamedTemporaryFile` and `TemporaryDirectory`.
…file-like and path-like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some stylistic suggestions and test simplifications.
| if hasattr(file, 'read') or hasattr(file, 'write'): | ||
| self._filePassed = 1 | ||
| self.fp = file | ||
| self.filename = getattr(file, 'name', None) | ||
| else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this have to to with tempfile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When given an argument that is both file-like and path-like, previously zipfile would treat the argument as path-like and attempt to re-open() it. On Windows that caused PermissionError to be raised because the NamedTemporaryFile was already open. This change makes zipfile use the file-like interface in preference to the path-like interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect some third-party code will fall into the same trap as zipfile did here, and so perhaps it's not viable to make NamedTemporaryFile path-like.
|
When you're done making the requested changes, leave the comment: |
Co-authored-by: Brett Cannon <[email protected]>
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this was forgotten about, ready to merge before the freeze in a fortnight?
tempfile.NamedTemporaryFile and TemporaryDirectory path-liketempfile.NamedTemporaryFile and TemporaryDirectory path-like
|
🤖 New build scheduled with the buildbot fleet by @hugovk for commit fc0ff04 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F114765%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
|
@barneygale I'm not sure if you want to still land this, but it will have to target 3.15 now. |
Add
__fspath__()methods totempfile.NamedTemporaryFileandTemporaryDirectory.📚 Documentation preview 📚: https://cpython-previews--114765.org.readthedocs.build/