Repository navigation
trim_image() does not correctly update WCS #281
Description
The trim_image() function returns a trimmed copy of the input CCDData object but does not correctly update the WCS. The CRPIX values appear to be modified accordingly but the NAXIS values are unchanged and consequently the trimmed image ends up with an invalid WCS, e.g.:
>>> test = ccdproc.CCDData.read(path, unit='adu')
>>> test.shape
(4268, 5215)
>>> test.wcs
WCS Keywords
Number of WCS axes: 2
CTYPE : 'RA---TAN' 'DEC--TAN'
CRVAL : 13.614671744880001 -37.589520634069999
CRPIX : 2608.0 2134.5
CD1_1 CD1_2 : -0.00079067726619539997 0.0
CD2_1 CD2_2 : 0.0 0.00079067726619539997
NAXIS : 5215 4268
>>> result = ccdproc.trim_image(test[100:500,250:850])
>>> result.shape
(400, 600)
>>> result.wcs
WCS Keywords
Number of WCS axes: 2
CTYPE : 'RA---TAN' 'DEC--TAN'
CRVAL : 13.614671744880001 -37.589520634069999
CRPIX : 2358.0 2034.5
CD1_1 CD1_2 : -0.00079067726619539997 0.0
CD2_1 CD2_2 : 0.0 0.00079067726619539997
NAXIS : 5215 4268
The documentation in https://github.com/astropy/ccdproc/blob/master/docs/ccdproc/ccddata.rst includes the statement "Either way, the wcs attribute is kept up to date if the ~ccdproc.ccddata.CCDData image is trimmed." however until this is fixed that's not quite true.
A look at the code suggests this is actually an upstream bug in astropy.wcs.
Activity
crawfordsm commentedon Jan 15, 2016
Thanks @AnthonyHorton for the report. The NAXIS should definitely be changed and the wcs updated (or at the very least a warning raised when trim is used with an object with wcs). I know there is a large pull request underway with
astropy.nddata, but if you have more details on what needs to change or can submit a PR (either here or at astropy.nddata), please go ahead.MSeifert04 commentedon Jan 15, 2016
I don't get how
trim_imagecan alter the WCS at all, since https://github.com/astropy/ccdproc/blob/master/ccdproc/core.py#L300 only slices data, mask and uncertainty. Even if it somehow tries to slice theCCDDatait would just forward the slicing to the attribute saved aswcs(https://github.com/astropy/astropy/blob/master/astropy/nddata/mixins/ndslicing.py#L32) so the problem probably resides insideastropy.wcs.WCS.AnthonyHorton commentedon Jan 15, 2016
I've done some poking around and it looks like the place to fix this is indeed
astropy.wcs.WCS.slice().ccdproc.trim_image()is relying on the slice functionality ofccdproc.CCDDatawhich is inherited fromastropy.nddata.NDDataArraywhich is inherited fromastropy.nddata.NDDatawhich relies on the individual classes of its attributes implementing their own slicing functionality which leads me toastropy.wcs.WCSand itsslice()method, which implements the effect on CRPIX correctly but not NAXIS.I'll have a go at fixing it up there and submit a PR.
MSeifert04 commentedon Jan 15, 2016
I just realized what happened.
@AnthonyHorton You have no
,between theCCDDataandsectionso you are usingtrim_imagejust as a mean of copy the image and adding a header keyword. Since the,is neglected in the documentation of the function (https://github.com/astropy/ccdproc/blob/master/ccdproc/core.py#L278) it seems like the preffered way but then what is the point of the part inside theif? https://github.com/astropy/ccdproc/blob/master/ccdproc/core.py#L300 (which does not even attempt to slice the wcs).AnthonyHorton commentedon Jan 15, 2016
@MSeifert04 As I understand it the optional second argument to
trim_imagesimply gives you the option of specifying the slice in FITS style (X before Y, 1 based) instead of numpy style (Y before X, 0 based). In my example I was just using numpy style slicing, hence no,. When used that way all the actual work is done by the slicing functionality inherited from elsewhere. That does try to trim the WCS, it's just doesn't do it properly.For the case where the second argument (fits_section) is given then some fixes would be needed in
ccdproc.trim_image()because, as you noted, the existing code only trims the data, mask and uncertainty and just copies the other attributes (unit, meta, wcs, flags). I can add slicing of the wcs attribute in here but for it to work properly it'll still need to be fixed inastropy.wcs.WCS.MSeifert04 commentedon Jan 15, 2016
@AnthonyHorton - No the bug be worse since inside the
ifthe function does not try to slice theWCSat all. Hence there theWCSwould not even have alteredCRPIXvalues.AnthonyHorton commentedon Jan 15, 2016
@MSeifert04 You're right, I hadn't looked at the
fits_sectionexecution path as I'd never used it. I'll add WCS slicing there as well as trying to get the implementation of WCS slicing fixed inastropy.wcs.WCS.MSeifert04 commentedon Jan 15, 2016
@AnthonyHorton - Probably it should be just a
trimmed = trimmed[python_slice]inside thefits_sectionexecution path instead of slicing all the seperate attributes. That way both letCCDData.__getdata__(or it's superclasses) take care of slicing and thus yield similar results.Nevertheless if you can fix and submit a PR for
astropy.wcs.WCSthat would be great - I could do a PR fortrim_image.mwcraig commentedon Feb 2, 2016
@AnthonyHorton @MSeifert04 -- was this closed by #282?
MSeifert04 commentedon Feb 2, 2016
@mwcraig No, unfortunatly the issue is still under discussion in astropy/astropy#4498.
MSeifert04 commentedon Oct 31, 2016
This should be fixed by astropy/astropy#5411.
@AnthonyHorton Can I close this issue?