Repository navigation
Table.read on FITS file triggers PyYAML import #7783
Description
In Gammapy some of our tests are failing now in the build where we check basic functionality without optional dependencies like PyYAML installed:
https://travis-ci.org/gammapy/gammapy/jobs/422455259#L2801
The problem is that Table.read on a FITS file will try to import yaml and throw an ImportError, even if I mention format="fits".
Minimal example that fails with Astropy master as of today( 9a99f5e ).
>>> from astropy.table import Table
>>> Table.read('/Users/deil/work/code/gammapy-extra/datasets/hess-dl3-dr1/obs-index.fits.gz', format='fits')
Traceback (most recent call last):
File "/Users/deil/work/code/astropy/astropy/table/meta.py", line 321, in get_header_from_yaml
import yaml
ModuleNotFoundError: No module named 'yaml'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/deil/work/code/astropy/astropy/table/table.py", line 2554, in read
out = io_registry.read(cls, *args, **kwargs)
File "/Users/deil/work/code/astropy/astropy/io/registry.py", line 517, in read
data = reader(*args, **kwargs)
File "/Users/deil/work/code/astropy/astropy/io/fits/connect.py", line 200, in read_table_fits
astropy_native=astropy_native)
File "/Users/deil/work/code/astropy/astropy/io/fits/connect.py", line 277, in read_table_fits
t = _decode_mixins(t)
File "/Users/deil/work/code/astropy/astropy/io/fits/connect.py", line 97, in _decode_mixins
info = meta.get_header_from_yaml(lines)
File "/Users/deil/work/code/astropy/astropy/table/meta.py", line 323, in get_header_from_yaml
raise ImportError('`import yaml` failed, PyYAML package '
ImportError: `import yaml` failed, PyYAML package is required for serializing mixin columns
I think given that PyYAML is an optional dependency, and FITS support is built into Astropy, this should be considered a bug?
cc @saimn @taldcroft
Activity
taldcroft commentedon Aug 30, 2018
Agreed this is a bug. The FITS file you are reading must have been written with serialized mixin columns and contain that metadata in the
COMMENTS. This is what triggers the attempt to read and decode that mixin metadata:astropy/astropy/io/fits/connect.py
Line 76 in 9a99f5e
It would be a trivial fix to put an
import yamlin thattryblock and also catchImportErrorat this point. The hardest part of the exercise would be writing the test. I guess you would need to create a new test FITS file with a serialized mixin (with yaml installed) and include that in the set of static test files, then do the test without yaml. Volunteers? 😄cdeil commentedon Aug 30, 2018
Oops, you are right. I got those by accident.
Just in case it's useful, the original file that caused the error is here:
https://github.com/gammapy/gammapy-extra/blob/e7a3c2db514fd7edea5c713c4fb6c19bb52539fb/datasets/hess-dl3-dr1/obs-index.fits.gz
https://github.com/gammapy/gammapy-extra/raw/e7a3c2db514fd7edea5c713c4fb6c19bb52539fb/datasets/hess-dl3-dr1/obs-index.fits.gz
taldcroft commentedon Aug 30, 2018
Nevertheless it shouldn't crash. I guess there is a question if a warning would be appropriate / helpful in that case.
saimn commentedon Aug 30, 2018
It is also possible to use mock to prevent the import :
saimn commentedon Aug 30, 2018
And I guess that this also concerns the other format that use the meta serialization (hdf5, ecsv) ?
astrofrog commentedon Sep 14, 2018
I haven't looked into this in detail, but why not simply modify
get_header_from_yamlso that it emits a warning that the header can't be read if PyYAML can't be imported? (rather than crash)Avoid crash when table contains mixin info and yaml is missing