From bcec9018dd5a85b565ba867af4340a0cdb117a09 Mon Sep 17 00:00:00 2001 From: samuelbray32 Date: Wed, 26 Nov 2025 11:46:04 -0800 Subject: [PATCH] revert_1454 --- CHANGELOG.md | 1 - src/spyglass/common/common_device.py | 2 - src/spyglass/common/common_ephys.py | 79 ---------------------------- tests/common/test_ephys.py | 29 ---------- 4 files changed, 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc3a6933b..e4f119ef8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/spyglass/common/common_device.py b/src/spyglass/common/common_device.py index 8a78d511b..f61c186ae 100644 --- a/src/spyglass/common/common_device.py +++ b/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 diff --git a/src/spyglass/common/common_ephys.py b/src/spyglass/common/common_ephys.py index d9ed0b3a8..d718cea1e 100644 --- a/src/spyglass/common/common_ephys.py +++ b/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)) diff --git a/tests/common/test_ephys.py b/tests/common/test_ephys.py index 3800c2896..6c07cde4d 100644 --- a/tests/common/test_ephys.py +++ b/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