AstropyDeprecationWarning: The class "Fits" has been renamed to "FITS" in version 7.0

[pllim]

astropy/astropy#16455 (comment)

gwcs/gwcs/wcs.py

Line 2035 in 53025a9

cunit = frame.unit[fidx].get_format_name(u.format.Fits).upper()

@neutrinoceros expressed interest to submit a patch.

[eerovaher]

Quick summary

The correct code that would avoid the deprecation warnings would be

-                     cunit = frame.unit[fidx].get_format_name(u.format.Fits).upper()
+                     cunit = frame.unit[fidx].get_format_name("fits").upper()

Trying to use formatter classes instead of the format names will not work as intended for all units, as can be demonstrated below:

>>> from astropy import units as u
>>> u.voxel.get_format_name("fits")
'voxel'
>>> u.voxel.get_format_name(u.format.Fits)
'vox'

If any of the affected units (at least u.count, u.deg_C, u.voxel) are relevant to the code here then there is an actual bug that is independent of the exact name of the FITS unit formatter class in astropy.

Detailed explanation

Writing down the name of every possible unit in every possible format astropy supports would be onerous. NamedUnit.get_format_name() tries to look up the specified format from the dictionary of format-specific names associated with the unit, but returns a default name if the format is not in the dictionary. What is relevant to the code here is that get_format_name() makes no attempts to check if the requested format is valid. Formatter classes themselves are never valid format names, so using them always returns the default name. Compare correct code using format names

>>> from astropy import units as u
>>> u.deg.get_format_name("fits")
'deg'
>>> u.deg.get_format_name("latex")
'{}^{\\circ}'
>>> u.deg.get_format_name("unicode")
'°'

with incorrect code using the formatter classes directly

>>> u.deg.get_format_name(u.format.Fits)
'deg'
>>> u.deg.get_format_name(u.format.Latex)
'deg'
>>> u.deg.get_format_name(u.format.Unicode)
'deg'

Often enough the name as specified by the FITS standard is the same as the default name astropy uses, but not always. I am not sure if the affected units are relevant to the code in question. It would be best if someone more familiar with the code here would come up with a regression test, if such a test is possible.

[pllim]

I also wonder if this problem would propagate to the fields in ASDF data schema, etc. 😬

[neutrinoceros]

@neutrinoceros expressed interest to submit a patch.

Indeed I did, but I was thinking of simply shoe-horning an if minversion("astropy",...): / else: which would have retained 1-to-1 behavior, but would admitedly not have been the proper fix: The reason this regression got us by surprise is that gwcs apparently uses private astropy API, so I agree the correct thing to do is to use public API instead. However if the behavior isn't 100% identical, it's probably best left to someone more expert as suggested by @eerovaher.

[eerovaher]

I had a look at the the whole block

gwcs/gwcs/wcs.py

Lines 2034 to 2037 in 53025a9

	if hasattr(frame.unit[fidx], 'get_format_name'):
	cunit = frame.unit[fidx].get_format_name(u.format.Fits).upper()
	else:
	cunit = ''

Composite units (such as "mas / yr" or "km / s") are handled in the else-branch, but I can't tell if this is deliberate or is it just a workaround for the fact that they don't have a get_format_name() method. If the else-branch is indeed a clumsy workaround then the whole if-else block could be replaced with a single line:

- if hasattr(frame.unit[fidx], 'get_format_name'): 
-     cunit = frame.unit[fidx].get_format_name(u.format.Fits).upper() 
- else: 
-     cunit = ''
+ cunit = frame.unit[fidx].to_string("fits").upper()

Note also that differently from get_format_name(), to_string() does check that the requested format is a valid one. But if it is important that for composite units cunit is an empty string then this suggestion will obviously not work.