Detect mismatch errors between group and group names when writing ElectrodeGroups

CHANGELOG.md

@@ -13,6 +13,8 @@
 * Fix a bug where data in `DeepLabCutInterface` failed to write when `ndx-pose` was not imported. [#1144](https://github.com/catalystneuro/neuroconv/pull/1144)
 * `SpikeGLXConverterPipe` converter now accepts multi-probe structures with multi-trigger and does not assume a specific folder structure [#1150](https://github.com/catalystneuro/neuroconv/pull/1150)
 * `SpikeGLXNIDQInterface` is no longer written as an ElectricalSeries [#1152](https://github.com/catalystneuro/neuroconv/pull/1152)
+* Fix a bug on ecephys interfaces where extra electrode group and devices were written if the property of the "group_name" was set in the recording extractor [#1164](https://github.com/catalystneuro/neuroconv/pull/1164)
+
 
 ## Features
 * Propagate the `unit_electrode_indices` argument from the spikeinterface tools to `BaseSortingExtractorInterface`. This allows users to map units to the electrode table when adding sorting data [PR #1124](https://github.com/catalystneuro/neuroconv/pull/1124)
@@ -29,6 +31,7 @@
 * Use pytest format for dandi tests to avoid window permission error on teardown [PR #1151](https://github.com/catalystneuro/neuroconv/pull/1151)
 * Added many docstrings for public functions [PR #1063](https://github.com/catalystneuro/neuroconv/pull/1063)
 * Clean up with warnings and deprecations in the testing framework [PR #1158](https://github.com/catalystneuro/neuroconv/pull/1158)
+* Detect mismatch errors between group and group names when writing ElectrodeGroups [PR #1165](https://github.com/catalystneuro/neuroconv/pull/1165)
 
 
 # v0.6.5 (November 1, 2024)

src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py

@@ -88,7 +88,9 @@ def get_metadata_schema(self) -> dict:
     def get_metadata(self) -> DeepDict:
         metadata = super().get_metadata()
 
-        channel_groups_array = self.recording_extractor.get_channel_groups()
+        from ...tools.spikeinterface.spikeinterface import _get_group_name
+
+        channel_groups_array = _get_group_name(recording=self.recording_extractor)
         unique_channel_groups = set(channel_groups_array) if channel_groups_array is not None else ["ElectrodeGroup"]
         electrode_metadata = [
             dict(name=str(group_id), description="no description", location="unknown", device="DeviceEcephys")

src/neuroconv/tools/spikeinterface/spikeinterface.py

@@ -245,11 +245,19 @@ def _get_group_name(recording: BaseRecording) -> np.ndarray:
         An array containing the group names. If the `group_name` property is not
         available, the channel groups will be returned. If the group names are
         empty, a default value 'ElectrodeGroup' will be used.
+
+    Raises
+    ------
+    ValueError
+        If the number of unique group names doesn't match the number of unique groups,
+        or if the mapping between group names and group numbers is inconsistent.
     """
     default_value = "ElectrodeGroup"
     group_names = recording.get_property("group_name")
+    groups = recording.get_channel_groups()
+
     if group_names is None:
-        group_names = recording.get_channel_groups()
+        group_names = groups
     if group_names is None:
         group_names = np.full(recording.get_num_channels(), fill_value=default_value)
 
@@ -259,6 +267,23 @@ def _get_group_name(recording: BaseRecording) -> np.ndarray:
     # If for any reason the group names are empty, fill them with the default
     group_names[group_names == ""] = default_value
 
+    # Validate group names against groups
+    if groups is not None:
+        unique_groups = np.unique(groups)
+        unique_names = np.unique(group_names)
+
+        if len(unique_names) != len(unique_groups):
+            raise ValueError("The number of group names must match the number of groups")
+
+        # Check consistency of group name to group number mapping
+        group_to_name_map = {}
+        for group, name in zip(groups, group_names):
+            if group in group_to_name_map:
+                if group_to_name_map[group] != name:
+                    raise ValueError("Inconsistent mapping between group numbers and group names")
+            else:
+                group_to_name_map[group] = name
+
     return group_names
 
 

src/neuroconv/tools/testing/mock_interfaces.py

@@ -220,6 +220,12 @@ def __init__(
             es_key=es_key,
         )
 
+        # Adding this as a safeguard before the spikeinterface changes are merged:
+        # https://github.com/SpikeInterface/spikeinterface/pull/3588
+        channel_ids = self.recording_extractor.get_channel_ids()
+        channel_ids_as_strings = [str(id) for id in channel_ids]
+        self.recording_extractor = self.recording_extractor.rename_channels(new_channel_ids=channel_ids_as_strings)
+
     def get_metadata(self) -> dict:
         """
         Returns the metadata dictionary for the current object.
@@ -272,6 +278,7 @@ def __init__(
         )
 
         # Sorting extractor to have string unit ids until is changed in SpikeInterface
+        # https://github.com/SpikeInterface/spikeinterface/pull/3588
         string_unit_ids = [str(id) for id in self.sorting_extractor.unit_ids]
         self.sorting_extractor = self.sorting_extractor.rename_units(new_unit_ids=string_unit_ids)
 

tests/test_ecephys/test_ecephys_interfaces.py

@@ -119,7 +119,7 @@ def test_electrode_indices_assertion_error_when_missing_table(self, setup_interf
 
 class TestRecordingInterface(RecordingExtractorInterfaceTestMixin):
     data_interface_cls = MockRecordingInterface
-    interface_kwargs = dict(durations=[0.100])
+    interface_kwargs = dict(num_channels=4, durations=[0.100])
 
     def test_stub(self, setup_interface):
         interface = self.interface
@@ -146,6 +146,18 @@ def test_always_write_timestamps(self, setup_interface):
         expected_timestamps = self.interface.recording_extractor.get_times()
         np.testing.assert_array_equal(electrical_series.timestamps[:], expected_timestamps)
 
+    def test_group_naming_not_adding_extra_devices(self, setup_interface):
+
+        interface = self.interface
+        recording_extractor = interface.recording_extractor
+        recording_extractor.set_channel_groups(groups=[0, 1, 2, 3])
+        recording_extractor.set_property(key="group_name", values=["group1", "group2", "group3", "group4"])
+
+        nwbfile = interface.create_nwbfile()
+
+        assert len(nwbfile.devices) == 1
+        assert len(nwbfile.electrode_groups) == 4
+
 
 class TestAssertions(TestCase):
     @pytest.mark.skipif(python_version.minor != 10, reason="Only testing with Python 3.10!")

tests/test_ecephys/test_tools_spikeinterface.py

@@ -24,6 +24,7 @@
 from neuroconv.tools.nwb_helpers import get_module
 from neuroconv.tools.spikeinterface import (
     add_electrical_series_to_nwbfile,
+    add_electrode_groups_to_nwbfile,
     add_electrodes_to_nwbfile,
     add_recording_to_nwbfile,
     add_sorting_to_nwbfile,
@@ -1071,6 +1072,29 @@ def test_missing_bool_values(self):
         assert np.array_equal(extracted_incomplete_property, expected_incomplete_property)
 
 
+class TestAddElectrodeGroups:
+    def test_group_naming_not_matching_group_number(self):
+        recording = generate_recording(num_channels=4)
+        recording.set_channel_groups(groups=[0, 1, 2, 3])
+        recording.set_property(key="group_name", values=["A", "A", "A", "A"])
+
+        nwbfile = mock_NWBFile()
+        with pytest.raises(ValueError, match="The number of group names must match the number of groups"):
+            add_electrode_groups_to_nwbfile(nwbfile=nwbfile, recording=recording)
+
+    def test_inconsistent_group_name_mapping(self):
+        recording = generate_recording(num_channels=3)
+        # Set up groups where the same group name is used for different group numbers
+        recording.set_channel_groups(groups=[0, 1, 0])
+        recording.set_property(
+            key="group_name", values=["A", "B", "B"]  # Inconsistent: group 0 maps to names "A" and "B"
+        )
+
+        nwbfile = mock_NWBFile()
+        with pytest.raises(ValueError, match="Inconsistent mapping between group numbers and group names"):
+            add_electrode_groups_to_nwbfile(nwbfile=nwbfile, recording=recording)
+
+
 class TestAddUnitsTable(TestCase):
     @classmethod
     def setUpClass(cls):