WCS: Add square pixel convenience function
A very frequent use pattern is, e.g.:
pixscale = wcs.utils.proj_plane_pixel_scales(cube_hi.wcs.celestial)[0]
in which the user: (1) assumes the pixels are square (2) wants only the linear transformation from pixel edge size to degrees
This is a somewhat dangerous pattern because it will fail silently if the pixels are non-square or distorted.
The proposed convenience method does what I want - it gives you the one-dimensional pixel scale without exposing you to the risk of misinterpreting rectangular or distorted pixels.
I can add tests for this, but I want to propose the change first.
For a detailed discussion, please see #3113, #3230, and #3304. In general, it seems to me that when people want a single pixel scale value, everyone has their own need for that value. Square pixels are the only case when a single value actually exists and makes sense (uncontroversially; well, unless pixels are round or equilateral triangles).
The function proposed in this PR basically is an (improved) resurrection of the removed celestial_pixel_scale() function.
in which the user: (1) assumes the pixels are square This is a somewhat dangerous pattern because it will fail silently if the pixels are non-square or distorted.
Users should not make such assumptions especially that the proj_plane_pixel_scales() function returns multiple (generally distinct) values: this function was not designed to intentionally return a list of copies of the same number.
Personally, I do not see much use for this function. Most "raw" images are distorted and the function proposed in this PR would not apply to those. This would work on resampled, rectified, images. But since I know a priori what image type I am working on, I have no trouble extracting first value from existing proj_plane_pixel_scales()[0].
And so it seems to me that the main role of this function is to notify users that a single pixel scale value is not appropriate for their use.
Thanks for reminding me of our old conversation; we really went deep at that point.
In the seven years since, I have ended up with the pattern above in my code many, many times. I view the role of the code not as just telling users their pixels aren't square, but also of improving code readability.
Many of the use cases that I skimmed through rely on the pixels being square for the subsequent code to make sense. The "recommended" patterns:
pixscale = wcs.utils.proj_plane_pixel_scales(ww.celestial)[0]
pixscale = wcs.utils.proj_plane_pixel_scales(ww.celestial).mean()
both carry the risk of imposing a square assumption on a rectangular pixel. While you might know a priori what image type you are working on, much of the code that people have written can be applied generally - and will produce incorrect or misleading results in the case 'raw', distorted images are given to code written assuming pixels are square.
Providing this function gives developers a way to clearly indicate whether code is meant to work only for square pixels.
Many of the use cases that I skimmed through rely on the pixels being square for the subsequent code to make sense. The "recommended" patterns:
pixscale = wcs.utils.proj_plane_pixel_scales(ww.celestial)[0] pixscale = wcs.utils.proj_plane_pixel_scales(ww.celestial).mean()both carry the risk of imposing a square assumption on a rectangular pixel. While you might know a priori what image type you are working on, much of the code that people have written can be applied generally - and will produce incorrect or misleading results in the case 'raw', distorted images are given to code written assuming pixels are square.
Providing this function gives developers a way to clearly indicate whether code is meant to work only for square pixels.
First, I would not "recommend" those patterns for anything. Second, I feel that when people write code that relies on specific assumptions, it is their responsibility to check that those assumptions upon which their algorithm relies hold.
What you propose, it seems to me, is to add narrow functions valid only for very specific circumstances and put the burden of checking those circumstances on those specialized functions every time we have a "general" function. My personal feeling is against this approach but if there is a larger appeal for such a narrow function, .... maybe...
Hi humans :wave: - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then.
In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.
If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.
If you believe I commented on this pull request incorrectly, please report this here.
I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!
If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.