Replace ic_electrode with icephys_electrode on NWBFile (#1200)

* Deprecate ic_electrode params and funcs on NWBFile with icephys_electrode params and funcs. Fix #1181

* Replace use of ic_electrode with icephys_electrode in integration tests

* Add tests to check that deprecation warnings are raised for ic_electrode uses

* Replace use of ic_electrode with icephys_electrode in the ICEphys tutorial

* Fix flake8

* Update tests/unit/test_icephys.py

Co-Authored-By: Ryan Ly <[email protected]>

* Simplify testing of warnings using self.assertWarnsWith as suggested by @rly

* Mover icephys_electrodes NWBFile constructor arg to the end to not break backward compatibility as suggest by @rly

Co-authored-by: Ryan Ly <[email protected]>

Author: oruebel

docs/gallery/domain/icephys.py

@@ -44,11 +44,11 @@
 #
 # Intracellular electrode metadata is represented by :py:class:`~pynwb.icephys.IntracellularElectrode` objects.
 # To create an electrode group, you can use the :py:class:`~pynwb.file.NWBFile` instance method
-# :py:meth:`~pynwb.file.NWBFile.create_ic_electrode`.
+# :py:meth:`~pynwb.file.NWBFile.create_icephys_electrode`.
 
-elec = nwbfile.create_ic_electrode(name="elec0",
-                                   description='a mock intracellular electrode',
-                                   device=device)
+elec = nwbfile.create_icephys_electrode(name="elec0",
+                                        description='a mock intracellular electrode',
+                                        device=device)
 
 #######################
 # Stimulus data
@@ -160,7 +160,7 @@
 ####################
 # We can also get back the electrode we added.
 
-elec = nwbfile.get_ic_electrode('elec0')
+elec = nwbfile.get_icephys_electrode('elec0')
 
 ####################
 # Alternatively, we can also get this electrode from the :py:class:`~pynwb.icephys.CurrentClampStimulusSeries`

src/pynwb/file.py

@@ -140,11 +140,11 @@ class NWBFile(MultiContainerInterface):
             'get': 'get_imaging_plane'
         },
         {
-            'attr': 'ic_electrodes',
-            'add': 'add_ic_electrode',
+            'attr': 'icephys_electrodes',
+            'add': 'add_icephys_electrode',
             'type': IntracellularElectrode,
-            'create': 'create_ic_electrode',
-            'get': 'get_ic_electrode'
+            'create': 'create_icephys_electrode',
+            'get': 'get_icephys_electrode'
         },
         {
             'attr': 'ogen_sites',
@@ -275,7 +275,8 @@ class NWBFile(MultiContainerInterface):
             {'name': 'electrode_groups', 'type': Iterable,
              'doc': 'the ElectrodeGroups that belong to this NWBFile', 'default': None},
             {'name': 'ic_electrodes', 'type': (list, tuple),
-             'doc': 'IntracellularElectrodes that belong to this NWBFile', 'default': None},
+             'doc': 'DEPRECATED use icephys_electrodes parameter instead. '
+                    'IntracellularElectrodes that belong to this NWBFile', 'default': None},
             {'name': 'sweep_table', 'type': SweepTable,
              'doc': 'the SweepTable that belong to this NWBFile', 'default': None},
             {'name': 'imaging_planes', 'type': (list, tuple),
@@ -287,7 +288,9 @@ class NWBFile(MultiContainerInterface):
             {'name': 'subject', 'type': Subject,
              'doc': 'subject metadata', 'default': None},
             {'name': 'scratch', 'type': (list, tuple),
-             'doc': 'scratch data', 'default': None})
+             'doc': 'scratch data', 'default': None},
+            {'name': 'icephys_electrodes', 'type': (list, tuple),
+             'doc': 'IntracellularElectrodes that belong to this NWBFile.', 'default': None})
     def __init__(self, **kwargs):
         kwargs['name'] = 'root'
         call_docval_func(super(NWBFile, self).__init__, kwargs)
@@ -322,7 +325,6 @@ def __init__(self, **kwargs):
             'electrodes',
             'electrode_groups',
             'devices',
-            'ic_electrodes',
             'imaging_planes',
             'ogen_sites',
             'intervals',
@@ -352,6 +354,14 @@ def __init__(self, **kwargs):
         for attr in fieldnames:
             setattr(self, attr, kwargs.get(attr, None))
 
+        # backwards-compatibility code for ic_electrodes / icephys_electrodes
+        ic_elec_val = kwargs.get('icephys_electrodes', None)
+        if ic_elec_val is None and kwargs.get('ic_electrodes', None) is not None:
+            ic_elec_val = kwargs.get('ic_electrodes', None)
+            warn("Use of the ic_electrodes parameter is deprecated. "
+                 "Use the icephys_electrodes parameter instead", DeprecationWarning)
+        setattr(self, 'icephys_electrodes', ic_elec_val)
+
         experimenter = kwargs.get('experimenter', None)
         if isinstance(experimenter, str):
             experimenter = (experimenter,)
@@ -404,6 +414,35 @@ def ec_electrodes(self):
         warn("replaced by NWBFile.electrodes", DeprecationWarning)
         return self.electrodes
 
+    @property
+    def ic_electrodes(self):
+        warn("deprecated. use NWBFile.icephys_electrodes instead", DeprecationWarning)
+        return self.icephys_electrodes
+
+    def add_ic_electrode(self, *args, **kwargs):
+        """
+        This method is deprecated and will be removed in future versions. Please
+        use :py:meth:`~pynwb.file.NWBFile.add_icephys_electrode` instead
+        """
+        warn("deprecated, use NWBFile.add_icephys_electrode instead", DeprecationWarning)
+        return self.add_icephys_electrode(*args, **kwargs)
+
+    def create_ic_electrode(self, *args, **kwargs):
+        """
+        This method is deprecated and will be removed in future versions. Please
+        use :py:meth:`~pynwb.file.NWBFile.create_icephys_electrode` instead
+        """
+        warn("deprecated, use NWBFile.create_icephys_electrode instead", DeprecationWarning)
+        return self.create_icephys_electrode(*args, **kwargs)
+
+    def get_ic_electrode(self, *args, **kwargs):
+        """
+        This method is deprecated and will be removed in future versions. Please
+        use :py:meth:`~pynwb.file.NWBFile.get_icephys_electrode` instead
+        """
+        warn("deprecated, use NWBFile.get_icephys_electrode instead", DeprecationWarning)
+        return self.get_icephys_electrode(*args, **kwargs)
+
     def __check_epochs(self):
         if self.epochs is None:
             self.epochs = TimeIntervals('epochs', 'experimental epochs')

src/pynwb/io/file.py

@@ -49,7 +49,7 @@ def __init__(self, spec):
 
         icephys_spec = general_spec.get_group('intracellular_ephys')
         self.unmap(icephys_spec)
-        self.map_spec('ic_electrodes', icephys_spec.get_neurodata_type('IntracellularElectrode'))
+        self.map_spec('icephys_electrodes', icephys_spec.get_neurodata_type('IntracellularElectrode'))
         self.map_spec('sweep_table', icephys_spec.get_neurodata_type('SweepTable'))
 
         # TODO map the filtering dataset to something or deprecate it

tests/integration/hdf5/test_icephys.py

@@ -22,12 +22,12 @@ def setUpContainer(self):
 
     def addContainer(self, nwbfile):
         """ Add the test IntracellularElectrode and Device to the given NWBFile """
-        nwbfile.add_ic_electrode(self.container)
+        nwbfile.add_icephys_electrode(self.container)
         nwbfile.add_device(self.device)
 
     def getContainer(self, nwbfile):
         """ Return the test IntracellularElectrode from the given NWBFile """
-        return nwbfile.get_ic_electrode(self.container.name)
+        return nwbfile.get_icephys_electrode(self.container.name)
 
 
 class TestPatchClampSeries(AcquisitionH5IOMixin, TestCase):
@@ -54,7 +54,7 @@ def addContainer(self, nwbfile):
         """
         Add the test PatchClampSeries as an acquisition and IntracellularElectrode and Device to the given NWBFile
         """
-        nwbfile.add_ic_electrode(self.elec)
+        nwbfile.add_icephys_electrode(self.elec)
         nwbfile.add_device(self.device)
         super().addContainer(nwbfile)
 
@@ -132,7 +132,7 @@ def addContainer(self, nwbfile):
         """
         nwbfile.sweep_table = self.container
         nwbfile.add_device(self.device)
-        nwbfile.add_ic_electrode(self.elec)
+        nwbfile.add_icephys_electrode(self.elec)
         nwbfile.add_acquisition(self.pcs)
 
     def getContainer(self, nwbfile):
@@ -182,7 +182,7 @@ def addContainer(self, nwbfile):
         """
         nwbfile.sweep_table = self.container
         nwbfile.add_device(self.device)
-        nwbfile.add_ic_electrode(self.elec)
+        nwbfile.add_icephys_electrode(self.elec)
 
         nwbfile.add_acquisition(self.pcs1)
         nwbfile.add_stimulus_template(self.pcs2a)

tests/unit/test_icephys.py

@@ -4,22 +4,97 @@
         VoltageClampSeries, VoltageClampStimulusSeries, IntracellularElectrode
 from pynwb.device import Device
 from pynwb.testing import TestCase
+from pynwb.file import NWBFile  # Needed to test icephys functionality defined on NWBFile
+from datetime import datetime
+from dateutil.tz import tzlocal
 
 
 def GetElectrode():
     device = Device(name='device_name')
-    elec = IntracellularElectrode('test_iS',
-                                  device,
-                                  'description',
-                                  'slice',
-                                  'seal',
-                                  'location',
-                                  'resistance',
-                                  'filtering',
-                                  'initial_access_resistance')
+    elec = IntracellularElectrode(
+        name='test_iS',
+        device=device,
+        description='description',
+        slice='slice',
+        seal='seal',
+        location='location',
+        resistance='resistance',
+        filtering='filtering',
+        initial_access_resistance='initial_access_resistance')
     return elec
 
 
+class NWBFileICEphys(TestCase):
+    """Test ICEphys-specific functionality on NWBFile"""
+    def setUp(self):
+        self.icephys_electrode = GetElectrode()
+
+    def test_ic_electrodes_parameter_deprecation(self):
+        # Make sure we warn when using the ic_electrodes parameter on NWBFile
+        msg = "Use of the ic_electrodes parameter is deprecated. Use the icephys_electrodes parameter instead"
+        with self.assertWarnsWith(DeprecationWarning, msg):
+            _ = NWBFile(
+                session_description='NWBFile icephys test',
+                identifier='NWB123',  # required
+                session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()),
+                ic_electrodes=[self.icephys_electrode, ])
+
+    def test_icephys_electrodes_parameter(self):
+        nwbfile = NWBFile(
+                session_description='NWBFile icephys test',
+                identifier='NWB123',  # required
+                session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()),
+                icephys_electrodes=[self.icephys_electrode, ])
+        self.assertEqual(nwbfile.get_icephys_electrode('test_iS'), self.icephys_electrode)
+
+    def test_add_ic_electrode_deprecation(self):
+        # Make sure we warn when using the add_ic_electrodes parameter on NWBFile
+        nwbfile = NWBFile(
+                session_description='NWBFile icephys test',
+                identifier='NWB123',  # required
+                session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()))
+
+        msg = "deprecated, use NWBFile.add_icephys_electrode instead"
+        with self.assertWarnsWith(DeprecationWarning, msg):
+            nwbfile.add_ic_electrode(self.icephys_electrode)
+
+    def test_ic_electrodes_attribute_deprecation(self):
+        nwbfile = NWBFile(
+            session_description='NWBFile icephys test',
+            identifier='NWB123',  # required
+            session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()),
+            icephys_electrodes=[self.icephys_electrode, ])
+        # make sure NWBFile.ic_electrodes property warns
+
+        msg = "deprecated. use NWBFile.icephys_electrodes instead"
+        with self.assertWarnsWith(DeprecationWarning, msg):
+            nwbfile.ic_electrodes
+
+        # make sure NWBFile.get_ic_electrode warns
+        msg = "deprecated, use NWBFile.get_icephys_electrode instead"
+        with self.assertWarnsWith(DeprecationWarning, msg):
+            nwbfile.get_ic_electrode(self.icephys_electrode.name)
+
+    def test_create_ic_electrode_deprecation(self):
+        nwbfile = NWBFile(
+            session_description='NWBFile icephys test',
+            identifier='NWB123',  # required
+            session_start_time=datetime(2017, 4, 3, 11, tzinfo=tzlocal()))
+        device = Device(name='device_name')
+        msg = "deprecated, use NWBFile.create_icephys_electrode instead"
+        with self.assertWarnsWith(DeprecationWarning, msg):
+            nwbfile.create_ic_electrode(
+                name='test_iS',
+                device=device,
+                description='description',
+                slice='slice',
+                seal='seal',
+                location='location',
+                resistance='resistance',
+                filtering='filtering',
+                initial_access_resistance='initial_access_resistance')
+
+
 class IntracellularElectrodeConstructor(TestCase):
 
     def test_constructor(self):