Change MeaningsTable.target from a link to an object-reference attribute

CHANGELOG.md

@@ -1,6 +1,9 @@
 # HDMF Changelog
 
-## HDMF 6.1.1 (Upcoming)
+## HDMF 6.2.0 (Upcoming)
+
+### Changed
+- Added support for hdmf-common schema 1.10.0, which changes ``MeaningsTable.target`` from a link to an object-reference attribute (``dtype`` with ``reftype: object``, ``target_type: VectorData``). Files written with the hdmf-common 1.9.0 ``MeaningsTable`` (a link named "target") are still read correctly via a backwards-compatibility mapping in ``MeaningsTableMap``. There is no change in the read/write API; the change is limited to the representation on disk. @rly [#1525](https://github.com/hdmf-dev/hdmf/pull/1525)
 
 ### Fixed
 - Fixed writing a 1D dataset of object references whose targets are `DynamicTable`s (or other sized, indexable containers). The shape of a reference (or compound) dataset is now taken from the reference array itself. @pauladkisson [#1533](https://github.com/hdmf-dev/hdmf/pull/1533)

src/hdmf/common/io/table.py

@@ -1,6 +1,6 @@
 from .. import register_map
-from ..table import DynamicTable, VectorData, VectorIndex, DynamicTableRegion
-from ...build import ObjectMapper, BuildManager, CustomClassGenerator
+from ..table import DynamicTable, VectorData, VectorIndex, DynamicTableRegion, MeaningsTable
+from ...build import ObjectMapper, BuildManager, CustomClassGenerator, GroupBuilder
 from ...spec import Spec
 from ...utils import docval, getargs
 
@@ -123,3 +123,48 @@ def _get_attrs_not_to_set_init(cls, classdict, parent_docval_args):
             column_names = [column_conf["name"] for column_conf in classdict["__columns__"]]
             attrs_not_to_set.update(column_names)
         return attrs_not_to_set
+
+
+@register_map(MeaningsTable)
+class MeaningsTableMap(DynamicTableMap):
+    """Object mapper for MeaningsTable.
+
+    In HDMF Common Schema 1.10.0+, ``MeaningsTable.target`` is stored as an object-reference attribute named "target".
+    hdmf-common 1.9.0 stored it as a link named "target". On read, a legacy "target" link is removed from the
+    builder so it is not matched as a VectorData column of the table, and the VectorData it points to
+    is supplied to the ``target`` constructor argument. Files that store "target" as the
+    object-reference attribute are handled by the default mapping.
+    """
+
+    def __init__(self, spec):
+        super().__init__(spec)
+        # Resolved target VectorData for the MeaningsTable builder currently being constructed,
+        # populated for legacy hdmf-common 1.9.0 files
+        self.__legacy_target = None
+
+    def construct(self, builder, manager, parent=None):
+        # Remove the legacy "target" link before the generic mapping runs, otherwise it is matched as
+        # a VectorData column of the table. This is too early for target_carg, so stash the resolved
+        # target for it to supply.
+        if isinstance(builder, GroupBuilder):
+            legacy_link = builder.links.pop('target', None)
+        else:
+            legacy_link = None
+        if legacy_link is not None:
+            builder.obj_type.pop('target', None)
+            self.__legacy_target = manager.construct(legacy_link.builder)
+        try:
+            return super().construct(builder, manager, parent)
+        finally:
+            # This mapper instance is shared across all MeaningsTable builders, so clear the stash,
+            # even on error, to avoid leaking this target into the next MeaningsTable constructed.
+            self.__legacy_target = None
+
+    @ObjectMapper.constructor_arg('target')
+    def target_carg(self, builder, manager):
+        """Supply ``target`` from a legacy hdmf-common 1.9.0 "target" link.
+
+        Returns the VectorData resolved from a removed "target" link, or None for files that store
+        "target" as an attribute, in which case the default object-reference resolution supplies it.
+        """
+        return self.__legacy_target

src/hdmf/common/table.py

@@ -1902,14 +1902,14 @@ def add_row(self, **kwargs):
 @register_class('MeaningsTable')
 class MeaningsTable(DynamicTable):
     """
-    A table to store information about the meanings of values in a linked VectorData object.
+    A table to store information about the meanings of values in a referenced VectorData object.
 
-    All possible values of the linked VectorData object should be present in the 'value' column
+    All possible values of the referenced VectorData object should be present in the 'value' column
     of this table, even if the value is not observed in the data. Additional columns may be
     added to store additional metadata about each value.
 
     The name of the MeaningsTable is automatically set to "{target.name}_meanings" based on
-    the linked VectorData object. For example, if the linked VectorData object is named
+    the referenced VectorData object. For example, if the referenced VectorData object is named
     "stimulus_type", the MeaningsTable will be named "stimulus_type_meanings".
     """
 
@@ -1918,7 +1918,7 @@ class MeaningsTable(DynamicTable):
     )
 
     __columns__ = (
-        {'name': 'value', 'description': 'The value in the linked VectorData object.', 'required': True},
+        {'name': 'value', 'description': 'The value in the referenced VectorData object.', 'required': True},
         {'name': 'meaning', 'description': 'The meaning of the value.', 'required': True},
     )
 
@@ -1942,7 +1942,7 @@ def __init__(self, **kwargs):
         super().__init__(**kwargs)
         self.target = target
 
-    @docval({'name': 'value', 'type': None, 'doc': 'the value in the linked VectorData object'},
+    @docval({'name': 'value', 'type': None, 'doc': 'the value in the referenced VectorData object'},
             {'name': 'meaning', 'type': str, 'doc': 'the meaning of the value'},
             {'name': 'id', 'type': int, 'doc': 'the ID for the row', 'default': None},
             {'name': 'enforce_unique_id', 'type': bool, 'doc': 'enforce that the id in the table must be unique',

tests/unit/common/test_table.py

@@ -3495,6 +3495,56 @@ def test_roundtrip_meanings_table_data(self):
         self.assertEqual(list(mt['meaning'].data), ['stimulus A', 'stimulus B', 'stimulus C'])
 
 
+class TestMeaningsTableLegacyTargetLinkRead(TestCase):
+    """Read a MeaningsTable written with the hdmf-common 1.9.0 "target" link layout.
+
+    hdmf-common 1.9.0 stored ``MeaningsTable.target`` as a link named "target"; 1.10.0 stores it as
+    an object-reference attribute named "target". A new-format file is written and then rewritten on
+    disk into the 1.9.0 layout (the "target" reference attribute is replaced with a "target" SoftLink
+    to the target column) so that MeaningsTableMap's backwards-compatibility path is exercised.
+    """
+
+    def setUp(self):
+        self.path = get_temp_filepath()
+
+    def tearDown(self):
+        remove_test_file(self.path)
+
+    def _write_legacy_file(self):
+        table = DynamicTable(name='test_table', description='a test table')
+        table.add_column(name='stimulus_type', description='stimulus type')
+        table.add_row(stimulus_type='a')
+        table.add_row(stimulus_type='b')
+        table.add_row(stimulus_type='a')
+
+        mt = MeaningsTable(target=table['stimulus_type'])
+        mt.add_row(value='a', meaning='stimulus A')
+        mt.add_row(value='b', meaning='stimulus B')
+        table.add_meanings_table(mt)
+
+        with HDF5IO(self.path, 'w', manager=get_manager()) as io:
+            io.write(table)
+
+        # rewrite into the hdmf-common 1.9.0 layout: replace the "target" object-reference attribute
+        # with a "target" SoftLink to the target column
+        with h5py.File(self.path, 'r+') as f:
+            group = f['meanings_tables/stimulus_type_meanings']
+            del group.attrs['target']
+            group['target'] = h5py.SoftLink('/stimulus_type')
+
+    def test_read_legacy_target_link(self):
+        self._write_legacy_file()
+        with HDF5IO(self.path, 'r', manager=get_manager()) as io:
+            read_table = io.read()
+            mt = read_table.get_meanings_table('stimulus_type_meanings')
+            # the target link resolves to the target column, not an extra column of the table
+            self.assertEqual(tuple(mt.colnames), ('value', 'meaning'))
+            self.assertEqual(len(mt), 2)
+            self.assertEqual(list(mt['value'].data), ['a', 'b'])
+            self.assertEqual(list(mt['meaning'].data), ['stimulus A', 'stimulus B'])
+            self.assertIs(mt.target, read_table['stimulus_type'])
+
+
 class TestMeaningsTableLengthMismatchRoundTrip(H5RoundTripMixin, TestCase):
     """Roundtrip when MeaningsTable row count differs from the target column row count.