Time mix-in column does not round-trip when written to FITS

[astrofrog]

Description

When making a table with a mixin Time column and writing it to FITS, the time column is not restored as a Time object when reading the file back.

Expected behavior

The column should be read back in as a Time object, as is the case for SkyCoord.

How to Reproduce

In [21]: from astropy.table import Table

In [22]: t1 = Table()

In [23]: t1['time'] = Time([1, 2, 3], format='mjd')

In [24]: t1.write('test_time.fits')

In [25]: t2 = Table.read('test_time.fits')

In [26]: t2['time']
Out[26]: 
<Column name='time' dtype='float64' shape=(2,) unit='d' length=3>
2400002.0 .. -0.5
 2400002.0 .. 0.5
2400004.0 .. -0.5

Versions

Linux-6.5.0-28-generic-x86_64-with-glibc2.35
Python 3.11.9 (main, Apr  6 2024, 17:59:24) [GCC 11.4.0]
astropy 7.0.0.dev108+gd5bdb4859e
Numpy 1.26.4
pyerfa 2.0.1.4
Scipy 1.11.2
Matplotlib 3.7.3

cc @taldcroft

[taldcroft]

@astrofrog - you need to read back with astropy_native=True. See https://docs.astropy.org/en/stable/io/unified.html#writing-and-reading-astropy-time-columns.

[astrofrog]

Thanks! Shouldn't this kwarg apply to SkyCoord too?

[taldcroft]

IIRC we added the astropy_native largely because there is native support in the FITS standard for some time column formats, so there is ambiguity about the representation when reading a FITS file. See: https://docs.astropy.org/en/stable/io/fits/usage/table.html#fits-tables-with-time-columns. There is a lot of complexity here that @aaryapatil might recall better than me.

For SkyCoord there is no such ambiguity and it round-trips without astropy_native being set.

[astrofrog]

Ok thanks! I wonder if we should consider renaming astropy_native to astropy_native_time?

[taldcroft]

Ok thanks! I wonder if we should consider renaming astropy_native to astropy_native_time?

Well that ship has sailed and I'm not sure it is worth the churn to rename it. There is also the possibility that it might apply for some future mixin which is not Time, though that in itself is not a compelling argument.

[github-actions]

Hi humans 👋 - this issue was labeled as Close? approximately 13 hours ago. If you think this issue should not be closed, a maintainer should remove the Close? label - otherwise, I will close this issue in 7 days.

If you believe I commented on this issue incorrectly, please report this here

[astrofrog]

@taldcroft thinking about this more, if we are writing the file and storing mixin metadata about the time column it should be unambiguous though when reading back in right? I understand the ambiguity for files not produced by astropy of course.

[taldcroft]

It turns out that FITS has support for time such that in your case there is no mixin metadata. @aaryapatil's GSoC project involved a deep dive into the FITS specification and mapping Time into the existing spec where possible for maximum compatibility. This is generally a good thing but does mean that reading back as Time is opt-in.

In [3]: In [21]: from astropy.table import Table
   ...: 
   ...: In [22]: t1 = Table()
   ...: 
   ...: In [23]: t1['time'] = Time([1, 2, 3], format='mjd')
   ...: 
   ...: In [24]: t1.write('test_time.fits')

In [4]:                                                                                                                       
Do you really want to exit ([y]/n)? 
(astropy) ➜  astropy git:(main) ✗ fitsheader test_time.fits 
# HDU 0 in test_time.fits:
SIMPLE  =                    T / conforms to FITS standard                      
BITPIX  =                    8 / array data type                                
NAXIS   =                    0 / number of array dimensions                     
EXTEND  =                    T                                                  

# HDU 1 in test_time.fits:
XTENSION= 'BINTABLE'           / binary table extension                         
BITPIX  =                    8 / array data type                                
NAXIS   =                    2 / number of array dimensions                     
NAXIS1  =                   16 / length of dimension 1                          
NAXIS2  =                    3 / length of dimension 2                          
PCOUNT  =                    0 / number of group parameters                     
GCOUNT  =                    1 / number of groups                               
TFIELDS =                    1 / number of table fields                         
TIMESYS = 'UTC     '           / Default time scale                             
JDREF   =                  0.0 / Time columns are jd = jd1 + jd2                
TREFPOS = 'TOPOCENTER'         / Time reference position                        
TTYPE1  = 'time    '                                                            
TFORM1  = '2D      '                                                            
TUNIT1  = 'd       '                                                            
TDIM1   = '(2)     '                                                            
TCTYP1  = 'UTC     '                                                            
TCUNI1  = 'd       '                  

Would it help to make the default for astropy_native be a configuration variable? Then you could set it once in the code and forget it.

[taldcroft]

And yes, I believe that if there is astropy-specific mixin metadata in the FITS file then an astropy object is returned.

[github-actions]

I'm going to close this issue as per my previous message, but if you feel that this issue should stay open, then feel free to re-open and remove the Close? label.

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here