[WIP] Create ElectrodeTable type

src/pynwb/ecephys.py

@@ -1,16 +1,54 @@
 import warnings
 from collections.abc import Iterable
 
-from hdmf.common import DynamicTableRegion
+from hdmf.common import DynamicTableRegion, DynamicTable
 from hdmf.data_utils import DataChunkIterator, assertEqualShape
-from hdmf.utils import docval, popargs, get_docval, popargs_to_dict, get_data_shape
+from hdmf.utils import docval, popargs, get_docval, popargs_to_dict, get_data_shape, AllowPositional
 
 from . import register_class, CORE_NAMESPACE
 from .base import TimeSeries
 from .core import NWBContainer, NWBDataInterface, MultiContainerInterface
 from .device import Device
 
 
+@register_class("ElectrodesTable", CORE_NAMESPACE)
+class ElectrodesTable(DynamicTable):
+
+    __columns__ = (
+        {"name": "location", "description": "The location of the electrode within the subject, e.g., a particular brain region."},
+        {"name": "group", "description": "A reference to the ElectrodeGroup that this electrode is part of."},
+        {"name": "group_name", "description": "The name of the ElectrodeGroup this electrode is a part of."},
+        {"name": "x", "description": "The x coordinate of the electrode location in the brain (+x is posterior)."},
+        {"name": "y", "description": "The y coordinate of the electrode location in the brain (+y is inferior)."},
+        {"name": "z", "description": "The z coordinate of the electrode location in the brain (+z is right)."},
+        {"name": "imp", "description": "The impedance of the electrode, in ohms."},
+        {"name": "filtering", "description": "Description of hardware filtering, including the filter name and frequency cutoffs."},
+        {"name": "rel_x", "description": "The x coordinate of the electrode location within the electrode group."},
+        {"name": "rel_y", "description": "The y coordinate of the electrode location within the electrode group."},
+        {"name": "rel_z", "description": "The z coordinate of the electrode location within the electrode group."},
+        {"name": "reference",
+         "description": ("Description of the reference electrode and/or reference scheme used for this "
+                         "electrode, e.g., 'stainless steel skull screw' or 'online common average referencing'.")},
+    )
+
+    @docval(
+        {"name": "name", "type": str, "doc": "The name of this electrodes table", "default": "electrodes"},
+        {"name": "description", "type": str, "doc": "description of this electrode group",
+         "default": "Metadata about extracellular electrodes"},
+        *get_docval(DynamicTable.__init__, "id", "columns", "colnames"),
+    )
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+
+    @docval(*get_docval(DynamicTable.add_row, "data", "id"),
+            {'name': 'enforce_unique_id', 'type': bool, 'doc': 'enforce that the id in the table must be unique',
+             'default': True},
+            allow_extra=True,
+            allow_positional=AllowPositional.ERROR)
+    def add_electrode(self, **kwargs):
+        super().add_row(**kwargs)
+
+
 @register_class('ElectrodeGroup', CORE_NAMESPACE)
 class ElectrodeGroup(NWBContainer):
     """Defines a related group of electrodes."""

src/pynwb/file.py

@@ -14,7 +14,7 @@
 from .base import TimeSeries, ProcessingModule
 from .device import Device
 from .epoch import TimeIntervals
-from .ecephys import ElectrodeGroup
+from .ecephys import ElectrodeGroup, ElectrodesTable
 from .icephys import (IntracellularElectrode, SweepTable, PatchClampSeries, IntracellularRecordingsTable,
                       SimultaneousRecordingsTable, SequentialRecordingsTable, RepetitionsTable,
                       ExperimentalConditionsTable)
@@ -265,7 +265,7 @@ class NWBFile(MultiContainerInterface):
                      'virus',
                      'stimulus_notes',
                      'lab',
-                     {'name': 'electrodes', 'child': True,  'required_name': 'electrodes'},
+                     {'name': 'electrodes', 'child': True, 'required_name': 'electrodes'},
                      {'name': 'epochs', 'child': True, 'required_name': 'epochs'},
                      {'name': 'trials', 'child': True, 'required_name': 'trials'},
                      {'name': 'units', 'child': True, 'required_name': 'units'},
@@ -374,7 +374,7 @@ class NWBFile(MultiContainerInterface):
              'doc': 'ProcessingModule objects belonging to this NWBFile', 'default': None},
             {'name': 'lab_meta_data', 'type': (list, tuple), 'default': None,
              'doc': 'an extension that contains lab-specific meta-data'},
-            {'name': 'electrodes', 'type': DynamicTable,
+            {'name': 'electrodes', 'type': ElectrodesTable,
              'doc': 'the ElectrodeTable that belongs to this NWBFile', 'default': None},
             {'name': 'electrode_groups', 'type': Iterable,
              'doc': 'the ElectrodeGroups that belong to this NWBFile', 'default': None},
@@ -642,7 +642,7 @@ def add_epoch(self, **kwargs):
 
     def __check_electrodes(self):
         if self.electrodes is None:
-            self.electrodes = ElectrodeTable()
+            self.electrodes = ElectrodesTable()
 
     @docval(*get_docval(DynamicTable.add_column))
     def add_electrode_column(self, **kwargs):
@@ -653,77 +653,16 @@ def add_electrode_column(self, **kwargs):
         self.__check_electrodes()
         self.electrodes.add_column(**kwargs)
 
-    @docval({'name': 'x', 'type': float, 'doc': 'the x coordinate of the position (+x is posterior)',
-             'default': None},
-            {'name': 'y', 'type': float, 'doc': 'the y coordinate of the position (+y is inferior)', 'default': None},
-            {'name': 'z', 'type': float, 'doc': 'the z coordinate of the position (+z is right)', 'default': None},
-            {'name': 'imp', 'type': float, 'doc': 'the impedance of the electrode, in ohms', 'default': None},
-            {'name': 'location', 'type': str,
-             'doc': 'the location of electrode within the subject e.g. brain region. Required.',
-             'default': None},
-            {'name': 'filtering', 'type': str,
-             'doc': 'description of hardware filtering, including the filter name and frequency cutoffs',
-             'default': None},
-            {'name': 'group', 'type': ElectrodeGroup,
-             'doc': 'the ElectrodeGroup object to add to this NWBFile. Required.',
-             'default': None},
-            {'name': 'id', 'type': int, 'doc': 'a unique identifier for the electrode', 'default': None},
-            {'name': 'rel_x', 'type': float, 'doc': 'the x coordinate within the electrode group', 'default': None},
-            {'name': 'rel_y', 'type': float, 'doc': 'the y coordinate within the electrode group', 'default': None},
-            {'name': 'rel_z', 'type': float, 'doc': 'the z coordinate within the electrode group', 'default': None},
-            {'name': 'reference', 'type': str, 'doc': 'Description of the reference electrode and/or reference scheme\
-                used for this  electrode, e.g.,"stainless steel skull screw" or "online common average referencing". ',
-                'default': None},
-            {'name': 'enforce_unique_id', 'type': bool, 'doc': 'enforce that the id in the table must be unique',
-             'default': True},
+    @docval(*get_docval(ElectrodesTable.add_electrode),
             allow_extra=True,
-            allow_positional=AllowPositional.WARNING)
+            allow_positional=AllowPositional.ERROR)
     def add_electrode(self, **kwargs):
         """
         Add an electrode to the electrodes table.
-        See :py:meth:`~hdmf.common.table.DynamicTable.add_row` for more details.
-
-        Required fields are *location* and
-        *group* and any columns that have been added
-        (through calls to `add_electrode_columns`).
+        See :py:meth:`~pynwb.ecephys.ElectrodesTable.add_electrode` for more details.
         """
         self.__check_electrodes()
-        d = _copy.copy(kwargs['data']) if kwargs.get('data') is not None else kwargs
-
-        # NOTE location and group are required arguments. in PyNWB 2.1.0 we made x, y, z optional arguments, and
-        # in order to avoid breaking API changes, the order of the arguments needed to be maintained even though
-        # these optional arguments came before the required arguments, so in docval these required arguments are
-        # displayed as optional when really they are required. this should be changed when positional arguments
-        # are not allowed
-        if not d['location']:
-            raise ValueError("The 'location' argument is required when creating an electrode.")
-        if not kwargs['group']:
-            raise ValueError("The 'group' argument is required when creating an electrode.")
-        if d.get('group_name', None) is None:
-            d['group_name'] = d['group'].name
-
-        new_cols = [('x', 'the x coordinate of the position (+x is posterior)'),
-                    ('y', 'the y coordinate of the position (+y is inferior)'),
-                    ('z', 'the z coordinate of the position (+z is right)'),
-                    ('imp', 'the impedance of the electrode, in ohms'),
-                    ('filtering', 'description of hardware filtering, including the filter name and frequency cutoffs'),
-                    ('rel_x', 'the x coordinate within the electrode group'),
-                    ('rel_y', 'the y coordinate within the electrode group'),
-                    ('rel_z', 'the z coordinate within the electrode group'),
-                    ('reference', 'Description of the reference electrode and/or reference scheme used for this \
-                        electrode, e.g.,"stainless steel skull screw" or "online common average referencing".')
-                    ]
-
-        # add column if the arg is supplied and column does not yet exist
-        # do not pass arg to add_row if arg is not supplied
-        for col_name, col_doc in new_cols:
-            if kwargs[col_name] is not None:
-                if col_name not in self.electrodes:
-                    self.electrodes.add_column(col_name, col_doc)
-            else:
-                d.pop(col_name)  # remove args from d if not set
-
-        self.electrodes.add_row(**d)
+        self.electrodes.add_electrode(**kwargs)
 
     @docval({'name': 'region', 'type': (slice, list, tuple), 'doc': 'the indices of the table'},
             {'name': 'description', 'type': str, 'doc': 'a brief description of what this electrode is'},
@@ -821,13 +760,15 @@ def add_invalid_time_interval(self, **kwargs):
     @docval({'name': 'electrode_table', 'type': DynamicTable, 'doc': 'the ElectrodeTable for this file'})
     def set_electrode_table(self, **kwargs):
         """
+        This method is deprecated as of PyNWB 3.0.0 and will be removed in a future release.
         Set the electrode table of this NWBFile to an existing ElectrodeTable
         """
+        warn("NWBFile.set_electrode_table has been replaced by the setter for NWBFile.electrodes.", DeprecationWarning)
         if self.electrodes is not None:
             msg = 'ElectrodeTable already exists, cannot overwrite'
             raise ValueError(msg)
         electrode_table = getargs('electrode_table', kwargs)
-        self.electrodes = electrode_table
+        self.electrodes = kwargs["electrode_table"]
 
     def _check_sweep_table(self):
         """
@@ -1164,6 +1105,9 @@ def _tablefunc(table_name, description, columns):
 
 def ElectrodeTable(name='electrodes',
                    description='metadata about extracellular electrodes'):
+    warn("The ElectrodeTable method is deprecated as of PyNWB 3.0.0 and will be removed in a future release. "
+         "Please use the ElectrodesTable class instead.",
+         DeprecationWarning)
     return _tablefunc(name, description,
                       [('location', 'the location of channel within the subject e.g. brain region'),
                        ('group', 'a reference to the ElectrodeGroup this electrode is a part of'),

src/pynwb/io/file.py

@@ -208,6 +208,34 @@ def publication_obj_attr(self, container, manager):
             ret = (container.related_publications,)
         return ret
 
+    @ObjectMapper.constructor_arg("electrodes")
+    def electrodes_carg(self, builder, manager):
+        electrodes_builder = builder.groups["general"].groups["extracellular_ephys"].groups["electrodes"]
+        breakpoint()
+        if electrodes_builder is None:
+            return None
+        if electrodes_builder.attributes['neurodata_type'] != 'ElectrodesTable':
+            # override builder attributes
+            electrodes_builder.attributes['neurodata_type'] = 'ElectrodesTable'
+            electrodes_builder.attributes['namespace'] = 'core'
+        return manager.construct(electrodes_builder)
+            # # construct new columns list
+            # columns = list()
+            # for dset_builder in builder.datasets.values():
+            #     dset_obj = manager.construct(dset_builder)  # these have already been constructed
+            #     # go through only the column datasets and replace the 'timeseries' column class in-place.
+            #     if isinstance(dset_obj, VectorData):
+            #         if dset_obj.name == 'timeseries':
+            #             # replacing the class in-place is possible because the VectorData and
+            #             # TimeSeriesReferenceVectorData have the same memory layout and the old and new
+            #             # schema are compatible (i.e., only the neurodata_type was changed in 2.5)
+            #             dset_obj.__class__ = TimeSeriesReferenceVectorData
+            #             # Execute init logic specific for TimeSeriesReferenceVectorData
+            #             dset_obj._init_internal()
+            #         columns.append(dset_obj)
+            # # overwrite the columns constructor argument
+            # return columns
+
 
 @register_map(Subject)
 class SubjectMap(ObjectMapper):

src/pynwb/testing/make_test_files.py

@@ -214,6 +214,74 @@ def _make_subject_without_age_reference():
     test_name = 'subject_no_age__reference'
     _write(test_name, nwbfile)
 
+def _make_old_electrodes_table_basic():
+    """Create a test file with the old electrodes table and only the required fields."""
+    nwbfile = NWBFile(session_description='ADDME',
+                      identifier='ADDME',
+                      session_start_time=datetime.now().astimezone())
+    device = nwbfile.create_device(name='dev1')
+    group = nwbfile.create_electrode_group(
+        name='tetrode1',
+        description='tetrode description',
+        location='tetrode location',
+        device=device,
+    )
+    nwbfile.add_electrode(location="CA1", group=group)
+    nwbfile.add_electrode(location="CA1", group=group)
+
+    test_name = 'old_electrodes_table_basic'
+    _write(test_name, nwbfile)
+
+def _make_old_electrodes_table_full():
+    """Create a test file with the old electrodes table and all fields."""
+    nwbfile = NWBFile(session_description='ADDME',
+                      identifier='ADDME',
+                      session_start_time=datetime.now().astimezone())
+    device = nwbfile.create_device(name='dev1')
+    group = nwbfile.create_electrode_group(
+        name='tetrode1',
+        description='tetrode description',
+        location='tetrode location',
+        device=device,
+    )
+    nwbfile.add_electrode_column(
+        name="verified_location",
+        description="location verified after histology"
+    )
+    nwbfile.add_electrode(
+        location="CA1",
+        group=group,
+        group_name="tetrode1",
+        x=1.0,
+        y=2.0,
+        z=3.0,
+        imp=4.0,
+        rel_x=0.1,
+        rel_y=0.2,
+        rel_z=0.3,
+        filtering='Low-pass filter at 300 Hz',
+        reference="None",
+        verified_location="CA1"
+    )
+    nwbfile.add_electrode(
+        location="CA1",
+        group=group,
+        group_name="tetrode1",
+        x=1.1,
+        y=2.1,
+        z=3.1,
+        imp=4.1,
+        rel_x=0.11,
+        rel_y=0.21,
+        rel_z=0.31,
+        filtering='Low-pass filter at 300 Hz',
+        reference="None",
+        verified_location="CA1"
+    )
+
+    test_name = 'old_electrodes_table_full'
+    _write(test_name, nwbfile)
+
 
 if __name__ == '__main__':
     # install these versions of PyNWB and run this script to generate new files
@@ -242,3 +310,9 @@ def _make_subject_without_age_reference():
 
     if __version__ == "2.2.0":
         _make_subject_without_age_reference()
+
+    # if __version__ == "2.3.2":
+    if 1:
+        _make_old_electrodes_table_basic()
+        _make_old_electrodes_table_full()
+

src/pynwb/testing/testh5io.py

@@ -228,7 +228,7 @@ def getContainer(self, nwbfile):
 
     def setUp(self):
         container_type = self.getContainerType().replace(" ", "_")
-        session_description = 'A file to test writing and reading a %s' % container_type
+        session_description = 'A file to test writing and reading %s' % container_type
         identifier = 'TEST_%s' % container_type
         session_start_time = datetime(1971, 1, 1, 12, tzinfo=tzutc())
         self.nwbfile = NWBFile(
@@ -251,7 +251,7 @@ def tearDown(self):
         remove_test_file(self.filename)
         remove_test_file(self.export_filename)
 
-    def getContainerType() -> str:
+    def getContainerType(self) -> str:
         """Return the name of the type of Container being tested, for test ID purposes."""
         raise NotImplementedError('Cannot run test unless getContainerType is implemented.')
 
@@ -369,10 +369,16 @@ def validate(self):
             with NWBHDF5IO(self.filename, mode='r') as io:
                 errors = pynwb_validate(io)
                 if errors:
-                    raise Exception("\n".join(errors))
+                    err_str = ""
+                    for err in errors:
+                        err_str = str(err) + "\n"
+                    raise Exception(err_str)
 
         if os.path.exists(self.export_filename):
             with NWBHDF5IO(self.filename, mode='r') as io:
                 errors = pynwb_validate(io)
                 if errors:
-                    raise Exception("\n".join(errors))
+                    err_str = ""
+                    for err in errors:
+                        err_str = str(err) + "\n"
+                    raise Exception(err_str)

tests/back_compat/test_read.py

@@ -3,6 +3,7 @@
 import warnings
 
 from pynwb import NWBHDF5IO, validate, TimeSeries
+from pynwb.ecephys import ElectrodesTable
 from pynwb.image import ImageSeries
 from pynwb.testing import TestCase
 
@@ -120,3 +121,11 @@ def test_read_subject_no_age__reference(self):
         with NWBHDF5IO(str(f), 'r') as io:
             read_nwbfile = io.read()
             self.assertIsNone(read_nwbfile.subject.age__reference)
+
+    def test_read_old_electrodes_table_basic(self):
+        """Test reading an old electrodes table (DynamicTable) with only required fields."""
+        f = Path(__file__).parent / '2.3.2_old_electrodes_table_basic.nwb'
+        with NWBHDF5IO(str(f), 'r') as io:
+            read_nwbfile = io.read()
+            self.assertIsInstance(read_nwbfile.electrodes, ElectrodesTable)
+            self.assertEqual(read_nwbfile.electrodes[0, "location"], "CA1")