From e305456cc4d1331d8629b3a0b39019b2b29f481e Mon Sep 17 00:00:00 2001 From: System Administrator Date: Sat, 25 Mar 2017 13:41:17 +0530 Subject: [PATCH 01/12] preliminary fix for quantity columns to be written to fits --- astropy/table/table.py | 5 ++--- astropy/table/tests/test_mixin.py | 17 +++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/astropy/table/table.py b/astropy/table/table.py index a029bcd5852..0e71cd647a4 100644 --- a/astropy/table/table.py +++ b/astropy/table/table.py @@ -852,10 +852,9 @@ def __bytes__(self): @property def has_mixin_columns(self): """ - True if table has any mixin columns (defined as columns that are not Column - subclasses) + True if QTable has any mixin columns other than Quantity subclasses. """ - return any(has_info_class(col, MixinInfo) for col in self.columns.values()) + return any(not isinstance(col, (BaseColumn, Quantity)) for col in self.columns.values()) def _add_as_mixin_column(self, col): """ diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index 691d69549a1..bf59ec546f5 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -26,7 +26,7 @@ from ... import units as u from .. import table_helpers from .conftest import MIXIN_COLS - +from ...units import Quantity def test_attributes(mixin_cols): """ @@ -110,13 +110,14 @@ def test_io_write_fail(mixin_cols): this just confirms no exceptions. """ t = QTable(mixin_cols) - for fmt in ('fits', 'votable', 'hdf5'): - if fmt == 'hdf5' and not HAS_H5PY: - continue - out = StringIO() - with pytest.raises(ValueError) as err: - t.write(out, format=fmt) - assert 'cannot write table with mixin column(s)' in str(err.value) + if all(not isinstance(col, Quantity) for col in t.columns.values()): + for fmt in ('fits', 'votable', 'hdf5'): + if fmt == 'hdf5' and not HAS_H5PY: + continue + out = StringIO() + with pytest.raises(ValueError) as err: + t.write(out, format=fmt) + assert 'cannot write table with mixin column(s)' in str(err.value) def test_join(table_types): From 9924a94486ccb28b7b1b4130cbe6961a37af5a9d Mon Sep 17 00:00:00 2001 From: System Administrator Date: Sun, 26 Mar 2017 12:53:59 +0530 Subject: [PATCH 02/12] Requested changes done and votable changes for Quantity columns --- CHANGES.rst | 2 + astropy/io/fits/convenience.py | 15 ++++--- astropy/io/misc/hdf5.py | 14 ++++--- astropy/io/votable/connect.py | 15 ++++--- astropy/io/votable/tree.py | 65 +++++++++++++++++++------------ astropy/table/table.py | 18 ++++++++- astropy/table/tests/test_mixin.py | 42 +++++++++++++------- 7 files changed, 114 insertions(+), 57 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 484050f1e52..9ef6dad0ffc 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -62,6 +62,8 @@ New Features the string gets truncated. This can occur because numpy string arrays are fixed-width and silently drop characters which do not fit within the fixed width. [#5624] + - Added functionality to allow ``astropy.units.Quantity`` to be written + as a normal column to FITS, hdf5, and votable files. [#5910] - ``astropy.time`` diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index b538cc6aeb7..b773f88e132 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -72,11 +72,12 @@ from .fitsrec import FITS_rec from ...units import Unit from ...units.format.fits import UnitScaleError +from ...units import Quantity from ...extern import six from ...extern.six import string_types from ...utils.exceptions import AstropyUserWarning from ...utils.decorators import deprecated_renamed_argument - +from ...table.column import BaseColumn __all__ = ['getheader', 'getdata', 'getval', 'setval', 'delval', 'writeto', 'append', 'update', 'info', 'tabledump', 'tableload', @@ -460,12 +461,14 @@ def table_to_hdu(table): # Avoid circular imports from .connect import is_column_keyword, REMOVE_KEYWORDS - # Tables with mixin columns are not supported + # Not all tables with mixin columns are supported if table.has_mixin_columns: - mixin_names = [name for name, col in table.columns.items() - if not isinstance(col, table.ColumnClass)] - raise ValueError('cannot write table with mixin column(s) {0}' - .format(mixin_names)) + # Only those columns which are instances of BaseColumn or Quantity can be written + unsupported_cols = table.columns.not_isinstance(BaseColumn, Quantity) + if unsupported_cols: + unsupported_names = [col.info.name for col in unsupported_cols] + raise ValueError('cannot write table with mixin column(s) {0}' + .format(unsupported_names)) # Create a new HDU object if table.masked: diff --git a/astropy/io/misc/hdf5.py b/astropy/io/misc/hdf5.py index 80831950642..10f48c24710 100644 --- a/astropy/io/misc/hdf5.py +++ b/astropy/io/misc/hdf5.py @@ -15,6 +15,8 @@ from ...utils.exceptions import AstropyUserWarning from ...extern import six from ...table import meta +from ...units import Quantity +from ...table.column import BaseColumn HDF5_SIGNATURE = b'\x89HDF\r\n\x1a\n' META_KEY = '__table_column_meta__' @@ -194,12 +196,14 @@ def write_table_hdf5(table, output, path=None, compression=False, except ImportError: raise Exception("h5py is required to read and write HDF5 files") - # Tables with mixin columns are not supported + # Not all tables with mixin columns are supported if table.has_mixin_columns: - mixin_names = [name for name, col in table.columns.items() - if not isinstance(col, table.ColumnClass)] - raise ValueError('cannot write table with mixin column(s) {0} to HDF5' - .format(mixin_names)) + # Only those columns which are instances of BaseColumn or Quantity can be written + unsupported_cols = table.columns.not_isinstance(BaseColumn, Quantity) + if unsupported_cols: + unsupported_names = [col.info.name for col in unsupported_cols] + raise ValueError('cannot write table with mixin column(s) {0} to HDF5' + .format(unsupported_names)) if path is None: raise ValueError("table path should be set via the path= argument") diff --git a/astropy/io/votable/connect.py b/astropy/io/votable/connect.py index 633cd572e31..a42eaeca022 100644 --- a/astropy/io/votable/connect.py +++ b/astropy/io/votable/connect.py @@ -10,7 +10,8 @@ from .tree import VOTableFile, Table as VOTable from .. import registry as io_registry from ...table import Table - +from ...table.column import BaseColumn +from ...units import Quantity def is_votable(origin, filepath, fileobj, *args, **kwargs): """ @@ -139,12 +140,14 @@ def write_table_votable(input, output, table_id=None, overwrite=False, ``tabledata``. See :ref:`votable-serialization`. """ - # Tables with mixin columns are not supported + # Not all tables with mixin columns are supported if input.has_mixin_columns: - mixin_names = [name for name, col in input.columns.items() - if not isinstance(col, input.ColumnClass)] - raise ValueError('cannot write table with mixin column(s) {0} to VOTable' - .format(mixin_names)) + # Only those columns which are instances of BaseColumn or Quantity can be written + unsupported_cols = input.columns.not_isinstance(BaseColumn, Quantity) + if unsupported_cols: + unsupported_names = [col.info.name for col in unsupported_cols] + raise ValueError('cannot write table with mixin column(s) {0} to VOTable' + .format(unsupported_names)) # Check if output file already exists if isinstance(output, six.string_types) and os.path.exists(output): diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index 7183f03ff0a..63e81d509a8 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -25,7 +25,7 @@ from ...utils.xml.writer import XMLWriter from ...utils.exceptions import AstropyDeprecationWarning from ...utils.misc import InheritDocstrings - +from ...units import Quantity from . import converters from .exceptions import (warn_or_raise, vo_warn, vo_raise, vo_reraise, warn_unknown_attrs, W06, W07, W08, W09, W10, W11, W12, @@ -1115,22 +1115,25 @@ def to_table_column(self, column): column.meta['values'] = meta def from_table_column(self, column): - if 'values' not in column.meta: - return - - meta = column.meta['values'] - for key in ['ID', 'null']: - val = meta.get(key, None) - if val is not None: - setattr(self, key, val) - if 'min' in meta: - self.min = meta['min']['value'] - self.min_inclusive = meta['min']['inclusive'] - if 'max' in meta: - self.max = meta['max']['value'] - self.max_inclusive = meta['max']['inclusive'] - if 'options' in meta: - self._options = list(meta['options'].items()) + if isinstance(column, Quantity): + column = column.info + if column.meta is not None: + if 'values' not in column.meta: + return + + meta = column.meta['values'] + for key in ['ID', 'null']: + val = meta.get(key, None) + if val is not None: + setattr(self, key, val) + if 'min' in meta: + self.min = meta['min']['value'] + self.min_inclusive = meta['min']['inclusive'] + if 'max' in meta: + self.max = meta['max']['value'] + self.max_inclusive = meta['max']['inclusive'] + if 'options' in meta: + self._options = list(meta['options'].items()) class Field(SimpleElement, _IDProperty, _NameProperty, _XtypeProperty, @@ -1546,26 +1549,38 @@ def from_table_column(cls, votable, column): Restores a `Field` instance from a given `astropy.table.Column` instance. """ + Q_column = None + if isinstance(column, Quantity): + Q_column = column + column = column.info kwargs = {} for key in ['ucd', 'width', 'precision', 'utype', 'xtype']: - val = column.meta.get(key, None) - if val is not None: - kwargs[key] = val + if column.meta is not None: + val = column.meta.get(key, None) + if val is not None: + kwargs[key] = val # TODO: Use the unit framework when available if column.unit is not None: kwargs['unit'] = column.unit kwargs['name'] = column.name - result = converters.table_column_to_votable_datatype(column) + if Q_column is not None: + result = converters.table_column_to_votable_datatype(Q_column) + else: + result = converters.table_column_to_votable_datatype(column) kwargs.update(result) field = cls(votable, **kwargs) if column.description is not None: field.description = column.description - field.values.from_table_column(column) - if 'links' in column.meta: - for link in column.meta['links']: - field.links.append(Link.from_table_column(link)) + if Q_column is not None: + field.values.from_table_column(Q_column) + else: + field.values.from_table_column(column) + if column.meta is not None: + if 'links' in column.meta: + for link in column.meta['links']: + field.links.append(Link.from_table_column(link)) # TODO: Parse format into precision and width return field diff --git a/astropy/table/table.py b/astropy/table/table.py index 0e71cd647a4..f170b1d0474 100644 --- a/astropy/table/table.py +++ b/astropy/table/table.py @@ -150,6 +150,19 @@ def keys(self): def values(self): return list(OrderedDict.values(self)) + def isinstance(self, *cls): + ''' + Return a list of columns which are instances of the specified classes. + ''' + cols = [col for col in self.values() if isinstance(col, cls)] + return cols + + def not_isinstance(self, *cls): + ''' + Return a list of columns which are not instances of the specified classes. + ''' + cols = [col for col in self.values() if not isinstance(col, cls)] + return cols class Table(object): """A class to represent tables of heterogeneous data. @@ -852,9 +865,10 @@ def __bytes__(self): @property def has_mixin_columns(self): """ - True if QTable has any mixin columns other than Quantity subclasses. + True if table has any mixin columns (defined as columns that are not Column + subclasses) """ - return any(not isinstance(col, (BaseColumn, Quantity)) for col in self.columns.values()) + return any(has_info_class(col, MixinInfo) for col in self.columns.values()) def _add_as_mixin_column(self, col): """ diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index bf59ec546f5..bc2f5952914 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -27,6 +27,7 @@ from .. import table_helpers from .conftest import MIXIN_COLS from ...units import Quantity +from ..column import BaseColumn def test_attributes(mixin_cols): """ @@ -100,24 +101,39 @@ def test_io_ascii_write(): if fmt['Write'] and '.fast_' not in fmt['Format']: out = StringIO() t.write(out, format=fmt['Format']) - + +def test_io_Quantity_write(): + """ + Test that table with Quantity mixin column can be written by io.votable, + io.fits, and io.misc.hdf5. No validation of the output is done, + this just confirms no exceptions. + """ + t = QTable() + t['a'] = Quantity([1,2,4], unit='Angstrom') + for fmt in ('fits', 'votable', 'hdf5'): + if fmt == 'hdf5' and not HAS_H5PY: + continue + out = StringIO() + t.write(out, format=fmt) def test_io_write_fail(mixin_cols): """ - Test that table with mixin column cannot be written by io.votable, - io.fits, and io.misc.hdf5 - every pure Python writer. No validation of the output is done, - this just confirms no exceptions. + Test that table with mixin column (excluding Quantity) cannot be written by io.votable, + io.fits, and io.misc.hdf5. """ t = QTable(mixin_cols) - if all(not isinstance(col, Quantity) for col in t.columns.values()): - for fmt in ('fits', 'votable', 'hdf5'): - if fmt == 'hdf5' and not HAS_H5PY: - continue - out = StringIO() - with pytest.raises(ValueError) as err: - t.write(out, format=fmt) - assert 'cannot write table with mixin column(s)' in str(err.value) + #Only do this test if there are unsupported column types (i.e. anything besides + #BaseColumn or Quantity subclasses. + unsupported_cols = t.columns.not_isinstance(BaseColumn, Quantity) + if not unsupported_cols: + pytest.skip("no unsupported column types") + for fmt in ('fits', 'votable', 'hdf5'): + if fmt == 'hdf5' and not HAS_H5PY: + continue + out = StringIO() + with pytest.raises(ValueError) as err: + t.write(out, format=fmt) + assert 'cannot write table with mixin column(s)' in str(err.value) def test_join(table_types): From 1e9e9fe47927ca76db01ace27fc0764ef1bce562 Mon Sep 17 00:00:00 2001 From: System Administrator Date: Mon, 27 Mar 2017 19:49:26 +0530 Subject: [PATCH 03/12] Changes requested --- astropy/io/fits/convenience.py | 3 +- astropy/io/votable/tree.py | 72 +++++++++++++------------------ astropy/table/table.py | 30 ++++++++++--- astropy/table/tests/test_mixin.py | 8 +++- 4 files changed, 63 insertions(+), 50 deletions(-) diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index b773f88e132..929d6e44835 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -77,7 +77,6 @@ from ...extern.six import string_types from ...utils.exceptions import AstropyUserWarning from ...utils.decorators import deprecated_renamed_argument -from ...table.column import BaseColumn __all__ = ['getheader', 'getdata', 'getval', 'setval', 'delval', 'writeto', 'append', 'update', 'info', 'tabledump', 'tableload', @@ -463,6 +462,8 @@ def table_to_hdu(table): # Not all tables with mixin columns are supported if table.has_mixin_columns: + from ...table.column import BaseColumn + # Only those columns which are instances of BaseColumn or Quantity can be written unsupported_cols = table.columns.not_isinstance(BaseColumn, Quantity) if unsupported_cols: diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index 63e81d509a8..e166e6fd7ef 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -1115,25 +1115,22 @@ def to_table_column(self, column): column.meta['values'] = meta def from_table_column(self, column): - if isinstance(column, Quantity): - column = column.info - if column.meta is not None: - if 'values' not in column.meta: - return - - meta = column.meta['values'] - for key in ['ID', 'null']: - val = meta.get(key, None) - if val is not None: - setattr(self, key, val) - if 'min' in meta: - self.min = meta['min']['value'] - self.min_inclusive = meta['min']['inclusive'] - if 'max' in meta: - self.max = meta['max']['value'] - self.max_inclusive = meta['max']['inclusive'] - if 'options' in meta: - self._options = list(meta['options'].items()) + if column.info.meta is None or 'values' not in column.info.meta: + return + + meta = column.info.meta['values'] + for key in ['ID', 'null']: + val = meta.get(key, None) + if val is not None: + setattr(self, key, val) + if 'min' in meta: + self.min = meta['min']['value'] + self.min_inclusive = meta['min']['inclusive'] + if 'max' in meta: + self.max = meta['max']['value'] + self.max_inclusive = meta['max']['inclusive'] + if 'options' in meta: + self._options = list(meta['options'].items()) class Field(SimpleElement, _IDProperty, _NameProperty, _XtypeProperty, @@ -1549,37 +1546,26 @@ def from_table_column(cls, votable, column): Restores a `Field` instance from a given `astropy.table.Column` instance. """ - Q_column = None - if isinstance(column, Quantity): - Q_column = column - column = column.info kwargs = {} + meta = column.info.meta or {} for key in ['ucd', 'width', 'precision', 'utype', 'xtype']: - if column.meta is not None: - val = column.meta.get(key, None) - if val is not None: - kwargs[key] = val + val = meta.get(key, None) + if val is not None: + kwargs[key] = val # TODO: Use the unit framework when available - if column.unit is not None: - kwargs['unit'] = column.unit - kwargs['name'] = column.name - if Q_column is not None: - result = converters.table_column_to_votable_datatype(Q_column) - else: - result = converters.table_column_to_votable_datatype(column) + if column.info.unit is not None: + kwargs['unit'] = column.info.unit + kwargs['name'] = column.info.name + result = converters.table_column_to_votable_datatype(column) kwargs.update(result) field = cls(votable, **kwargs) - if column.description is not None: - field.description = column.description - if Q_column is not None: - field.values.from_table_column(Q_column) - else: - field.values.from_table_column(column) - if column.meta is not None: - if 'links' in column.meta: - for link in column.meta['links']: + if column.info.description is not None: + field.description = column.info.description + field.values.from_table_column(column) + if 'links' in meta: + for link in meta['links']: field.links.append(Link.from_table_column(link)) # TODO: Parse format into precision and width diff --git a/astropy/table/table.py b/astropy/table/table.py index f170b1d0474..1263b63b25c 100644 --- a/astropy/table/table.py +++ b/astropy/table/table.py @@ -151,16 +151,36 @@ def values(self): return list(OrderedDict.values(self)) def isinstance(self, *cls): - ''' + """ Return a list of columns which are instances of the specified classes. - ''' + + Parameters + ---------- + *cls : argument list of Column classes + List of Column (including mixin) classes. + + Returns + ------- + col_list : list of Columns + List of Column objects which are instances of given classes. + """ cols = [col for col in self.values() if isinstance(col, cls)] return cols def not_isinstance(self, *cls): - ''' + """ Return a list of columns which are not instances of the specified classes. - ''' + + Parameters + ---------- + *cls : argument list of Column classes + List of Column (including mixin) classes. + + Returns + ------- + col_list : list of Columns + List of Column objects which are not instances of given classes. + """ cols = [col for col in self.values() if not isinstance(col, cls)] return cols @@ -866,7 +886,7 @@ def __bytes__(self): def has_mixin_columns(self): """ True if table has any mixin columns (defined as columns that are not Column - subclasses) + subclasses). """ return any(has_info_class(col, MixinInfo) for col in self.columns.values()) diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index bc2f5952914..3a43962f8f4 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -29,6 +29,7 @@ from ...units import Quantity from ..column import BaseColumn + def test_attributes(mixin_cols): """ Required attributes for a column can be set. @@ -101,7 +102,8 @@ def test_io_ascii_write(): if fmt['Write'] and '.fast_' not in fmt['Format']: out = StringIO() t.write(out, format=fmt['Format']) - + + def test_io_Quantity_write(): """ Test that table with Quantity mixin column can be written by io.votable, @@ -114,8 +116,12 @@ def test_io_Quantity_write(): if fmt == 'hdf5' and not HAS_H5PY: continue out = StringIO() + if fmt == 'hdf5': + t.write(out, format=fmt, path='data') + continue t.write(out, format=fmt) + def test_io_write_fail(mixin_cols): """ Test that table with mixin column (excluding Quantity) cannot be written by io.votable, From b161978cb9bd71939916eb37a46d7885e88f06ea Mon Sep 17 00:00:00 2001 From: System Administrator Date: Mon, 27 Mar 2017 19:56:30 +0530 Subject: [PATCH 04/12] Few changes --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 9ef6dad0ffc..a596601529b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -63,7 +63,7 @@ New Features arrays are fixed-width and silently drop characters which do not fit within the fixed width. [#5624] - Added functionality to allow ``astropy.units.Quantity`` to be written - as a normal column to FITS, hdf5, and votable files. [#5910] + as a normal column to FITS, HDF5, and votable files. [#5910] - ``astropy.time`` From e307ad4cc0a88fe6d12481be1b4010f7ca989a92 Mon Sep 17 00:00:00 2001 From: System Administrator Date: Mon, 27 Mar 2017 20:03:23 +0530 Subject: [PATCH 05/12] Indentation fix --- astropy/io/fits/convenience.py | 1 + astropy/io/votable/tree.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index 929d6e44835..2052b4003e7 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -78,6 +78,7 @@ from ...utils.exceptions import AstropyUserWarning from ...utils.decorators import deprecated_renamed_argument + __all__ = ['getheader', 'getdata', 'getval', 'setval', 'delval', 'writeto', 'append', 'update', 'info', 'tabledump', 'tableload', 'table_to_hdu', 'printdiff'] diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index e166e6fd7ef..6253a4f2e86 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -1566,7 +1566,7 @@ def from_table_column(cls, votable, column): field.values.from_table_column(column) if 'links' in meta: for link in meta['links']: - field.links.append(Link.from_table_column(link)) + field.links.append(Link.from_table_column(link)) # TODO: Parse format into precision and width return field From 38cda35bd248818e2666bc83d8601b90b4753b2f Mon Sep 17 00:00:00 2001 From: System Administrator Date: Mon, 27 Mar 2017 20:07:29 +0530 Subject: [PATCH 06/12] Circular import comment --- astropy/io/fits/convenience.py | 1 + 1 file changed, 1 insertion(+) diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index 2052b4003e7..21287d0fb96 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -463,6 +463,7 @@ def table_to_hdu(table): # Not all tables with mixin columns are supported if table.has_mixin_columns: + #the import is done here, in order to avoid a circular import from ...table.column import BaseColumn # Only those columns which are instances of BaseColumn or Quantity can be written From d4804399061784c5fe14aaf639650e7cfc29a61c Mon Sep 17 00:00:00 2001 From: System Administrator Date: Tue, 28 Mar 2017 10:53:00 +0530 Subject: [PATCH 07/12] Test Changes and Import comment --- astropy/io/fits/convenience.py | 2 +- astropy/table/tests/test_mixin.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index 21287d0fb96..ddd8b1fe197 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -463,7 +463,7 @@ def table_to_hdu(table): # Not all tables with mixin columns are supported if table.has_mixin_columns: - #the import is done here, in order to avoid a circular import + #the import is done here, in order to avoid it at build time as erfa is not yet available then from ...table.column import BaseColumn # Only those columns which are instances of BaseColumn or Quantity can be written diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index 3a43962f8f4..db82ef90940 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -104,7 +104,7 @@ def test_io_ascii_write(): t.write(out, format=fmt['Format']) -def test_io_Quantity_write(): +def test_io_Quantity_write(tmpdir): """ Test that table with Quantity mixin column can be written by io.votable, io.fits, and io.misc.hdf5. No validation of the output is done, @@ -112,14 +112,20 @@ def test_io_Quantity_write(): """ t = QTable() t['a'] = Quantity([1,2,4], unit='Angstrom') + + filename = tmpdir.join("table-tmp").strpath + open(filename, 'w').close() + for fmt in ('fits', 'votable', 'hdf5'): if fmt == 'hdf5' and not HAS_H5PY: continue out = StringIO() if fmt == 'hdf5': - t.write(out, format=fmt, path='data') + t.write(filename, format=fmt, path='data', overwrite=True) continue - t.write(out, format=fmt) + t.write(filename, format=fmt, overwrite=True) + if fmt == 'fits': + assert QTable.read(filename, format=fmt)['a'].unit == 'Angstrom' def test_io_write_fail(mixin_cols): From 3a7af1e0a93797e48ca9d07a704e1bc3247a6d71 Mon Sep 17 00:00:00 2001 From: System Administrator Date: Tue, 28 Mar 2017 12:39:53 +0530 Subject: [PATCH 08/12] remove whitespace --- astropy/table/tests/test_mixin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index db82ef90940..eb8b9386bae 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -112,10 +112,10 @@ def test_io_Quantity_write(tmpdir): """ t = QTable() t['a'] = Quantity([1,2,4], unit='Angstrom') - + filename = tmpdir.join("table-tmp").strpath open(filename, 'w').close() - + for fmt in ('fits', 'votable', 'hdf5'): if fmt == 'hdf5' and not HAS_H5PY: continue From fda7b16172f3866aa8b970947e4b29dfd088e935 Mon Sep 17 00:00:00 2001 From: AustereCuriosity Date: Wed, 19 Apr 2017 23:27:37 +0530 Subject: [PATCH 09/12] Minor changes --- CHANGES.rst | 1 + astropy/io/votable/tree.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index a596601529b..1deb57784e9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -62,6 +62,7 @@ New Features the string gets truncated. This can occur because numpy string arrays are fixed-width and silently drop characters which do not fit within the fixed width. [#5624] + - Added functionality to allow ``astropy.units.Quantity`` to be written as a normal column to FITS, HDF5, and votable files. [#5910] diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index 6253a4f2e86..d6546f1b4a4 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -25,7 +25,6 @@ from ...utils.xml.writer import XMLWriter from ...utils.exceptions import AstropyDeprecationWarning from ...utils.misc import InheritDocstrings -from ...units import Quantity from . import converters from .exceptions import (warn_or_raise, vo_warn, vo_raise, vo_reraise, warn_unknown_attrs, W06, W07, W08, W09, W10, W11, W12, From 0fd7c063d6d0b1eecfcaeac62182e46be9e16742 Mon Sep 17 00:00:00 2001 From: AustereCuriosity Date: Thu, 20 Apr 2017 00:52:08 +0530 Subject: [PATCH 10/12] Added tests and a few changes --- astropy/io/fits/tests/test_connect.py | 7 ++++--- astropy/io/votable/tree.py | 1 + astropy/table/tests/test_mixin.py | 13 +++++++------ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/astropy/io/fits/tests/test_connect.py b/astropy/io/fits/tests/test_connect.py index 8cf78f6d670..3100d0f36b0 100644 --- a/astropy/io/fits/tests/test_connect.py +++ b/astropy/io/fits/tests/test_connect.py @@ -92,13 +92,14 @@ def test_simple_noextension(self, tmpdir): t2 = Table.read(filename) assert equal_data(t1, t2) - def test_with_units(self, tmpdir): + @pytest.mark.parametrize('table_type', (Table, QTable)) + def test_with_units(self, table_type, tmpdir): filename = str(tmpdir.join('test_with_units.fits')) - t1 = Table(self.data) + t1 = table_type(self.data) t1['a'].unit = u.m t1['c'].unit = u.km / u.s t1.write(filename, overwrite=True) - t2 = Table.read(filename) + t2 = table_type.read(filename) assert equal_data(t1, t2) assert t2['a'].unit == u.m assert t2['c'].unit == u.km / u.s diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index d6546f1b4a4..46bb6835317 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -25,6 +25,7 @@ from ...utils.xml.writer import XMLWriter from ...utils.exceptions import AstropyDeprecationWarning from ...utils.misc import InheritDocstrings + from . import converters from .exceptions import (warn_or_raise, vo_warn, vo_raise, vo_reraise, warn_unknown_attrs, W06, W07, W08, W09, W10, W11, W12, diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index eb8b9386bae..913492d7ccc 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -24,10 +24,9 @@ from ... import time from ... import coordinates from ... import units as u +from ..column import BaseColumn from .. import table_helpers from .conftest import MIXIN_COLS -from ...units import Quantity -from ..column import BaseColumn def test_attributes(mixin_cols): @@ -104,14 +103,14 @@ def test_io_ascii_write(): t.write(out, format=fmt['Format']) -def test_io_Quantity_write(tmpdir): +def test_io_quantity_write(tmpdir): """ Test that table with Quantity mixin column can be written by io.votable, io.fits, and io.misc.hdf5. No validation of the output is done, this just confirms no exceptions. """ t = QTable() - t['a'] = Quantity([1,2,4], unit='Angstrom') + t['a'] = u.Quantity([1,2,4], unit='Angstrom') filename = tmpdir.join("table-tmp").strpath open(filename, 'w').close() @@ -125,7 +124,9 @@ def test_io_Quantity_write(tmpdir): continue t.write(filename, format=fmt, overwrite=True) if fmt == 'fits': - assert QTable.read(filename, format=fmt)['a'].unit == 'Angstrom' + qt = QTable.read(filename, format=fmt) + assert isinstance(qt['a'], u.Quantity) + assert qt['a'].unit == 'Angstrom' def test_io_write_fail(mixin_cols): @@ -136,7 +137,7 @@ def test_io_write_fail(mixin_cols): t = QTable(mixin_cols) #Only do this test if there are unsupported column types (i.e. anything besides #BaseColumn or Quantity subclasses. - unsupported_cols = t.columns.not_isinstance(BaseColumn, Quantity) + unsupported_cols = t.columns.not_isinstance(BaseColumn, u.Quantity) if not unsupported_cols: pytest.skip("no unsupported column types") for fmt in ('fits', 'votable', 'hdf5'): From f7b42a964d51302b50cb25d1e1735705671b543a Mon Sep 17 00:00:00 2001 From: AustereCuriosity Date: Thu, 20 Apr 2017 01:10:19 +0530 Subject: [PATCH 11/12] Minor error --- astropy/io/fits/tests/test_connect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/astropy/io/fits/tests/test_connect.py b/astropy/io/fits/tests/test_connect.py index 3100d0f36b0..c808b6f63b3 100644 --- a/astropy/io/fits/tests/test_connect.py +++ b/astropy/io/fits/tests/test_connect.py @@ -10,7 +10,7 @@ from .... import units as u from ....extern.six.moves import range, zip -from ....table import Table +from ....table import Table, QTable from ....tests.helper import pytest, catch_warnings from ....units.format.fits import UnitScaleError From 9d914372300d8bda4c7426fe2bfdbfc74c9ee063 Mon Sep 17 00:00:00 2001 From: AustereCuriosity Date: Fri, 21 Apr 2017 20:15:08 +0530 Subject: [PATCH 12/12] Remove all changes for votable and HDF5 --- CHANGES.rst | 2 +- astropy/io/fits/convenience.py | 2 +- astropy/io/misc/hdf5.py | 14 +++++--------- astropy/io/votable/connect.py | 15 ++++++--------- astropy/io/votable/tree.py | 21 ++++++++++----------- astropy/table/table.py | 12 ++++++------ astropy/table/tests/test_mixin.py | 23 ++++++++++++----------- 7 files changed, 41 insertions(+), 48 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 1deb57784e9..c8f04911e73 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -64,7 +64,7 @@ New Features fit within the fixed width. [#5624] - Added functionality to allow ``astropy.units.Quantity`` to be written - as a normal column to FITS, HDF5, and votable files. [#5910] + as a normal column to FITS files. [#5910] - ``astropy.time`` diff --git a/astropy/io/fits/convenience.py b/astropy/io/fits/convenience.py index ddd8b1fe197..a59d7e4b2e0 100644 --- a/astropy/io/fits/convenience.py +++ b/astropy/io/fits/convenience.py @@ -467,7 +467,7 @@ def table_to_hdu(table): from ...table.column import BaseColumn # Only those columns which are instances of BaseColumn or Quantity can be written - unsupported_cols = table.columns.not_isinstance(BaseColumn, Quantity) + unsupported_cols = table.columns.not_isinstance((BaseColumn, Quantity)) if unsupported_cols: unsupported_names = [col.info.name for col in unsupported_cols] raise ValueError('cannot write table with mixin column(s) {0}' diff --git a/astropy/io/misc/hdf5.py b/astropy/io/misc/hdf5.py index 10f48c24710..80831950642 100644 --- a/astropy/io/misc/hdf5.py +++ b/astropy/io/misc/hdf5.py @@ -15,8 +15,6 @@ from ...utils.exceptions import AstropyUserWarning from ...extern import six from ...table import meta -from ...units import Quantity -from ...table.column import BaseColumn HDF5_SIGNATURE = b'\x89HDF\r\n\x1a\n' META_KEY = '__table_column_meta__' @@ -196,14 +194,12 @@ def write_table_hdf5(table, output, path=None, compression=False, except ImportError: raise Exception("h5py is required to read and write HDF5 files") - # Not all tables with mixin columns are supported + # Tables with mixin columns are not supported if table.has_mixin_columns: - # Only those columns which are instances of BaseColumn or Quantity can be written - unsupported_cols = table.columns.not_isinstance(BaseColumn, Quantity) - if unsupported_cols: - unsupported_names = [col.info.name for col in unsupported_cols] - raise ValueError('cannot write table with mixin column(s) {0} to HDF5' - .format(unsupported_names)) + mixin_names = [name for name, col in table.columns.items() + if not isinstance(col, table.ColumnClass)] + raise ValueError('cannot write table with mixin column(s) {0} to HDF5' + .format(mixin_names)) if path is None: raise ValueError("table path should be set via the path= argument") diff --git a/astropy/io/votable/connect.py b/astropy/io/votable/connect.py index a42eaeca022..633cd572e31 100644 --- a/astropy/io/votable/connect.py +++ b/astropy/io/votable/connect.py @@ -10,8 +10,7 @@ from .tree import VOTableFile, Table as VOTable from .. import registry as io_registry from ...table import Table -from ...table.column import BaseColumn -from ...units import Quantity + def is_votable(origin, filepath, fileobj, *args, **kwargs): """ @@ -140,14 +139,12 @@ def write_table_votable(input, output, table_id=None, overwrite=False, ``tabledata``. See :ref:`votable-serialization`. """ - # Not all tables with mixin columns are supported + # Tables with mixin columns are not supported if input.has_mixin_columns: - # Only those columns which are instances of BaseColumn or Quantity can be written - unsupported_cols = input.columns.not_isinstance(BaseColumn, Quantity) - if unsupported_cols: - unsupported_names = [col.info.name for col in unsupported_cols] - raise ValueError('cannot write table with mixin column(s) {0} to VOTable' - .format(unsupported_names)) + mixin_names = [name for name, col in input.columns.items() + if not isinstance(col, input.ColumnClass)] + raise ValueError('cannot write table with mixin column(s) {0} to VOTable' + .format(mixin_names)) # Check if output file already exists if isinstance(output, six.string_types) and os.path.exists(output): diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index 46bb6835317..7183f03ff0a 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -1115,10 +1115,10 @@ def to_table_column(self, column): column.meta['values'] = meta def from_table_column(self, column): - if column.info.meta is None or 'values' not in column.info.meta: + if 'values' not in column.meta: return - meta = column.info.meta['values'] + meta = column.meta['values'] for key in ['ID', 'null']: val = meta.get(key, None) if val is not None: @@ -1547,25 +1547,24 @@ def from_table_column(cls, votable, column): `astropy.table.Column` instance. """ kwargs = {} - meta = column.info.meta or {} for key in ['ucd', 'width', 'precision', 'utype', 'xtype']: - val = meta.get(key, None) + val = column.meta.get(key, None) if val is not None: kwargs[key] = val # TODO: Use the unit framework when available - if column.info.unit is not None: - kwargs['unit'] = column.info.unit - kwargs['name'] = column.info.name + if column.unit is not None: + kwargs['unit'] = column.unit + kwargs['name'] = column.name result = converters.table_column_to_votable_datatype(column) kwargs.update(result) field = cls(votable, **kwargs) - if column.info.description is not None: - field.description = column.info.description + if column.description is not None: + field.description = column.description field.values.from_table_column(column) - if 'links' in meta: - for link in meta['links']: + if 'links' in column.meta: + for link in column.meta['links']: field.links.append(Link.from_table_column(link)) # TODO: Parse format into precision and width diff --git a/astropy/table/table.py b/astropy/table/table.py index 1263b63b25c..1e47544d0cb 100644 --- a/astropy/table/table.py +++ b/astropy/table/table.py @@ -150,14 +150,14 @@ def keys(self): def values(self): return list(OrderedDict.values(self)) - def isinstance(self, *cls): + def isinstance(self, cls): """ Return a list of columns which are instances of the specified classes. Parameters ---------- - *cls : argument list of Column classes - List of Column (including mixin) classes. + cls : class or tuple of classes + Column class (including mixin) or tuple of Column classes. Returns ------- @@ -167,14 +167,14 @@ def isinstance(self, *cls): cols = [col for col in self.values() if isinstance(col, cls)] return cols - def not_isinstance(self, *cls): + def not_isinstance(self, cls): """ Return a list of columns which are not instances of the specified classes. Parameters ---------- - *cls : argument list of Column classes - List of Column (including mixin) classes. + cls : class or tuple of classes + Column class (including mixin) or tuple of Column classes. Returns ------- diff --git a/astropy/table/tests/test_mixin.py b/astropy/table/tests/test_mixin.py index 913492d7ccc..1214a8eed72 100644 --- a/astropy/table/tests/test_mixin.py +++ b/astropy/table/tests/test_mixin.py @@ -105,9 +105,10 @@ def test_io_ascii_write(): def test_io_quantity_write(tmpdir): """ - Test that table with Quantity mixin column can be written by io.votable, - io.fits, and io.misc.hdf5. No validation of the output is done, - this just confirms no exceptions. + Test that table with Quantity mixin column can be written by io.fits, + but not by io.votable and io.misc.hdf5. Validation of the output is done. + Test that io.fits writes a table containing Quantity mixin columns that can + be round-tripped (metadata unit). """ t = QTable() t['a'] = u.Quantity([1,2,4], unit='Angstrom') @@ -116,17 +117,17 @@ def test_io_quantity_write(tmpdir): open(filename, 'w').close() for fmt in ('fits', 'votable', 'hdf5'): - if fmt == 'hdf5' and not HAS_H5PY: - continue - out = StringIO() - if fmt == 'hdf5': - t.write(filename, format=fmt, path='data', overwrite=True) - continue - t.write(filename, format=fmt, overwrite=True) if fmt == 'fits': + t.write(filename, format=fmt, overwrite=True) qt = QTable.read(filename, format=fmt) assert isinstance(qt['a'], u.Quantity) assert qt['a'].unit == 'Angstrom' + continue + if fmt == 'hdf5' and not HAS_H5PY: + continue + with pytest.raises(ValueError) as err: + t.write(filename, format=fmt, overwrite=True) + assert 'cannot write table with mixin column(s)' in str(err.value) def test_io_write_fail(mixin_cols): @@ -137,7 +138,7 @@ def test_io_write_fail(mixin_cols): t = QTable(mixin_cols) #Only do this test if there are unsupported column types (i.e. anything besides #BaseColumn or Quantity subclasses. - unsupported_cols = t.columns.not_isinstance(BaseColumn, u.Quantity) + unsupported_cols = t.columns.not_isinstance((BaseColumn, u.Quantity)) if not unsupported_cols: pytest.skip("no unsupported column types") for fmt in ('fits', 'votable', 'hdf5'):