Allow writing of all MaskedColumn to FITS. - #11194

p.s. Test failure is because it now requires yaml to run - they pass locally. Clearly, there should be one with and one without yaml.

I'm not sure this is a good route, for the reason you mentioned about int types.

The recommended route for masked float data is to use the data_mask serialization method. Is that not sufficient? Maybe there should be a warning for masked non-int data to tell the user to use data_mask?

In [3]: t                                                                                                                                         
Out[3]: 
<Table masked=True length=3>
  a      b     c  
int64 float64 str1
----- ------- ----
   --     1.0    c
    2     2.0   --
    3      --    e

In [4]: t.write('junk.fits', overwrite=True)                                                                                                      

In [5]: t2 = Table.read('junk.fits')                                                                                                              

In [6]: t2                                                                                                                                        
Out[6]: 
<Table masked=True length=3>
  a      b      c   
int64 float64 bytes1
----- ------- ------
   --     1.0      c
    2     2.0      N
    3     nan      e

In the example above it seems like the masked str values should go to '' per the FITS standard. The float is already doing the right thing and going to NaN, but it should become masked on construction.

Ah, if there is a standard, that is definitely the way to go! And better for my Masked class, since it does not carry a fill_value around...

As part of trying to look at where this might happen:

@taldcroft - looking more at the conversion of tables to FITS, could it be that something was missed in changing the meaning of table.masked? Am I right in thinking that in convenience.py, one should have changed table.masked to table.has_masked_values()? See

could it be that something was missed in changing the meaning of table.masked?

I think you are right. But there is still more that seems just broken, even when table.masked is True.

In [7]: t                                                                                                                             
Out[7]: 
<Table masked=True length=3>
  a      b     c  
int64 float64 str3
----- ------- ----
   --     1.0  abc
    2     2.0   --
    3      --  asd

In [8]: t.write('junk.fits', overwrite=True)                                                                                          

In [9]: t2 = Table.read('junk.fits')                                                                                                  

In [10]: t2                                                                                                                           
Out[10]: 
<Table masked=True length=3>
  a      b      c   
int64 float64 bytes3
----- ------- ------
   --     1.0    abc
    2     2.0    N/A
    3     nan    asd

Things that seem wrong:

• The nan in b should be converted to a masked value when reading.
• The masked value in c should be filled with "" for writing and then that "" should be masked for reading. (From the FITS standard for character data: "Null strings are defined by the presence of an ASCII NULL as the first character." In FITS parlance a null value is what we call a masked value.

This comment below completely confuses me. From the code above it seems that it can be reached only for int-type data, but the comment refers to character data.

So it looks like both on the writing and on the reading side there are some problems, with the writing one perhaps just the oversight of .masked. The comment I think is just another oversight, from a large pyfits update - see https://github.com/astropy/astropy/pull/1676/files#diff-2b263cbbb7d9b3215b1a7180dcd1b909c7ee9f78cf1e2e53834ee3b4d6b837b3

Need to see where the reading comes from...

Somehow I had thought that writing losslessly to FITS was working better than this. Note that meta['__attributes__'] (hence hidden columns) is also broken when writing to FITS. Maybe we need to just automatically fall through to using the FITS serialization machinery for values in meta that cannot be stored as a FITS keyword.

A less invasive solution to MaskedColumn not being properly stored is in #11222.

A separate question perhaps remains whether the fill value should be serialized, if only for the data_mask serialization.