Numerical Inverse needs to be a bit careful about units

[WilliamJamieson]

Recently, ndcube has started seeing failures using GWCS dev in one of its tests, in particular test_2d_skycoord_no_mesh. After investigating I have determined that this failure is due to the changes from #457 in combination with those from sunpy/ndcube#803.

What has happened is sunpy/ndcube#803 changed an x-fail to something checking a specific exception; namely, a UnitsError resulting from a conversion problem. However, it did this without determining why that exception was being raised. Ultimately, #457 changed the nature of this error, though happening in the same place, to a TypeError.

This has prompted me to do a more complete investigation into what is going on here. What I have found is that the original error (checked by sunpy/ndcube#803), is the result of a mismatch between the input argument units to the forward transform relative to what it was expecting; namely,dimensionless was passed, but pix was expected by the transform. This resulted in a UnitsError for conversion difficulties.

#457 altered how the forward transform was being evaluated enough so that the WCS object is now able to guess the units that need to be on the inputs to the forward transform and attempts to apply those units to the inputs (either adding or converting if no units are present); implemented by,

gwcs/gwcs/wcs.py

Lines 466 to 468 in 4e438c6

	input_is_quantity = any(isinstance(a, u.Quantity) for a in args)
	if not input_is_quantity and transform.uses_quantity:
	args = self._add_units_input(args, from_frame)

This results in a successful evaluation of the forward transform being successfully evaluated.

However, we still get a failure in numerical_inverse at

gwcs/gwcs/wcs.py

Line 1046 in 4e438c6

l1, phi1 = np.deg2rad(self.__call__(*(crpix - 0.5)))

This failure is now do to the fact that np.deg2rad cannot handle astropy quantities, rather than being unable to evaluate the transform. Indeed, it appears that the transform is in fact outputting values with degrees units which is what seems to be expected there.

Thus one solution is to be slightly more careful throughout numerical inverse and properly cast the units to the correct one (namely degrees) and then strip the quantity away from the value so that it can continue as expected. We should consider if this is the appropriate solution.

Also note, that while this eliminates any errors being raised as part of test_2d_skycoord_no_mesh it still will result in a test failure there because the numerical inverse appears to fail to converge in this case resulting in a NaN value rather than round tripping the value. This may indicate an issue with numerical_inverse, or it may indicate a simple issue with the problem posed in the test itself. Indeed, in reviewing the ndcube commit history indicates that the round trip aspect of this test was never passing.