Repository navigation
Fully implement IOBase abstract on SpooledTemporaryFile #70363
Description
Activity
GFernie commented
on Jan 21, 2016 GFerniemannequinMannequinAuthorSpooledTemporaryFile does not fully satisfy the abstract for IOBase.
Namely,seekable,readable, andwritableare missing.This was discovered when seeking a SpooledTemporaryFile-backed lzma file.
You may quickly repro this:
lzma.open(SpooledTemporaryFile()).seek(0)It would be nice to add test cases for this.
Looking at <https://docs.python.org/release/3.4.3/library/tempfile.html#tempfile.SpooledTemporaryFile\>, there is a version changed notice for the truncate() method. So perhaps this should be added as a new feature for 3.6. On the other hand, the documentation doesn’t mention the specific API, so I would assume the relevant IOBase subclass, and this would be a bug.
It may also be worth implementing BufferedIOBase and TextIOBase. (It seems buffering=0 isn’t reliable, e.g. rollover with limited disk space, so it may not be worth implementing RawIOBase.)
To implement BufferedIOBase, “read1” and “readinto1” should be added.
To implement TextIOBase, “errors” should be added, and “newlines” should only list translated newlines, not the “newline” constructor argument.
Technically, “detach” should also be implemented, although it doesn’t have to do anything useful.
Martin, the PR has one 'approved' review, not from you. You appear to be requesting changes here, but it is not clear to me which are 'maybe' and which are 'must'.
It is not obvious to me whether this should be treated as enhancement or behavior issue, but backporting is moot until something is merged into master.
I was making suggestions, not demanding anything. Except for the quirk with __del__, Gary’s changes (revision fb28362) look okay to add on their own as a bug fix.
I wouldn’t claim that IOBase is “fully implemented” however, until the return values for “seek” and “truncate” are fixed.
The underlying problem is the original SpooledTemporaryFile imitated Python 2’s file class/type, and wasn’t completely adapted for Python 3’s file classes.
nubirstein commented
on Oct 30, 2018 nubirsteinmannequinMannequinShould I have been added my request there? Anyway I do suffer from lack of 'seekable()' implementation there
nubirstein commented
on Oct 30, 2018 nubirsteinmannequinMannequinMy last comment meant to land somewhere else, but nonetheless it is related to this topic, so:
SpooledTemporaryFile class from lib/tempfile.py still does not implement seekable() method. It could be like this (just two lines of code and my Flask.Request tests with sending files started again to work on 3.7:
def seekable(self): return self._file.seekable()
Is it possible to add this method?
danieldjewell commented
on May 13, 2020 danieldjewellmannequinMannequinTo add something additional here:
The current documentation for tempfile.SpooledTemporaryFile indicates "This function operates exactly as TemporaryFile() does, except that data is spooled in memory until the file size exceeds max_size[...]" (see https://docs.python.org/3/library/tempfile.html)
Except that SpooledTemporaryFile *doesn't* act _exactly_ like TemporaryFile() - as documented here. TemporaryFile() returns an "_io.BufferedRandom" which implements all of the expected "file-like" goodies - like [.readable, .seekable, ...etc]. SpooledTemporaryFile does not.
In comparing to the 2.x docs, the text for SpooledTemporaryFile() appears to be identical or nearly identical to the 3.8.x current docs. This goes in line with what has already been discussed here.
At a _very minimum_ the documentation should be updated to reflect the current differences between TemporaryFile() and SpooledTemporaryFile().
Perhaps an easier change would be to extend TemporaryFile() to have a parameter that enables functionality similar to SpooledTemporaryFile? Namely, *memory-only* storage up to a max_size? Or perhaps there is an alternate solution that already exists?
Ultimately, the functionality that appears to be missing is to be able to easily create a file-like object backed primarily by memory for reading/writing data ... (i.e. one 100% compatible with 'the usual' file objects returned by open()...)
ztane commented
on Oct 30, 2020 ztanemannequinMannequinAnother test case:
import tempfile import io import json with tempfile.SpooledTemporaryFile(max_size=2**20) as f: tf = io.TextIOWrapper(f, encoding='utf-8') json.dump({}, fp=tf)
I was writing json to a file-like object that I need to read in as binary (to upload to S3). Originally the code used BytesIO and I thought it would be wise to actually spool this to disk as I was operating with possible limited RAM... except that of course it didn't work.
ztane commented
on Oct 30, 2020 ztanemannequinMannequin... to further clarify, it is disappointing that either BytesIO or TemporaryFile would work alone, but the one that merges these two doesn't.
I believe the PR is in good shape.
Can someone with expertise in tempfile review it?It would also be useful if the people who had a bug could test the new code.
Éric, you might use git log or git blame to see who that is active has patched the relevant file in the last few years.
Also, which of the two patches.
Irit, you just patched Temp file doc, can you look at the PR code?
Irit, you just patched Temp file doc, can you look at the PR code?
I don't consider myself and expert here, but I left a comment on PR29560 just to be a good sport.
- added a commit that references this issue
on May 3, 2022 - added a commit that references this issue
on Jul 31, 2023
io.IOBaseinterface forSpooledTemporaryFile#29560Note: 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:
bugs.python.org fields: