astropy.io.fits.table_to_hdu unexpectedly crashes on certain units

[weaverba137]

Consider the following snippet:

#!/usr/bin/env python
#
import warnings
import numpy as np

from astropy.io.fits import BinTableHDU, table_to_hdu
from astropy.table import Table, Column
from astropy.units import Unit
from astropy.utils.exceptions import AstropyUserWarning

#
# Create a table with a mix of units.
#
n = 20
table = Table()
table.add_column(Column(name='ID', length=n, dtype='i4',
                        data=np.zeros(n)-1))
table.add_column(Column(name='MAG', length=n, dtype='f4',
                        data=np.zeros(n)-1, unit='mag'))
table.add_column(Column(name='FLUX_G', length=n, dtype='f4',
                        unit='nanomaggies'))
table.add_column(Column(name='OIIFLUX', length=n, dtype='f4',
                        data=np.zeros(n)-1, unit='erg/(s*cm2)'))
table.add_column(Column(name='EWOII', length=n, dtype='f4',
                        data=np.zeros(n)-1, unit='Angstrom'))
table.add_column(Column(name='VDISP', length=n, dtype='f4',
                        data=np.zeros(n)-1, unit='km/s'))
table.add_column(Column(name='OIIIHBETA', length=n, dtype='f4',
                        data=np.zeros(n)-1, unit='dex'))
print(table)
#
# Below is a stripped-down version of:
#
#     hdu = table_to_hdu(table)
#
hdu = BinTableHDU.from_columns(np.array(table.filled()))
print(hdu.columns)
for col in hdu.columns:
    unit = table[col.name].unit
    print(col.name, unit)
    if unit is not None:
        try:
            col.unit = unit.to_string(format='fits')
        except ValueError:
            warnings.warn(
                    "The unit '{0}' could not be saved to FITS format".format(
                        unit.to_string()), AstropyUserWarning)

        # Try creating a Unit to issue a warning if the unit is not FITS compliant
        Unit(col.unit, format='fits', parse_strict='warn')

which outputs:

ID None
MAG mag
FLUX_G nanomaggies
WARNING: UnitsWarning: 'nanomaggies' did not parse as fits unit: At col 0, Unit 'nanomaggies' not supported by the FITS standard.  [astropy.units.core]
OIIFLUX erg / (cm2 s)
EWOII Angstrom
VDISP km / s
OIIIHBETA dex
WARNING: The unit 'dex' could not be saved to FITS format [__main__]
Traceback (most recent call last):
  File "test_units.py", line 47, in <module>
    Unit(col.unit, format='fits', parse_strict='warn')
  File "/home/weaver/Documents/local/products/anaconda/envs/astropy2/lib/python3.6/site-packages/astropy/units/core.py", line 1828, in __call__
    raise TypeError("None is not a valid Unit")
TypeError: None is not a valid Unit

So basically, the unit 'dex' is causing table_to_hdu to crash on Tables that contain that unit. I accept that 'dex' may not be compatible with the FITS standard, but it is rather commonly used by astronomers, so wouldn't it be better in this case if the call to Unit() were bypassed by putting the Unit() call in an else: block? That is:

    if unit is not None:
        try:
            col.unit = unit.to_string(format='fits')
        except ValueError:
            warnings.warn(
                    "The unit '{0}' could not be saved to FITS format".format(
                        unit.to_string()), AstropyUserWarning)
        else:
            # Try creating a Unit to issue a warning if the unit is not FITS compliant
            Unit(col.unit, format='fits', parse_strict='warn')

[weaverba137]

PS, there is another strange thing to note. 'nanomaggies' gets a warning as an invalid FITS unit, which is fine, it is a non-standard flux unit. However, 'nanomaggies' still gets added to the FITS binary table as the unit for that column! For example:

ColDefs(
    name = 'ID'; format = 'J'
    name = 'MAG'; format = 'E'; unit = 'mag'
    name = 'FLUX_G'; format = 'E'; unit = 'nanomaggies'
    name = 'OIIFLUX'; format = 'E'; unit = 'cm-2 erg s-1'
    name = 'EWOII'; format = 'E'; unit = 'Angstrom'
    name = 'VDISP'; format = 'E'; unit = 'km s-1'
    name = 'OIIIHBETA'; format = 'E'
)

This is after adding the else: block I suggested above.

So why is 'nanomaggies' allowed to pass through, but 'dex' is not?

[weaverba137]

OK, looks like all I need to do is replace 'dex' with 'Dex'. Still, that's kind of unexpected, and my question about 'nanomaggies' still applies.

[mhvk]

@weaverba137 - see #3822 for the long-standing issue related to this. A contribution would be most welcome, as clearly I am not getting to it myself...

[weaverba137]

I will take a look.

[mhvk]

closing in favour of #3822