diff --git a/docs/gallery/domain/brain_observatory.py b/docs/gallery/domain/brain_observatory.py index 11e490251..2c2600177 100644 --- a/docs/gallery/domain/brain_observatory.py +++ b/docs/gallery/domain/brain_observatory.py @@ -74,7 +74,7 @@ data=dataset.get_stimulus_template(stimulus), unit='NA', format='raw', - timestamps=[0.0]) + timestamps=timestamps[dataset.get_stimulus_table(stimulus).start.values]) image_index = IndexSeries( name=stimulus, data=dataset.get_stimulus_table(stimulus).frame.values, diff --git a/src/pynwb/base.py b/src/pynwb/base.py index 747c039d8..2b049e0dd 100644 --- a/src/pynwb/base.py +++ b/src/pynwb/base.py @@ -189,28 +189,34 @@ def __init__(self, **kwargs): else: raise TypeError("either 'timestamps' or 'rate' must be specified") - if not self._check_time_series_dimension(): - warn("%s '%s': Length of data does not match length of timestamps. Your data may be transposed. " - "Time should be on the 0th dimension" % (self.__class__.__name__, self.name)) + self._error_on_new_warn_on_construct( + error_msg=self._check_time_series_dimension() + ) def _check_time_series_dimension(self): """Check that the 0th dimension of data equals the length of timestamps, when applicable. """ if self.timestamps is None: - return True + return data_shape = get_data_shape(data=self.fields["data"], strict_no_data_load=True) timestamps_shape = get_data_shape(data=self.fields["timestamps"], strict_no_data_load=True) # skip check if shape of data or timestamps cannot be computed if data_shape is None or timestamps_shape is None: - return True + return # skip check if length of the first dimension is not known if data_shape[0] is None or timestamps_shape[0] is None: - return True + return - return data_shape[0] == timestamps_shape[0] + if data_shape[0] == timestamps_shape[0]: + return + + return ( + "%s '%s': Length of data does not match length of timestamps. Your data may be transposed. " + "Time should be on the 0th dimension" % (self.__class__.__name__, self.name) + ) @property def num_samples(self): diff --git a/src/pynwb/image.py b/src/pynwb/image.py index d8953d397..4b6a40f63 100644 --- a/src/pynwb/image.py +++ b/src/pynwb/image.py @@ -102,13 +102,9 @@ def __init__(self, **kwargs): DeprecationWarning, ) - if not self._check_image_series_dimension(): - warnings.warn( - "%s '%s': Length of data does not match length of timestamps. Your data may be transposed. " - "Time should be on the 0th dimension" - % (self.__class__.__name__, self.name) - ) - + self._error_on_new_warn_on_construct( + error_msg=self._check_image_series_dimension() + ) self._error_on_new_warn_on_construct( error_msg=self._check_external_file_starting_frame_length() ) @@ -135,17 +131,17 @@ def _check_time_series_dimension(self): """Override _check_time_series_dimension to do nothing. The _check_image_series_dimension method will be called instead. """ - return True + return def _check_image_series_dimension(self): """Check that the 0th dimension of data equals the length of timestamps, when applicable. ImageSeries objects can have an external file instead of data stored. The external file cannot be - queried for the number of frames it contains, so this check will return True when an external file + queried for the number of frames it contains, so this check will return when an external file is provided. Otherwise, this function calls the parent class' _check_time_series_dimension method. """ if self.external_file is not None: - return True + return return super()._check_time_series_dimension() def _check_external_file_starting_frame_length(self): diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index a27a90e96..6c20e4d71 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -373,12 +373,13 @@ def test_conflicting_time_args(self): timestamps=[0.3, 0.4, 0.5], ) - def test_dimension_warning(self): + def test_timestamps_data_length_error_raised(self): + """Test that TimeSeries cannot be created with timestamps and data of different lengths.""" msg = ( "TimeSeries 'test_ts2': Length of data does not match length of timestamps. Your data may be " "transposed. Time should be on the 0th dimension" ) - with self.assertWarnsWith(UserWarning, msg): + with self.assertRaisesWith(ValueError, msg): TimeSeries( name="test_ts2", data=[10, 11, 12], @@ -386,6 +387,39 @@ def test_dimension_warning(self): timestamps=[0.3, 0.4, 0.5, 0.6, 0.7, 0.8], ) + def test_timestamps_data_length_warning_construct_mode(self): + """ + Test that warning is raised when the length of data does not match the length of + timestamps in case that the TimeSeries in construct mode (i.e., during read). + """ + msg = ( + "TimeSeries 'test_ts2': Length of data does not match length of timestamps. Your data may be " + "transposed. Time should be on the 0th dimension" + ) + for timestamps in [[0], [1, 2, 3, 4]]: + with self.subTest(): + # Create the time series in construct mode, modelling the behavior + # of the ObjectMapper on read while avoiding having to create, write, + # and read and entire NWB file + obj = TimeSeries.__new__( + TimeSeries, + container_source=None, + parent=None, + object_id="test", + in_construct_mode=True, + ) + with self.assertWarnsWith(UserWarning, msg): + obj.__init__( + name="test_ts2", + data=[10, 11, 12], + unit="grams", + timestamps=timestamps, + ) + # Disable construct mode. Since we are not using this object anymore + # this is not strictly necessary but is good style in case we expand + # the test later on. + obj._in_construct_mode = False + class TestImage(TestCase): def test_init(self): diff --git a/tests/unit/test_image.py b/tests/unit/test_image.py index 7ac426be8..89fe9bbb2 100644 --- a/tests/unit/test_image.py +++ b/tests/unit/test_image.py @@ -82,14 +82,13 @@ def test_external_file_no_unit(self): ) self.assertEqual(iS.unit, ImageSeries.DEFAULT_UNIT) - def test_dimension_warning(self): - """Test that a warning is raised when the dimensions of the data are not the - same as the dimensions of the timestamps.""" + def test_dimension_error(self): + """Test that ImageSeries cannot be created with timestamps and data of different lengths.""" msg = ( "ImageSeries 'test_iS': Length of data does not match length of timestamps. Your data may be " "transposed. Time should be on the 0th dimension" ) - with self.assertWarnsWith(UserWarning, msg): + with self.assertRaisesWith(ValueError, msg): ImageSeries( name='test_iS', data=np.ones((3, 3, 3)), @@ -98,9 +97,14 @@ def test_dimension_warning(self): ) def test_dimension_warning_external_file_with_timestamps(self): - """Test that a warning is not raised when external file is used with timestamps.""" + """Test that warning is not raised when external file is used with timestamps.""" + obj = ImageSeries.__new__(ImageSeries, + container_source=None, + parent=None, + object_id="test", + in_construct_mode=True) with warnings.catch_warnings(record=True) as w: - ImageSeries( + obj.__init__( name='test_iS', external_file=['external_file'], format='external', @@ -109,6 +113,10 @@ def test_dimension_warning_external_file_with_timestamps(self): timestamps=[1, 2, 3, 4] ) self.assertEqual(w, []) + # Disable construct mode. Since we are not using this object any more + # this is not strictly necessary but is good style in case we expand + # the test later on + obj._in_construct_mode = False def test_dimension_warning_external_file_with_rate(self): """Test that a warning is not raised when external file is used with rate."""