Skip to content
This repository has been archived by the owner on Dec 19, 2017. It is now read-only.

Bug assigning a mmap'd array to the data of another file #25

Closed
embray opened this issue Jul 9, 2013 · 2 comments
Closed

Bug assigning a mmap'd array to the data of another file #25

embray opened this issue Jul 9, 2013 · 2 comments
Assignees
Milestone

Comments

@embray
Copy link
Collaborator

embray commented Jul 9, 2013

This is a corner case I found with the help of Sara Ogaz.

If I have two FITS files A and B opened with memmap=True (the default) and A open in update mode, if I do something like A[0].data = B[0].data, and then A.flush() this should replace the data for A's first HDU with the data from B's first HDU.

But in fact that doesn't happen. Somewhere along the line A loses track of the fact that its data was replaced with a completely different array. When it gets to writing the data it actually just calls .flush() on B's data. Since B is open read-only this actually silently does nothing, and doesn't modify A at all.

This is a simple test that reproduces the bug:

import pyfits
import numpy as np

arr1 = np.arange(10)
arr2 = arr1 * 2

hdu1 = pyfits.PrimaryHDU(data=arr1)
hdu1.writeto('test1.fits', clobber=True)
hdu2 = pyfits.PrimaryHDU(data=arr2)
hdu2.writeto('test2.fits', clobber=True)

hdul1 = pyfits.open('test1.fits', mode='update', memmap=True)
hdul2 = pyfits.open('test2.fits', memmap=True)
hdul1[0].data = hdul2[0].data
hdul1.close()
hdul2.close()

hdul1 = pyfits.open('test1.fits')

assert np.all(hdul1[0].data == arr2)

This is actually a pretty serious bug because not only does it fail; it fails silently. So it should be fixed ASAP.

@ghost ghost assigned embray Jul 9, 2013
@embray
Copy link
Collaborator Author

embray commented Jul 9, 2013

This seems to be a problem for tables as well, so I will add a test case that addresses tables.

@embray
Copy link
Collaborator Author

embray commented Jul 11, 2013

This is fixed; meant to do a PR or something but I just sort of out of habit did the merge locally. See 66505e3...02783f3

@embray embray closed this as completed Jul 11, 2013
jwoillez pushed a commit to jwoillez/astropy that referenced this issue Oct 30, 2013
embray added a commit to embray/PyFITS that referenced this issue Nov 9, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant