add code review suggestions

src/pynwb/core.py

@@ -38,11 +38,8 @@ def _error_on_new_warn_on_construct(self, error_msg: str):
         Raise an error when a check is violated on instance creation.
         To ensure backwards compatibility, this method throws a warning
         instead of raising an error when reading from a file, ensuring that
-        files with invalid data can be read. If error_msg is set to None
-        the function will simply return without further action.
+        files with invalid data can be read.
         """
-        if error_msg is None:
-            return
         if not self._in_construct_mode:
             raise ValueError(error_msg)
         warn(error_msg)
@@ -52,11 +49,7 @@ def _error_on_new_pass_on_construct(self, error_msg: str):
         Raise an error when a check is violated on instance creation.
         When reading from a file, do nothing, ensuring that files with
         invalid data or deprecated neurodata types can be read.
-        If error_msg is set to None the function will simply return
-        without further action.
         """
-        if error_msg is None:
-            return
         if not self._in_construct_mode:
             raise ValueError(error_msg)
 

src/pynwb/file.py

@@ -382,7 +382,8 @@ class NWBFile(MultiContainerInterface, HERDManager):
              'doc': 'the ElectrodeGroups that belong to this NWBFile', 'default': None},
             {'name': 'ic_electrodes', 'type': (list, tuple),
              'doc': 'DEPRECATED use icephys_electrodes parameter instead. '
-                    'IntracellularElectrodes that belong to this NWBFile', 'default': None},  # TODO remove this arg in PyNWB 4.0
+                    'IntracellularElectrodes that belong to this NWBFile', 'default': None},  
+                    # TODO remove this arg in PyNWB 4.0
             {'name': 'sweep_table', 'type': SweepTable,
              'doc': '[DEPRECATED] Use IntracellularRecordingsTable instead. '
                     'The SweepTable that belong to this NWBFile', 'default': None},
@@ -491,7 +492,7 @@ def __init__(self, **kwargs):
         # backwards-compatibility code for ic_electrodes / icephys_electrodes
         icephys_electrodes = args_to_set['icephys_electrodes']
         ic_electrodes = args_to_set['ic_electrodes']
-        if icephys_electrodes is None and ic_electrodes is not None:
+        if ic_electrodes is not None:
             self._error_on_new_pass_on_construct(error_msg=("Use of the ic_electrodes parameter is deprecated "
                                                             "and will be removed in PyNWB 4.0. "
                                                             "Use the icephys_electrodes parameter instead"))

src/pynwb/image.py

@@ -106,13 +106,17 @@ def __init__(self, **kwargs):
                 % (self.__class__.__name__, self.name)
             )
 
-        self._error_on_new_warn_on_construct(
-            error_msg=self._check_external_file_starting_frame_length()
-        )
-        self._error_on_new_warn_on_construct(
-            error_msg=self._check_external_file_format()
-        )
-        self._error_on_new_warn_on_construct(error_msg=self._check_external_file_data())
+        error_msg = self._check_external_file_starting_frame_length()
+        if error_msg:
+            self._error_on_new_warn_on_construct(error_msg=error_msg)
+
+        error_msg = self._check_external_file_format()
+        if error_msg:
+            self._error_on_new_warn_on_construct(error_msg=error_msg)
+
+        error_msg = self._check_external_file_data()
+        if error_msg:
+            self._error_on_new_warn_on_construct(error_msg=error_msg)
 
     def _change_external_file_format(self):
         """

tests/unit/test_icephys_metadata_tables.py

@@ -1241,7 +1241,7 @@ def test_deprecation_icephys_filtering_on_init(self):
                 identifier='EXAMPLE_ID',
                 session_start_time=datetime.now(tzlocal()),
                 icephys_filtering='test filtering')
-        msg = ("Use of icephys_filtering is deprecated. "
+        msg = ("Use of icephys_filtering is deprecated and will be removed in PyNWB 4.0. "
                "Use the IntracellularElectrode.filtering field instead")        
 
         with self.assertRaisesWith(ValueError, msg):
@@ -1262,7 +1262,7 @@ def test_icephys_filtering_roundtrip(self):
             session_start_time=datetime.now(tzlocal())
         )
         # set the icephys_filtering attribute and make sure we get a deprecation warning
-        msg = ("Use of icephys_filtering is deprecated. "
+        msg = ("Use of icephys_filtering is deprecated and will be removed in PyNWB 4.0. "
                "Use the IntracellularElectrode.filtering field instead")
         with self.assertRaisesWith(ValueError, msg):
             nwbfile.icephys_filtering = 'test filtering'