Change timeseries data checks to warn instead of error on read (#1793)

* change timeseries rate errors to warn on read (#1786, #1721) * add test for starting time and update CHANGELOG --------- Co-authored-by: Steph Prince <[email protected]>
NeurodataWithoutBorders · Nov 30, 2023 · 87a7c57 · 87a7c57
1 parent 5e8ddc2
commit 87a7c57
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - For `NWBHDF5IO()`, change the default of arg `load_namespaces` from `False` to `True`. @bendichter [#1748](https://github.com/NeurodataWithoutBorders/pynwb/pull/1748)
 - Add `NWBHDF5IO.can_read()`. @bendichter [#1703](https://github.com/NeurodataWithoutBorders/pynwb/pull/1703)
 - Add `pynwb.get_nwbfile_version()`. @bendichter [#1703](https://github.com/NeurodataWithoutBorders/pynwb/pull/1703)
+- Updated timeseries data checks to warn instead of error when reading invalid files. @stephprince [#1793](https://github.com/NeurodataWithoutBorders/pynwb/pull/1793)
 
 ## PyNWB 2.5.0 (August 18, 2023)
 

diff --git a/src/pynwb/base.py b/src/pynwb/base.py
@@ -174,15 +174,23 @@ def __init__(self, **kwargs):
         timestamps = args_to_process['timestamps']
         if timestamps is not None:
             if self.rate is not None:
-                raise ValueError('Specifying rate and timestamps is not supported.')
+                self._error_on_new_warn_on_construct(
+                    error_msg='Specifying rate and timestamps is not supported.'
+                )
             if self.starting_time is not None:
-                raise ValueError('Specifying starting_time and timestamps is not supported.')
+                self._error_on_new_warn_on_construct(
+                    error_msg='Specifying starting_time and timestamps is not supported.'
+                )
             self.fields['timestamps'] = timestamps
             self.timestamps_unit = self.__time_unit
             self.interval = 1
             if isinstance(timestamps, TimeSeries):
                 timestamps.__add_link('timestamp_link', self)
         elif self.rate is not None:
+            if self.rate <= 0:
+                self._error_on_new_warn_on_construct(
+                    error_msg='Rate must be a positive value.'
+                )
             if self.starting_time is None:  # override default if rate is provided but not starting time
                 self.starting_time = 0.0
             self.starting_time_unit = self.__time_unit

diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
@@ -405,6 +405,63 @@ def test_get_data_in_units(self):
         ts = mock_TimeSeries(data=[1., 2., 3.])
         assert_array_equal(ts.get_data_in_units(), [1., 2., 3.])
 
+    def test_non_positive_rate(self):
+        with self.assertRaisesWith(ValueError, 'Rate must be a positive value.'):
+            TimeSeries(name='test_ts', data=list(), unit='volts', rate=-1.0)
+        with self.assertRaisesWith(ValueError, 'Rate must be a positive value.'):
+            TimeSeries(name='test_ts1', data=list(), unit='volts', rate=0.0)
+
+    def test_file_with_non_positive_rate_in_construct_mode(self):
+        """Test that UserWarning is raised when rate is 0 or negative
+         while being in construct mode (i.e,. on data read)."""
+        obj = TimeSeries.__new__(TimeSeries,
+                                 container_source=None,
+                                 parent=None,
+                                 object_id="test",
+                                 in_construct_mode=True)
+        with self.assertWarnsWith(warn_type=UserWarning, exc_msg='Rate must be a positive value.'):
+            obj.__init__(
+                name="test_ts",
+                data=list(),
+                unit="volts",
+                rate=-1.0
+            )
+
+    def test_file_with_rate_and_timestamps_in_construct_mode(self):
+        """Test that UserWarning is raised when rate and timestamps are both specified
+         while being in construct mode (i.e,. on data read)."""
+        obj = TimeSeries.__new__(TimeSeries,
+                                 container_source=None,
+                                 parent=None,
+                                 object_id="test",
+                                 in_construct_mode=True)
+        with self.assertWarnsWith(warn_type=UserWarning, exc_msg='Specifying rate and timestamps is not supported.'):
+            obj.__init__(
+                name="test_ts",
+                data=[11, 12, 13, 14, 15],
+                unit="volts",
+                rate=1.0,
+                timestamps=[1, 2, 3, 4, 5]
+            )
+
+    def test_file_with_starting_time_and_timestamps_in_construct_mode(self):
+        """Test that UserWarning is raised when starting_time and timestamps are both specified
+         while being in construct mode (i.e,. on data read)."""
+        obj = TimeSeries.__new__(TimeSeries,
+                                 container_source=None,
+                                 parent=None,
+                                 object_id="test",
+                                 in_construct_mode=True)
+        with self.assertWarnsWith(warn_type=UserWarning,
+                                  exc_msg='Specifying starting_time and timestamps is not supported.'):
+            obj.__init__(
+                name="test_ts",
+                data=[11, 12, 13, 14, 15],
+                unit="volts",
+                starting_time=1.0,
+                timestamps=[1, 2, 3, 4, 5]
+            )
+
 
 class TestImage(TestCase):
     def test_init(self):