Adding new rows to fits table with unsigned ints results in data change following write and readback #3921

Activity

1. embray

   added

   io.fits

   Bug

   Affects-release

   on Jul 7, 2015
2. embray commented on Jul 7, 2015

   

   embray

   on Jul 7, 2015

   Member

   More actions

   Thanks for the detailed report--this definitely looks like a bug. Probably somewhere in the from_columns step.

   Though this is one of those cases where I would again strongly, strongly urge anyone to use the Astropy Table interface where possible to do any kind of table manipulations, rather than trying to work with Numpy arrays like tables. For example this works:

   >>> # Unfortunately have to open the file with fits.open first since the uint=True parameter
   >>> # isn't accepted by Table.read.  It probably should be (though that will be more of a moot
   >>> # point once #3916 is merged)
   >>> hdul = fits.open('S.fits', uint=True)
   >>> table = Table.read(hdul)
   >>> for _ in range(9):
   ...     table.add_row(t[0])
   ...
   >>> table.write('S2.fits')
   >>> hdul2 = fits.open('S2.fits', uint=True)
   >>> hdul2[1].data
   FITS_rec([ (999999, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
          (0, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)], 
         dtype=[('OBT_TIME', '>i8'), ('UTC_TIME', 'S104'), ('MJD_TIME', '>f8'), ('HK_STAT_SID', '>i2'), ('HK_STAT_MODE', '>i2'), ('FixVal13', '>i2'), ('HK_STAT_FPM_POWER', '>f8'), ('HK_STAT_DAT_BUFFER_OVERFLOW', '>f8'), ('HK_STAT_SCU_MAIN_RED', '>f8'), ('HK_STAT_LAST_SPWLINK_ERROR', '>i2'), ('HK_STAT_LAST_ERROR_ID', '>i2'), ('HK_STAT_LAST_ERROR_FREQUENCY', '>i2'), ('HK_STAT_NUM_CMD_RECEIVED', '>i2'), ('HK_STAT_NUM_CMD_EXECUTED', '>i2'), ('HK_STAT_NUM_DATA_SENT', '>i2'), ('HK_STAT_SCU_PROC_DUTY_CYCLE', '>i2'), ('HK_STAT_SCU_NUM_AHB_ERROR', '>i2'), ('HK_STAT_SCU_NUM_AHB_CORRECTABLE_ERROR', '>i2'), ('HK_TEMP_SEM_SCU', '>i4'), ('HK_TEMP_SEM_PCU', '>i4')])
   >>> hdul2[1].columns
   ColDefs(
       name = 'OBT_TIME'; format = 'K'; unit = 'sec'; null = 999999
       name = 'UTC_TIME'; format = '104A'; unit = 'UTC'
       name = 'MJD_TIME'; format = 'D'; unit = 'day'
       name = 'HK_STAT_SID'; format = 'I'; null = 16959
       name = 'HK_STAT_MODE'; format = 'I'; null = 16959; bzero = 32768
       name = 'FixVal13'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_FPM_POWER'; format = 'D'
       name = 'HK_STAT_DAT_BUFFER_OVERFLOW'; format = 'D'
       name = 'HK_STAT_SCU_MAIN_RED'; format = 'D'
       name = 'HK_STAT_LAST_SPWLINK_ERROR'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_LAST_ERROR_ID'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_LAST_ERROR_FREQUENCY'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_NUM_CMD_RECEIVED'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_NUM_CMD_EXECUTED'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_NUM_DATA_SENT'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_SCU_PROC_DUTY_CYCLE'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_SCU_NUM_AHB_ERROR'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_STAT_SCU_NUM_AHB_CORRECTABLE_ERROR'; format = 'I'; null = 16959; bzero = 32768
       name = 'HK_TEMP_SEM_SCU'; format = 'J'; null = 999999
       name = 'HK_TEMP_SEM_PCU'; format = 'J'; null = 999999
   )

   (Incidentally, @taldcroft do you know if there is an open issue to add a Table.add_rows method or something of the like? Being able to add multiple new rows to a table at once would seem more efficient than calling add_row a bunch of times in a loop...)
3. taldcroft commented on Jul 7, 2015

   

   taldcroft

   on Jul 7, 2015

   Member

   More actions

   @embray - I think this came up once before. Right you can do:

   t = Table(...)  # the original table
   t_rows = Table(rows=new_rows, names=t.colnames)  # assuming a list of rows
   t = table.vstack([t, t_rows])
   

   This is the expected idiom for adding chunks of rows at a time.

   The issue with having an add_rows is that multiple rows effectively constitutes a new table. Trying to extend the existing add_row method could (in theory) be done by allowing the current vals and mask args to be lists, but I worry this could get messy in the code and the API. (I might be wrong, I haven't thought about it in detail).
4. taldcroft commented on Jul 7, 2015

   

   taldcroft

   on Jul 7, 2015

   Member

   More actions

   @embray - I think the FITS documentation should include your recommendation, in a big Note box right at the top in the Introduction, to use Table.read / write for most simple "I want to read / write a single-HDU bin table" cases. I know users who never realized this because they just go to the docs with the intent "I want to read a FITS file" and end up in the io.fits package docs.
5. embray commented on Jul 7, 2015

   

   embray

   on Jul 7, 2015

   Member

   More actions

   @taldcroft I agree--it's very much an FAQ, and should be in the FAQ along with a noticeable link in the docs. I should be getting some time soon to deal with the FITS table stuff once and for all, I think, too.
6. embray commented on Jul 7, 2015

   

   embray

   on Jul 7, 2015

   Member

   More actions

   Thanks also for the vstack idiom--that seems fine. It does seem to me that could be packaged into method that returns a new table (though I also don't see how adding one row is any different from adding multiple rows when it comes to creating a new table, if you want I'll take a look at what's involved)
7. embray commented on Mar 18, 2016

   

   embray

   on Mar 18, 2016

   Member

   More actions

   I'm not 100% sure but I think the bug reported here is the same: #3921

   Ultimately it has to do with unsigned integer data not being updated correctly when writing out a FITS table.
8. drdavella

   closed this as completedin #6956on Jan 24, 2018
9. saimn

   reopened this on Jan 24, 2018
10. saimn commented on Jan 24, 2018

    

    saimn

    on Jan 24, 2018

    Contributor

    More actions

    Reopening as #6956 does not fix this one.
11. pllim

    removed

    Affects-release

    on Mar 30, 2020
12. cmarmo commented on Dec 14, 2022

    

    cmarmo

    on Dec 14, 2022

    Last edited by cmarmo

    Member

    More actions

    Hello, the files used to investigate the issue are no longer available.
    I have reproduced the script above as follows

    from astropy.io import fits
    import numpy as np
    
    data = fits.FITS_rec(np.array([ (-1, '1970-01-01T00:00:00.000000', np.nan, 0, 0, 0, 0.0, 0.0, 0.0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)], 
          dtype=[('OBT_TIME', '>i8'), ('UTC_TIME', 'S26'), ('MJD_TIME', '>f8'), ('HK_STAT_SID', '>i2'), ('HK_STAT_MODE', '>i2'), ('FixVal13', '>i2'), ('HK_STAT_FPM_POWER', 'u1'), ('HK_STAT_DAT_BUFFER_OVERFLOW', 'u1'), ('HK_STAT_SCU_MAIN_RED', 'u1'), ('HK_STAT_LAST_SPWLINK_ERROR', '>i2'), ('HK_STAT_LAST_ERROR_ID', '>i2'), ('HK_STAT_LAST_ERROR_FREQUENCY', '>i2'), ('HK_STAT_NUM_CMD_RECEIVED', '>i2'), ('HK_STAT_NUM_CMD_EXECUTED', '>i2'), ('HK_STAT_NUM_DATA_SENT', '>i2'), ('HK_STAT_SCU_PROC_DUTY_CYCLE', '>i2'), ('HK_STAT_SCU_NUM_AHB_ERROR', '>i2'), ('HK_STAT_SCU_NUM_AHB_CORRECTABLE_ERROR', '>i2'), ('HK_TEMP_SEM_SCU', '>i4'), ('HK_TEMP_SEM_PCU', '>i4')]))
    
    H=fits.BinTableHDU.from_columns(data, nrows=1)
    H.writeto("S.fits", overwrite=True)
    
    H=fits.open("S.fits",uint=True)
    print(H[1].data)
    
    Z=fits.BinTableHDU.from_columns(data, nrows=10)
    for c in Z.data.names: Z.data[c][1:]=Z.data[c][0]
    print(Z.data)
    
    H[1]=Z
    H.writeto("S2.fits", overwrite=True)
    H.close()
    
    K=fits.open("S2.fits",uint=True)
    print(K[1].data)

    Running this script gives

    [(-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)]
    [(-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)]
    [(-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
     (-1, '1970-01-01T00:00:00.000000', nan, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)]
    

    Therefore, I'm under the impression that the bug has been fixed at some point.
13. cmarmo commented on Dec 14, 2022

    

    cmarmo

    on Dec 14, 2022

    Member

    More actions

    This is with

    >>> import astropy
    >>> astropy.__version__
    '5.3.dev24+gd9bdd0dd0'
14. saimn commented on Dec 16, 2022

    

    saimn

    on Dec 16, 2022

    Contributor

    More actions

    Thanks for checking this @cmarmo, closing!

    👍React with 👍1Reacted by Chiara Marmo
15. saimn

    closed this as completedon Dec 16, 2022