Skip to content

Commit 40ca691

Browse files
pllimbsipocz
authored andcommitted
Merge pull request #6177 from DougBurke/bugfix-for-issue-6188
Remove ResourceWarnings when fits.open is given a non-FITS file (fix #6168)
1 parent 8791d45 commit 40ca691

4 files changed

Lines changed: 73 additions & 1 deletion

File tree

‎CHANGES.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,9 @@ Bug Fixes
418418

419419
- Use more sensible fix values for invalid NAXISj header values. [#5935]
420420

421+
- Close file on error to avoid creating a ``ResourceWarning`` warning
422+
about an unclosed file. [#6168, #6177]
423+
421424
- ``astropy.io.misc``
422425

423426
- ``astropy.io.registry``

‎astropy/io/fits/file.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ def __init__(self, fileobj=None, mode=None, memmap=None, overwrite=False,
9595
self.readonly = False
9696
self.writeonly = False
9797
self.simulateonly = True
98+
self.close_on_error = False
9899
return
99100
else:
100101
self.simulateonly = False
@@ -132,6 +133,10 @@ def __init__(self, fileobj=None, mode=None, memmap=None, overwrite=False,
132133
# Underlying fileobj is a file-like object, but an actual file object
133134
self.file_like = False
134135

136+
# Should the object be closed on error: see
137+
# https://github.com/astropy/astropy/issues/6168
138+
self.close_on_error = False
139+
135140
# More defaults to be adjusted below as necessary
136141
self.compression = None
137142
self.readonly = False
@@ -358,6 +363,7 @@ def close(self):
358363
self._mmap = None
359364

360365
self.closed = True
366+
self.close_on_error = False
361367

362368
def _maybe_close_mmap(self, refcount_delta=0):
363369
"""
@@ -497,6 +503,7 @@ def _open_filename(self, filename, mode, overwrite):
497503
self._file = bz2.BZ2File(self.name, bzip2_mode)
498504
else:
499505
self._file = fileobj_open(self.name, IO_FITS_MODES[mode])
506+
self.close_on_error = True
500507

501508
# Make certain we're back at the beginning of the file
502509
# BZ2File does not support seek when the file is open for writing, but

‎astropy/io/fits/hdu/hdulist.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,9 +1019,14 @@ def _readfrom(cls, fileobj=None, data=None, mode=None,
10191019

10201020
# Make sure at least the PRIMARY HDU can be read
10211021
read_one = hdulist._read_next_hdu()
1022+
10221023
# If we're trying to read only and no header units were found,
1023-
# raise and exception
1024+
# raise an exception
10241025
if not read_one and mode in ('readonly', 'denywrite'):
1026+
# Close the file if necessary (issue #6168)
1027+
if hdulist._file.close_on_error:
1028+
hdulist._file.close()
1029+
10251030
raise IOError('Empty or corrupt FITS file')
10261031

10271032
if not lazy_load_hdus:
@@ -1093,6 +1098,12 @@ def _read_next_hdu(self):
10931098
self._read_all = True
10941099
return False
10951100
except IOError:
1101+
# Close the file: see
1102+
# https://github.com/astropy/astropy/issues/6168
1103+
#
1104+
if self._file.close_on_error:
1105+
self._file.close()
1106+
10961107
if fileobj.writeonly:
10971108
self._read_all = True
10981109
return False

‎astropy/io/fits/tests/test_hdulist.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from ..verify import VerifyError
1313
from ....extern.six.moves import range
14+
from ....extern import six
1415
from ....io import fits
1516
from ....tests.helper import raises, catch_warnings, ignore_warnings
1617
from ....utils.exceptions import AstropyUserWarning, AstropyDeprecationWarning
@@ -913,3 +914,53 @@ def test_proper_error_raised_on_non_fits_file_with_unicode(self):
913914
# This should raise an IOError because there is no end card.
914915
with pytest.raises(IOError):
915916
fits.open(filename)
917+
918+
@pytest.mark.skipif(six.PY2,
919+
reason='ResourceWarning is not created in Python 2')
920+
def test_no_resource_warning_raised_on_non_fits_file(self):
921+
"""
922+
Regression test for https://github.com/astropy/astropy/issues/6168
923+
924+
The ResourceWarning shows up when (in python 3+) you try to
925+
open a non-FITS file when using a filename.
926+
"""
927+
928+
# To avoid creating the file multiple times the tests are
929+
# all included in one test file. See the discussion to the
930+
# PR at https://github.com/astropy/astropy/issues/6168
931+
#
932+
filename = self.temp('not-fits.fits')
933+
with open(filename, mode='w') as f:
934+
f.write('# header line\n')
935+
f.write('0.1 0.2\n')
936+
937+
# Opening the file should raise an OSError however the file
938+
# is opened (there are two distinct code paths, depending on
939+
# whether ignore_missing_end is True or False).
940+
#
941+
# Explicit tests are added to make sure the file handle is not
942+
# closed when passed in to fits.open. In this case the ResourceWarning
943+
# was not raised, but a check is still included.
944+
#
945+
with catch_warnings(ResourceWarning) as ws:
946+
947+
# Make sure that files opened by the user are not closed
948+
with open(filename, mode='rb') as f:
949+
with pytest.raises(OSError):
950+
fits.open(f, ignore_missing_end=False)
951+
952+
assert not f.closed
953+
954+
with open(filename, mode='rb') as f:
955+
with pytest.raises(OSError):
956+
fits.open(f, ignore_missing_end=True)
957+
958+
assert not f.closed
959+
960+
with pytest.raises(OSError):
961+
fits.open(filename, ignore_missing_end=False)
962+
963+
with pytest.raises(OSError):
964+
fits.open(filename, ignore_missing_end=True)
965+
966+
assert len(ws) == 0

0 commit comments

Comments
 (0)