From ed99b627239c6cd72a261c2c9225bd74c7d7a176 Mon Sep 17 00:00:00 2001 From: Steph Prince Date: Wed, 29 Nov 2023 11:22:48 -0800 Subject: [PATCH 1/2] change timeseries rate errors to warn on read (#1786, #1721) --- src/pynwb/base.py | 12 ++++++++++-- tests/unit/test_base.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/pynwb/base.py b/src/pynwb/base.py index bec8903d5..5514288e8 100644 --- 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 index 805f946ec..712f25753 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -405,6 +405,45 @@ 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] + ) + class TestImage(TestCase): def test_init(self): From e8bd8f88406fab714a49e3bb71c01d6854991104 Mon Sep 17 00:00:00 2001 From: Steph Prince Date: Wed, 29 Nov 2023 15:54:13 -0800 Subject: [PATCH 2/2] add test for starting time and update CHANGELOG --- CHANGELOG.md | 1 + tests/unit/test_base.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaef47607..62122888a 100644 --- 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/tests/unit/test_base.py b/tests/unit/test_base.py index 712f25753..899628190 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -444,6 +444,24 @@ def test_file_with_rate_and_timestamps_in_construct_mode(self): 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):