revert_1454

CHANGELOG.md

@@ -61,7 +61,6 @@ DecodingParameters().alter()
         from NWB #1433
     - Add custom/dynamic `AnalysisNwbfile` creation #1435
     - Allow nullable `DataAcquisitionDevice` foreign keys #1455
-    - Improve error transparency on duplicate `Electrode` ids #1454
     - Remove pre-existing `Units` from created analysis nwb files #1453
     - Allow multiple VideoFile entries during ingestion #1462
     - Handle epoch formats with varying zero-padding #1459

src/spyglass/common/common_device.py

@@ -421,8 +421,6 @@ class Electrode(SpyglassIngestion, dj.Part):
         # Electrode configuration, with ID, contact size, X/Y/Z coordinates
         -> Probe.Shank
         probe_electrode: int          # electrode ID from ShanksElectrode.name
-                                      # Should be globally unique across all
-                                      # probes and shanks for clarity (see #1447)
         ---
         contact_size = NULL: float    # (um) contact size
         rel_x = NULL: float           # (um) x coordinate of electrode

src/spyglass/common/common_ephys.py

@@ -200,10 +200,6 @@ def make(self, key):
                     key.update(electrode_config_dicts[elect_id])
             electrode_inserts.append(key.copy())
 
-        # Validate electrode ID uniqueness (issue #1447)
-        if not self._test_mode:
-            _validate_electrode_ids(electrode_inserts, nwb_file_name)
-
         self.insert(
             electrode_inserts,
             skip_duplicates=True,
@@ -961,78 +957,3 @@ def fetch1_dataframe(self, *attrs, **kwargs) -> pd.DataFrame:
                 filtered_nwb["filtered_data"].timestamps, name="time"
             ),
         )
-
-
-def _validate_electrode_ids(electrode_inserts, nwb_file_name):
-    """Validate that electrode IDs (probe_electrode values) are globally unique.
-
-    Checks for duplicate probe_electrode values across all electrodes in the
-    session. While Spyglass technically allows duplicate probe_electrode values
-    across different shanks (since the primary key includes probe_shank),
-    having duplicates can lead to confusing foreign key errors and makes it
-    unclear which electrode is being referenced.
-
-    Parameters
-    ----------
-    electrode_inserts : list of dict
-        List of electrode dictionaries to be inserted
-    nwb_file_name : str
-        Name of the NWB file being processed
-
-    Raises
-    ------
-    ValueError
-        If duplicate probe_electrode values are detected, with detailed
-        information about which electrodes are duplicated and suggestions for
-        fixing the issue.
-
-    See Also
-    --------
-    https://github.com/LorenFrankLab/spyglass/issues/1447
-    """
-    probe_electrodes = [
-        e
-        for e in electrode_inserts
-        if "probe_electrode" in e and "probe_id" in e
-    ]
-
-    if not probe_electrodes:
-        return  # No probe electrodes to validate
-
-    # Track probe_electrode values by location
-    # probe_electrode -> list of (probe_id, probe_shank, nwb_electrode_id)
-    elec_locs = defaultdict(list)
-    for electrode in probe_electrodes:
-        elec_locs[electrode["probe_electrode"]].append(
-            (
-                electrode["probe_id"],
-                electrode.get("probe_shank", "unknown"),
-                electrode.get("electrode_id", "unknown"),
-            )
-        )
-
-    duplicates = {e: locs for e, locs in elec_locs.items() if len(locs) > 1}
-
-    if not duplicates:
-        return  # All probe_electrode values are unique
-
-    error_lines = [
-        f"Duplicate electrode IDs detected in NWB file '{nwb_file_name}'.",
-        "The following IDs are duplicated:",
-    ]
-
-    for elec_id, locations in sorted(duplicates.items()):
-        error_lines.append(
-            f"  Electrode ID {elec_id} appears {len(locations)} times:"
-        )
-        for probe_id, shank, nwb_id in locations:
-            error_lines.append(
-                f"    - Probe '{probe_id}', Shank {shank}, NWB index {nwb_id}"
-            )
-
-    error_lines.append(
-        "To resolve, please ensure that each electrode has a unique "
-        + "'probe_electrode' value across all probes and shanks. "
-    )
-
-    raise ValueError("\n".join(error_lines))

tests/common/test_ephys.py

@@ -1,16 +1,5 @@
-import numpy as np
 import pytest
-from ndx_franklab_novela import (
-    DataAcqDevice,
-    NwbElectrodeGroup,
-    Probe,
-    Shank,
-    ShanksElectrode,
-)
 from numpy import array_equal
-from pynwb import NWBHDF5IO
-from pynwb.testing.mock.ecephys import mock_ElectricalSeries
-from pynwb.testing.mock.file import mock_NWBFile, mock_Subject
 
 from ..conftest import TEARDOWN
 
@@ -65,21 +54,3 @@ def test_set_lfp_electrodes(mini_insert, common_ephys, mini_copy_name):
 @pytest.mark.skip(reason="Not testing V0: common lfp")
 def test_lfp():
     pass
-
-
-def test_duplicate_electrode_ids_error(common_ephys):
-    """Test duplicate electrode IDs (probe_electrode) produce clear error."""
-    validate = common_ephys._validate_electrode_ids
-    bad_inserts = [
-        dict(probe_electrode=i, probe_id=j) for i in [1, 2] for j in [1, 2]
-    ]
-
-    # Attempt insertion - expect clear ValueError about duplicate electrode IDs
-    with pytest.raises(ValueError) as exc_info:
-        validate(bad_inserts, "some_file")
-
-    # Verify we get a clear, informative error message
-    error_message = str(exc_info.value)
-    assert "Duplicate electrode IDs detected" in error_message
-    assert "Electrode ID 1 appears 2 times" in error_message
-    assert "Electrode ID 2 appears 2 times" in error_message