Improve CFReader destructor#3336
Improve CFReader destructor#3336evertrol wants to merge 2 commits intoSciTools:masterfrom evertrol:cfreader-improve-destructor
Conversation
This prevents errors when cleaning up CFReader during garbage collection.
lib/iris/tests/test_cf.py
Outdated
| with io.StringIO() as buf: | ||
| with contextlib.redirect_stderr(buf): | ||
| try: | ||
| reader = cf.CFReader(fp.name) |
There was a problem hiding this comment.
F841 local variable 'reader' is assigned to but never used
lib/iris/tests/test_cf.py
Outdated
| except OSError: | ||
| pass | ||
| try: | ||
| cubes = iris.load_cubes(fp.name) |
There was a problem hiding this comment.
F841 local variable 'cubes' is assigned to but never used
SciTools-assistant
added
the
Blocked: CLA needed
|
I'm not entirely happy with the unit test:
|
This test is slightly complicated: it tests that no errors occur during garbage collection of the CFReader; that is, during use of the `__del__` method. Any errors that may occur, however, will not be raised, but just printed to standard error, as a safety measure (since a destructor should always run and complete). Thus, the standard error is captured and its length verified to be zero. For the actual test, we create a netCDF4 file with a proper signature (HDF5 actually), but otherwise broken; the correct signature (magic number) is needed for use in the indirect `load_cubes` call, which first attempts to determine the file type before instantiating a `CFReader` object. A final remark: this test does not fit very well in the unittest class, since it doesn't rely on external data. For clarity, however, it is kept within the TestCFReader class.
|
Hello @evertrol . |
|
I'm curious about the copyright year thing, because I don't understand it fully: does Travis check for modified file that the copyright (but code-commented) line contains the current year? How does this work for files that haven't been touched for a few years, but contain two dates (e.g., Is the copyright for the entire project not simply renewed when a new version of Iris is released, as per the README (I missed a LICENSE file)? |
|
@pp-mo Thanks for that. Nice to see the default license will be BSD 3-clause (in an LICENSE file), for SciTools projects. Though the end-year of the copyright remains confusing to me (perhaps it shouldn't be |
|
I'm going to close and hand off this PR, since I'm not inclined to sign the CLA: that is implicit by submitting this PR. |
6 participants
This improves the CFReader's destructor, by testing for
self._datasetto exist. It fixes #3312.