Use NaN for missing values in FITS when using Table.write

[astrofrog]

At least, when no other fill value is provided. Currently, it defaults to 1.e20.

[ashishdastro]

I'll try to look into it

[embray]

👎 on using NaN as the default fill value for all tables. I don't think it's an appropriate value to use as a default for missing values. I think for now it should just be used for FITS since it's the only appropriate way to do this for FITS (and, as Mark Taylor pointed out, what the FITS standard prescribes).

[astrofrog]

Ok - sounds good. @ashishdastro - you should try and implement a fix for this in astropy/io/fits/connect.py for now.

[ashishdastro]

ok

[shaileshahuja]

Hi, I was trying this out, but fixing it this fails one of the tests in test_connect.py
Currently col.null value can only be set for integer values. If you set it for float, while reading the file, the reader checks and raises an exception.

[astrofrog]

@shaileshahuja - for float columns, you should not set col.null because FITS doesn't allow NULL to be specified for these columns. The correct approach is to simply check if the table column is masked, and if so, check if the fill value is 1e20, and if it is, then use the filled array but using NaN instead of 1e20. I can provide more specific pointers if needed.

[shaileshahuja]

The default fill value in numpy.ndarray (which the BaseColumn class extends) can only be 1e20 when dtype = float64. So if the column is integer type, then it will not be 1e20 anyway.

So I need to loop though the table columns, and call filled(np.nan) for each column if

1. it's a float type column, and
2. filled_value is 1e20

Then, I create the BinTableHDU with the filled table. Is that what you want?
col.null should be left untouched then?

[astrofrog]

@shaileshahuja - yes, that's exactly it! You might want to check what the behavior is for float32 as well.

[shaileshahuja]

For float32 default value is 1.0000000200408773e+20. It is not getting converted to Nan right now. Should I manually check for this as well?

[astrofrog]

@shaileshahuja - yes, that would be great!

[astrofrog]

Closed by #2196