has_distortion needs to be clearly documented - #3112

Right - this really just means whether there are any distortion terms on top of the spherical projection. I agree the documentation should be improved.

PR attached.

👍

I guess it is my fault for not being very clear about my concerns in my initial post. The problem is that the name has_distortions suggests to me (without having any documentation) that by checking this attribute one could answer the question: "is the image distorted" (actually, "is WCS distorted" - not the actual pixels in the image).

From the SIP Convention for Representing FITS Image Distortions: "The CDi_j keywords encode skew as well as rotation and scaling. The CD-matrix values together with the higher-order distortion polynomials, as in Equations 1, 2, and 3, define a unique transformation from pixel coordinates to the plane-of-projection."

That is, the linear part of the SIP is encoded in the CD matrix and if there are no higher-order coefficients in the SIP you might have a situation in which wcs.sip is None but the image is "distorted" because, e.g., of the skew.

I was hoping (based on the name) that this function will check that really not distortions are present but found that it checks for that some specific attributes are None, which is OK as long as it is made clear that the image can still have linear distortions even though has_distortions returns False. Possibly other people may believe that has_distortions gives the definitive answer. Maybe has_distortion_attributes would be a more appropriate name for this specific implementation.

I apologize for not being clear in formulating this issue in the first place.

@astrofrog: Maybe you can comment as it appears you introduced the function.

I'd be fine with renaming the method if it helps clarify things. It was implemented to use in astropy/wcs/utils.py for the skycoord_to_pixel and reverse functions. The idea was that I think if cubes also had extra distortions beyond the spherical projection, this would cause issues because the celestial slices wouldn't all be nicely contained in a cube 'slice'. But I'd be happy for us to re-examing this, rename the method, etc.

@astrofrog "The idea was that I think if cubes also had extra distortions beyond the spherical projection, this would cause issues because the celestial slices wouldn't all be nicely contained in a cube 'slice'."

I do not know the exact issue that you are facing (e.g., what are these "cubes"), but it sounds that has_distortions in the current form may not be what you need (because linear distortions could be encoded into the CD matrix). In that case you need a more thorough check that would include checking the CD matrix. On the other hand, if all you need to know is whether higher than linear order distortions are present, then this function is OK.

Actually, in the current form, this function is useful to determine if higher than linear order distortions are present and could be used to replace the following long check in wcs._all_world2pix():

        # Check that an iterative solution is required at all
        # (when any of the non-CD-matrix-based corrections are
        # present). If not required return the initial
        # approximation (pix0).
        if self.sip is None and \
           self.cpdis1 is None and self.cpdis2 is None and \
           self.det2im1 is None and self.det2im2 is None:
            # No non-WCS corrections detected so
            # simply return initial approximation:
            return pix0

Still, I think the current function name is misleading but I am not very prolific with function names so I cannot suggest a good replacement.

@mcara - sorry, I should have been clearer regarding my comment on cubes. Imagine you have a spectral cube, so two celestial dimensions (the two first dimensions), and one spectral dimension. The question I want to answer is, are the celestial coordinates of each pixel in each spectral slice the same?. There are three things that matter here:

• If the transformation from pixel to world coordinates is simply an affine transformation, described by an arbitrary 3x3 CD matrix, then the spectral slices are 'independent' if and only if:

CD[2,0] == 0 and CD[2,1] == 0 and CD[0,2] == 0 and CD[1,2] == 0

that is, if there are no cross terms between dimensions 0/1 and dimensions 2. Of course, there could still be cross terms between dimensions 0 and 1 but that is fine. The point is that the celestial coordinates of pixels in different slices are the same from slice to slice.

• If the celestial transformation includes a spherical projection, then we are still fine because the curved coordinate lines are only curved in the spatial dimension.
• However, imagine you now have the case where the CD matrix is like an identity matrix, so all cross terms are 0. We can again forget about what the projection is in the celestial dimensions since this does not impact the behavior as a function of wavelength. But now say that we include SIP or other distortion terms - then it's possible to make it so that planes of constant spectral coordinate are not perfectly lined up with the spectral slices.

Now from my understanding of astropy.wcs (and @mdboom can correct me if I'm wrong), the issue of the cross terms is automatically taken into account when one does WCS.sub([WCSSUB_CELESTIAL]) in that it will crash if there are cross terms. However, we still need to catch the last case and check whether there are any additional distortions beyond the 'simple' behavior of a CD matrix plus a spherical projection in the celestial dimension. The purpose of has_distortions was to check if there are any additional distortions beyond the CD matrix + standard spherical projections.

Bear in mind that you can have non-linear transformations from the spherical projections (e.g. TAN, SIN, etc.) so the if statement only checks that there are not additional non-linear transformations on top of the already existing ones. Does this make more sense?

@astrofrog - Thank you very much for this detailed answer! Because you are interested in "...additional distortions beyond the 'simple' behavior of a CD matrix" it is clear that this function does what you need. As I said, I did not know the specifics of your particular problem...

@mcara - I agree. By the way, note that even with a CD matrix with no cross terms and without any SIP or other similar distortion terms, you can have non-linear mapping of pixel to world coordinates in the celestial plane (just in case it matters for your application).

@mdboom - can you think of a name for the method that would make its scope clearer?

Thinking about this more, I think I'd prefer to re-privatize it or tuck it away somewhere. Its usage is pretty specific to skycoord_to_pixel. As far as I can tell, it has never existed in a released version, so we can just silently move it back to astropy.wcs.utils.

Sounds good to me!

Great. Then I think once Travis ok's this, this is ready to merge.

Moving to 1.0 since I'm too lazy to move the changelog entry.