You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fully implement IOBase abstract on SpooledTemporaryFile #70363
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
nubirsteinmannequin
Mannequin
More actions
Should I have been added my request there? Anyway I do suffer from lack of 'seekable()' implementation there
nubirstein commented on Oct 30, 2018
nubirsteinmannequin
Mannequin
More actions
My 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:
defseekable(self):
returnself._file.seekable()
Is it possible to add this method?
danieldjewell commented on May 13, 2020
danieldjewellmannequin
Mannequin
More actions
To 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()...)
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
ztanemannequin
Mannequin
More actions
... to further clarify, it is disappointing that either BytesIO or TemporaryFile would work alone, but the one that merges these two doesn't.
Activity
GFernie commentedon Jan 21, 2016
SpooledTemporaryFile 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)vadmium commentedon Jan 22, 2016
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.
vadmium commentedon Jan 19, 2018
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.
terryjreedy commentedon Jun 8, 2018
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.
vadmium commentedon Jun 9, 2018
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 commentedon Oct 30, 2018
Should I have been added my request there? Anyway I do suffer from lack of 'seekable()' implementation there
nubirstein commentedon Oct 30, 2018
My 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:
Is it possible to add this method?
danieldjewell commentedon May 13, 2020
To 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 commentedon Oct 30, 2020
Another test case:
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 commentedon Oct 30, 2020
... to further clarify, it is disappointing that either BytesIO or TemporaryFile would work alone, but the one that merges these two doesn't.
merwok commentedon Feb 25, 2022
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.
terryjreedy commentedon Feb 25, 2022
É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.
terryjreedy commentedon Feb 25, 2022
Also, which of the two patches.
Irit, you just patched Temp file doc, can you look at the PR code?
iritkatriel commentedon Feb 28, 2022
I don't consider myself and expert here, but I left a comment on PR29560 just to be a good sport.
gh-70363: Implement `io.IOBase` interface for `SpooledTemporaryFile` (G…
pythongh-70363: Emscripten cannot fstat renamed spool file
gh-70363: Emscripten cannot fstat renamed spool file (GH-92354)
Fixed SpooledTemporaryFile on Python <3.11