bpo-26175: Implement io.IOBase interface for SpooledTemporaryFile - #29560
bpo-26175: Implement io.IOBase interface for SpooledTemporaryFile#29560methane merged 6 commits into
Conversation
Sorry, something went wrong.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
Sorry, something went wrong.
|
@pR0Ps I guess you ignored the change requests from the other PR, shouldn't call __del__ like that; see here. Also IMO you'd want to give the warning regardless of the file having been rolled over to disk or not. BytesIO does not warn in its __del__. Otherwise you'd have warnings that are triggered by the phase of moon. |
Sorry, something went wrong.
|
@ztane Made the requested change to the __del__ method and added a test to ensure that a ResourceWarning is generated in both a rolled and unrolled state. |
Sorry, something went wrong.
|
Rebased on latest main. |
Sorry, something went wrong.
Since the underlying file-like objects (either `io.BytesIO`, `io.StringIO`, or a true file object) all implement the `io.IOBase` interface, the `SpooledTemporaryFile` should as well. Additionally, since the underlying file object will either be an instance of an `io.BufferedIOBase` (for binary mode) or an `io.TextIOBase` (for text mode), methods for these classes were also implemented. In every case, the required methods and properties are simply delegated to the underlying file object. Co-authored-by: Gary Fernie <Gary.Fernie@skyscanner.net>
|
In the future, please avoid force pushes on Python repositories, they make reviewing less pleasant: https://devguide.python.org/pullrequest/ |
Sorry, something went wrong.
Sorry, I've re-pushed the change as a separate commit and will do the same in the future. |
Sorry, something went wrong.
|
@pR0Ps Thanks! When PR is updated with a usual push, everybody subscribed can open the PR from "Unread notifications" and see only these updates of a diff. For example, a full PR4881073 contains 38 insertions but I do not need to reread it from a scratch after a (bunch of) commit(s) was added today:
With a force-push, however, GitHub gives up and forces a reviewer to open a list of commits and manually scan for all changes:
When there are a few commits per day, it's ok. However, in this repo with a dosen of changes per hour, such manual works would become a second job. |
Sorry, something went wrong.
|
@erlend-aasland Can you take a look please? |
Sorry, something went wrong.
python/cpython#29560 Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Since the underlying file-like objects (either io.BytesIO, io.StringIO, or a true file object) all implement the io.IOBase interface, the SpooledTemporaryFile should as well.
Additionally, since the underlying file object will either be an instance of an io.BufferedIOBase (for binary mode) or an io.TextIOBase (for text mode), methods for these classes were also implemented.
In every case, the required methods and properties are simply delegated to the underlying file object.
Co-authored-by: Gary Fernie <Gary.Fernie@skyscanner.net>
This is a rebased and reworked followup of the seemingly-abandoned #3249
https://bugs.python.org/issue26175