Close fp if we created it #121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If we opened it, we are responsible for closing it. This also rids us of ResourceWarnings popping up in newer python versions. Of course, actually, the creator of an OleFileIO is repsonsible for calling close() before it goes out of scope. But if the error happens in open() called from __init__, then caller has no chance to call close().
We got rid of the ResourceWarning now, but sometimes it could be helpful to teach people proper exception handling... Just not when open() was called from __init__
Clarify in doc string that fp is owned by caller
Unfortunately, most interesting behaviour cannot be tested automatically since __del__ can be run at any time and after init failure object is not assigned
Closed
christian-intra2net
force-pushed
the
close-fp-if-we-created-it
branch
from
8ee44d8
to
0e34cee
Compare
|
Tried to implement tests for issuing the warning, but del really cannot be predicted and not realiably controlled using module gc nor a forked interpreter session (e.g. not called at session end in py2.7) |
decalage2
approved these changes
May 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This solves issue #120 ("Clarify who owns fp")
Clarify that fp is owned by whoever created it. Meaning that if an OleFileIO was created/opened with an open file handle as argument, it will never close it. On the other hand, if an OleFileIO was created/opened with a filename, it opens that file internally and therefore takes care to always close the handle.
However, the user of an OleFileIO is responsible for calling close() on it. If she fails to do so a warning is issued to help improve caller code.
Unittests verify that this works as expected and will work in the future. Doc strings also include hints.
This avoids ResourceWarnings caused by unclosed file handles in OleFileIO.