Raise error if expected attribute does not exist on a Container

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):

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'")

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

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)

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)
 

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",