diff --git a/src/hdmf/build/map.py b/src/hdmf/build/map.py index 9b93aeee7..a5104af7b 100644 --- a/src/hdmf/build/map.py +++ b/src/hdmf/build/map.py @@ -726,16 +726,15 @@ def get_attr_value(self, **kwargs): return None attr_val = self.__get_override_attr(attr_name, container, manager) if attr_val is None: - # TODO: A message like this should be used to warn users when an expected attribute - # does not exist on a Container object - # - # if not hasattr(container, attr_name): - # msg = "Container '%s' (%s) does not have attribute '%s'" \ - # % (container.name, type(container), attr_name) - # #warnings.warn(msg) - attr_val = getattr(container, attr_name, None) + try: + attr_val = getattr(container, attr_name) + except AttributeError: + # raise error if an expected attribute (based on the spec) does not exist on a Container object + msg = "Container '%s' (%s) does not have attribute '%s'" % (container.name, type(container), attr_name) + raise Exception(msg) if attr_val is not None: attr_val = self.__convert_value(attr_val, spec) + # else: attr_val is an attribute on the Container and its value is None return attr_val def __convert_value(self, value, spec): diff --git a/src/hdmf/common/table.py b/src/hdmf/common/table.py index 8278619cb..856d944dc 100644 --- a/src/hdmf/common/table.py +++ b/src/hdmf/common/table.py @@ -252,6 +252,9 @@ def __init__(self, **kwargs): col_dict = dict() self.__indices = dict() for col in self.columns: + if hasattr(self, col.name): + raise ValueError("Column name '%s' is not allowed because it is already an attribute" % col.name) + setattr(self, col.name, col) if isinstance(col, VectorData): existing = col_dict.get(col.name) # if we added this column using its index, ignore this column @@ -272,10 +275,16 @@ def __init__(self, **kwargs): self.__df_cols = [self.id] + [col_dict[name] for name in self.colnames] self.__colids = {name: i+1 for i, name in enumerate(self.colnames)} for col in self.__columns__: - if col.get('required', False) and col['name'] not in self.__colids: - self.add_column(col['name'], col['description'], - index=col.get('index', False), - table=col.get('table', False)) + if col['name'] not in self.__colids: + if col.get('required', False): + self.add_column(col['name'], col['description'], + index=col.get('index', False), + table=col.get('table', False)) + + else: # create column name attributes (set to None) on the object even if column is not required + setattr(self, col['name'], None) + if col.get('index', False): + setattr(self, col['name'] + '_index', None) @staticmethod def __build_columns(columns, df=None): @@ -402,6 +411,7 @@ def add_column(self, **kwargs): col = cls(**ckwargs) col.parent = self columns = [col] + setattr(self, name, col) # Add index if it's been specified if index is not False: @@ -421,6 +431,7 @@ def add_column(self, **kwargs): # else, the ObjectMapper will create a link from self (parent) to col_index (child with existing parent) col = col_index self.__indices[col_index.name] = col_index + setattr(self, col_index.name, col_index) if len(col) != len(self.id): raise ValueError("column must have the same number of rows as 'id'") diff --git a/tests/unit/build_tests/test_io_manager.py b/tests/unit/build_tests/test_io_manager.py index 04b9a8c16..4ccef76f4 100644 --- a/tests/unit/build_tests/test_io_manager.py +++ b/tests/unit/build_tests/test_io_manager.py @@ -217,6 +217,7 @@ def setUpBucketMapper(self): class BucketMapper(ObjectMapper): def __init__(self, spec): super(BucketMapper, self).__init__(spec) + self.unmap(spec.get_group('foo_holder')) self.map_spec('foos', spec.get_group('foo_holder').get_data_type('Foo')) return BucketMapper @@ -253,6 +254,8 @@ def setUpBucketMapper(self): class BucketMapper(ObjectMapper): def __init__(self, spec): super(BucketMapper, self).__init__(spec) + self.unmap(spec.get_group('foo_holder_holder')) + self.unmap(spec.get_group('foo_holder_holder').get_group('foo_holder')) self.map_spec('foos', spec.get_group('foo_holder_holder').get_group('foo_holder').get_data_type('Foo')) return BucketMapper diff --git a/tests/unit/build_tests/test_io_map.py b/tests/unit/build_tests/test_io_map.py index c25787392..ae839dd73 100644 --- a/tests/unit/build_tests/test_io_map.py +++ b/tests/unit/build_tests/test_io_map.py @@ -150,6 +150,13 @@ class MyMap(ObjectMapper): self.assertIsInstance(mapper, MyMap) +class BarMapper(ObjectMapper): + def __init__(self, spec): + super(BarMapper, self).__init__(spec) + data_spec = spec.get_dataset('data') + self.map_spec('attr2', data_spec.get_attribute('attr2')) + + class TestMapStrings(unittest.TestCase): def customSetUp(self, bar_spec): @@ -170,7 +177,7 @@ def test_build_1d(self): 'attr2', 'an example integer attribute', 'int')])], attributes=[AttributeSpec('attr1', 'an example string attribute', 'text')]) type_map = self.customSetUp(bar_spec) - type_map.register_map(Bar, ObjectMapper) + type_map.register_map(Bar, BarMapper) bar_inst = Bar('my_bar', ['a', 'b', 'c', 'd'], 'value1', 10) builder = type_map.build(bar_inst) self.assertEqual(builder.get('data').data, ['a', 'b', 'c', 'd']) @@ -183,7 +190,7 @@ def test_build_scalar(self): 'attr2', 'an example integer attribute', 'int')])], attributes=[AttributeSpec('attr1', 'an example string attribute', 'text')]) type_map = self.customSetUp(bar_spec) - type_map.register_map(Bar, ObjectMapper) + type_map.register_map(Bar, BarMapper) bar_inst = Bar('my_bar', ['a', 'b', 'c', 'd'], 'value1', 10) builder = type_map.build(bar_inst) self.assertEqual(builder.get('data').data, "['a', 'b', 'c', 'd']") @@ -196,7 +203,7 @@ def test_build_dataio(self): 'attr2', 'an example integer attribute', 'int')])], attributes=[AttributeSpec('attr1', 'an example string attribute', 'text')]) type_map = self.customSetUp(bar_spec) - type_map.register_map(Bar, ObjectMapper) + type_map.register_map(Bar, BarMapper) bar_inst = Bar('my_bar', H5DataIO(['a', 'b', 'c', 'd'], chunks=True), 'value1', 10) builder = type_map.build(bar_inst) self.assertIsInstance(builder.get('data').data, H5DataIO) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index 3e7c5a819..21eba1dce 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -59,6 +59,7 @@ def check_table(self, table): self.assertEqual(table.columns[1].data, [10.0, 20.0, 30.0, 40.0, 50.0]) self.assertEqual(table.columns[2].data, ['cat', 'dog', 'bird', 'fish', 'lizard']) self.assertEqual(table.id.data, [0, 1, 2, 3, 4]) + self.assertTrue(hasattr(table, 'baz')) def test_constructor_ids_default(self): columns = [VectorData(name=s['name'], description=s['description'], data=d) @@ -107,6 +108,7 @@ def test_add_column(self): table = self.with_spec() table.add_column(name='qux', description='qux column') self.assertEqual(table.colnames, ('foo', 'bar', 'baz', 'qux')) + self.assertTrue(hasattr(table, 'qux')) def test_getitem_row_num(self): table = self.with_spec() @@ -156,8 +158,7 @@ def test_pandas_roundtrip(self): table = DynamicTable.from_dataframe(df, 'foo') obtained = table.to_dataframe() - - assert df.equals(obtained) + self.assertTrue(df.equals(obtained)) def test_to_dataframe(self): table = self.with_columns_and_data() @@ -167,7 +168,7 @@ def test_to_dataframe(self): 'baz': ['cat', 'dog', 'bird', 'fish', 'lizard'] }) obtained_df = table.to_dataframe() - assert expected_df.equals(obtained_df) + self.assertTrue(expected_df.equals(obtained_df)) def test_from_dataframe(self): df = pd.DataFrame({ @@ -179,6 +180,17 @@ def test_from_dataframe(self): obtained_table = DynamicTable.from_dataframe(df, 'test') self.check_table(obtained_table) + def test_from_dataframe_dup_attr(self): + df = pd.DataFrame({ + 'foo': [1, 2, 3, 4, 5], + 'bar': [10.0, 20.0, 30.0, 40.0, 50.0], + 'description': ['cat', 'dog', 'bird', 'fish', 'lizard'] + }).loc[:, ('foo', 'bar', 'description')] + + msg = "Column name 'description' is not allowed because it is already an attribute" + with self.assertRaisesRegex(ValueError, msg): + DynamicTable.from_dataframe(df, 'test') + def test_missing_columns(self): table = self.with_spec() with self.assertRaises(ValueError): @@ -227,9 +239,9 @@ def test_dynamic_table_iteration(self): def test_nd_array_to_df(self): data = np.array([[1, 1, 1], [2, 2, 2], [3, 3, 3]]) - col = VectorData(name='name', description='desc', data=data) + col = VectorData(name='data', description='desc', data=data) df = DynamicTable('test', 'desc', np.arange(3, dtype='int'), (col, )).to_dataframe() - df2 = pd.DataFrame({'name': [x for x in data]}, + df2 = pd.DataFrame({'data': [x for x in data]}, index=pd.Index(name='id', data=[0, 1, 2])) pd.testing.assert_frame_equal(df, df2) diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index c3c0062ff..cd52436df 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -654,7 +654,9 @@ def __init__(self, spec): class BucketMapper(ObjectMapper): def __init__(self, spec): super(BucketMapper, self).__init__(spec) - foo_spec = spec.get_group('foo_holder').get_data_type('Foo') + foo_holder_spec = spec.get_group('foo_holder') + self.unmap(foo_holder_spec) + foo_spec = foo_holder_spec.get_data_type('Foo') self.map_spec('foos', foo_spec) file_spec = GroupSpec("A file of Foos contained in FooBuckets",