Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FITS_rec._scale_back(): Incorrectly checks heapsize for TFORMn='Q' #14810

Merged
merged 1 commit into from May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion astropy/io/fits/fitsrec.py
Expand Up @@ -1130,7 +1130,7 @@ def _scale_back(self, update_heap_pointers=True):
# Even if this VLA has not been read or updated, we need to
# include the size of its constituent arrays in the heap size
# total
if heapsize >= 2**31:
if type(recformat) == _FormatP and heapsize >= 2**31:
raise ValueError(
"The heapsize limit for 'P' format has been reached. "
"Please consider using the 'Q' format for your file."
Expand Down
23 changes: 23 additions & 0 deletions astropy/io/fits/tests/test_table.py
Expand Up @@ -3230,6 +3230,29 @@
):
t.writeto(self.temp("matrix.fits"))

@pytest.mark.skipif(sys.maxsize < 2**32, reason="requires 64-bit system")
@pytest.mark.skipif(sys.platform == "win32", reason="Cannot test on Windows")
@pytest.mark.hugemem
def test_heapsize_Q_limit(self):
"""
Regression test for https://github.com/astropy/astropy/issues/14808

Check if the error is no longer raised when the heap size is bigger than what can be
indexed with a 32 bit signed int.
"""

# a matrix with variable length array elements is created
nelem = 2**28
matrix = np.zeros(1, dtype=np.object_)
matrix[0] = np.arange(0.0, float(nelem + 1))

Check warning on line 3247 in astropy/io/fits/tests/test_table.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/fits/tests/test_table.py#L3245-L3247

Added lines #L3245 - L3247 were not covered by tests

col = fits.Column(name="MATRIX", format=f"QD({nelem})", unit="", array=matrix)

Check warning on line 3249 in astropy/io/fits/tests/test_table.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/fits/tests/test_table.py#L3249

Added line #L3249 was not covered by tests

t = fits.BinTableHDU.from_columns([col])
t.name = "MATRIX"

Check warning on line 3252 in astropy/io/fits/tests/test_table.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/fits/tests/test_table.py#L3251-L3252

Added lines #L3251 - L3252 were not covered by tests

t.writeto(self.temp("matrix.fits"))

Check warning on line 3254 in astropy/io/fits/tests/test_table.py

View check run for this annotation

Codecov / codecov/patch

astropy/io/fits/tests/test_table.py#L3254

Added line #L3254 was not covered by tests

def test_empty_vla_raw_data(self):
"""
Regression test for https://github.com/astropy/astropy/issues/12881
Expand Down
2 changes: 2 additions & 0 deletions docs/changes/io.fits/14810.bugfix.rst
@@ -0,0 +1,2 @@
Fixes an issue where FITS_rec was incorrectly raising a ValueError exception when the heapsize was greater than 2**31
when the Column type was 'Q' instead of 'P'.