Fix bad validation check for postion in ElectrodeGroup.__init__

CHANGELOG.md

@@ -1,5 +1,11 @@
 # PyNWB Changelog
 
+## PyNWB 2.5.1 (Upcoming)
+
+### Bug fixes
+- Fixed bug in how `ElectrodGroup.__init__` validates its `position` argument. @oruebel [#1770](https://github.com/NeurodataWithoutBorders/pynwb/pull/1770)
+
+
 ## PyNWB 2.5.0 (August 18, 2023)
 
 ### Enhancements and minor changes

src/pynwb/ecephys.py

@@ -26,13 +26,29 @@
             {'name': 'location', 'type': str, 'doc': 'description of location of this electrode group'},
             {'name': 'device', 'type': Device, 'doc': 'the device that was used to record from this electrode group'},
             {'name': 'position', 'type': 'array_data',
-             'doc': 'stereotaxic position of this electrode group (x, y, z)', 'default': None})
+             'doc': 'Compound dataset with stereotaxic position of this electrode group (x, y, z). '
+                    'Each element of the data array must have three elements or the dtype of the '
+                    'array must be `(float, float, float)', 'default': None})
     def __init__(self, **kwargs):
         args_to_set = popargs_to_dict(('description', 'location', 'device', 'position'), kwargs)
         super().__init__(**kwargs)
-        if args_to_set['position'] and len(args_to_set['position']) != 3:
-            raise ValueError('ElectrodeGroup position argument must have three elements: x, y, z, but received: %s'
-                             % str(args_to_set['position']))
+        # position is a compound dataset, i.e., this must be an array with a compound data type of three floats
+        # or each element in the list must at least have three entries
+        if args_to_set['position'] is not None and len(args_to_set['position']) > 0:
+            position_dtype_valid = True
+            if hasattr(args_to_set['position'], 'dtype'):  # If we have a dtype, then check that it is valid
+                if len(args_to_set['position'].dtype) != 3:
+                    print("H1", len(args_to_set['position'].dtype))
+                    position_dtype_valid = False
+            if position_dtype_valid:  # If we have list of element, then check that the elements are of lenght 3
+                try:
+                    if len(args_to_set['position'][0]) != 3:
+                        position_dtype_valid = False
+                except TypeError:  # len not supported by first_position
+                    position_dtype_valid = False
+            if not position_dtype_valid:
+                raise ValueError('ElectrodeGroup position dataset must have three components (x, y, z) '
+                                 'for each array element, but received: %s' % str(args_to_set['position']))
         for key, val in args_to_set.items():
             setattr(self, key, val)
 

tests/integration/hdf5/test_ecephys.py

@@ -25,7 +25,8 @@ def setUpContainer(self):
         eg = ElectrodeGroup(name='elec1',
                             description='a test ElectrodeGroup',
                             location='a nonexistent place',
-                            device=self.dev1)
+                            device=self.dev1,
+                            position=[(1., 2., 3.), ])
         return eg
 
     def addContainer(self, nwbfile):

tests/unit/test_ecephys.py

@@ -154,16 +154,23 @@ class ElectrodeGroupConstructor(TestCase):
 
     def test_init(self):
         dev1 = Device('dev1')
-        group = ElectrodeGroup('elec1', 'electrode description', 'electrode location', dev1, (1, 2, 3))
+        group = ElectrodeGroup(name='elec1',
+                               description='electrode description',
+                               location='electrode location',
+                               device=dev1,
+                               position=[(1, 2, 3), ])
         self.assertEqual(group.name, 'elec1')
         self.assertEqual(group.description, 'electrode description')
         self.assertEqual(group.location, 'electrode location')
         self.assertEqual(group.device, dev1)
-        self.assertEqual(group.position, (1, 2, 3))
+        self.assertEqual(group.position, [(1, 2, 3), ])
 
     def test_init_position_none(self):
         dev1 = Device('dev1')
-        group = ElectrodeGroup('elec1', 'electrode description', 'electrode location', dev1)
+        group = ElectrodeGroup(name='elec1',
+                               description='electrode description',
+                               location='electrode location',
+                               device=dev1)
         self.assertEqual(group.name, 'elec1')
         self.assertEqual(group.description, 'electrode description')
         self.assertEqual(group.location, 'electrode location')
@@ -173,7 +180,29 @@ def test_init_position_none(self):
     def test_init_position_bad(self):
         dev1 = Device('dev1')
         with self.assertRaises(ValueError):
-            ElectrodeGroup('elec1', 'electrode description', 'electrode location', dev1, (1, 2))
+            ElectrodeGroup(name='elec1',
+                           description='electrode description',
+                           location='electrode location',
+                           device=dev1,
+                           position=(1, 2))
+        with self.assertRaises(ValueError):
+            ElectrodeGroup(name='elec1',
+                           description='electrode description',
+                           location='electrode location',
+                           device=dev1,
+                           position=(1, 2, 3))
+        with self.assertRaises(ValueError):
+            ElectrodeGroup(name='elec1',
+                           description='electrode description',
+                           location='electrode location',
+                           device=dev1,
+                           position=[(1, 2), ])
+        with self.assertRaises(ValueError):
+            ElectrodeGroup(name='elec1',
+                           description='electrode description',
+                           location='electrode location',
+                           device=dev1,
+                           position=np.array([(1., 2.)], dtype=np.dtype([('x', float), ('y', float)])))
 
 
 class EventDetectionConstructor(TestCase):