Ensure proper close after open in export_od #469
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #469 +/- ##
==========================================
+ Coverage 67.21% 67.55% +0.34%
==========================================
Files 26 26
Lines 3096 3098 +2
Branches 522 521 -1
==========================================
+ Hits 2081 2093 +12
+ Misses 873 864 -9
+ Partials 142 141 -1
|
Co-authored-by: Erlend E. Aasland <[email protected]>
| else: | ||
| raise NotImplementedError(f"No support for the {doc_type!r} format") |
There was a problem hiding this comment.
Hm, I noticed this a little bit too late... this is a little bit unconventional; NotImplementedError is not meant for such use. IMO, ObjectDictionaryError would be more appropriate.
There was a problem hiding this comment.
Right. Agreed. It's not always easy to "just fix" something like the close issue and then uncovering other things that needs fixing too. ;)
There was a problem hiding this comment.
Yeah, one fix quickly leads to the discovery of other issues :) I'll create an issue about this.
There was a problem hiding this comment.
For the record; before this PR, this check was an assert (IOW, AssertionError, unless running with -O or -OO).
This PR increases the robustness on file open in export_od(). It ensures that if the function opens a file, it will close it.
PS! The buried import is itching to be fixed, but let's not do that in this PR, please.