Add target_tables argument to DynamicTable.__init__ (#971)

Co-authored-by: Ryan Ly <rly@lbl.gov>
Co-authored-by: Oliver Ruebel <oruebel@users.noreply.github.com>

CHANGELOG.md

@@ -1,5 +1,11 @@
 # HDMF Changelog
 
+## HDMF 3.11.0 (Upcoming)
+
+### Enhancements
+- Added `target_tables` attribute to `DynamicTable` to allow users to specify the target table of any predefined
+`DynamicTableRegion` columns of a `DynamicTable` subclass. @rly [#971](https://github.com/hdmf-dev/hdmf/pull/971)
+
 ## HDMF 3.10.0 (October 3, 2023)
 
 Since version 3.9.1 should have been released as 3.10.0 but failed to release on PyPI and conda-forge, this release

docs/gallery/plot_dynamictable_howto.py

@@ -318,6 +318,41 @@
     columns=[dtr_idx, indexed_dtr_col],
 )
 
+###############################################################################
+# Setting the target table of a DynamicTableRegion column of a DynamicTable
+# -------------------------------------------------------------------------
+# A subclass of DynamicTable might have a pre-defined DynamicTableRegion column.
+# To write this column correctly, the "table" attribute of the column must be set so
+# that users know to what table the row index values reference. Because the target
+# table could be any table, the "table" attribute must be set explicitly. There are three
+# ways to do so. First, you can use the ``target_tables`` argument of the
+# DynamicTable constructor as shown below. This argument
+# is a dictionary mapping the name of the DynamicTableRegion column to
+# the target table. Secondly, the target table can be set after the DynamicTable
+# has been initialized using ``my_table.my_column.table = other_table``. Finally,
+# you can create the DynamicTableRegion column and pass the ``table``
+# attribute to `DynamicTableRegion.__init__` and then pass the column to
+# `DynamicTable.__init__` using the `columns` argument. However, this approach
+# is not recommended for columns defined in the schema, because it is up to
+# the user to ensure that the column is created in accordance with the schema.
+
+class SubTable(DynamicTable):
+    __columns__ = (
+        {'name': 'dtr', 'description': 'required region', 'required': True, 'table': True},
+    )
+
+referenced_table = DynamicTable(
+    name='referenced_table',
+    description='an example table',
+)
+
+sub_table = SubTable(
+    name='sub_table',
+    description='an example table',
+    target_tables={'dtr': referenced_table},
+)
+# now the target table of the DynamicTableRegion column 'dtr' is set to `referenced_table`
+
 ###############################################################################
 # Creating an expandable table
 # ----------------------------

src/hdmf/common/io/table.py

@@ -2,7 +2,7 @@
 from ..table import DynamicTable, VectorData, VectorIndex, DynamicTableRegion
 from ...build import ObjectMapper, BuildManager, CustomClassGenerator
 from ...spec import Spec
-from ...utils import docval, getargs, popargs, AllowPositional
+from ...utils import docval, getargs
 
 
 @register_map(DynamicTable)
@@ -111,55 +111,3 @@ def post_process(cls, classdict, bases, docval_args, spec):
         columns = classdict.get('__columns__')
         if columns is not None:
             classdict['__columns__'] = tuple(columns)
-
-    @classmethod
-    def set_init(cls, classdict, bases, docval_args, not_inherited_fields, name):
-        if '__columns__' not in classdict:
-            return
-
-        base_init = classdict.get('__init__')
-        if base_init is None:  # pragma: no cover
-            raise ValueError("Generated class dictionary is missing base __init__ method.")
-
-        # add a specialized docval arg for __init__ for specifying targets for DTRs
-        docval_args_local = docval_args.copy()
-        target_tables_dvarg = dict(
-            name='target_tables',
-            doc=('dict mapping DynamicTableRegion column name to the table that the DTR points to. The column is '
-                 'added to the table if it is not already present (i.e., when it is optional).'),
-            type=dict,
-            default=None
-        )
-        cls._add_to_docval_args(docval_args_local, target_tables_dvarg, err_if_present=True)
-
-        @docval(*docval_args_local, allow_positional=AllowPositional.WARNING)
-        def __init__(self, **kwargs):
-            target_tables = popargs('target_tables', kwargs)
-            base_init(self, **kwargs)
-
-            # set target attribute on DTR
-            if target_tables:
-                for colname, table in target_tables.items():
-                    if colname not in self:  # column has not yet been added (it is optional)
-                        column_conf = None
-                        for conf in self.__columns__:
-                            if conf['name'] == colname:
-                                column_conf = conf
-                                break
-                        if column_conf is None:
-                            raise ValueError("'%s' is not the name of a predefined column of table %s."
-                                             % (colname, self))
-                        if not column_conf.get('table', False):
-                            raise ValueError("Column '%s' must be a DynamicTableRegion to have a target table."
-                                             % colname)
-                        self.add_column(name=column_conf['name'],
-                                        description=column_conf['description'],
-                                        index=column_conf.get('index', False),
-                                        table=True)
-                    if isinstance(self[colname], VectorIndex):
-                        col = self[colname].target
-                    else:
-                        col = self[colname]
-                    col.table = table
-
-        classdict['__init__'] = __init__

src/hdmf/common/table.py

@@ -292,9 +292,15 @@ def __gather_columns(cls, name, bases, classdict):
             {'name': 'colnames', 'type': 'array_data',
              'doc': 'the ordered names of the columns in this table. columns must also be provided.',
              'default': None},
+            {'name': 'target_tables',
+             'doc': ('dict mapping DynamicTableRegion column name to the table that the DTR points to. The column is '
+                     'added to the table if it is not already present (i.e., when it is optional).'),
+             'type': dict,
+             'default': None},
             allow_positional=AllowPositional.WARNING)
     def __init__(self, **kwargs):  # noqa: C901
         id, columns, desc, colnames = popargs('id', 'columns', 'description', 'colnames', kwargs)
+        target_tables = popargs('target_tables', kwargs)
         super().__init__(**kwargs)
         self.description = desc
 
@@ -468,6 +474,10 @@ def __init__(self, **kwargs):  # noqa: C901
         self.__colids = {name: i + 1 for i, name in enumerate(self.colnames)}
         self._init_class_columns()
 
+        if target_tables:
+            self._set_dtr_targets(target_tables)
+
+
     def __set_table_attr(self, col):
         if hasattr(self, col.name) and col.name not in self.__uninit_cols:
             msg = ("An attribute '%s' already exists on %s '%s' so this column cannot be accessed as an attribute, "
@@ -516,6 +526,40 @@ def _init_class_columns(self):
                         self.__uninit_cols[col['name'] + '_elements'] = col
                         setattr(self, col['name'] + '_elements', None)
 
+    def _set_dtr_targets(self, target_tables: dict):
+        """Set the target tables for DynamicTableRegion columns.
+
+        If a column is not yet initialized, it is initialized with the target table.
+        """
+        for colname, table in target_tables.items():
+            if colname not in self:  # column has not yet been added (it is optional)
+                column_conf = None
+                for conf in self.__columns__:
+                    if conf['name'] == colname:
+                        column_conf = conf
+                        break
+                if column_conf is None:
+                    raise ValueError("'%s' is not the name of a predefined column of table %s."
+                                        % (colname, self))
+                if not column_conf.get('table', False):
+                    raise ValueError("Column '%s' must be a DynamicTableRegion to have a target table."
+                                        % colname)
+                self.add_column(name=column_conf['name'],
+                                description=column_conf['description'],
+                                index=column_conf.get('index', False),
+                                table=True)
+            if isinstance(self[colname], VectorIndex):
+                col = self[colname].target
+            else:
+                col = self[colname]
+            if not isinstance(col, DynamicTableRegion):
+                raise ValueError("Column '%s' must be a DynamicTableRegion to have a target table." % colname)
+            # if columns are passed in, then the "table" attribute may have already been set
+            if col.table is not None and col.table is not table:
+                raise ValueError("Column '%s' already has a target table that is not the passed table." % colname)
+            if col.table is None:
+                col.table = table
+
     @staticmethod
     def __build_columns(columns, df=None):
         """

tests/unit/common/test_table.py

@@ -1598,6 +1598,97 @@ def test_init_columns_add_dup_column(self):
         with self.assertRaisesWith(ValueError, msg):
             SubTable(name='subtable', description='subtable description', columns=[col1_ind, col1])
 
+    def test_no_set_target_tables(self):
+        """Test that the target table of a predefined DTR column is None."""
+        table = SubTable(name='subtable', description='subtable description')
+        self.assertIsNone(table.col5.table)
+
+    def test_set_target_tables(self):
+        """Test setting target tables for predefined DTR columns."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        table2 = SubTable(
+            name='subtable2',
+            description='subtable description',
+            target_tables={
+                'col5': table1,
+                'col6': table1,
+                'col7': table1,
+                'col8': table1,
+            },
+        )
+        self.assertIs(table2.col5.table, table1)
+        self.assertIs(table2.col6.table, table1)
+        self.assertIs(table2.col7.table, table1)
+        self.assertIs(table2.col8.table, table1)
+
+    def test_set_target_tables_unknown_col(self):
+        """Test setting target tables for unknown columns."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        msg = r"'bad_col' is not the name of a predefined column of table subtable2 .*"
+        with self.assertRaisesRegex(ValueError, msg):
+            SubTable(
+                name='subtable2',
+                description='subtable description',
+                target_tables={
+                    'bad_col': table1,
+                },
+            )
+
+    def test_set_target_tables_bad_init_col(self):
+        """Test setting target tables for predefined, required non-DTR columns."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        msg = "Column 'col1' must be a DynamicTableRegion to have a target table."
+        with self.assertRaisesWith(ValueError, msg):
+            SubTable(
+                name='subtable2',
+                description='subtable description',
+                target_tables={
+                    'col1': table1,
+                },
+            )
+
+    def test_set_target_tables_bad_opt_col(self):
+        """Test setting target tables for predefined, optional non-DTR columns."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        msg = "Column 'col2' must be a DynamicTableRegion to have a target table."
+        with self.assertRaisesWith(ValueError, msg):
+            SubTable(
+                name='subtable2',
+                description='subtable description',
+                target_tables={
+                    'col2': table1,
+                },
+            )
+
+    def test_set_target_tables_existing_col_mismatch(self):
+        """Test setting target tables for an existing DTR column with a mismatched, existing target table."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        table2 = SubTable(name='subtable2', description='subtable description')
+        dtr = DynamicTableRegion(name='dtr', data=[], description='desc', table=table1)
+        msg = "Column 'dtr' already has a target table that is not the passed table."
+        with self.assertRaisesWith(ValueError, msg):
+            SubTable(
+                name='subtable3',
+                description='subtable description',
+                columns=[dtr],
+                target_tables={
+                    'dtr': table2,
+                },
+            )
+
+    def test_set_target_tables_existing_col_match(self):
+        """Test setting target tables for an existing DTR column with a matching, existing target table."""
+        table1 = SubTable(name='subtable1', description='subtable description')
+        dtr = DynamicTableRegion(name='dtr', data=[], description='desc', table=table1)
+        SubTable(
+            name='subtable2',
+            description='subtable description',
+            columns=[dtr],
+            target_tables={
+                'dtr': table1,
+            },
+        )
+
 
 class TestEnumData(TestCase):