You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@mcara wrote some WCS manipulation codes for me that used to work but are now broken with the latest dev version. Relevant snippets below.
# Shrink the imagedata=zoom(data, zoom_factor)
# Adjust WCSslice_factor=1/zoom_factorold_wcs=wcs.WCS(hdr) # To account for distortion, add "pf" as 2nd arg# Supress RuntimeWarning about ignoring cdelt because cd is present.withwarnings.catch_warnings():
warnings.simplefilter('ignore', RuntimeWarning)
# Update if croppingnew_wcs=old_wcs.slice((np.s_[::slice_factor], np.s_[::slice_factor]))
Traceback (most recent call last):
...
File ".../stginga/utils.py", line 349, in scale_image
(np.s_[::slice_factor], np.s_[::slice_factor]))
File ".../astropy-1.3.dev16480-py3.5-linux-x86_64.egg/astropy/wcs/wcs.py", line 2983, in slice
nitems = len(builtins.range(self._naxis[wcs_index])[iview])
TypeError: slice indices must be integers or None or have an __index__ method
More often than not, floating point indices are bugs/unintended. Numpy are even phasing those out and being strict about them. Why should we go in the opposite direction?
Yes, for my case, using int(1 / zoom_factor) seems to work. I cannot comment on zoom_factor > 1. I just use the code written by someone else as-is. I'm glad the fix for my case is simple. Thanks!
@astrofrog I think that's another case of WCS without image and WCS with image. If you zoom with interpolation the new points might represent floating point indices of the original array (not sure if scipy.ndimage.zoom qualifies) and the value of that index is calculated by interpolation. If you now slice the WCS with integers you'll truncate the real index and loose accuracy.
If you zoom without interpolation I totally agree and casting to integers (either round or truncate) is necessary!
@MSeifert04 - thanks for the clarification - let's discuss this further in #5455. I'm open to the idea of allowing floating point slicing but I think we need to have a separate class for WCS and WCS + NAXIS given the very different behaviors (see latest comment in #5455)
Activity
MSeifert04 commentedon Nov 29, 2016
Convince @nden and @astrofrog that #5455 should be merged. 😄
astrofrog commentedon Nov 29, 2016
Seriously though, what is wrong with simply requiring:
?
@pllim - does that solve your issue?
More often than not, floating point indices are bugs/unintended. Numpy are even phasing those out and being strict about them. Why should we go in the opposite direction?
MSeifert04 commentedon Nov 29, 2016
For any
zoom_factor >= 1theint(1 / zoom_factor)will be 0 and thus leave an "empty" wcs.astrofrog commentedon Nov 29, 2016
@MSeifert04 I agree but in that case, what does slicing with slice e.g.
slice(None, None, 0.3)mean?astrofrog commentedon Nov 29, 2016
(since @pllim wrote
# Shrink the imageI am assuming zoom_factor is < 1 in her use case)pllim commentedon Nov 29, 2016
Yes, for my case, using
int(1 / zoom_factor)seems to work. I cannot comment onzoom_factor > 1. I just use the code written by someone else as-is. I'm glad the fix for my case is simple. Thanks!pllim commentedon Nov 29, 2016
Although... it might not be accurate but should not be a problem for my use case.
MSeifert04 commentedon Nov 29, 2016
@astrofrog I think that's another case of WCS without image and WCS with image. If you zoom with interpolation the new points might represent floating point indices of the original array (not sure if
scipy.ndimage.zoomqualifies) and the value of that index is calculated by interpolation. If you now slice theWCSwith integers you'll truncate the real index and loose accuracy.If you zoom without interpolation I totally agree and casting to integers (either round or truncate) is necessary!
astrofrog commentedon Nov 29, 2016
@MSeifert04 - thanks for the clarification - let's discuss this further in #5455. I'm open to the idea of allowing floating point slicing but I think we need to have a separate class for WCS and WCS + NAXIS given the very different behaviors (see latest comment in #5455)
MSeifert04 commentedon Nov 29, 2016
I think #5455 should be merged (not necessarily as-is) just to remain backwards-compatible, so better to discuss this in #5461?