astropy
TableHDU.from_columns possible bug
Hi,
I think I found a bug in TableHDU.from_columns(). The documentation says that it will take a BinTableHDU as input, but when I try this I get the following error. This is Astropy 3.0 but it was also happening in the later dev version I was running ('3.1.dev21605'). It looks like the columns attribute is getting checked with __getatrr__() and isn't in the KEYWORD_ATTRIBUTES tuple.
<ipython-input-2-12e68d4c959a> in round_trip_ascii(filename, ext)
3 table = Table.read(filename, hdu=ext)
4 temp_table = fits.table_to_hdu(table)
----> 5 new_table = fits.TableHDU.from_columns(temp_table)
6 #hdu = fits.TableHDU.from_columns([col1, col2, col3])
7 fits_file[ext] = new_table
~/miniconda3/envs/irafdev/lib/python3.5/site-packages/astropy/io/fits/hdu/table.py in from_columns(cls, columns, header, nrows, fill, character_as_bytes, **kwargs)
126 coldefs = cls._columns_type(columns)
127 data = FITS_rec.from_columns(coldefs, nrows=nrows, fill=fill,
--> 128 character_as_bytes=character_as_bytes)
129 hdu = cls(data=data, header=header, character_as_bytes=character_as_bytes, **kwargs)
130 coldefs._add_listener(hdu)
~/miniconda3/envs/irafdev/lib/python3.5/site-packages/astropy/io/fits/fitsrec.py in from_columns(cls, columns, nrows, fill, character_as_bytes)
305
306 # read the delayed data
--> 307 for column in columns:
308 arr = column.array
309 if isinstance(arr, Delayed):
~/miniconda3/envs/irafdev/lib/python3.5/site-packages/astropy/io/fits/column.py in __getitem__(self, key)
1565 key = _get_index(self.names, key)
1566
-> 1567 x = self.columns[key]
1568 if _is_int(key):
1569 return x
~/miniconda3/envs/irafdev/lib/python3.5/site-packages/astropy/io/fits/column.py in __getattr__(self, name)
1506 attr.append(val if val is not None else '')
1507 return attr
-> 1508 raise AttributeError(name)
1509
1510 @lazyproperty
AttributeError: columns
Wait... is this just because it still wants me to pull out the columns from the BinTableHDU myself? If so, the doc strings could use some updating.
Just so someone who isn't a maintainer who might be interested to tackle this is able to, please provide a minimal example to reproduce the error, and also a link to the documentation in question. Thank you!
It looks like TableHDU.from_columns(hdu.columns) works, and BinTableHDU.from_columns(hdu) as well, so TableHDU.from_columns(hdu) should also be valid I guess. So I think it's a real bug, not a docstring issue.
sample code:
temp_table = fits.table_to_hdu(astropy_table)
new_ascii_HDU = fits.TableHDU.from_columns(temp_table)
API doc: http://docs.astropy.org/en/stable/io/fits/api/tables.html#astropy.io.fits.TableHDU.from_columns
sad trombone. Tried `TableHDU.from_columns(hdu.columns) as well on my sample file and I get a different error, thought I'd document that here as well.
table = Table.read(filename, hdu=ext)
temp_table = fits.table_to_hdu(table)
new_table = fits.TableHDU.from_columns(temp_table.columns)
<ipython-input-13-e104b5bac7bc> in round_trip_ascii(filename, ext)
4 temp_table = fits.table_to_hdu(table)
5 print(temp_table.columns)
----> 6 new_table = fits.TableHDU.from_columns(temp_table.columns)
7 #hdu = fits.TableHDU.from_columns([col1, col2, col3])
8 fits_file[ext] = new_table
~/miniconda3/envs/astropydev/lib/python3.6/site-packages/astropy-3.1.dev21606-py3.6-macosx-10.7-x86_64.egg/astropy/io/fits/hdu/table.py in from_columns(cls, columns, header, nrows, fill, character_as_bytes, **kwargs)
126 coldefs = cls._columns_type(columns)
127 data = FITS_rec.from_columns(coldefs, nrows=nrows, fill=fill,
--> 128 character_as_bytes=character_as_bytes)
129 hdu = cls(data=data, header=header, character_as_bytes=character_as_bytes, **kwargs)
130 coldefs._add_listener(hdu)
~/miniconda3/envs/astropydev/lib/python3.6/site-packages/astropy-3.1.dev21606-py3.6-macosx-10.7-x86_64.egg/astropy/io/fits/fitsrec.py in from_columns(cls, columns, nrows, fill, character_as_bytes)
471 # https://github.com/spacetelescope/PyFITS/issues/99
472 for idx in range(len(columns)):
--> 473 columns._arrays[idx] = data.field(idx)
474
475 return data
~/miniconda3/envs/astropydev/lib/python3.6/site-packages/astropy-3.1.dev21606-py3.6-macosx-10.7-x86_64.egg/astropy/io/fits/fitsrec.py in field(self, key)
707 # Handle all other column data types which are fixed-width
708 # fields
--> 709 converted = self._convert_other(column, field, recformat)
710
711 # Note: Never assign values directly into the self._converted dict;
~/miniconda3/envs/astropydev/lib/python3.6/site-packages/astropy-3.1.dev21606-py3.6-macosx-10.7-x86_64.egg/astropy/io/fits/fitsrec.py in _convert_other(self, column, field, recformat)
925 # TODO: This also needs to be fixed in the effort to make Columns
926 # responsible for scaling their arrays to/from FITS native values
--> 927 if not column.ascii and column.format.p_format:
928 format_code = column.format.p_format
929 else:
AttributeError: '_AsciiColumnFormat' object has no attribute 'p_format'
I've been digging into this but I'm currently stuck. Let me try and summarise my findings so far.
First, I've rewritten the MRE as 6 tests, 4 of which already pass on main, and the two failing ones show the bug from different perspective. Namely, test_from_hdu[TableHDU] exercises public API only, and test_core_issue[TableHDU] reproduces the exception by doing less, but using private API.
import pytest
from astropy.io import fits
@pytest.mark.parametrize("cls", [fits.BinTableHDU, fits.TableHDU])
class Test7330:
# see https://github.com/astropy/astropy/issues/7330
def test_from_columns(self, cls):
hdu = fits.BinTableHDU()
cls.from_columns(hdu.columns)
def test_from_hdu(self, cls):
hdu = fits.BinTableHDU()
cls.from_columns(hdu)
def test_core_issue(self, cls):
hdu = fits.BinTableHDU()
coldefs = cls._columns_type(hdu)
assert isinstance(coldefs, fits.column.ColDefs)
assert hasattr(coldefs, "columns")
Let's now focus on test_core_issue[TableHDU]. I note that TableHDU._columns_type is astropy.io.fits.column._AsciiColDefs. This class inherits from astropy.io.fits.column.ColDefs, which has an interesting __new__ method. In effect, this means that in some cases, one might call _AsciiColDefs(hdu) and get a ColDefs instance in return (not an instance of the child class _AsciiColDefs). Now what I do not understand at the moment is that, in the cases I'm studying neither _AsciiColDefs.__init__ nor ColDefs.__init__ appear to be called following ColDefs.__new__, so we seemingly get an uninitialised (and broken) ColDefs instance that, in particular, doesn't have columns attribute.
At the time of writing I have no clue why this happens, but I'm confident that this is key to resolving this bug.
It looks like TableHDU.from_columns(hdu.columns) works, and BinTableHDU.from_columns(hdu) as well, so TableHDU.from_columns(hdu) should also be valid I guess. So I think it's a real bug, not a docstring issue.
I also want to warmly thank @saimn for pointing this out, this was extremely helpful from an outsider's perspective !
