astropy / ccdproc

Astropy affiliated package for reducing optical/IR CCD data
https://ccdproc.readthedocs.io
BSD 3-Clause "New" or "Revised" License
89 stars 87 forks source link

FITS files not updated for ImageFileCollection.ccds(overwrite=True) #453

Closed mkelley closed 5 years ago

mkelley commented 7 years ago

Hello,

When using the CCDData generator and the overwrite=True option, changes to the returned object are not updated in the FITS file. However, the hdu generator does work as expected:

import numpy as np
from astropy.io import fits
import astropy.units as u
import ccdproc

# generate and save an empty image
im = np.zeros((10, 10))
h = fits.Header()
h['BUNIT'] = 'adu'
print('Original array:', im.mean())
fits.writeto('test.fits', im, h)

# load the image into an image collection, return the CCDData, add 1.0, and overwrite
ic = ccdproc.ImageFileCollection(location='.', filenames=['test.fits'])
for ccd in ic.ccds(overwrite=True):
    ccd += 1 * u.adu
    print('Updated CCDData:', ccd.mean())

print('Saved CCDData:', fits.getdata('test.fits').mean())

# Instead iterate over hdus, add 1.0, and overwrite
for hdu in ic.hdus(overwrite=True):
    hdu.data += 1
    print('Updated HDU:', hdu.data.mean())

print('Saved HDU:', fits.getdata('test.fits').mean())

Results:

$ python3 test.py
Original array: 0.0
Updated CCDData: 1.0 adu
Saved CCDData: 0.0
Updated HDU: 1.0
Saved HDU: 1.0

The saved CCDData mean is 0.0, but I expected 1.0.

astropy 2.0.dev17751, ccdproc 1.3.dev1066, and numpy 1.11.0.

Thanks for the help, Mike

MSeifert04 commented 7 years ago

Thank you for the report and especially for the standalone example. 👍 Very easy to verify and it made it pretty easy to debug.

Actually there are two issues here: One is that you are correct and the ccds-generator doesn't update the original. However the second problem is that ccd += 1 creates a numpy-array. So you didn't modify ccd but just created a new object.

I'll submit a fix for the first issue but the second one is impossible to fix without creating in-place arithmetic operations (like __iadd__).

mkelley commented 7 years ago

OK, now I get it, ccd.data += 1 would be the right call.

mwcraig commented 5 years ago

@mkelley -- is it ok to close this now?

crawfordsm commented 5 years ago

Closing this, but please re-open or create a new issue if something more is needed.