Fix #574 Do not allow AlignedDynamicTable as category for an AlignedD…

…ynamicTable

docs/gallery/plot_aligneddynamictable.py

@@ -185,9 +185,14 @@
 # Adding a category
 # ^^^^^^^^^^^^^^^^^
 #
-# To add a new :py:class:`~hdmf.common.table.DynamicTable` as a category, 
+# To add a new :py:class:`~hdmf.common.table.DynamicTable` as a category,
 # we use :py:func:`~hdmf.common.alignedtable.AlignedDynamicTable.add_category`.
 #
+# .. note::
+#    Only regular ``DynamicTables`` are allowed as category tables. Using
+#    an ``AlignedDynamicTable`` as a category for another  ``AlignedDynamicTable``
+#    is currently not supported.
+#
 
 # create a new category DynamicTable for the work address
 subcol1 = VectorData(

src/hdmf/common/alignedtable.py

@@ -25,14 +25,26 @@ class AlignedDynamicTable(DynamicTable):
 
     @docval(*get_docval(DynamicTable.__init__),
             {'name': 'category_tables', 'type': list,
-             'doc': 'List of DynamicTables to be added to the container', 'default': None},
+             'doc': 'List of DynamicTables to be added to the container. NOTE: Only regular '
+                    'DynamicTables are allowed. Using AlignedDynamicTable as a category for '
+                    'AlignedDynamicTable is currently not supported.', 'default': None},
             {'name': 'categories', 'type': 'array_data',
              'doc': 'List of names with the ordering of category tables', 'default': None})
-    def __init__(self, **kwargs):
+    def __init__(self, **kwargs):  # noqa: C901
         in_category_tables = popargs('category_tables', kwargs)
         in_categories = popargs('categories', kwargs)
+        if in_category_tables is not None:
+            # Error check to make sure that all category_table are regular DynamicTable
+            for i, v in enumerate(in_category_tables):
+                if not isinstance(v, DynamicTable):
+                    raise ValueError("Category table with index %i is not a DynamicTable" % i)
+                if isinstance(v, AlignedDynamicTable):
+                    raise ValueError("Category table with index %i is an AlignedDynamicTable. "
+                                     "Nesting of AlignedDynamicTable is currently not supported." % i)
+        # set in_categories from the in_category_tables if it is empy
         if in_categories is None and in_category_tables is not None:
             in_categories = [tab.name for tab in in_category_tables]
+        # check that if categories is given that we also have category_tables
         if in_categories is not None and in_category_tables is None:
             raise ValueError("Categories provided but no category_tables given")
         # at this point both in_categories and in_category_tables should either both be None or both be a list
@@ -119,14 +131,18 @@ def add_category(self, **kwargs):
         other category tables). I.e., if the AlignedDynamicTable is already populated with data
         then we have to populate the new category with the corresponding data before adding it.
 
-        :raises: ValueError is raised if the input table does not have the same number of rows as the main table
+        :raises: ValueError is raised if the input table does not have the same number of rows as the main table.
+                 ValueErrir is raised if the table is an AlignedDynamicTable instead of regular DynamicTable.
         """
         category = getargs('category', kwargs)
         if len(category) != len(self):
             raise ValueError('New category DynamicTable does not align, it has %i rows expected %i' %
                              (len(category), len(self)))
         if category.name in self.category_tables:
             raise ValueError("Category %s already in the table" % category.name)
+        if isinstance(category, AlignedDynamicTable):
+            raise ValueError("Category is an AlignedDynamicTable. Nesting of AlignedDynamicTable "
+                             "is currently not supported.")
         self.category_tables[category.name] = category
         category.parent = self
 

tests/unit/common/test_alignedtable.py

@@ -189,6 +189,26 @@ def test_init_with_duplicate_custom_categories(self):
                 description='Test aligned container',
                 category_tables=categories)
 
+    def test_init_with_bad_custom_categories(self):
+        """Test that we cannot provide a category that is not a DynamicTable"""
+        num_rows = 10
+        categories = [  # good category
+                      DynamicTable(name='test1',
+                                   description="test1 description",
+                                   columns=[VectorData(name='test1'+t,
+                                                       description='test1' + t + ' description',
+                                                       data=np.arange(num_rows)) for t in ['c1', 'c2', 'c3']]
+                                   ),
+                      # use a list as a bad category example
+                      [0, 1, 2]]
+        with self.assertRaises(ValueError) as ve:
+            AlignedDynamicTable(
+                name='test_aligned_table',
+                description='Test aligned container',
+                category_tables=categories)
+            self.assertEqual(str(ve.exception),
+                             "Category table with index 1 is not a DynamicTable")
+
     def test_round_trip_container(self):
         """Test read and write the container by itself"""
         category_names = ['test1', 'test2', 'test3']
@@ -447,3 +467,43 @@ def test_to_dataframe(self):
         tdf_cols = tdf.columns.tolist()
         for v in zip(expected_cols, tdf_cols):
             self.assertTupleEqual(v[0], v[1])
+
+    def test_nested_aligned_dynamic_table_not_allowed(self):
+        """
+        Test that using and AlignedDynamicTable as category for an AlignedDynamicTable is not allowed
+        """
+        # create an AlignedDynamicTable as category
+        subsubcol1 = VectorData(name='sub_sub_column1', description='test sub sub column', data=['test11', 'test12'])
+        sub_category = DynamicTable(name='sub_category1', description='test subcategory table', columns=[subsubcol1, ])
+        subcol1 = VectorData(name='sub_column1', description='test-subcolumn', data=['test1', 'test2'])
+        adt_category = AlignedDynamicTable(
+            name='category1',
+            description='test using AlignedDynamicTable as a category',
+            columns=[subcol1, ],
+            category_tables=[sub_category, ])
+
+        # Create a regular column for our main AlignedDynamicTable
+        col1 = VectorData(name='column1', description='regular test column', data=['test1', 'test2'])
+
+        # test 1: Make sure we can't add the AlignedDynamicTable category on init
+        with self.assertRaises(ValueError) as ve:
+            # create the nested AlignedDynamicTable with our adt_category as a sub-category
+            AlignedDynamicTable(
+                name='nested_adt',
+                description='test nesting AlignedDynamicTable',
+                columns=[col1, ],
+                category_tables=[adt_category, ])
+            self.assertEqual(str(ve.exception),
+                             "Category table with index %i is an AlignedDynamicTable. "
+                             "Nesting of AlignedDynamicTable is currently not supported." % 0)
+
+        # test 2: Make sure we can't add the AlignedDynamicTable category via add_category
+        adt = AlignedDynamicTable(
+            name='nested_adt',
+            description='test nesting AlignedDynamicTable',
+            columns=[col1, ])
+        with self.assertRaises(ValueError) as ve:
+            adt.add_category(adt_category)
+            self.assertEqual(str(ve.exception),
+                             "Category is an AlignedDynamicTable. Nesting of AlignedDynamicTable "
+                             "is currently not supported.")