[BUG] Fix TimeSeries data does not match length of timestamps should raise an error when created

docs/gallery/domain/brain_observatory.py

@@ -18,6 +18,8 @@
 
 # sphinx_gallery_thumbnail_path = 'figures/gallery_thumbnails_allenbrainobservatory.png'
 
+import numpy as np
+
 import allensdk.brain_observatory.stimulus_info as si
 from allensdk.core.brain_observatory_cache import BrainObservatoryCache
 
@@ -71,8 +73,9 @@
         data=dataset.get_stimulus_template(stimulus),
         unit="NA",
         format="raw",
-        timestamps=[0.0],
+        rate=np.nan,
     )
+
     image_index = IndexSeries(
         name=stimulus,
         data=dataset.get_stimulus_table(stimulus).frame.values,

src/pynwb/base.py

@@ -199,28 +199,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):

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,7 +131,7 @@ 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.
@@ -145,7 +141,7 @@ def _check_image_series_dimension(self):
         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):

tests/unit/test_base.py

@@ -375,12 +375,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],
@@ -478,6 +479,39 @@ def test_repr_html(self):
         pm.add(ts2)
         self.assertIn('(link to processing/test_ts1/timestamps)', pm._repr_html_())
 
+    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):

tests/unit/test_image.py

@@ -84,14 +84,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)),
@@ -100,9 +99,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',
@@ -111,6 +115,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."""